sysroot: Stabilize deployment finalization, add API #3090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cgwalters
force-pushed
the
stabilize-locking
branch
from
38cfb61
to
04de1dd
Compare
jlebon
reviewed
Nov 20, 2023
src/libostree/ostree-deployment.c
Outdated
| * @self: Deployment | ||
| * | ||
| * Returns: `TRUE` if deployment is queued to be "finalized" at shutdown time, but requires | ||
| * additional action. Since: 2023.7 |
There was a problem hiding this comment.
Suggested change
| * additional action. Since: 2023.7 | |
| * additional action. Since: 2023.8 |
There was a problem hiding this comment.
This is usually on its own line; not sure if it affects gobject introspection if it's not.
cgwalters
force-pushed
the
stabilize-locking
branch
from
04de1dd
to
8d58901
Compare
jlebon
reviewed
Nov 22, 2023
cgwalters
force-pushed
the
stabilize-locking
branch
from
8d58901
to
36477ea
Compare
Hmm yes possibly, though I'm not reproducing this here. One thing this really brings to the fore is the huge messiness around gpg verification vs direct deployments and how |
Right now `ostree admin status` errors out in this case, but `rpm-ostree status` doesn't. The former behavior is probably more of a bug, work around it for now.
It's helpful to see which deployment has an error.
It's about time we do this; deployment finalization locking is a useful feature. An absolutely key thing here is that we've slowly been moving towards the deployments as the primary "source of truth". Specifically in bootc for example, we will GC container images not referenced by a deployment. This is then neecessary to support a "pull but don't apply automatically" model. This stabilizes the existing `ostree admin deploy --lock-finalization` CLI, and adds a new `ostree admin unlock-finalization`. We still check the old lock file path, but there's a new boolean value as part of the staged deployment data which is intended to be the source of truth in the future. At some point then we can drop the rpm-ostree lockfile handling. Closes: ostreedev#3025
cgwalters
force-pushed
the
stabilize-locking
branch
from
36477ea
to
28cc761
Compare
jlebon
approved these changes
Nov 27, 2023
|
|
||
| <listitem><para> | ||
| The deployment will not be "finalized" by default on shutdown; to later | ||
| queue it, use <literal>ostree admin unlock-finalization</literal>. |
There was a problem hiding this comment.
Suggested change
| queue it, use <literal>ostree admin unlock-finalization</literal>. | |
| queue it, use <literal>ostree admin lock-finalization --unlock</literal>. |
There was a problem hiding this comment.
Good catch, thanks for the careful review!
| will be set into a "finalization locked" state, which means it will not be queued for the next boot by default. | ||
| </para> | ||
| <para> | ||
| This is the same as the <literal>--lock-finalization</literal> argument for <literal>ostree admin deploy</literal>. |
There was a problem hiding this comment.
Feels like we should be more explicit about the race condition of not doing this. Maybe e.g.
Suggested change
| This is the same as the <literal>--lock-finalization</literal> argument for <literal>ostree admin deploy</literal>. | |
| This is the same as the <literal>--lock-finalization</literal> argument for <literal>ostree admin deploy</literal>, which is the recommended way to use this feature in a race-free way. |
?
|
Followup in #3100 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's about time we do this; deployment finalization locking is a useful feature. An absolutely key thing here is that we've slowly been moving towards the deployments as the primary "source of truth".
Specifically in bootc for example, we will GC container images not referenced by a deployment.
This is then neecessary to support a "pull but don't apply automatically" model.
Closes: #3025