cli: Add tag-aware upgrade operations#2094
cli: Add tag-aware upgrade operations#2094gursewak1997 wants to merge 1 commit intobootc-dev:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces new bootc upgrade CLI options: --tag to specify a target image tag for an upgrade, and --list-tags to display available tags for the currently booted image's repository. It includes new helper functions derive_image_with_tag and list_tags_for_current_image to support this functionality, along with corresponding unit tests and documentation updates. A review comment suggests refactoring duplicated logic within the upgrade function for retrieving the current image and handling cases where it's not defined, as well as making error messages more specific.
4e09e3a to
893c24a
Compare
crates/lib/src/cli.rs
Outdated
| let repo = format!("docker://{}", repo_name); | ||
|
|
||
| // Use skopeo to list tags | ||
| let output = tokio::process::Command::new("skopeo") |
There was a problem hiding this comment.
One problem is this won't use the ostree/bootc pull secret by default; we have a whole containers-image-proxy abstraction that we need to use in general for this.
Though the next problem here is that code doesn't expose an abstraction for this...but should.
There was a problem hiding this comment.
You marked this resolved but I don't think it is right? IOW if the image is behind a pull secret then this will fail. Look at the containers-image-proxy code that we use. We need to pass the ostree auth file to skopeo here.
There was a problem hiding this comment.
Bigger picture can we just drop this list-tags functionality for now and instead suggest to users to directly run skopeo inspect on their own?
I know it's less ergonomic and this whole thing is just about ergonomics, but hopefully the --tag is enough.
My worry is basically again:
- We must handle authfiles and such using the ostree/bootc one
- This feels like a kind of scope creep into general "inspect the remote container" reference but users can already just run
skopeowith their own options if that's what they want.
Maybe we could add skopeo inspect --booted for example? But that still doesn't solve the authfile discrepancy.
(I know the authfile thing is also quite annoying...see also containers/image#2600 )
There was a problem hiding this comment.
I see. Dropped the list-tags functionality for now
crates/lib/src/cli.rs
Outdated
| anyhow::bail!("Failed to list tags: {}", stderr); | ||
| } | ||
|
|
||
| let stdout = String::from_utf8_lossy(&output.stdout); |
There was a problem hiding this comment.
For things like this it would be a lot cleaner to use skopeo inspect which outputs JSON. I think it's almost always wrong to use from_utf8_lossy.
Perhaps this might be best done with upgrade --check? We could also cache the latest available tags there.
There was a problem hiding this comment.
I've switched to skopeo inspect with JSON parsing (parsing the RepoTags field) instead of from_utf8_lossy. Also integrated it with upgrade --check as an optional --show-tags flag.
Caching the tags during check is a good idea. I can add that if you think it's worth the added complexity.
4c5ca28 to
c7274c6
Compare
crates/lib/src/cli.rs
Outdated
| } | ||
|
|
||
| /// Derive a new image reference by replacing the tag of the current image | ||
| fn derive_image_with_tag(current: &ImageReference, new_tag: &str) -> Result<ImageReference> { |
There was a problem hiding this comment.
Hmm I think this should probably be a method on ImageReference instead of being all the way at the top in the CLI code.
That said I am pretty sure for containers-storage we can also create a Reference instead of reimplementing parsing.
The complicated one is oci because it's filesystem paths and : creates ambiguity; there's some complex code IIRC in containers/container-libs around this in Go which we may need to replicate. I think this came up elsewhere.
crates/lib/src/cli.rs
Outdated
| let repo = format!("docker://{}", repo_name); | ||
|
|
||
| // Use skopeo to list tags | ||
| let output = tokio::process::Command::new("skopeo") |
There was a problem hiding this comment.
You marked this resolved but I don't think it is right? IOW if the image is behind a pull secret then this will fail. Look at the containers-image-proxy code that we use. We need to pass the ostree auth file to skopeo here.
crates/lib/src/cli.rs
Outdated
| let repo = format!("docker://{}", repo_name); | ||
|
|
||
| // Use skopeo to list tags | ||
| let output = tokio::process::Command::new("skopeo") |
There was a problem hiding this comment.
Bigger picture can we just drop this list-tags functionality for now and instead suggest to users to directly run skopeo inspect on their own?
I know it's less ergonomic and this whole thing is just about ergonomics, but hopefully the --tag is enough.
My worry is basically again:
- We must handle authfiles and such using the ostree/bootc one
- This feels like a kind of scope creep into general "inspect the remote container" reference but users can already just run
skopeowith their own options if that's what they want.
Maybe we could add skopeo inspect --booted for example? But that still doesn't solve the authfile discrepancy.
(I know the authfile thing is also quite annoying...see also containers/image#2600 )
c7274c6 to
c4b371e
Compare
Implements bootc upgrade --tag <TAG> to upgrade to a different tag of the current image without typing the full registry path. Implementation: - Added ImageReference::with_tag() method in spec.rs - Uses OCI Reference API for registry and containers-storage transports - Falls back to string manipulation for oci: and other complex transports - Properly strips digests when replacing tags - Works with all transport types Testing: - Unit tests cover registry, containers-storage, digest handling, and edge cases - Integration test (test-upgrade-tag.nu) uses containers-storage to verify full tag-switching workflow across reboots without registry dependency Documentation updated with --tag examples. Assisted-by: Claude (Sonnet 4.5) Signed-off-by: gursewak1997 <gursmangat@gmail.com>
c4b371e to
a32853b
Compare
Implements
bootc upgrade --tag <TAG>to upgrade to a different tagof the current image without typing the full registry path.
Implementation:
ImageReference::with_tag()method in spec.rsTesting:
full tag-switching workflow across reboots without registry dependency