-
Notifications
You must be signed in to change notification settings - Fork 119
spec, status: Add architecture #1239
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
Conversation
Now obviously something to bikeshed further is we use the OCI arch names which come from Docker where they just punted on and said "use whatever Go uses" which...yeah is not the same as |
Ouch.. yeah. The split isn't ideal but I don't have a better suggestion than what you're going with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in a local build
[core@cosa-devsh ~]$ rpm-ostree status
State: idle
Deployments:
● ostree-remote-image:fedora:docker://quay.io/fedora/fedora-coreos:rawhide
Digest: sha256:fce2f34d80a04c67b01d6c8633e91a18a39c985e6402ebc1d5538b5c2496b674
Version: 43.20250328.dev.1 (2025-03-28T19:55:54Z)
[core@cosa-devsh ~]$
[core@cosa-devsh ~]$ sudo /usr/local/bin/bootc status
● Booted image: quay.io/fedora/fedora-coreos:rawhide
Digest: sha256:fce2f34d80a04c67b01d6c8633e91a18a39c985e6402ebc1d5538b5c2496b674 (amd64)
Version: 43.20250328.dev.1 (2025-03-28T19:55:54Z)
The argument here is twofold: - We used to show it with ostree refs because ostree doesn't have manifest lists, and people may have gotten used to seeing it in e.g. bug reports - Highlight that the digest of the image is always the digest of the per-arch image; we currently peel and discard manifest lists, which may not be obvious. Closes: bootc-dev#1238 Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the idea!
A small bikeshed: I would prefer a dedicated field in the human-readable output.
Image, Digest, Architecture and Version all seem like a good candidates.
Hmm - why? Just visual consistency? The rationale behind the current choice (appending to an existing field) is for most use cases the admin is going to know it - and also it's always going to be the same for each deployment, so the redundancy multiplies. I think the current "conciseness" of status is a feature. Per the commit message, the architecture is also associated with the digest. We also group the timestamp with the version today under the idea that it's often similar data. |
I agree with @cgwalters here. Since this is the human readable output I do prefer conciseness. When I originally opened #1238 I was struggling myself with how we could implement it without adding a new field and I think what Colin proposed was a good solution. |
I guess the other option would be to maintain a map ( |
The field says "digest", so I expect a digest, not an architecture. I think the architecture is harder to find like that. Having a dedicated field would help me find the information faster. |
@vrothberg how strong is your opinion here? Is it simply a preference or do you think it's worth blocking this from getting in? |
Just a preference, no need to block at all :) |
Oh we have that mapping code in a bunch of places; one is https://github.com/coreos/stream-metadata-go/blob/main/arch/arch.go But I also added mapping from the OCI architecture to the Rust architecture (ref https://docs.rs/oci-spec/latest/src/oci_spec/image/mod.rs.html#435 ) which is almost the same thing as the GNU architectures except for the powerpc/ppc64le split (though I didn't look at RISC-V which is probably its own ball of fun). Anyways though there's I think a strong argument to stick with the OCI (Go) arch here; if you want the other one it's just a |
As far as positioning/layout it's for sure bikeshed-able but I prefer this as is. I'm more interested in aligning higher level tooling on the problem reporting domain, I don't think |
👍
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
The argument here twofold:
Closes: #1238