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

Run eos-firstboot a second time on dual-boot systems #408

Closed
wants to merge 1 commit into from

Conversation

wjt
Copy link
Member

@wjt wjt commented Jun 4, 2024

In new dual-boot installs of 6.0.0, eos-firstboot did not correctly grow the filesystem due to the bug fixed in commit
56aea35.

We now want to run it a second time, only on dual-boot systems. Ideally we would like to only trigger this on dual-boot systems that were affected by this bug but it's harder to detect that case than to run eos-firstboot again and have resize2fs gracefully do nothing.

When booting a non-live image-booted system which has not previously run this unit, remove the flag file (if present) that prevents eos-firstboot.service from running more than once, then touch another flag file to prevent running this unit again. The service is ordered Before=eos-firstboot.service and is Type=oneshot so we are guaranteed that its ExecStart= line has completed before eos-firstboot.service's ConditionPathExists check is evaluated. From systemd.unit(5):

The conditions and asserts are checked at the time the queued start
job is to be executed. The ordering dependencies are still respected,
so other units are still pulled in and ordered as if this unit was
successfully activated, and the conditions and asserts are executed
the precise moment the unit would normally start and thus can validate
system state after the units ordered before completed initialization.

On a fresh installation of an image containing this unit, /var/eos-booted does not exist; so this unit has no effect but is harmless.

On a system originally installed with 6.0.0, eos-firstboot.service will be retriggered once and will successfully resize the filesystem to fill the partition.

On a system originally installed with a release prior to 6.0.0, eos-firstboot.service will be retriggered once and will successfully take no action.

https://phabricator.endlessm.com/T35432

@wjt wjt requested a review from starnight June 4, 2024 10:17
Makefile.am Outdated Show resolved Hide resolved
In new dual-boot installs of 6.0.0, eos-firstboot did not correctly grow
the filesystem due to the bug fixed in commit
56aea35.

We now want to run it a second time, only on dual-boot systems. Ideally
we would like to only trigger this on dual-boot systems that were
affected by this bug but it's harder to detect that case than to run
eos-firstboot again and have resize2fs gracefully do nothing.

When booting a non-live image-booted system which has not previously run
this unit, remove the flag file (if present) that prevents
eos-firstboot.service from running more than once, then touch another
flag file to prevent running this unit again. The service is ordered
Before=eos-firstboot.service and is Type=oneshot so we are guaranteed
that its ExecStart= line has completed before eos-firstboot.service's
ConditionPathExists check is evaluated. From systemd.unit(5):

> The conditions and asserts are checked at the time the queued start
> job is to be executed. The ordering dependencies are still respected,
> so other units are still pulled in and ordered as if this unit was
> successfully activated, and the conditions and asserts are executed
> the precise moment the unit would normally start and thus can validate
> system state after the units ordered before completed initialization.

On a fresh installation of an image containing this unit,
/var/eos-booted does not exist; so this unit has no effect but is
harmless.

On a system originally installed with 6.0.0, eos-firstboot.service will be
retriggered once and will successfully resize the filesystem to fill the
partition.

On a system originally installed with a release prior to 6.0.0,
eos-firstboot.service will be retriggered once and will successfully
take no action.

https://phabricator.endlessm.com/T35432
@wjt wjt force-pushed the T35432-grow-rootfs-on-existing-dual-boot branch from 8b58bbb to b36a307 Compare June 4, 2024 10:28
@wjt wjt marked this pull request as ready for review June 4, 2024 10:29
Copy link
Member

@dbnicholson dbnicholson left a comment

Choose a reason for hiding this comment

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

If we're going to unconditionally re-run eos-firstboot.service, why not just change the stamp file it uses? I.e., s,/var/eos-booted,/var/lib/%N, in eos-firstboot and eos-firstboot.service? You could even have it delete the old /var/eos-booted, which is a bit of an eyesore.

DefaultDependencies=no
After=sysinit.target local-fs.target
Before=basic.target eos-firstboot.service
ConditionPathExists=!/var/%N
Copy link
Member

Choose a reason for hiding this comment

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

I think Rob has previously expressed regret about dropping these files directly in /var. I.e., maybe this should be /var/lib/%N.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we're going to unconditionally re-run eos-firstboot.service, why not just change the stamp file it uses?

Because I want to only re-run it on dual-boot systems.

But yes, that would be easier. It's harmless to rerun (except in terms of time wasted). It would also be an excuse to clean up the ugly top-level-of-/var stamp file. Nice idea.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed the implications of ConditionKernelCommandLine=endless.image.device.

Copy link
Member Author

Choose a reason for hiding this comment

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

I implemented that alternative at #410.

@wjt wjt marked this pull request as draft June 4, 2024 17:51
@wjt
Copy link
Member Author

wjt commented Jun 4, 2024

Converting this to draft, I'll rework it tomorrow as described above.

@wjt
Copy link
Member Author

wjt commented Jun 6, 2024

#410 was merged instead.

@wjt wjt closed this Jun 6, 2024
@wjt wjt deleted the T35432-grow-rootfs-on-existing-dual-boot branch June 6, 2024 15:25
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.

2 participants