ostree-sysroot: make simple_write_deployment smarter #1110
Closed
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
Also convert ot-admin-builtin-deploy.c. Prep for more work there.
This is a follow-up to ostreedev#1097. We make simple_write_deployment smart enough so that it can be used for rpm-ostree's purposes. This is mostly an upstreaming of logic that already existed there. Notably we correctly append NOT_DEFAULT deployments *after* the booted deployment and we now support RETAIN_PENDING and RETAIN_ROLLBACK flags to have more granularity on deployment pruning. Expose these new flags on the CLI using new options (as well as expose the previously existing NOT_DEFAULT flag as --not-as-default). I couldn't add tests for --retain-pending because the merge deployment is always the topmost one. Though I did check that it worked in a VM.
|
I only glanced at this quickly so far (though first patch looks obviously correct); but am I correct in thinking that we can't quite use this in rpm-ostree yet since we also have the livefs logic? EDIT: I see the rpm-ostree patch. |
|
Usage in coreos/rpm-ostree#942 |
cgwalters
reviewed
Aug 24, 2017
src/libostree/ostree-sysroot.c
Outdated
| * to put the new deployment right after the current default in the | ||
| * order. | ||
| */ | ||
| if (!added_new) |
There was a problem hiding this comment.
Hm, don't we need this to handle the initial-deployment case?
|
I'd wondered why it didn't break the tests, it turns out this API isn't called for initial deployments right now. Which is OK, but I'd still say worth supporting. |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 25, 2017
This is a follow-up to #1097. We make simple_write_deployment smart enough so that it can be used for rpm-ostree's purposes. This is mostly an upstreaming of logic that already existed there. Notably we correctly append NOT_DEFAULT deployments *after* the booted deployment and we now support RETAIN_PENDING and RETAIN_ROLLBACK flags to have more granularity on deployment pruning. Expose these new flags on the CLI using new options (as well as expose the previously existing NOT_DEFAULT flag as --not-as-default). I couldn't add tests for --retain-pending because the merge deployment is always the topmost one. Though I did check that it worked in a VM. Closes: #1110 Approved by: cgwalters
|
☀️ Test successful - status-atomicjenkins |
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.
This is a follow-up to #1097.
We make
simple_write_deploymentsmart enough so that it can be used forrpm-ostree's purposes. This is mostly an upstreaming of logic that
already existed there.
Notably we correctly append
NOT_DEFAULTdeployments after the booteddeployment and we now support
RETAIN_PENDINGandRETAIN_ROLLBACKflagsto have more granularity on deployment pruning.
Expose these new flags on the CLI using new options (as well as expose
the previously existing
NOT_DEFAULTflag as--not-as-default).I couldn't add tests for
--retain-pendingbecause the merge deploymentis always the topmost one. Though I did check that it worked in a VM.