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

feat: allow rescue/emergency boot with grub cmdline args #488

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

bsherman
Copy link
Contributor

@bsherman bsherman commented Feb 4, 2024

This uses a systemd-generator to dyamically write a drop-in config for the rescue and emergency services only when they are requested via the kernel cmdline, which requires console/grub access. This allows use of these modes with the default Fedora state of a password locked root user, but does not auto-allow root access in the case of a failed fsck-check, which can also drop into the emergency shell.

Relates: #470

This uses a systemd-generator to dyamically write a drop-in config for
the rescue and emergency services only when they are requested via the
kernel cmdline, which requires console/grub access. This allows use of
these modes with the default Fedora state of a password locked root
user, but does not auto-allow root access in the case of a failed
fsck-check, which can also drop into the emergency shell.

Relates: #470
@bsherman
Copy link
Contributor Author

bsherman commented Feb 4, 2024

@ublue-os/approver please do NOT merge this until we've collected feedback. :-) Thank you.

@bsherman
Copy link
Contributor Author

bsherman commented Feb 4, 2024

Some notes for reference:

@cgwalters
Copy link

Looks sane to me. Arguably we could try to put this in systemd upstream too.

@bsherman
Copy link
Contributor Author

bsherman commented Feb 5, 2024

Looks sane to me. Arguably we could try to put this in systemd upstream too.

I'm not sure how easy it is to get merged into different projects. At the least, I figured we'd include it here and I'll PR to CoreOS.

Then we'd have running use cases before pushing further upstream.

@bsherman bsherman requested a review from a team February 5, 2024 18:57
Copy link
Member

@KyleGospo KyleGospo left a comment

Choose a reason for hiding this comment

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

Tested well on my end!

@bsherman bsherman requested a review from a team February 5, 2024 19:03
Copy link

@TriMoon TriMoon left a comment

Choose a reason for hiding this comment

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

Not to be nit-picking but....
Title should be "kernel" instead of "grub", as it is not specific to Grub...

Plus it will boot, i guess, in a root shell?
(So mention that also)

Copy link
Member

@EyeCantCU EyeCantCU left a comment

Choose a reason for hiding this comment

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

Works over here as well :)

@bsherman bsherman added this pull request to the merge queue Feb 6, 2024
Merged via the queue into main with commit c2ad2bf Feb 6, 2024
24 checks passed
@bsherman bsherman deleted the enable-rescue-emergency-mode-with-locked-root branch February 6, 2024 03:16
@bsherman
Copy link
Contributor Author

bsherman commented Feb 6, 2024

Arguably we could try to put this in systemd upstream too.

@cgwalters , I had planned to PR this into CoreOS as a replacement for the current static drop-ins. Would prefer I first attempt going upstream to systemd?

@cgwalters
Copy link

Landing in FCOS first sounds totally sane to me.

bsherman added a commit that referenced this pull request Feb 23, 2024
I'm confused that the original PR ( #488 ) failed to COPY this file
via the Containerfile, thus for the last few weeks this has not worked
as intended.

Given several people tested it, I'm not sure how we missed that, but
this resolves it regardless.
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.

None yet

5 participants