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

overlay.d: add service to check for unique-boot after ignition #1540

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

nikita-dubrovskii
Copy link
Contributor

@nikita-dubrovskii nikita-dubrovskii commented Feb 23, 2022

After ignition's stages partition table could be modified, and therefore
kernel may have an old picture of disks and partitions. During verification
we force kernel to reread the partition table for each disk and call udevsettle,
which in turn leads to modification of "/dev/disk/by-/" symlinks and systemd
may send SIGTERM to services with "Require=dev-disk-by\x2dlabel-boot.device".
To avoid that this checks for the unique fs labeled 'boot' after ignition's
disks stage was finished but before mounting stage.

Issue: coreos/fedora-coreos-tracker#1105

@nikita-dubrovskii nikita-dubrovskii force-pushed the issue-1105 branch 2 times, most recently from 16684d5 to 62c3369 Compare March 8, 2022 14:06
@nikita-dubrovskii nikita-dubrovskii changed the title WIP: overlay.d: add service to check for unique-boot after ignition overlay.d: add service to check for unique-boot after ignition Mar 8, 2022
cgwalters
cgwalters previously approved these changes Mar 8, 2022
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I didn't do an in-depth analysis here, but this seems sane to me.

jlebon
jlebon previously approved these changes Mar 17, 2022
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

@jlebon
Copy link
Member

jlebon commented Mar 17, 2022

coreos.boot-mirror is failing with

Starting CoreOS Ensure Unique Boot Filesystem...
Error: System has 0 devices with a filesystem labeled 'boot': []
coreos-ignition-unique-boot.service: Main process exited, code=exited, status=1/FAILURE

which is interesting. This runs right after ignition-disks which does do udevadm settle. And --rereadpt also does a udevadm settle, though based on this it's possible we didn't actually wait for anything at all.

Hmm, but actually I think we could have verify-unique-fs-label just keep waiting until the label shows up after re-reading all partition tables. This would be the equivalent of Requires=dev-disk-by\x2dlabel-boot.device and After=dev-disk-by\x2dlabel-boot.device, except we can't do that directly otherwise we'll hit the exact issue this PR intended to solve.

@nikita-dubrovskii nikita-dubrovskii force-pushed the issue-1105 branch 2 times, most recently from f145d16 to dae4c54 Compare March 25, 2022 17:11
@nikita-dubrovskii
Copy link
Contributor Author

using this with coreos/coreos-installer#813 fixes CI (locally)

@jlebon
Copy link
Member

jlebon commented Apr 25, 2022

This is pending a new coreos-installer release.

After ignition's stages partition table could be modified, and therefore
kernel may have an old picture of disks and partitions. During verification
we force kernel to reread the partition table for each disk and call udevsettle,
which in turn leads to modification of "/dev/disk/by-*/*" symlinks and systemd
may send SIGTERM to services with "Require=dev-disk-by\x2dlabel-boot.device".
To avoid that this checks for the unique fs labeled 'boot' after ignition's
disks stage was finished but before mounting stage.

Issue: coreos/fedora-coreos-tracker#1105

Signed-off-by: Nikita Dubrovskii <[email protected]>
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

We have new enough coreos-installer now, so this is unblocked.
LGTM!

@jlebon
Copy link
Member

jlebon commented Jun 28, 2022

I restarted CI here to make sure it tests on top of the latest. Let's rerun it multiple times before merging this to try to make sure we're not introducing flakiness again. Edit: I've now enabled auto-merge after restarting the test three times.

Ideally, we should add tests for both coreos-unique-boot.service and this new service. I think we already have some tests in kola which expect failure in the initrd. OK, filed coreos/coreos-assembler#2953 for this.

@jlebon jlebon enabled auto-merge (rebase) June 28, 2022 16:47
@jlebon jlebon merged commit b4c81c4 into coreos:testing-devel Jun 28, 2022
@nikita-dubrovskii nikita-dubrovskii deleted the issue-1105 branch June 29, 2022 04:34
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