Merge ~xnox/cloud-initramfs-tools:master into cloud-initramfs-tools:master

Proposed by Dimitri John Ledkov
Status: Work in progress
Proposed branch: ~xnox/cloud-initramfs-tools:master
Merge into: cloud-initramfs-tools:master
Diff against target: 56 lines (+13/-1)
2 files modified
debian/changelog (+9/-1)
overlayroot/scripts/init-bottom/overlayroot (+4/-0)
Reviewer Review Type Date Requested Status
Dimitri John Ledkov (community) Disapprove
Scott Moser Pending
Ryan Harper Pending
cloud-initramfs-tools Pending
Review via email: mp+389202@code.launchpad.net

Commit message

Allow booting ephemeral rw overlayroot squashfs without fstab.

- Do not attempt to overlayrootify non-existing fstab, and mount
  rootfs rw thus removing need to generate any fstab entries; remount
  ro; for systemd to generate mount unit; and remount as rw again.

LP: #1890803

To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

How can I retest this with curtin's vmtests?

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

I'd call this a MAAS bug too, why does it specify "ro" if it clearly wants ephemeral "rw" layer.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Or does this too miss something?

I'm struggling to see reasons to (re)mount the overlayfs layer 3 times.

Revision history for this message
Ryan Harper (raharper) wrote :

> How can I retest this with curtin's vmtests?

In the bug I showed the repack instructions, I'll leave repacking the boot-initrd from MAAS to you.

Once done, you can

% git clone lp:curtin
% cd curtin
% make vmtest-deps
# run this you can control-c it after it syncs the groovy kernel/initrd/squashfs
% rm -rf output/; CURTIN_VMTEST_IMAGE_SYNC=1 ./tools/jenkins-runner tests/vmtests/test_basic.py:GroovyTestBasic
# replace the boot-initrd with your repack
% mv /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd.orig
% cp boot-initrd.fixed /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd
% rm -rf ./output; ./tools/jenkins-runner tests/vmtests/test_basic.py:GroovyTestBasic

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Thanks!

On Thu, 13 Aug 2020 at 00:31, Ryan Harper <email address hidden> wrote:
>
> > How can I retest this with curtin's vmtests?
>
> In the bug I showed the repack instructions, I'll leave repacking the boot-initrd from MAAS to you.
>
> Once done, you can
>
> % git clone lp:curtin
> % cd curtin
> % make vmtest-deps
> # run this you can control-c it after it syncs the groovy kernel/initrd/squashfs
> % rm -rf output/; CURTIN_VMTEST_IMAGE_SYNC=1 ./tools/jenkins-runner tests/vmtests/test_basic.py:GroovyTestBasic
> # replace the boot-initrd with your repack
> % mv /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd.orig
> % cp boot-initrd.fixed /srv/images/groovy/amd64/20200811/ga-20.10/boot-initrd
> % rm -rf ./output; ./tools/jenkins-runner tests/vmtests/test_basic.py:GroovyTestBasic
>
> --
> https://code.launchpad.net/~xnox/cloud-initramfs-tools/+git/cloud-initramfs-tools/+merge/389202
> You are the owner of ~xnox/cloud-initramfs-tools:master.

--
Regards,

Dimitri.

Revision history for this message
Scott Moser (smoser) wrote :

'ro' is common on 95% of linux kernel command lines since 1995.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

That's why Dimitri gets to build the OS the bootloader the kernel cmdline
and how it is operated. 😀

I find it weird that everything constantly requests to mount rootfs "ro"
only to ignore it later, and remount rewrite.

"ro" only makes sense without initrd, not with.

And once it was mounted "rw" it makes even less sense to remount it "ro" to
remount "rw" again. Clearly we should be better than that 95% of the time.

On Thu, 13 Aug 2020, 01:43 Scott Moser, <email address hidden> wrote:

> 'ro' is common on 95% of linux kernel command lines since 1995.
>
> --
>
> https://code.launchpad.net/~xnox/cloud-initramfs-tools/+git/cloud-initramfs-tools/+merge/389202
> You are the owner of ~xnox/cloud-initramfs-tools:master.
>

Revision history for this message
Scott Moser (smoser) wrote :

I do have to agree that 'ro' is a archaic thing.
but overlayroot is correctly respecting it.

If you want to change kernel command lines, then you can/should do that. But don't *break* overlayroot.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

So, at this point i think rharpers merge is better. It's accurate (reading /proc/mounts) and is more minimal change than this one (thus easier to review/test/validate/land/etc).

Please reject this merge proposal.

review: Disapprove

Unmerged commits

bef4640... by Dimitri John Ledkov

Allow booting ephemeral rw overlayroot squashfs without fstab.

 - Do not attempt to overlayrootify non-existing fstab, and mount
 rootfs rw thus removing need to generate any fstab entries; remount
 ro; for systemd to generate mount unit; and remount as rw again.

LP: #1890803

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index a9c6a6c..8182b00 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,9 +1,17 @@
1cloud-initramfs-tools (0.45ubuntu1) UNRELEASED; urgency=medium1cloud-initramfs-tools (0.45ubuntu1) UNRELEASED; urgency=medium
22
3 [ Scott Moser ]
3 * Add dependency on flock for growroot's use of growpart.4 * Add dependency on flock for growroot's use of growpart.
4 (LP: #1834875)5 (LP: #1834875)
56
6 -- Scott Moser <smoser@ubuntu.com> Mon, 24 Feb 2020 20:22:30 -05007 [ Dimitri John Ledkov ]
8 * Allow booting ephemeral rw overlayroot squashfs without fstab. LP:
9 #1890803
10 - Do not attempt to overlayrootify non-existing fstab, and mount
11 rootfs rw thus removing need to generate any fstab entries; remount
12 ro; for systemd to generate mount unit; and remount as rw again.
13
14 -- Dimitri John Ledkov <xnox@ubuntu.com> Wed, 12 Aug 2020 23:31:07 +0100
715
8cloud-initramfs-tools (0.44ubuntu1) disco; urgency=medium16cloud-initramfs-tools (0.44ubuntu1) disco; urgency=medium
917
diff --git a/overlayroot/scripts/init-bottom/overlayroot b/overlayroot/scripts/init-bottom/overlayroot
index c13c02b..b926c85 100755
--- a/overlayroot/scripts/init-bottom/overlayroot
+++ b/overlayroot/scripts/init-bottom/overlayroot
@@ -804,10 +804,12 @@ cat <<EOF >${ROOTMNT}/etc/fstab
804EOF804EOF
805805
806[ $? -eq 0 ] || log_fail "failed to modify /etc/fstab (step 1)"806[ $? -eq 0 ] || log_fail "failed to modify /etc/fstab (step 1)"
807if [ -e "${ROOTMNT}${root_ro}/etc/fstab" ]; then
807overlayrootify_fstab "${ROOTMNT}${root_ro}/etc/fstab" "$root_ro" \808overlayrootify_fstab "${ROOTMNT}${root_ro}/etc/fstab" "$root_ro" \
808 "$root_rw" "$dir_prefix" "$recurse" "$swap" "${overlayroot_driver}" \809 "$root_rw" "$dir_prefix" "$recurse" "$swap" "${overlayroot_driver}" \
809 >> "${ROOTMNT}/etc/fstab" ||810 >> "${ROOTMNT}/etc/fstab" ||
810 log_fail "failed to modify /etc/fstab (step 2)"811 log_fail "failed to modify /etc/fstab (step 2)"
812fi
811813
812# we have to make the directories in ${root_rw} because if they do not814# we have to make the directories in ${root_rw} because if they do not
813# exist, then the 'upper=' argument to overlayfs will fail.815# exist, then the 'upper=' argument to overlayfs will fail.
@@ -820,6 +822,7 @@ if [ "${overlayroot_driver}" = "overlayfs" ]; then
820 log_fail "failed to fix upstart for overlayfs"822 log_fail "failed to fix upstart for overlayfs"
821fi823fi
822824
825if [ -e "${ROOTMNT}${root_ro}/etc/fstab" ]; then
823# if / is supposed to be mounted read-only (cmdline with 'ro')826# if / is supposed to be mounted read-only (cmdline with 'ro')
824# then mount our overlayfs as read-only just to be more normal827# then mount our overlayfs as read-only just to be more normal
825read cmdline < /proc/cmdline828read cmdline < /proc/cmdline
@@ -829,6 +832,7 @@ if [ "${cmdline#* ro }" != "$cmdline" ]; then
829 log_fail "failed to remount overlayroot read-only"832 log_fail "failed to remount overlayroot read-only"
830 debug "mounted $ROOTMNT read-only per kernel cmdline"833 debug "mounted $ROOTMNT read-only per kernel cmdline"
831fi834fi
835fi
832836
833msg="configured root with '$overlayroot' using ${overlayroot_driver} per"837msg="configured root with '$overlayroot' using ${overlayroot_driver} per"
834msg="$msg ${used_desc}"838msg="$msg ${used_desc}"

Subscribers

People subscribed via source and target branches