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

composepost: Support rootfs.transient=yes #4719

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

cgwalters
Copy link
Member

This pairs with ostreedev/ostree#3114

Basically we want to detect the case where the OS has opted-in to this new mode and not symlink things.

I originally thought we could implement this by just moving all the toplevel directories, but then I hit on the fact that because the filesystem package is creating all the toplevel directories in lua script which we ignore...that doesn't work.

So we need to keep making them by hand.

@cgwalters
Copy link
Member Author

/retest

@cgwalters
Copy link
Member Author

Hmm thinking about this more we almost certainly still want the /home -> /var/home and /root -> /var/roothome links even in this mode by default. The transient rootfs is I think mainly about "solving" RPMs installing to /opt etc. Making user home directories ephemeral breaks SSH key injection etc.

@cgwalters
Copy link
Member Author

OK tested this more, also extended the unit tests.

@jlebon
Copy link
Member

jlebon commented Dec 8, 2023

I think there are two separate features here:

  1. support for adding content to the OSTree commit in e.g. /opt or /foo
  2. support for whether to make r/w transient some parts of the deployment

Those two things seem related but still mostly independent. /opt is special in that you want both, but e.g. you could want transient /usr even, or not want it on /foo. Would it make sense to make those bits more independently configurable?

@cgwalters
Copy link
Member Author

Transient /usr would need to be a new knob in ostree's prepare-root.conf right? But that would be basically orthogonal to this as far as I can see. How would it change the code here?

or not want it on /foo.

Can you give a specific example? Are you saying that we would want to somehow still support deny-listing specific toplevel directories from being added? Or for being...read-only?

Note today without ostree using overlayfs/composefs, any new toplevel directories today added to a commit default to writable - and effectively transient because they will go away on the next OS update.

@jlebon
Copy link
Member

jlebon commented Dec 11, 2023

For posterity, note I've taken the discussion here to ostreedev/ostree#3113.

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.

A messy detail here is that it creates a rift between the container flow and the client-side layering flow. The client-side layering code moves /opt to /usr/lib/opt, so having /opt be transient doesn't help there. I think if we go with the rebased upperdir approach in the future, we'll have to decide how to harmonize /opt handling. (I.e. either stop moving it to /usr/lib/opt client-side, or change the container path to move /opt to /usr/lib/opt on import.)

rust/src/ostree_prepareroot.rs Show resolved Hide resolved
rust/src/composepost.rs Show resolved Hide resolved
rust/src/composepost.rs Show resolved Hide resolved
rust/src/composepost.rs Show resolved Hide resolved
This pairs with ostreedev/ostree#3114

Basically we want to detect the case where the OS has opted-in
to this new mode and *not* symlink things.

I originally thought we could implement this by just moving
all the toplevel directories, but then I hit on the fact that
because the `filesystem` package is creating all the toplevel
directories in lua script which we ignore...that doesn't work.

So we need to keep making them by hand.
tests/compose/test-rootfs-transient.sh Dismissed Show dismissed Hide dismissed
tests/compose/test-rootfs-transient.sh Dismissed Show dismissed Hide dismissed
@cgwalters
Copy link
Member Author

  • Error out on tmp-is-dir: false
  • Add a compose test

@cgwalters cgwalters merged commit 0751f34 into coreos:main Dec 12, 2023
16 of 17 checks passed
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

2 participants