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

tests: fix fsck on boot on arm devices #9655

Merged
45 changes: 34 additions & 11 deletions tests/core/fsck-on-boot/task.yaml
Original file line number Diff line number Diff line change
@@ -1,24 +1,37 @@
summary: the boot base provides essential fsck programs

details: |
Snapd uses vfat on certain essential boot partitions, due to external
requirements imposed by the bootloader architecture. This test verifies that
the boot process is capable of detecting unclean vfat and fixing it before
such file system is mounted. This is an essential property to ensure
longevity of devices that rely on write to vfat to operate.

prepare: |
tests.cleanup prepare

execute: |
unmount_vfat() {
if os.query is-core16; then
# Refer to the core 16 PC gadget for details:
# Refer to the core 16 gadgets for details:
# https://github.com/snapcore/pc-amd64-gadget/blob/16/gadget.yaml
umount /boot/efi
umount /boot/grub
# https://github.com/snapcore/pi2-gadget/blob/master/gadget.yaml
if [[ "$SPREAD_SYSTEM" == ubuntu-core-16-arm-* ]]; then
umount /boot/uboot
else
umount /boot/efi
umount /boot/grub
fi
elif os.query is-core18; then
# Refer to the core 18 PC gadget for details:
# Refer to the core 18 gadgets for details:
# https://github.com/snapcore/pc-amd64-gadget/blob/18/gadget.yaml
umount /boot/efi
umount /boot/grub
# https://github.com/snapcore/pi2-gadget/blob/18/gadget.yaml
if [[ "$SPREAD_SYSTEM" == ubuntu-core-18-arm-* ]]; then
umount /boot/uboot
else
umount /boot/efi
umount /boot/grub
fi
elif os.query is-core20; then
# TODO:UC20 The property of having to keep a mounted vfat at all time
# is not the most fortunate. Any power loss will result in a dirty
mvo5 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -31,7 +44,11 @@ execute: |
# unset RecoverySystem in the modeenv, and it's not necessary for
# seeding anymore
if mountpoint /run/mnt/snapd >/dev/null; then
umount /run/mnt/ubuntu-seed/systems/*/snaps/snapd_*.snap
if [[ "$SPREAD_SYSTEM" == ubuntu-core-20-arm-* ]]; then
umount /run/mnt/ubuntu-seed/snaps/snapd_*.snap
else
umount /run/mnt/ubuntu-seed/systems/*/snaps/snapd_*.snap
fi
fi
umount /var/lib/snapd/seed
umount /run/mnt/ubuntu-seed
Expand All @@ -54,7 +71,11 @@ execute: |
# Use offset 65 as FAT32 kicks in for devices larger than 32MB
printf "\x01" > one
tests.cleanup defer rm -f one
dd if=one of=/dev/sda2 seek=65 bs=1 count=1 conv=notrunc
if os.query is-core16 || os.query is-core18; then
dd if=/dev/disk/by-label/system-boot of=dirty skip=65 bs=1 count=1 conv=notrunc
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nitpick, but instead of duplicating the whole dd command, you could instead just have the if set an environment variable for the label like this:

if os.query is-core16 || os.query is-core18; then
  LABEL=system-boot
elif os.query is-core20; then
  LABEL=ubuntu-seed
else
  echo "unknown core system, please update test"
  exit 1
fi
dd if="/dev/disk/by-label/$LABEL" of=dirty skip=65 bs=1 count=1 conv=notrunc

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can skip the else branch given that the check for uc22 et al is also done above too

Copy link
Contributor

Choose a reason for hiding this comment

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

/me notes that this also would be a good use for canonical/spread#83 if that ever becomes a thing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

totally agree, should be great to be able to define vars by system

elif os.query is-core20; then
dd if=/dev/disk/by-label/ubuntu-seed of=dirty skip=65 bs=1 count=1 conv=notrunc
fi
tests.cleanup pop

# Reboot to give the early boot process a chance to fix the corruption.
Expand All @@ -76,12 +97,14 @@ execute: |
# chain.
unmount_vfat
cat /proc/self/mountinfo >boot1-after-umount.log
dd if=/dev/sda2 of=dirty skip=65 bs=1 count=1 conv=notrunc
if os.query is-core16 || os.query is-core18; then
dd if=/dev/disk/by-label/system-boot of=dirty skip=65 bs=1 count=1 conv=notrunc
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment about LABEL

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

elif os.query is-core20; then
dd if=/dev/disk/by-label/ubuntu-seed of=dirty skip=65 bs=1 count=1 conv=notrunc
fi
test "$(od -t x1 -A n dirty)" = " 00" # NOTE: the leading space is relevant

# Reboot to restore mount points.
REBOOT
;;
esac
restore: |
tests.cleanup restore
mvo5 marked this conversation as resolved.
Show resolved Hide resolved