Skip to content

Conversation

cgwalters
Copy link
Collaborator

Add a systemd generator to fixup Anaconda's /etc/fstab

This is a giant and hacky workaround for
ostreedev/ostree#3193

The better real fix is probably in either systemd or anaconda
(more realistically both) but let's paper over things here for now.

Having code to run as a generator will likely be useful in the
future anyways.

Signed-off-by: Colin Walters [email protected]


/// A workaround for https://github.com/ostreedev/ostree/issues/3193
/// as generated by anaconda.
#[context("Updating /etc/fstab for anaconda+composefs")]
pub(crate) fn fixup_etc_fstab(root: &Dir) -> Result<()> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have to say one really nice thing about pervasively using cap-std is that it makes it trivial to unit-test things like this in a super reliable fashion without accidentally trying to edit the system global /etc/fstab when running cargo test and also without going all the way to running containers. (Which we should do too of course...)

@cgwalters
Copy link
Collaborator Author

@henrywang we aren't testing anaconda in our integration tests yet, right? Do you know offhand how hard that'd be to wire up?

A great thing to do here would be to change the tests to verify no systemd units fail across the board, which is the goal of this change.

@henrywang
Copy link
Collaborator

@henrywang we aren't testing anaconda in our integration tests yet, right? Do you know offhand how hard that'd be to wire up?

A great thing to do here would be to change the tests to verify no systemd units fail across the board, which is the goal of this change.

We have anaconda test in virt-s1/bootc-workflow-test with a daily run already. https://github.com/virt-s1/bootc-workflow-test/blob/main/anaconda.sh. Adding anaconda test into bootc repo is not hard.

@cgwalters
Copy link
Collaborator Author

Ah OK. Yeah I think it could be an opt-in? Well...here's what I'd propose...let's land this change and then it will show up very quickly in the -dev images, and the workflow test runs against our -dev images and we can verify it works there?

@henrywang
Copy link
Collaborator

Ah OK. Yeah I think it could be an opt-in? Well...here's what I'd propose...let's land this change and then it will show up very quickly in the -dev images, and the workflow test runs against our -dev images and we can verify it works there?

Exactly. -dev image will be tested every day, or it can be manually triggered as well.

@henrywang
Copy link
Collaborator

henrywang commented Mar 22, 2024

@cgwalters One more question. Does anaconda support bootc install already? Still has a discussion about bootc install support in rhinstaller/anaconda#5197. Thanks.

@cgwalters
Copy link
Collaborator Author

@cgwalters One more question. Does anaconda support bootc install already? Thanks.

No, that's rhinstaller/anaconda#5197

@cgwalters cgwalters force-pushed the generator-reconcile branch from 6a793c1 to 4e7e665 Compare March 22, 2024 16:30
This is a giant and hacky workaround for
ostreedev/ostree#3193

The better real fix is probably in either systemd or anaconda
(more realistically both) but let's paper over things here for now.

Having code to run as a generator will likely be useful in the
future anyways.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters cgwalters force-pushed the generator-reconcile branch from 4e7e665 to d46b072 Compare March 22, 2024 17:57
@jeckersb jeckersb self-requested a review March 25, 2024 14:02
Copy link
Collaborator

@jeckersb jeckersb left a comment

Choose a reason for hiding this comment

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

LGTM, love the tests!

@jeckersb jeckersb merged commit 2a35059 into bootc-dev:main Mar 25, 2024
@henrywang
Copy link
Collaborator

@cgwalters Anaconda test passed with bootc-202403251937.g0c250ea906-1.el9.x86_64 . Test log: virt-s1/bootc-workflow-report#205

@cgwalters
Copy link
Collaborator Author

Yep cool, I no longer see the systemd-remount-fs.service failures in that -dev run.

@runcom
Copy link
Contributor

runcom commented Oct 23, 2024

The better real fix is probably in either systemd or anaconda
(more realistically both)

@cgwalters are there tracker issues for this? just curious to follow and maybe help on this one to have a proper fix

@cgwalters
Copy link
Collaborator Author

Thanks for the ping, yes I didn't file a tracker for this because it's a bit unclear exactly where it could be fixed, but I'd love to have your help!

I think the best fix is:

  • Change anaconda not to write / into /etc/fstab for bootc/ostree systems by default

But there's more words in ostreedev/ostree#3193 (comment)

@runcom
Copy link
Contributor

runcom commented Oct 23, 2024

Change anaconda not to write / into /etc/fstab for bootc/ostree systems by default

I understand this is slightly different than just adding ‘ro’ like this patch does for the root filesystem 🤔 but I can poke around the anaconda code to achieve your fix and see what happens during some testing too.

@champtar
Copy link
Contributor

champtar commented Jan 10, 2025

I think the best fix is:
* Change anaconda not to write / into /etc/fstab for bootc/ostree systems by default

I agree, as setting ro if you are not using composefs will break the system, just commenting the '/' line works in both cases.
If you upgrade from a composefs enabled version (with bootc magic generator) to a composefs disabled version everything breaks.

@champtar
Copy link
Contributor

Doing some testing implementing a similar script and upgrading from really old versions,
to work this actually depends on having rw kargs as we are Before=systemd-remount-fs.service and ostree-remount.service is After=systemd-remount-fs.service, so without rw /etc is read-only and it fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants