Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

factory: mount users imported from hybrid system during recovery mode #263

Merged

Conversation

andrewphelpsj
Copy link
Member

These files are generated by snap-bootstrap when booting a hybrid system into recovery mode. The files contain a combination of the login information from the hybrid rootfs and the base.

@@ -20,3 +20,12 @@ if [ ! -d /run/mnt/data/system-data/var/lib/snapd/kernel/"$kernelName"/"$rev" ];
/run/mnt/kernel/modules /usr/lib/modules none bind,x-initrd.mount 0 0
EOF
fi

if [ -f /run/snapd/hybrid-users/passwd ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should check also if we are in recover mode (you can use get-mode for this)


if [ -f /run/snapd/hybrid-users/passwd ]; then
cat <<'EOF' >>/sysroot/etc/fstab
/run/snapd/hybrid-users/passwd /etc/passwd none bind,x-initrd.mount 0 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x-initrd.mount is not needed, the mounts can happen after switch root

@@ -21,11 +21,12 @@ if [ ! -d /run/mnt/data/system-data/var/lib/snapd/kernel/"$kernelName"/"$rev" ];
EOF
fi

if [ -f /run/snapd/hybrid-users/passwd ]; then
mode="$(/usr/libexec/core/get-mode mode)" || mode="$(/usr/libexec/core/get-arg snapd_recovery_mode)" || mode="unknown"
if [ -f /run/snapd/hybrid-users/passwd -a "${mode}" = "recover" ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using -a was deprecated, please use [...] && [...]

Copy link
Member

@alfonsosanchezbeato alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@@ -20,3 +20,13 @@ if [ ! -d /run/mnt/data/system-data/var/lib/snapd/kernel/"$kernelName"/"$rev" ];
/run/mnt/kernel/modules /usr/lib/modules none bind,x-initrd.mount 0 0
EOF
fi

mode="$(/usr/libexec/core/get-mode mode)" || mode="$(/usr/libexec/core/get-arg snapd_recovery_mode)" || mode="unknown"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really like to see a comment describing why we need to do this before this section

These files are generated by snap-bootstrap when booting a hybrid system
into recovery mode. The files contain a combination of the login information
from the hybrid rootfs and the base.
Copy link
Collaborator

@valentindavid valentindavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is fine. But could you add a reference to the PR in snapd that adds this feature?

@andrewphelpsj
Copy link
Member Author

@valentindavid the two PRs are here: canonical/snapd#14521, canonical/snapd#13581

@alfonsosanchezbeato alfonsosanchezbeato merged commit 9db3f5e into canonical:main Oct 10, 2024
3 checks passed
@alfonsosanchezbeato
Copy link
Member

@andrewphelpsj this will need a backport to the core24 branch (we recently branched)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants