Skip to content

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

Merged
merged 1 commit into from
Apr 3, 2025
Merged

Conversation

cgwalters
Copy link
Collaborator

The argument here twofold:

  • We used to show it with ostree refs because ostree doesn't have manifest lists
  • Actually we do want to 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: #1238

Copilot

This comment was marked as resolved.

@cgwalters cgwalters enabled auto-merge March 28, 2025 21:50
@cgwalters
Copy link
Collaborator Author

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 uname on at least Fedora derivatives. And yeah, what I see looking at https://quay.io/repository/fedora/fedora-bootc is a mix because the uploads use the uname-arch but quay.io of course renders the manifests in each image (e.g. here) with the OCI arch. So...we might as well give in and use what OCI uses.

@dustymabe
Copy link

Now obviously something to bikeshed further is we use the OCI arch names

Ouch.. yeah. The split isn't ideal but I don't have a better suggestion than what you're going with.

Copy link

@dustymabe dustymabe left a 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>
Copy link
Contributor

@vrothberg vrothberg left a 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.

@cgwalters
Copy link
Collaborator Author

A small bikeshed: I would prefer a dedicated field in the human-readable output.

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.

@dustymabe
Copy link

dustymabe commented Mar 31, 2025

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.

@dustymabe
Copy link

dustymabe commented Mar 31, 2025

Ouch.. yeah. The split isn't ideal but I don't have a better suggestion than what you're going with.

I guess the other option would be to maintain a map (amd64 -> x86_64, arm64 -> aarch64) for those two where it's different, which I'm starting to warm up to.

@vrothberg
Copy link
Contributor

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.

@dustymabe
Copy link

@vrothberg how strong is your opinion here?

Is it simply a preference or do you think it's worth blocking this from getting in?

@vrothberg
Copy link
Contributor

Just a preference, no need to block at all :)

@cgwalters
Copy link
Collaborator Author

I guess the other option would be to maintain a map (amd64 -> x86_64, arm64 -> aarch64) for those two where it's different, which I'm starting to warm up to.

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 uname away (and the Rust architecture name isn't as prominent/relevant except it's just what one has by default in a Rust program, not the other two, so we need to deal with mapping from it mainly).

@cgwalters
Copy link
Collaborator Author

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 bootc status should grow too much into that space right now; it should just report what it controls mainly.

@dustymabe
Copy link

Anyways though there's I think a strong argument to stick with the OCI (Go) arch here

👍

As far as positioning/layout it's for sure bikeshed-able but I prefer this as is.

👍

Copy link

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@cgwalters cgwalters requested a review from ckyrouac April 3, 2025 13:36
@cgwalters cgwalters merged commit f01fbae into bootc-dev:main Apr 3, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consider adding arch info to bootc status output
4 participants