You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2024/04/22 15:20:23 UTC
[PR] fix(trait): define synthetic Kit [camel-k]
squakez opened a new pull request, #5399:
URL: https://github.com/apache/camel-k/pull/5399
While working on #5378 I realized the concept of external Kit is used widely also for the concept of what is we can define as a synthetic Kit. For this user generated container image (likely not from the operator build) we cannot make any assumption when it come to certain traits which require the presence of a CamelCatalog bound to a given runtime provider and version.
This PR would define the new type of kit, giving the possibility to the user to leverage `container.imageWasKit=true` configuration to instruct the operator that the image imported (ie, via `kamel run --image`) was coming from a Kit, hence, it can make all the assumptions about it. Conversely, in presence of a "synthetic" kit, the operator cannot make any assumption, above all when coming to the execution of JVM trait.
Prerequisite of #5378
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
fix(trait): define synthetic Kit
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#issuecomment-2074260618
I wonder if this should be a kit only property/annotation rather than a trait, in fact even if the container image was built by camel-k, the absence of a kit may not make it 100% usable (my memory may not be 100% up to date, but the kit was also supposed to carry some traits, even some runtime traits)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#discussion_r1575046561
##########
e2e/advanced/operator_id_filtering_test.go:
##########
@@ -100,7 +100,7 @@ func TestOperatorIDFiltering(t *testing.T) {
// Save resources by deleting "moving" integration
g.Expect(Kamel(t, ctx, "delete", "moving", "-n", ns).Execute()).To(Succeed())
- g.Expect(KamelRunWithID(t, ctx, "operator-x", ns, "files/yaml.yaml", "--name", "pre-built", "--force", "-t", fmt.Sprintf("container.image=%s", image), "-t", "jvm.enabled=true").Execute()).To(Succeed())
+ g.Expect(KamelRunWithID(t, ctx, "operator-x", ns, "files/yaml.yaml", "--name", "pre-built", "--force", "-t", fmt.Sprintf("container.image=%s", image), "-t", "container.imageWasKit=true").Execute()).To(Succeed())
Review Comment:
You probably need to use `container.image-was-kit` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #5399:
URL: https://github.com/apache/camel-k/pull/5399
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#issuecomment-2074249494
@lburgazzoli that's exactly the goal of this PR, regardless the name used for the parameter. Definitely, `image-format` sounds better and I'll work to do this change which result more descriptive. With this, we clarify the intent of an external IntegrationKit, which is either created via the operator (hence, reusable with a bound CamelCatalog, making all the assumptions around it) or created by the user with any other mean (hence, not reusable as it misses the CamelCatalog which generated it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#issuecomment-2074429132
> I understand that you're proposing to force the possibility to run a container image previously built by the operator only with the associated IntegrationKit (which must be copied from its source), is it correct? This feature already exists (ie, `kamel run --kit`) and this PR should not have affected it at all.
>
It is a little bit broader than this because as today you can instruct camel-k to run an integration against a pre-existing container image in two ways:
1. by creating an external `IntegrationKit` with the given image and reference it in the `Integration` spec
2. by setting the image on the `container` trait (which then generates an external `IntegrationKit` behind the scenes, then point 1)
3. by copying the `IntegrationKit` and reference it in the `Integration` spec
Now I think is where things gets a little bit confusing as yes you can use `kamel run --kit`, but such kit may or may not be built using a camel-k compatible image so you could potentially have to set the image format i.e. with `kamel run --kit ... --container.image-format=...`, even if you are referencing a kit which, by definition, is expected to describe an image.
This is why I think, the information about the image format is more a Kit specific trait (maybe it can be part of the spec, along with the image property).
> the main reason of this PR was to address certain failing tests that were assuming that we can run an Integration with a `container.image` coming off from a previous external IntegrationKit.
I think @valdar did some work in https://github.com/apache/camel-k/pull/5119 to fix this issue , may be worth having a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#issuecomment-2074171903
Sorry for being late but wonder if a better name would have been `container.image-format = camel-k|unknown|...` or similar, as
- it is possible to manually create a kit with an external image (I think there is also a cli command)
- it is possible to build a perfectly compatible image offline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(trait): define synthetic Kit [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5399:
URL: https://github.com/apache/camel-k/pull/5399#issuecomment-2074300667
I think we're talking about slight different things then. The problem we have is that when the user is running an Integration with `container.image` defined, the operator cannot know if this image was either built previously by the operator or by any external system. This is happening because the user can only input the container image, so, the operator cannot make any assumption around it, unless the user provide a further configuration to tell the operator that this container image is coming from a previous IntegrationKit.
I understand that you're proposing to force the possibility to run a container image previously built by the operator only with the associated IntegrationKit (which must be copied from its source), is it correct? This feature already exists (ie, `kamel run --kit`) and this PR should not have affected it at all.
In that case what we need to do is just removing the new parameter included in this PR (only the parameter as the rest of the PR goes in the direction to let a user run an opaque container image without the execution of those traits that need an IntegrationKit). And we need to adjust our test, as, the main reason of this PR was to address certain failing tests that were assuming that we can run an Integration with a `container.image` coming off from a previous external IntegrationKit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org