Skip to content
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

Initial support for container image updates #878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cgwalters
Copy link
Member

This is part of coreos/fedora-coreos-tracker#1263

If we're booted into a container image, then instead of looking for the special fedora-coreos.stream ostree commit metadata, we can do the much more obvious and natural thing of looking at the container image tag.

@cgwalters
Copy link
Member Author

This requires
coreos/rpm-ostree#4123
And it also needs us to add version tags to the container image, e.g. we should make quay.io/fedora/fedora-coreos:37.20221021.1.0 real.
And we also need support in rpm-ostree for rpm-ostree deploy tag= to work when tracking a container image reference.

@cgwalters
Copy link
Member Author

(It's kind of heavyweight here to add the ostree-ext dependency solely to parse the container image reference string. A bit tempting to explicitly separate out the parts from that in the status JSON...or I guess we could split out a tiny crate for that)

@cgwalters
Copy link
Member Author

Or, of course...ultimately, perhaps zincati's functionality should be folded into rpm-ostree.

cgwalters added a commit to cgwalters/zincati that referenced this pull request Nov 2, 2022
Prep for handling the container case in
coreos#878
@cgwalters
Copy link
Member Author

Split the prep patches to #879

This is part of coreos/fedora-coreos-tracker#1263

If we're booted into a container image, then instead of looking
for the special `fedora-coreos.stream` ostree commit metadata,
we can do the much more obvious and natural thing of looking at the
container image tag.
@cgwalters cgwalters marked this pull request as ready for review November 18, 2022 15:12
@cgwalters
Copy link
Member Author

OK rebased 🏄 and lifting draft - I think we can ship this.

@cgwalters
Copy link
Member Author

What will happen here now is zincati will switch the node back from container → ostree, which is obviously not what we want. Fixing that however requires a lot more work - we need to plumb through corresponding container image digests into the cincinnati metadata or/in-addition we add tags to the registry corresponding to the desired version.

But...I think this is arguably better than having zincati just fail today.

@lucab
Copy link
Contributor

lucab commented Nov 18, 2022

we can do the much more obvious and natural thing of looking at the container image tag

I think this may scratch the immediate itch but it's likely not a good foundation to build upon.

Overall we have been quite successful uncoupling each single dotted release from the moving tip of the stream.
I'm unsure why we aren't publishing dotted tags right now (e.g. quay.io/fedora/fedora-coreos:36.20221030.3.0), but I can see FCOS going in that direction in the future. And it will clash with this.

Also, the main usecase for image containerization is that people will end up pushing their own images, in which case we can't really rely on tags nor match FCOS upstream refspec.

Maybe I'm misunderstanding something, but I think even right now the ostree commit we ship in the container on quay does carry the fedora-coreos.stream metadata key. Is it being lost on the host after rebase, and you are trying to fix that here? Or are you trying to tackle the case of derived customized images which may not carry the metadata?

@cgwalters
Copy link
Member Author

Maybe I'm misunderstanding something, but I think even right now the ostree commit we ship in the container on quay does carry the fedora-coreos.stream metadata key. Is it being lost on the host after rebase, and you are trying to fix that here?

It's not lost exactly, but the deployed commit doesn't have it, it's in a base layer. I do think it's more natural to parse the container tag.

Overall we have been quite successful uncoupling each single dotted release from the moving tip of the stream.
I'm unsure why we aren't publishing dotted tags right now (e.g. quay.io/fedora/fedora-coreos:36.20221030.3.0), but I can see FCOS going in that direction in the future. And it will clash with this.

How so? I think we'd probably express this semantically by doing an explicit versioned fetch with out changing the origin. That's what happens today with rpm-ostree deploy revision=$version. So we'd likely do the same thing and have rpm-ostree deploy tag=$version, but the output will still display quay.io/fedora/fedora-coreos:stable e.g.

Also, the main usecase for image containerization is that people will end up pushing their own images, in which case we can't really rely on tags nor match FCOS upstream refspec.

Well...yes but I'm not trying to scope that in quite yet. The immediate goal is to take the steps necessary for us to switch over to using containers as a transport by default.

@jlebon
Copy link
Member

jlebon commented Nov 18, 2022

Would be good to flesh out the design for coreos/fedora-coreos-tracker#1263 before doing implementation changes. I'm also hesitant to bind us to certain conventions before tackling the larger issue of "layering + automatic updates".

@jlebon
Copy link
Member

jlebon commented Nov 18, 2022

Fixing that however requires a lot more work - we need to plumb through corresponding container image digests into the cincinnati metadata or/in-addition we add tags to the registry corresponding to the desired version.

Right, I think that'd be a good avenue. The graph schema is generic over the payload type and has a org.fedoraproject.coreos.scheme for this purpose. It's currently set to checksum, but in a container world, it could be set to e.g. sha256digest where the payload is then the container digest to feed to rpm-ostree. ISTM like the existing interface of rpm-ostree deploy revision=$X could be extended to support container digests to fit into this model.

That way Zincati doesn't really need to know in details about container things, which AFAIK matches the current status for OSTree things.

But again, the "layering" aspect of this changes things and how all this intersects with that use case needs some careful thought.

@jlebon
Copy link
Member

jlebon commented Nov 18, 2022

Though one thing we could do I guess is to gate this PR behind some sufficiently scary flag/env var?

But as an aside, I do think the fedora-coreos.stream metadata is important enough that it should be kept both in the layered commit and probably also promoted to an image label, even if we don't end up using it for Zincati's purposes.

@cgwalters
Copy link
Member Author

But as an aside, I do think the fedora-coreos.stream metadata is important enough that it should be kept both in the layered commit and probably also promoted to an image label, even if we don't end up using it for Zincati's purposes.

Well...part of the idea of all this is to get away from "ostree stuff". We could try to set up a system where metadata on the original commit does propagate to the local merge commit, but...in the end I think what we want is to just have it in the container image metadata, so: coreos/coreos-assembler#3214

Then zincati can parse the container metadata using standard APIs.

All of this helps lead to a world where a zincati-like tool can also be used for application container (e.g. podman) updates. And it should help us align with how e.g. OCP is already using Cincinnati to target a container image (not an ostree commit).

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jan 11, 2023
jlebon pushed a commit to coreos/coreos-assembler that referenced this pull request Jan 11, 2023
@kaplan-michael
Copy link

Hello, Is this already "usable" in some way(custom build rpm-ostree, zinacati, etc.)
I don't mind if it's still unstable, mostly want to try it out early.

Thanks a lot

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.

4 participants