Add fsck to dracut modules #1352

mudler · 2022-05-20T14:51:58Z

Also sets up fsck.repair=yes and fsck.mode=force by default.
Derivatives can opt-out by providing their own bootargs.cfg

Fixes: #1323

Signed-off-by: Ettore Di Giacinto edigiacinto@suse.com

mudler · 2022-05-20T14:57:42Z

packages/immutable-rootfs/30cos-immutable-rootfs/module-setup.sh

@@ -29,7 +29,7 @@ install() {
    # Include utilities required for cos-setup services,
    # probably a devoted cos-setup dracut module makes sense
    inst_multiple -o \
-        partprobe sync udevadm lsblk sgdisk parted mkfs.ext2 mkfs.ext3 mkfs.ext4 mkfs.vfat mkfs.fat mkfs.xfs blkid e2fsck resize2fs mount xfs_growfs umount
+        /usr/lib/systemd/systemd-fsck partprobe sync udevadm lsblk sgdisk parted mkfs.ext2 mkfs.ext3 mkfs.ext4 mkfs.vfat mkfs.fat mkfs.xfs blkid e2fsck resize2fs mount xfs_growfs umount


this should be already included - BUT - I feel more safe being explicit here. I'm not sure if there is any reason to NOT do it.

I don't think there is any reason not to do it. However this is part of the 00systemd module here https://github.com/dracutdevs/dracut/blob/1befc6416743a527824f0f2cc25471e86a6f8a79/modules.d/00systemd/module-setup.sh#L33-L41 so from a dracut perspective I guess the proper way of enforcing it would be to require systemd module in depends function. I doubt we explicitly require systemd module anywhere, assuming it is already there, which could not be the case.

So, I'd suggest adding systemd in depends, then keep this systemd-fsckinclusion here is good to explicitly tell we make use of systemd-fsck. I am wondering if shouldn't we use the ${systemdutildir} variable instead of the absolute path, just incase there are divergences on distros or the installation path of systemd evolves.

Fixed, thanks for the pointers!

mudler · 2022-05-21T10:25:38Z

mmm seems the tpm.mount in tumbleweed is in /usr/lib/systemd/system/tmp.mount as opposed to leap where is in /usr/share/systemd/tmp.mount

mudler · 2022-05-23T10:43:09Z

packages/immutable-rootfs/30cos-immutable-rootfs/cos-loop-img.sh

            exit 0
        else
            umount "${cos_state}"
        fi
    done
 }

+function dofsCheck {
+    # Iterate over current partitions


@davidcassany here while testing I corrupted COS_PERSISTENT in a way that no labels were properly detected anymore, but by running efsck on the disk id manually I could restore them just fine. When no labels are identified, it just skips all our loops, as we loop over labels

Since this could happen, I'm back at scanning all the partitions found. I'm not sure if this is maybe too invasive on some front and maybe better to make it opt-in - but on the other hand I don't see why someone would disable a fscheck integrity utility running on boot. I guess, in that case fsck.mode=skip could be passed, so I didn't felt to add any option to instrument it even more.

mudler · 2022-05-24T12:55:59Z

who did forgot to bump initrd? me!

mudler · 2022-05-24T14:00:11Z

ok, tested locally and seems to work just fine, I've enabled it now by default on the default bootargs.cfg that we ship. Will try now to add a test for it 🤞

mudler · 2022-05-24T14:21:26Z

Just waiting for tests to pass, but should be good for review as I've checked locally and works

mudler · 2022-05-25T07:44:39Z

oh my, tests went green

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Derivatives can opt-out from it by specifying a custom bootargs.cfg Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Itxaka · 2022-05-25T08:00:55Z

packages/immutable-rootfs/build.yaml

+- | 
+    /bin/bash -c " \
+    if [ -e /usr/share/systemd/tmp.mount ]; then \
+      cp /usr/share/systemd/tmp.mount /etc/systemd/system; \


is this something to do with the underlying distro change?

could be, note that TW alreay has this mountpoint by default, probably SLE for rancher too. The old logic was valid when opensuse meant leap, if we include TW, this logic is safer.

yup exactly, this happens to be in two different places - tw and leap have it differently

davidcassany · 2022-05-25T08:23:19Z

mmm seems the tpm.mount in tumbleweed is in /usr/lib/systemd/system/tmp.mount as opposed to leap where is in /usr/share/systemd/tmp.mount

Yes this is just because Leap has not done the switch to tmpfs mount in /tmp by default yet. This is something we should check for teal now that I am thinking about it, whast is SLE for Rancher using for /tmp by default?

davidcassany

Looks good to me, nice 👍

Just a couple of nits regarding the module dependencies and systemd-fsck absolute path usage.

davidcassany · 2022-05-25T08:26:18Z

packages/base-dracut-modules/50-cos-initrd.conf

@@ -2,4 +2,4 @@ hostonly_cmdline="no"
 hostonly="no"
 compress="xz"
 omit_dracutmodules+=" multipath "
-add_dracutmodules+=" livenet dmsquash-live "
+add_dracutmodules+=" fs-lib livenet dmsquash-live "


mmm I am not so sure about it. If this is a dependency of systemd-fsck I doubt it should be here. Instead better add it in depends() in packages/immutable-rootfs/30cos-immutable-rootfs/module-setup.sh, so each installation of the immutable-rootfs module will also pull and include this dracut module.

davidcassany · 2022-05-25T08:30:00Z

packages/immutable-rootfs/30cos-immutable-rootfs/cos-loop-img.sh

+
+            # FSCHECK if cos_root_perm == "ro" on both
+            if [ "$cos_root_perm" == "ro" ]; then
+               /usr/lib/systemd/systemd-fsck "/dev/disk/by-label/${label}"


This absolute path frightens me... I am not sure if we have the ${systemdutilsdir} variable here, I doubt it though. Probably we could install a symlink under /sbin in module_setup.sh and just use systemd-fsck here?

I know, it frightens me as well, but the fact that systemd-fsck is not having a binary in /sbin etc hold me off from doing any special setup. I just wonder why is not installed as a normal binary to begin with ?!

Seems it is? https://github.com/dracutdevs/dracut/blob/d502d2a816ba8f8329b3d8616bd2a7e82a0ad21f/modules.d/00systemd/module-setup.sh#L39

mmh but might not be just available when calling it inside scripts. I just wonder how dracut handles it as its used already?

should be fixed now!

davidcassany · 2022-05-25T08:31:00Z

packages/immutable-rootfs/30cos-immutable-rootfs/module-setup.sh

 depends() {
-    echo rootfs-block dm
+    echo rootfs-block dm fs-lib


If fs-lib is here, then it should not be needed in dracut.conf file.

ouch good catch, I think it slipped in while I was testing

davidcassany · 2022-05-25T08:33:26Z

packages/immutable-rootfs/build.yaml

+- | 
+    /bin/bash -c " \
+    if [ -e /usr/share/systemd/tmp.mount ]; then \
+      cp /usr/share/systemd/tmp.mount /etc/systemd/system; \


could be, note that TW alreay has this mountpoint by default, probably SLE for rancher too. The old logic was valid when opensuse meant leap, if we include TW, this logic is safer.

davidcassany · 2022-05-25T08:35:41Z

packages/grub2/config/bootargs.cfg.tmpl

@@ -2,7 +2,7 @@ set kernel=/boot/vmlinuz
 if [ -n "$recoverylabel" ]; then
    set kernelcmd="console=tty1 console=ttyS0 root=live:LABEL=$recoverylabel rd.live.dir=/ rd.live.squashimg=$img panic=5 rd.neednet=1 rd.cos.oemlabel=@OEM_LABEL@"
 else
-    set kernelcmd="console=tty1 console=ttyS0 root=LABEL=$label cos-img/filename=$img panic=5 security=selinux selinux=1 rd.neednet=1 rd.cos.oemlabel=@OEM_LABEL@"
+    set kernelcmd="console=tty1 console=ttyS0 root=LABEL=$label cos-img/filename=$img panic=5 security=selinux selinux=1 rd.neednet=1 rd.cos.oemlabel=@OEM_LABEL@ fsck.mode=force fsck.repair=yes"


I don't see any fsck.repair definition, hence I guess this is something that systemd-fsck parses, right? If this something custom from our side I'd suggest defining it with the rd.cos... prefix, I think it is not the case though, just to double check.

yup, those are read by systemd-fsck directly, so we don't need to do any special handling ourselves 🥳

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

mjura

LGTM

davidcassany

Nice!

mudler commented May 20, 2022

View reviewed changes

mudler force-pushed the fsck branch from 16c45c2 to 278195f Compare May 23, 2022 10:30

mudler commented May 23, 2022

View reviewed changes

mudler force-pushed the fsck branch 2 times, most recently from ee3d14c to 1399503 Compare May 24, 2022 12:41

mudler force-pushed the fsck branch from 1399503 to 0039128 Compare May 24, 2022 13:59

mudler marked this pull request as ready for review May 24, 2022 14:21

mudler requested review from davidcassany, Itxaka and mjura May 24, 2022 14:21

mudler added 6 commits May 25, 2022 09:52

Add fsck to dracut modules

949af5d

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Call fsck on each part found

c0dbdc3

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Handle tmp mount copy from different locations

9284c9d

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Enable fsck by default

5697990

Derivatives can opt-out from it by specifying a custom bootargs.cfg Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Add test for fs integrity checks

f0632ba

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Bump grub config package

877c82d

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

mudler force-pushed the fsck branch from e472d18 to 877c82d Compare May 25, 2022 07:52

Fine tune tests

7d838a4

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Itxaka reviewed May 25, 2022

View reviewed changes

Itxaka approved these changes May 25, 2022

View reviewed changes

davidcassany approved these changes May 25, 2022

View reviewed changes

mudler added 3 commits May 25, 2022 11:46

Incorporate feedback from review

8b0e5e3

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Properly install systemd-fsck

d7e8df3

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

Incorporate feedback from review

aaa808a

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>

mudler enabled auto-merge (squash) May 25, 2022 10:10

mjura approved these changes May 25, 2022

View reviewed changes

davidcassany approved these changes May 25, 2022

View reviewed changes

mudler merged commit 363b822 into master May 25, 2022

mudler deleted the fsck branch May 25, 2022 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fsck to dracut modules #1352

Add fsck to dracut modules #1352

mudler commented May 20, 2022 •

edited

Loading

mudler May 20, 2022

davidcassany May 25, 2022

mudler May 25, 2022

mudler commented May 21, 2022

mudler May 23, 2022 •

edited

Loading

mudler commented May 24, 2022

mudler commented May 24, 2022

mudler commented May 24, 2022

mudler commented May 25, 2022 •

edited

Loading

Itxaka May 25, 2022 •

edited

Loading

davidcassany May 25, 2022

mudler May 25, 2022

davidcassany commented May 25, 2022

davidcassany left a comment

davidcassany May 25, 2022

davidcassany May 25, 2022

mudler May 25, 2022

mudler May 25, 2022

mudler May 25, 2022 •

edited

Loading

mudler May 25, 2022

davidcassany May 25, 2022

mudler May 25, 2022

davidcassany May 25, 2022

davidcassany May 25, 2022

mudler May 25, 2022

mjura left a comment

davidcassany left a comment

Add fsck to dracut modules #1352

Add fsck to dracut modules #1352

Conversation

mudler commented May 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mudler commented May 21, 2022

mudler May 23, 2022 • edited Loading

Choose a reason for hiding this comment

mudler commented May 24, 2022

mudler commented May 24, 2022

mudler commented May 24, 2022

mudler commented May 25, 2022 • edited Loading

Itxaka May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidcassany commented May 25, 2022

davidcassany left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mudler May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjura left a comment

Choose a reason for hiding this comment

davidcassany left a comment

Choose a reason for hiding this comment

mudler commented May 20, 2022 •

edited

Loading

mudler May 23, 2022 •

edited

Loading

mudler commented May 25, 2022 •

edited

Loading

Itxaka May 25, 2022 •

edited

Loading

mudler May 25, 2022 •

edited

Loading