-
Notifications
You must be signed in to change notification settings - Fork 152
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
Add new registry section #2664
Add new registry section #2664
Conversation
There are two aspects that are not covered by this PR
|
1c23c70
to
f7bf346
Compare
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'm not sure this is the model we want. Why is the registry the top-level element with containers inside? Why isn't this part of sources, and then we can add containers and flatpaks to the same section packages are set? Also, why do we wait until firstboot to load them into container storage?
I'm answering in reverse order:
The reason for this is because loading a container archive into the storage area of the container backend requires the configured backend to be up and active. For example in podman your storage backend could be btrfs based and living in a snapshot which would not be able to be created/managed while we are building the offline image root tree. For e.g docker it's required that the docker daemon runs to actually load a container into the storage space, same for containerd... etc etc etc. It simply comes with too many restrictions when we want to load the container into the storage backend while it is not up and running. I don't want this limitation and therefore the model is fetch offline, load online. The price to pay for this model is storage space but that can be handled.
The reason why I used a new toplevel element for this is because our sources model is based on I'm open for other definition semantics but would like to ask for an example as you see it. |
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.
The "request changes" setting is for the spelling, all other comments are suggestions to consider, primarily from my perspective as a user of the feature.
I'd also like @travier to look at this and give feedback because I think we can kill two birds with one stone here and figure out the OCI Flatpak stuff that is needed for Fedora too. |
ok, the final definition section looks like the following example <registry source="registry.opensuse.org">
<containers backend="docker" path="home/marcus.schaefer/delta_containers/containers_tw">
<container name="joe"/>
</containers>
<containers backend="podman" path="home/marcus.schaefer/delta_containers/containers_tw">
<container name="mc"/>
<container name="lynx"/>
</containers>
</registry> If we need to add more to handle flatpacks let me know |
Is there a reason we can't add registry sources as repositories with |
The reason is that a registry is not a repository in the meaning of kiwi. I asked you to make an example how you would express it. So I believe you had something like this in mind <repository type="oci">
<source path="registry.opensuse.org/home/marcus.schaefer/delta_containers/containers_tw"/>
</repository>
<packages>
<container name="joe"/>
<container name="foo"/>
</packages> If the container In addition if we intermingle the registry handling with the repositories we have to touch all package manager backends to ignore type="oci". I'm sorry I see this as two different things. As for the commandline, we would need to add new commandline switches that says --add-registry, --set-registry if need be but also not intermix this with the existing --add-repo, --set-repo options |
next we would have to add a backend information (podman/docker/containerd, etc...) to the repository element which is an information that does not exist for repositories but for registries. So we would add attributes to an element which is pointless for package repositories. And vice versa many repository attributes are pointless for registries. That doesn't sound like a good semantic to me |
Also there might be additional changes on top of this feature which we currently don't know about. I would like to keep the repository and package management code free from any code change related to registry handling |
So my confusion about the registry section is that it's reasonable to do
And also, how do we handle when the "registry" tag is used multiple times in different snippets? Is it merged? Does it conflict? What do we do here? And what about non-OCI things like Incus? And non-OCI Flatpaks? |
I can't follow you <registry source="registry.opensuse.org">
<containers backend="podman">
<container name="joe"/>
<container name="foo" tag="some"/>
<container name="bar" fetch_only="true"/>
</containers>
<containers backend="docker" path=take/that>
<container name="some"/>
</containers>
</registry>
<registry source="registry.fedora.org">
<containers backend="podman">
<container name="xxx"/>
</containers>
</registry> translates to:
Where do you see issues ? and as I said I consider this very different than the repository handling and don't want to mix it |
What happens when we have two But about the registry thing, why wouldn't we just do: <containers backend="podman">
<container name="registry.opensuse.org/joe"/>
<container name="registry.opensuse.org/foo" tag="some"/>
<container name="registry.opensuse.org/bar" fetch_only="true"/>
</containers>
<containers backend="docker" path="take/that">
<container name="docker.io/library/some"/>
</containers> My point is I don't think we need |
Correct, but it gets a bit tiresome when there is more than 1 container and one has to type or generate the same registry path over an over again. Having a collector element that specifies the registry from which to pull a bunch of containers is a convenience feature for the user creting the kiwi description.
I do not see a reason why merging logic should be implemented or the should be any conflict.
|
@rjschwei I don't want us to be trapped with awkward semantics just because we didn't think of it like we are with |
And if we want to have the "convenience" of a registry tag, then I'd rather it be inside the |
Why do you want to type, or make the user type |
Now that I think about it more, why wouldn't we just define the registry as an attribute of <containers backend="podman" source="registry.opensuse.org">
<container name="joe"/>
<container name="foo" tag="some"/>
<container name="bar" fetch_only="true"/>
</containers>
<containers backend="docker" source="docker.io/library" path="take/that">
<container name="some"/>
</containers> |
Yes. We are not in the business of introducing custom things for every single thing. So maximizing reuse of our tags and attributes is beneficial. |
I have no strong preference between
which is what I think your proposing or the order in the PR. That said, it does look at bit odd to have the |
Shouldn't the path go wit the |
I would be fine with this change |
You would be able to define multiple containers sections with different registries, and then set them up as appropriate. This is just more compact than the current representation in the PR. |
I'm fine with the more compact definition. I'll change it ... after some rest, today evening or by tomorrow when I'm back at work |
We also need the |
like with the packages, yes I will add that |
Hmm, looking again at Robert's suggestion, that also moves the backend definition as an attribute to the containers element, which invalidates my idea of nesting the containers into a backend definition. In this case also the path element can stay in the container element because none of the advantages for having a backend element exists anymore. Actually this is very similar to what I started with, but I finally would like to move forward here and as it seems you both can live with that I'm going to add this suggestion |
Is there any specific reason that we are not using the URL format from https://github.com/containers/image/blob/main/docs/containers-transports.5.md for referencing the container images? Not all containers come from registries so making this a required element feels weird. What's the reason behind the podman/docker option? Both should handle the same container images fine. |
Overall, I like this option. I think I should be able to use it to make things simpler for the ostree/bootable container path. |
Thinking about this more, I think we should not focus on how the import is made but more on how the container image is stored in the image. The idea would be to specify where the image should go and under which format and then it would be up to the user doing the image to handle how to import it. This would let us pre-setup the containers for the backends that support it. Example:
|
In reply to @travier idea
We can have a conversation if it would be beneficial to add additional attributes to influence some of the fixed settings, but I would do this on real demand in followup pull requests. Thanks |
I can't follow you on this. We use the official supported transport URL. The final URL is constructed out of the
With the backend name we determine several settings, how to reach the endpoint, which tools to use, how to load the
I'm afraid as I tried to explain that a direct import while building is very limited and is only causing trouble I only saw the way via a one-shot firstboot service on boot. If you can be more explicit what you have in mind by |
What I meant by
So if I understand correctly, this will always generate a "docker://" URL and copy to an OCI archive. If you can not specify that you want to take it from the current local storage for example then this either means pulling it every time (to make sure it is the latest version) or caching it locally (and then needing a way to force a refresh). This is part of why I suggested that we don't split those and use the full transport URL (at lest for the source) so that the user can decide and manage that on their own. |
To clarify, I'm only sharing thoughts here and you should feel free to ignore anything I say :). I don't think we'll be able to use that directly for the bootable container cases in this current form. |
Nope this is the first time that we do this and I also think it smells a bit, but as I said I don't see another way except for letting the user alone with it. btw if you specify all containers with |
If the backend is set to either |
We currently fetch the container at build time of the image and store it there. On first boot it gets loaded. Assuming you build the image on a Monday and deploy it on a Friday, the container could have been updated in the registry yes, but the same applies to packages taken from a a repository. I was not quite clear what you mean by |
I take this serious as it covers use cases which are the important part. A feature of no use is useless right :) But I agree this is not working towards the |
We still need it even for |
But I did not think about the fact that there's multiple input source types supported by podman and docker. Since the |
|
Yes, that would be the build host's docker or podman image store. If I know that I'm going to build X images with a specific container in it, instead of pulling it X times into each of the images, I would pull it once in my local storage and then point the XML config to it instead. Or that could be for images that are not pushed to a registry at all. |
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.
A few more wording and description suggestions. LGTM
ok this means your local system needs a registry server e.g the CNCF distribution registry configured as a proxy. You would pull your container once and in your image build you could reference via
right which means So I guess this use case would be covered. There is just not much influence where in the system the container would be stored and the current selection of backends would allow for the format to be oci-archive only. But as I said we can add more granular settings later when required |
Sure, that could work, but it's super inefficient. If I already have the OCI archive on the build host, copying it into the rootfs of the destination image is "free" for filesystems that do copy-on-write for example. Note that all those transports are supported by skopeo for example so it's not code in kiwi.
I was more thinking about images that you've just built locally and are on the build host but not a in registry. It's independent of how you would import them in the final system. |
sure and to add this data into an image you don't need anything from what we are discussing here. I'm not sure if we are following the same line of thoughts. The feature provided in this PR is all about making container(s) from registrie(s) available in the local storage of an image such that a backend system be it podman, docker, containerd, etc etc can consume it without the need to pull them in first. If you have everything available on your local machine and just want to share data with an image build you can do this in various ways but it will also only work on this machine and nowhere else. Thus there is no reason to specify a source definition in a kiwi description, you could just go with a kiwi script that runs some cp, skopeo or podman image save or some other command to get the data into the image. Maybe we can take this topic offline ? It confuses me to be honest and I'm not sure if it helps with the review of the PR to be merged For me the most important part is, if we all agree that the current description schema serves the purpose for the feature for which @rjschwei has opened a request for and that it can be used/adapted to other scenarios that @Conan-Kudo and @travier have in mind ? I mean it's not all that of a rocket science, we fetch containers and load them and the description elements to describe this should be flexible enough to fetch from everywhere and to load into anything, which I believe is possible the way we describe it. Thoughts ? |
Allow to specify references to OCI containers in the image description like in the following example: <containers source="registry.suse.com" backend="podman"> <container name="some" tag="some" path="/some/path"/> </containers> During the kiwi process the containers are fetched into a temporary location and a systemd service is configured to one time load the containers into the local registry at first boot of the system. This Fixes #2663
b8c2135
to
a349c05
Compare
The implementation in this PR works for me and covers #2663 as far as I am concerned. We have some immediate use cases and a couple in the not too distant future, as such getting this over the finish line soon would be great. Thanks @schaefi for the effort an dthe quick implementation. @Conan-Kudo any other concerns? |
LGTM and I merged it. It's extensible enough that I think we can add Incus and Flatpak with minimal fuss down the road. |
Allow to specify references to OCI containers in the image description like in the following example:
During the kiwi process the containers are fetched into a temporary location and a systemd service is configured to one time load the containers into the local registry at first boot of the system.
This Fixes #2663