feat: use contaier image as kamelet repository #4350
Conversation
| return "", err | ||
| } | ||
|
|
||
| if _, err = oras.Copy(ctx, or, nt.Tag(), store, nt.Tag(), oras.DefaultCopyOptions); err != nil { |
There was a problem hiding this comment.
This is not super optimized as it unpack the entire image in a temporary folder but I have not yet found a good way to filter out the content.
There was a problem hiding this comment.
Yeah. Well, we can always clean, keeping only what we need. As this would happen on operator startup, I guess it's not a big deal.
There was a problem hiding this comment.
If the image only contains kamelet files, the overhead sounds negligible
There was a problem hiding this comment.
If the image only contains kamelet files, the overhead sounds negligible
correct, if we assume that the repository includes only kamelet files, then it won't be a big deal.
| } | ||
|
|
||
| func (r *ociKameletRepository) pullAll(ctx context.Context) { | ||
| r.once.Do(func() { |
There was a problem hiding this comment.
At this stage, the repository is only loaded once, so any new container image is not taken into account till the restart of the camel-k operator.
There was a problem hiding this comment.
We may eventually create a KameletRepository type which is reconciled and take care of the sync when any change happen. However, it would be a future optimization.
There was a problem hiding this comment.
Thanks for the work, I think the idea and the implementation are a good step forward. There are just a couple of consideration we need to take into account tough.
- is about authentication to the registry. If we implement this, we will have to probably deal with it as well (but I am pretty sure it's something you are already taking into account).
- As Kamelets are already a dedicated entity no longer bound to Camel K, I think we need to find the time to migrate all the
pkg/kamelet/repositoryand the same API definition to the same Kamelet Catalog project. From Camel K we'd have a way to synch with such definition, according to the version we're willing to use. This mean that all this logic may be moved to some other Camel K package. It's not an important detail at this stage of development, but I think it's important for all to already think on this future action.
| return "", err | ||
| } | ||
|
|
||
| if _, err = oras.Copy(ctx, or, nt.Tag(), store, nt.Tag(), oras.DefaultCopyOptions); err != nil { |
There was a problem hiding this comment.
Yeah. Well, we can always clean, keeping only what we need. As this would happen on operator startup, I guess it's not a big deal.
| } | ||
|
|
||
| func (r *ociKameletRepository) pullAll(ctx context.Context) { | ||
| r.once.Do(func() { |
There was a problem hiding this comment.
We may eventually create a KameletRepository type which is reconciled and take care of the sync when any change happen. However, it would be a future optimization.
| } | ||
|
|
||
| func (r *ociKameletRepository) pullAll(ctx context.Context) { | ||
| r.once.Do(func() { |
There was a problem hiding this comment.
We could imagine to support different image pull policies, in a similar way Kubernetes does.
A less advanced alternative would be to rely on a rate limiter to invalidate the cached data, e.g.:
rateLimiter = rate.NewLimiter(rate.Every(period), 1)
if !rateLimiter.Allow() {
return
}
...There was a problem hiding this comment.
Right I was about to add pull policies but noticed that as today the way a repository is defined is limited as it is essentially an uri :
camel-k/pkg/apis/camel/v1/integrationplatform_types.go
Lines 146 to 149 in 2417203
So I wonder if this must be changed in something more type safe, in line to what has been done for traits, like:
type IntegrationPlatformKameletRepositorySpec struct {
GitHub *GitHubKameletRepo `json:"github,omitempty"`
OCI *OCIKameletRepo `json:"oci,omitempty"`
}So we can add additional repo specific option, as example the authentication option for the oci repo.
The only issue is that by definition, if the tag of the image is latest, then the pull policy should become Always so I was thinking to use a rate limiter alike apporach that would also cache and check the checksum/digest of the manifest before actually pulling the layers.
There was a problem hiding this comment.
+1 for structuring the repository definitions.
+1 to comply with Kubernetes imagePullPolicy defaulting: https://kubernetes.io/docs/concepts/containers/images/#imagepullpolicy-defaulting.
There was a problem hiding this comment.
Giving that, I believe the IntegrationPlatform reconcile loop should be in charge of resolving the container image & so on.
One question though, is how to deal with errors: should the integration platform have a related repository condition set to false and the Ready condition set to false ?
There was a problem hiding this comment.
I think that we'll probably have to manage that as soon as we introduce certain logic into the IP. Right now, the only possibility we have for an IP to be in error is that it cannot connect to the registry for any reason (although we are not doing that check).
| } | ||
|
|
||
| func (r *ociKameletRepository) Get(ctx context.Context, name string) (*camelv1.Kamelet, error) { | ||
| r.pullAll(ctx) |
There was a problem hiding this comment.
I wonder what's the state of seekable / addressable image that would enable to lazily pull blobs / layers when needed. I think that would be applicable to our use case, but it doesn't seem it has landed into the OCI specs yet. Or maybe the repository image / manifest could already be built in a way that would serve that need.
There was a problem hiding this comment.
You could store a Kamelet in a layer and have it referenced in the Manifest. That way if you could pull a single Kamelet by getting it's address in the Manifest. The only drawback is the 127 layer limit so you could "only" store 127 Kamelets per image ...
There was a problem hiding this comment.
So theoretically it should be possible to inspect the manifest and lookup the layer i.e. by using some annotations, however I think the main decision factor would be how we would like to package the kamelets:
- 1 kamelet per layer
- 1 kamelet group per layer
- all in one layer (as tar file)
As today to keep things simpler, I would opt for option 3
There was a problem hiding this comment.
I would go with 3 for the moment. To avoid other complexities
There was a problem hiding this comment.
+1 for 3) to keep things simple 👍🏼.
|
Looks great! As Antonin said, eager to see everything in action! I also think it would be cool if you could pull the image via docker/podman: Right now I'm getting I think I stumbled this when working on custom built images and it might be the way the Manifest was built. As a side note, since we are already publishing the Kamelets on Maven it might be cool to support that too if we don't already. |
I started working some time ago on pulling the Kamelets from maven (see #3919). I stopped the development because it was unclear to me the way we wanted to support the bundling. If the effort is not that much, we may think to support several strategies as you suggest, I'd be +1 on that. |
|
Awesome stuff! |
As today, I've blindly followed the |
Yes this has intentional been skipped as some other changes may be required, so I didn't want to add to many stuffs before we agree if this is a good thing to do and how to move forward |
For sure! IIRC that's one of the reasons I didn't use ORAS and went for spectrum and Google's go-containerregistry ... I think it boils down to the list of registries that support the ORAS spec is pretty limited. That being said there might be a workaround ... |
I think I did some exploration with go-containerregistry in the past, but I have not seen an option to define a custom media type or annotations for the layers, do you know if that's possible ? |
Yes it does! |
Yeah, that is essentially also the conclusion in #2732, but I had hope for things to have become better, but looks like it is not. The alternative would be to use some annotations to help distinguish between the type of layers but, given the spec from the ORAS Artifacts have been merged to the OpenContainers specs I had an hope to get things more widely supported :) Happy to switch to |
Sure! So for building the Image I would just use Spectrum or Crane tbh.. both are built on top of or Crane This will create an Image with just the Kamelet files in a single layer And for the Operator part I would use the equivalent of which is just a Crane Pull followed by an Export. The mutate.Export is used which returns a stream. This allows you to not create a temporary file when extracting the Kamelets: for example. For authentication, you can reuse the Spectrum auth we already have... |
|
This PR has been automatically marked as stale due to 90 days of inactivity. |
|
bump |
|
This PR has been automatically marked as stale due to 90 days of inactivity. |
This is a PR opened mainly for discussion with some POC quality code that I had in my repo for some time now.
The goal is to explore the option of using OCI containers and registries as a repository for kamelets (relates to #2732) leveraging oras
Release Note