From 9d35897f2adc8533c99b28d48e5e7a1681c34c09 Mon Sep 17 00:00:00 2001 From: Gaelle Fournier Date: Wed, 7 Feb 2024 17:51:31 +0100 Subject: [PATCH] fix(core): Externally built Integrations run command * change the check to evaluate if the kit is external or not instead of 'camel-k-kit' naming convention * let the possibility for the user to explicitly enable the trait jvm and add a condition to inform the user that it is disabled by default * enable jvm trait explicitly on promote command --- .../pages/installation/registry/registry.adoc | 2 +- .../ROOT/partials/apis/camel-k-crds.adoc | 4 +- docs/modules/traits/pages/jvm.adoc | 4 +- e2e/common/runtimes/runtimes_test.go | 6 +-- .../operator_id_filtering_test.go | 2 +- e2e/install/cli/global_test.go | 2 +- .../camel/v1/integrationkit_types_support.go | 5 ++ pkg/apis/camel/v1/trait/jvm.go | 4 +- pkg/cmd/promote.go | 13 +++++ pkg/cmd/promote_test.go | 12 +++++ .../integrationkit_controller.go | 2 +- pkg/trait/container.go | 6 --- pkg/trait/jvm.go | 12 +++-- pkg/trait/jvm_test.go | 50 +++++++++++++++++++ pkg/trait/trait_condition_types.go | 5 ++ resources/traits.yaml | 11 ++-- 16 files changed, 111 insertions(+), 29 deletions(-) diff --git a/docs/modules/ROOT/pages/installation/registry/registry.adoc b/docs/modules/ROOT/pages/installation/registry/registry.adoc index 5f5e2f60ee..2a8099d554 100644 --- a/docs/modules/ROOT/pages/installation/registry/registry.adoc +++ b/docs/modules/ROOT/pages/installation/registry/registry.adoc @@ -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: `[:]//camel-k-kit-@sha256:`, 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: `[:]//kit-@sha256:`, 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). diff --git a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc index cd452887fd..1d504da9ab 100644 --- a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc +++ b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc @@ -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"] diff --git a/docs/modules/traits/pages/jvm.adoc b/docs/modules/traits/pages/jvm.adoc index 6e2654c445..5a2cc1540f 100755 --- a/docs/modules/traits/pages/jvm.adoc +++ b/docs/modules/traits/pages/jvm.adoc @@ -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**. diff --git a/e2e/common/runtimes/runtimes_test.go b/e2e/common/runtimes/runtimes_test.go index f9556232bf..87f6394e17 100644 --- a/e2e/common/runtimes/runtimes_test.go +++ b/e2e/common/runtimes/runtimes_test.go @@ -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)")) }) @@ -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")) }) @@ -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")) }) diff --git a/e2e/commonwithcustominstall/operator_id_filtering_test.go b/e2e/commonwithcustominstall/operator_id_filtering_test.go index 5164096cbf..719fcb0d8f 100644 --- a/e2e/commonwithcustominstall/operator_id_filtering_test.go +++ b/e2e/commonwithcustominstall/operator_id_filtering_test.go @@ -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)) diff --git a/e2e/install/cli/global_test.go b/e2e/install/cli/global_test.go index 27ad161594..4b5afe3ad8 100644 --- a/e2e/install/cli/global_test.go +++ b/e2e/install/cli/global_test.go @@ -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")) diff --git a/pkg/apis/camel/v1/integrationkit_types_support.go b/pkg/apis/camel/v1/integrationkit_types_support.go index d86b47744a..6bcde2c037 100644 --- a/pkg/apis/camel/v1/integrationkit_types_support.go +++ b/pkg/apis/camel/v1/integrationkit_types_support.go @@ -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 { diff --git a/pkg/apis/camel/v1/trait/jvm.go b/pkg/apis/camel/v1/trait/jvm.go index 2a08fcf0b2..b439381c35 100644 --- a/pkg/apis/camel/v1/trait/jvm.go +++ b/pkg/apis/camel/v1/trait/jvm.go @@ -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 { diff --git a/pkg/cmd/promote.go b/pkg/cmd/promote.go index b15cc08382..562b4dac56 100644 --- a/pkg/cmd/promote.go +++ b/pkg/cmd/promote.go @@ -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" ) @@ -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 { + dst.Spec.Traits.JVM = &traitv1.JVMTrait{} + } + if dst.Spec.Traits.JVM.Enabled == nil { + dst.Spec.Traits.JVM.Enabled = pointer.Bool(true) + } return &dst } @@ -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 } diff --git a/pkg/cmd/promote_test.go b/pkg/cmd/promote_test.go index 0cf9093d88..50616d5811 100644 --- a/pkg/cmd/promote_test.go +++ b/pkg/cmd/promote_test.go @@ -101,6 +101,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true status: traits: {} `, output) @@ -142,6 +144,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true sink: {} source: {} status: {} @@ -198,6 +202,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true status: traits: {} `, output) @@ -243,6 +249,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true sink: {} source: {} status: {} @@ -318,6 +326,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true status: traits: {} `, output) @@ -341,6 +351,8 @@ spec: traits: container: image: my-special-image + jvm: + enabled: true status: traits: {} `, output) diff --git a/pkg/controller/integrationkit/integrationkit_controller.go b/pkg/controller/integrationkit/integrationkit_controller.go index bece3c5419..56244785f2 100644 --- a/pkg/controller/integrationkit/integrationkit_controller.go +++ b/pkg/controller/integrationkit/integrationkit_controller.go @@ -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) } diff --git a/pkg/trait/container.go b/pkg/trait/container.go index 37429a60fb..16f1e72edd 100644 --- a/pkg/trait/container.go +++ b/pkg/trait/container.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "path/filepath" - "strings" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" @@ -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-") -} diff --git a/pkg/trait/jvm.go b/pkg/trait/jvm.go index ffcf2ccd98..ab11617ff8 100644 --- a/pkg/trait/jvm.go +++ b/pkg/trait/jvm.go @@ -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 } } @@ -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. diff --git a/pkg/trait/jvm_test.go b/pkg/trait/jvm_test.go index 610d5000ba..991d121f82 100644 --- a/pkg/trait/jvm_test.go +++ b/pkg/trait/jvm_test.go @@ -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) diff --git a/pkg/trait/trait_condition_types.go b/pkg/trait/trait_condition_types.go index c027bf5ecf..05f9ac4a40 100644 --- a/pkg/trait/trait_condition_types.go +++ b/pkg/trait/trait_condition_types.go @@ -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" ) @@ -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)) } diff --git a/resources/traits.yaml b/resources/traits.yaml index ddf226878e..bd6c3c5d15 100755 --- a/resources/traits.yaml +++ b/resources/traits.yaml @@ -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