Page MenuHomeVyOS Platform
Create Task
Maniphest T5690

Change to definition of environment variable 'vyos_rootfs_dir' is incorrect
Closed, ResolvedPublicBUG
Actions

Description

The change to the definition of the environment variable in https://vyos.dev/T5461 is incorrect in the case that an image is added with a custom name. More generally, for future uses of this env var, one will want to distinguish between vyos_rootfs_dir before and after bind_mount_boot, with appropriate names --- here we only address its current (and only) use in the context of https://vyos.dev/T5440 by reverting the commit revising the definition (6b03f686406); the original definition suffices for its use.

Symptom when updating 1.3.4 to Sagitta:

vyos@vyos:~$ show system image
The system currently has the following image(s) installed:

   1: 1.3.4 (default boot)

vyos@vyos:~$ add system image vyos-1.4-rolling-202310261853-amd64.iso
Checking SHA256 checksums of files on the ISO image... OK.
Done!
What would you like to name this image? [1.4-rolling-202310261853]: some-other-name-vyos-1.4
OK.  This image will be named: some-other-name-vyos-1.4
Installing "some-other-name-vyos-1.4" image.
...
Oct 26 19:33:41 vyos vyos-router[1089]: Starting VyOS router: migrate
Oct 26 19:33:41 vyos vyos-router[1688]: cp: cannot stat '/usr/lib/live/mount/rootfs/some-other-name-vyos-1.4.squashfs/opt/vyatta/etc/config/scripts/vyos-preconfig-bootup.script': No such file or directory
Oct 26 19:33:41 vyos vyos-router[1689]: chgrp: cannot access '/opt/vyatta/etc/config/scripts/vyos-preconfig-bootup.script': No such file or directory
Oct 26 19:33:41 vyos vyos-router[1690]: chmod: cannot access '/opt/vyatta/etc/config/scripts/vyos-preconfig-bootup.script': No such file or directory

Since this is called before bind_mount, one wants the original definition:

root@vyos:/home/vyos# mount -t squashfs | cut -d' ' -f3
/usr/lib/live/mount/rootfs/1.4-rolling-202310261853.squashfs

not the revised definition using:

root@vyos:/home/vyos# cat /proc/cmdline | sed -e s+^.*vyos-union=/boot/++ | sed -e 's/ .*$//'
some-other-name-vyos-1.4

Again, for any future uses the env var should be split into cases for searching
/usr/lib/live/mount/rootfs/
respectively
/usr/lib/live/mount/persistence/boot
for version before bind_mount respectively image-name after.

Details

Version
-
Is it a breaking change?
Perfectly compatible
Issue type
Bug (incorrect behavior)

Related Objects

Event Timeline

jestabro created this task.Oct 26 2023, 8:07 PM
jestabro triaged this task as Normal priority.
jestabro created this object in space S1 VyOS Public.
jestabro added projects: VyOS 1.5 Circinus, VyOS 1.4 Sagitta.
jestabro changed the task status from Open to In progress.Oct 26 2023, 8:45 PM
jestabro raised the priority of this task from Normal to Urgent!.
jestabro edited a custom field.Oct 26 2023, 8:48 PM
jestabro changed Is it a breaking change? from Unspecified (possibly destroys the router) to Perfectly compatible.
jestabro changed Issue type from Unspecified (please specify) to Bug (incorrect behavior).
pasik subscribed.Oct 26 2023, 8:50 PM
Apachez subscribed.Oct 27 2023, 10:37 AM

So in short https://vyos.dev/T5440 will be broken again?

jestabro added a comment.Oct 27 2023, 1:52 PM

Nope, it works fine, now; your revision to the change re-broke it.

Apachez added a comment.Oct 27 2023, 2:16 PM

But before the revert by T5690 today T5440 worked perfectly fine so what was "re-broken"?

For below example I have installed 1.5-rolling-202310240118 with the custom name 1.5-CUSTOM-231024.

Suggestions (that is perform all 2 suggestions as mentioned below):

Let vyos_rootfs_dir be as the original pre T5461 was. That is let the revert in T5690 remain.

Example content:

/usr/lib/live/mount/rootfs/1.5-rolling-202310240118.squashfs

root@vyos:~# mount -t squashfs | cut -d' ' -f3
/usr/lib/live/mount/rootfs/1.5-rolling-202310240118.squashfs

Create a new variable and name it vyos_persistence_dir

Example content:

/usr/lib/live/mount/persistence/boot/1.5-CUSTOM-231024

root@vyos:~# cat /proc/cmdline | sed -e s+^.*vyos-union=++ | sed -e 's/ .*$//'
/boot/1.5-CUSTOM-231024

That is file https://github.com/vyos/vyatta-op/blob/current/etc/default/vyatta.in would have this added to create the variable mentioned in suggestion 2 above:

if test -z "$vyos_persistence_dir" ; then
    IMAGE_NAME=$(cat /proc/cmdline | sed -e s+^.*vyos-union=++ | sed -e 's/ .*$//')
    declare -x -r vyos_persistence_dir="/usr/lib/live/mount/persistence/${IMAGE_NAME}"
fi

Anyone against? :-)

This way we have vyos_rootfs_dir which is the mounted rootfs (filesystem.squashfs from ISO is renamed to the VyOS version during install/upgrade) which is readonly. Can be used as source to copy files and directories if needed.

And vyos_persistence_dir which is the actual directory on the permanent storage in the box running VyOS (which uses the custom name if such was selected during install/upgrade).

Apachez added a comment.Oct 27 2023, 2:23 PM

Update:

Looking through https://vyos.dev/T5457 I now get what you meant by "re-broke it".

The break comes with if the admin choose to use a custom name which the updated vyos_rootfs_dir didnt accounted for.

Will add a separate task for adding vyos_persistence_dir as a variable.

Apachez added a comment.Oct 27 2023, 2:30 PM

One way however to make the variable more robust in case there are for whatever reason more than one squashfs mounted object available is to select the one who is "loop0".

Like so:

if test -z "$vyos_rootfs_dir" ; then
    ROOTFS=$(mount -t squashfs | grep loop0 | cut -d' ' -f3)
    declare -x -r vyos_rootfs_dir="${ROOTFS}"
fi
Apachez mentioned this in T5693: Adding variable vyos_persistence_dir (and improve variable vyos_rootfs_dir).Oct 27 2023, 2:39 PM
Apachez added a comment.Oct 27 2023, 4:09 PM

PR created: https://github.com/vyos/vyatta-op/pull/79

jestabro closed this task as Resolved.Nov 1 2023, 12:44 AM
jestabro moved this task from Open to Finished on the VyOS 1.5 Circinus board.
jestabro moved this task from Open to Finished on the VyOS 1.4 Sagitta board.