Rollback Fixes#2213
Open
Johan-Liebert1 wants to merge 5 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements dropping staged deployments on rollback to match ostree's behavior, updates boot entry sorting logic to sort based on the bootloader type (by sort-key for systemd-boot and by filename for GRUB), and adds corresponding unit and integration tests. The reviewer identified a robustness issue in rollback.rs where stale staged entries might not be cleaned up if host.status.staged is None and the rollback could fail unnecessarily if the staged deployment file is already missing. Additionally, a maintainability issue was found in status.rs where the comments in the GRUB sorting test contradict the actual assertions and sorting behavior.
Johan-Liebert1
force-pushed
the
rollback-fix
branch
from
a266a4c to
e07ffbd
Compare
Collaborator
Author
|
GHA seems to be having issues |
cgwalters
reviewed
May 26, 2026
| // Ostree will drop any staged deployment on rollback | ||
| // We follow the same approach for now | ||
| if let Some(..) = &host.status.staged { | ||
| println!("Removing currently staged deployment"); |
Collaborator
There was a problem hiding this comment.
I'd like to try to centralize reporting more in the future and get away from just random println!. This isn't the only case of course.
One thing we could probably do instead here is log to the journal, which would have other benefits.
Johan-Liebert1
force-pushed
the
rollback-fix
branch
from
e07ffbd to
3f8eaf8
Compare
Similar to ostree, if we find any staged deployment while performing a rollback, we'll get rid of the staged deployment. The staged deployment still exists on disk and will be GC'd later Fixes: bootc-dev#2208 Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Add a case in the rollback test which tests rollback when there is a staged deployment present. This is a test for bootc-dev#2208 Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Johan-Liebert1
force-pushed
the
rollback-fix
branch
from
3f8eaf8 to
9ac67f1
Compare
Update `get_sorted_type1_boot_entries_helper` to implement sorting
logic based on bootloader type
- systemd-boot: Sort by sort-key (using BLSConfig::cmp which handles
sort-key ascending, then version descending)
- GRUB: Sort by filename in descending order (ignoring sort-key fields)
Unit Tests generated by ClaudeCode (Opus)
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Instead of blindly selecting the "second" one in a list of sorted boot entries as the rollback and failing if there are more than one rollback candidate, sort the rollback candidates in the same order as the boot entries and take the first one as rollback. All the remaining deployments become `other_deployments`. This is especially useful if and when we implement pinned deployments for composefs Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Johan-Liebert1
force-pushed
the
rollback-fix
branch
2 times, most recently
from
2408645 to
9e127ff
Compare
cgwalters
previously approved these changes
Jun 1, 2026
Johan-Liebert1
force-pushed
the
rollback-fix
branch
from
9e127ff to
830e90a
Compare
If we are unable to find the origin file for a deployment, we were immediately exiting with an error which rendered any bootc operation useless. Instead, just log a warning in the journal and continue with the specified operation so that we don't completely stop working if the system is in an unpredictable state Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Johan-Liebert1
force-pushed
the
rollback-fix
branch
from
830e90a to
d08c253
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
cfs/rollback: Remove staged entry on rollback
Similar to ostree, if we find any staged deployment while performing
a rollback, we'll get rid of the staged deployment. The staged
deployment still exists on disk and will be GC'd later
Fixes: #2208
cfs/status: Implement bootloader-specific sorting
Update
get_sorted_type1_boot_entries_helperto implement sortinglogic based on bootloader type
sort-key ascending, then version descending)
Unit Tests generated by ClaudeCode (Opus)