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

fix(core): Externally built Integrations run command configuration from jvm trait #5151

Merged
merged 1 commit into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ You can therefore update the values in the `IntegrationPlatform` in order to per
== Container registry requirements
Each platform may have its default registry of choice. And each container registry may have a slight different configuration. Please, be aware that we won't be able to support all the available solutions.

The only requirement we have is that the registry must be able to produce/consume images with the following tagging convention: `<registry-host>[:<registry-port>]/<k8s-namespace>/camel-k-kit-<hash-code>@sha256:<sha256-code>`, ie `10.110.251.124/default/camel-k-kit-ck0612dahvgs73ffe5g0@sha256:3c9589dd093b689aee6bf5c2d35aa1fce9d0e76d5bb7da8b61d87e7a1ed6f36a`.
The only requirement we have is that the registry must be able to produce/consume images with the following tagging convention: `<registry-host>[:<registry-port>]/<k8s-namespace>/kit-<hash-code>@sha256:<sha256-code>`, ie `10.110.251.124/default/kit-ck0612dahvgs73ffe5g0@sha256:3c9589dd093b689aee6bf5c2d35aa1fce9d0e76d5bb7da8b61d87e7a1ed6f36a`.

This should be within the standard convention adopted by https://docs.docker.com/engine/reference/commandline/pull/#pull-an-image-by-digest-immutable-identifier[pulling a Docker image by digest] (immutable).

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/partials/apis/camel-k-crds.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -6836,11 +6836,11 @@ Forces the value for labels `sidecar.istio.io/inject`. By default the label is s

* <<#_camel_apache_org_v1_Traits, Traits>>

The JVM trait is used to configure the JVM that runs the Integration. This trait can be configured only for Integration and related IntegrationKits
The JVM trait is used to configure the JVM that runs the Integration. This trait is configured only for Integration and related IntegrationKits
(bound to a container image) built by Camel K operator. If the system detects the usage of a different container image (ie, built externally), then, the
trait is disabled by the platform.

NOTE: the platform will skip the trait configuration for those container image matching `camel-k-kit-` name.
WARNING: you can still enable the trait explicitly even when it is disabled by the platform but you should be aware that some configurations could fail.


[cols="2,2a",options="header"]
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/traits/pages/jvm.adoc
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
= Jvm Trait

// Start of autogenerated code - DO NOT EDIT! (description)
The JVM trait is used to configure the JVM that runs the Integration. This trait can be configured only for Integration and related IntegrationKits
The JVM trait is used to configure the JVM that runs the Integration. This trait is configured only for Integration and related IntegrationKits
(bound to a container image) built by Camel K operator. If the system detects the usage of a different container image (ie, built externally), then, the
trait is disabled by the platform.

NOTE: the platform will skip the trait configuration for those container image matching `camel-k-kit-` name.
WARNING: you can still enable the trait explicitly even when it is disabled by the platform but you should be aware that some configurations could fail.


This trait is available in the following profiles: **Kubernetes, Knative, OpenShift**.
Expand Down
6 changes: 3 additions & 3 deletions e2e/common/runtimes/runtimes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestSourceLessIntegrations(t *testing.T) {
Expect(KamelRunWithID(operatorID, ns, "--image", "docker.io/squakez/my-camel-main:1.0.0", "--resource", "configmap:my-cm-sourceless@/tmp/app/data").Execute()).To(Succeed())
Eventually(IntegrationPodPhase(ns, itName), TestTimeoutShort).Should(Equal(corev1.PodRunning))
Eventually(IntegrationConditionStatus(ns, itName, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: container image was not built via Camel K operator"))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: integration kit was not created via Camel K operator"))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring(cmData["my-file.txt"]))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring("Apache Camel (Main)"))
})
Expand All @@ -54,7 +54,7 @@ func TestSourceLessIntegrations(t *testing.T) {
Expect(KamelRunWithID(operatorID, ns, "--image", "docker.io/squakez/my-camel-sb:1.0.0", "--resource", "configmap:my-cm-sourceless@/tmp/app/data").Execute()).To(Succeed())
Eventually(IntegrationPodPhase(ns, itName), TestTimeoutShort).Should(Equal(corev1.PodRunning))
Eventually(IntegrationConditionStatus(ns, itName, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: container image was not built via Camel K operator"))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: integration kit was not created via Camel K operator"))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring(cmData["my-file.txt"]))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring("Spring Boot"))
})
Expand All @@ -64,7 +64,7 @@ func TestSourceLessIntegrations(t *testing.T) {
Expect(KamelRunWithID(operatorID, ns, "--image", "docker.io/squakez/my-camel-quarkus:1.0.0", "--resource", "configmap:my-cm-sourceless@/tmp/app/data").Execute()).To(Succeed())
Eventually(IntegrationPodPhase(ns, itName), TestTimeoutShort).Should(Equal(corev1.PodRunning))
Eventually(IntegrationConditionStatus(ns, itName, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: container image was not built via Camel K operator"))
Eventually(IntegrationCondition(ns, itName, v1.IntegrationConditionTraitInfo)().Message).Should(Equal("explicitly disabled by the platform: integration kit was not created via Camel K operator"))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring(cmData["my-file.txt"]))
Eventually(IntegrationLogs(ns, itName), TestTimeoutShort).Should(ContainSubstring("powered by Quarkus"))
})
Expand Down
2 changes: 1 addition & 1 deletion e2e/commonwithcustominstall/operator_id_filtering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestOperatorIDFiltering(t *testing.T) {
Expect(Kamel("delete", "moving", "-n", ns).Execute()).To(Succeed())

Expect(KamelRun(ns, "files/yaml.yaml", "--name", "pre-built", "--force",
"-t", fmt.Sprintf("container.image=%s", image)).Execute()).To(Succeed())
"-t", fmt.Sprintf("container.image=%s", image), "-t", "jvm.enabled=true").Execute()).To(Succeed())
Consistently(IntegrationPhase(ns, "pre-built"), 10*time.Second).Should(BeEmpty())
Expect(AssignIntegrationToOperator(ns, "pre-built", operator2)).To(Succeed())
Eventually(IntegrationPhase(ns, "pre-built"), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning))
Expand Down
2 changes: 1 addition & 1 deletion e2e/install/cli/global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestRunGlobalInstall(t *testing.T) {
}
Expect(TestClient().Create(TestContext, &externalKit)).Should(BeNil())

Expect(KamelRun(ns5, "files/Java.java", "--name", "ext", "--kit", "external").Execute()).To(Succeed())
Expect(KamelRun(ns5, "files/Java.java", "--name", "ext", "--kit", "external", "-t", "jvm.enabled=true").Execute()).To(Succeed())
Eventually(IntegrationPodPhase(ns5, "ext"), TestTimeoutLong).Should(Equal(corev1.PodRunning))
Eventually(IntegrationLogs(ns5, "ext"), TestTimeoutShort).Should(ContainSubstring("Magicstring!"))
Expect(IntegrationKit(ns5, "ext")()).Should(Equal("external"))
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/camel/v1/integrationkit_types_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ func (in *IntegrationKit) HasHigherPriorityThan(kit *IntegrationKit) bool {
return p1 > p2
}

// IsExternal returns true for external IntegrationKits (integration kit not created by the operator).
func (in *IntegrationKit) IsExternal() bool {
return in.Labels[IntegrationKitTypeLabel] == IntegrationKitTypeExternal
}

// GetCondition returns the condition with the provided type.
func (in *IntegrationKitStatus) GetCondition(condType IntegrationKitConditionType) *IntegrationKitCondition {
for i := range in.Conditions {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/camel/v1/trait/jvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ limitations under the License.

package trait

// The JVM trait is used to configure the JVM that runs the Integration. This trait can be configured only for Integration and related IntegrationKits
// The JVM trait is used to configure the JVM that runs the Integration. This trait is configured only for Integration and related IntegrationKits
// (bound to a container image) built by Camel K operator. If the system detects the usage of a different container image (ie, built externally), then, the
// trait is disabled by the platform.
//
// NOTE: the platform will skip the trait configuration for those container image matching `camel-k-kit-` name.
// WARNING: you can still enable the trait explicitly even when it is disabled by the platform but you should be aware that some configurations could fail.
//
// +camel-k:trait=jvm.
type JVMTrait struct {
Expand Down
13 changes: 13 additions & 0 deletions pkg/cmd/promote.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
rbacv1 "k8s.io/api/rbac/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -468,6 +469,12 @@ func (o *promoteCmdOptions) editIntegration(it *v1.Integration) *v1.Integration
dst.Spec.Traits.Container = &traitv1.ContainerTrait{}
}
dst.Spec.Traits.Container.Image = contImage
if dst.Spec.Traits.JVM == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this check. We know the promote tests are kind of flaky, no need to add anything in the command IMO. The container has to be initialized because it could raise a nil exception, but not the JVM trait, for which, we must keep the default setting.

Copy link
Contributor Author

@gansheer gansheer Feb 13, 2024

Choose a reason for hiding this comment

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

It is not a flacky test in this case, the promote command run the "promoted" integration with the trait Container.Image, so it becomes an external kit and does not start correctly. Since the trait was not explicilty set in the original integration, we are relying on default behavior, which will be different for original and promoted integration. This aims to fix this issue.

The alternative would be to add support for the traits on the comand options or add the copy of the integration kit created for the image.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. We can proceed then.

dst.Spec.Traits.JVM = &traitv1.JVMTrait{}
}
if dst.Spec.Traits.JVM.Enabled == nil {
dst.Spec.Traits.JVM.Enabled = pointer.Bool(true)
}
return &dst
}

Expand Down Expand Up @@ -513,6 +520,12 @@ func (o *promoteCmdOptions) editPipe(kb *v1.Pipe, it *v1.Integration) *v1.Pipe {
dst.Spec.Integration.Traits.Container = &traitv1.ContainerTrait{}
}
dst.Spec.Integration.Traits.Container.Image = contImage
if dst.Spec.Integration.Traits.JVM == nil {
dst.Spec.Integration.Traits.JVM = &traitv1.JVMTrait{}
}
if dst.Spec.Integration.Traits.JVM.Enabled == nil {
dst.Spec.Integration.Traits.JVM.Enabled = pointer.Bool(true)
}
if dst.Spec.Source.Ref != nil {
dst.Spec.Source.Ref.Namespace = o.To
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/cmd/promote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
status:
traits: {}
`, output)
Expand Down Expand Up @@ -142,6 +144,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
sink: {}
source: {}
status: {}
Expand Down Expand Up @@ -198,6 +202,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
status:
traits: {}
`, output)
Expand Down Expand Up @@ -243,6 +249,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
sink: {}
source: {}
status: {}
Expand Down Expand Up @@ -318,6 +326,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
status:
traits: {}
`, output)
Expand All @@ -341,6 +351,8 @@ spec:
traits:
container:
image: my-special-image
jvm:
enabled: true
status:
traits: {}
`, output)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/integrationkit/integrationkit_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (r *reconcileIntegrationKit) Reconcile(ctx context.Context, request reconci

if target.Status.Phase == v1.IntegrationKitPhaseNone || target.Status.Phase == v1.IntegrationKitPhaseWaitingForPlatform {
rlog.Debug("Preparing to shift integration kit phase")
if target.Labels[v1.IntegrationKitTypeLabel] == v1.IntegrationKitTypeExternal {
if target.IsExternal() {
target.Status.Phase = v1.IntegrationKitPhaseInitialization
return r.update(ctx, &instance, target)
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/trait/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"path/filepath"
"strings"

appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
Expand Down Expand Up @@ -348,8 +347,3 @@ func (t *containerTrait) configureSecurityContext(e *Environment, container *cor
}
}
}

// It's a user provided image if it does not match the naming convention used by Camel K Integration Kits.
func (t *containerTrait) hasUserProvidedImage() bool {
return t.Image != "" && !strings.Contains(t.Image, "camel-k-kit-")
}
12 changes: 7 additions & 5 deletions pkg/trait/jvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,12 @@ func (t *jvmTrait) Configure(e *Environment) (bool, *TraitCondition, error) {
return false, newIntegrationConditionPlatformDisabledWithMessage("quarkus native build"), nil
}
}
// The JVM trait must be disabled if it's a user based build (for which we do not control the way to handle JVM parameters)
if ct := e.Catalog.GetTrait(containerTraitID); ct != nil {
if ct, ok := ct.(*containerTrait); ok && ct.hasUserProvidedImage() {
return false, newIntegrationConditionPlatformDisabledWithMessage("container image was not built via Camel K operator"), nil

if e.IntegrationKit != nil && e.IntegrationKit.IsExternal() {
if pointer.BoolDeref(t.Enabled, false) {
return true, NewIntegrationConditionUserEnabledWithMessage("integration kit was not created via Camel K operator"), nil
} else {
return false, newIntegrationConditionPlatformDisabledWithMessage("integration kit was not created via Camel K operator"), nil
}
}

Expand Down Expand Up @@ -113,7 +115,7 @@ func (t *jvmTrait) Apply(e *Environment) error {
classpath.Add(artifact.Target)
}

if kit.Labels[v1.IntegrationKitTypeLabel] == v1.IntegrationKitTypeExternal {
if kit.IsExternal() {
// In case of an external created kit, we do not have any information about
// the classpath, so we assume the all jars in /deployments/dependencies/ have
// to be taken into account.
Expand Down
50 changes: 50 additions & 0 deletions pkg/trait/jvm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,56 @@ func TestConfigureJvmTraitInWrongIntegrationKitPhaseDoesNotSucceed(t *testing.T)
assert.Nil(t, condition)
}

func TestConfigureJvmTraitInWrongJvmDisabled(t *testing.T) {
trait, environment := createNominalJvmTest(v1.IntegrationKitTypePlatform)
trait.Enabled = pointer.Bool(false)

expectedCondition := NewIntegrationCondition(
v1.IntegrationConditionTraitInfo,
corev1.ConditionTrue,
"TraitConfiguration",
"explicitly disabled by the user",
)
configured, condition, err := trait.Configure(environment)
assert.Nil(t, err)
assert.False(t, configured)
assert.NotNil(t, condition)
assert.Equal(t, expectedCondition, condition)
}

func TestConfigureJvmTraitInWrongIntegrationKitPhaseExternal(t *testing.T) {
trait, environment := createNominalJvmTest(v1.IntegrationKitTypeExternal)

expectedCondition := NewIntegrationCondition(
v1.IntegrationConditionTraitInfo,
corev1.ConditionTrue,
"TraitConfiguration",
"explicitly disabled by the platform: integration kit was not created via Camel K operator",
)
configured, condition, err := trait.Configure(environment)
assert.Nil(t, err)
assert.False(t, configured)
assert.NotNil(t, condition)
assert.Equal(t, expectedCondition, condition)
}

func TestConfigureJvmTraitInRightIntegrationKitPhaseExternalAndJvmEnabled(t *testing.T) {
trait, environment := createNominalJvmTest(v1.IntegrationKitTypeExternal)
trait.Enabled = pointer.Bool(true)

expectedCondition := NewIntegrationCondition(
v1.IntegrationConditionTraitInfo,
corev1.ConditionTrue,
"TraitConfiguration",
"explicitly enabled by the user: integration kit was not created via Camel K operator",
)
configured, condition, err := trait.Configure(environment)
assert.Nil(t, err)
assert.True(t, configured)
assert.NotNil(t, condition)
assert.Equal(t, expectedCondition, condition)
}

func TestApplyJvmTraitWithDeploymentResource(t *testing.T) {
trait, environment := createNominalJvmTest(v1.IntegrationKitTypePlatform)

Expand Down
5 changes: 5 additions & 0 deletions pkg/trait/trait_condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
const (
traitConfigurationReason = "TraitConfiguration"
userDisabledMessage = "explicitly disabled by the user"
userEnabledMessage = "explicitly enabled by the user"
platformDisabledMessage = "explicitly disabled by the platform"
)

Expand All @@ -54,6 +55,10 @@ func NewIntegrationConditionUserDisabled() *TraitCondition {
return NewIntegrationCondition(v1.IntegrationConditionTraitInfo, corev1.ConditionTrue, traitConfigurationReason, userDisabledMessage)
}

func NewIntegrationConditionUserEnabledWithMessage(message string) *TraitCondition {
return NewIntegrationCondition(v1.IntegrationConditionTraitInfo, corev1.ConditionTrue, traitConfigurationReason, fmt.Sprintf("%s: %s", userEnabledMessage, message))
}

func newIntegrationConditionPlatformDisabledWithMessage(message string) *TraitCondition {
return NewIntegrationCondition(v1.IntegrationConditionTraitInfo, corev1.ConditionTrue, traitConfigurationReason, fmt.Sprintf("%s: %s", platformDisabledMessage, message))
}
Expand Down
11 changes: 6 additions & 5 deletions resources/traits.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -880,11 +880,12 @@ traits:
- Knative
- OpenShift
description: 'The JVM trait is used to configure the JVM that runs the Integration.
This trait can be configured only for Integration and related IntegrationKits
(bound to a container image) built by Camel K operator. If the system detects
the usage of a different container image (ie, built externally), then, the trait
is disabled by the platform. NOTE: the platform will skip the trait configuration
for those container image matching `camel-k-kit-` name.'
This trait is configured only for Integration and related IntegrationKits (bound
to a container image) built by Camel K operator. If the system detects the usage
of a different container image (ie, built externally), then, the trait is disabled
by the platform. WARNING: you can still enable the trait explicitly even when
it is disabled by the platform but you should be aware that some configurations
could fail.'
properties:
- name: enabled
type: bool
Expand Down
Loading