From 432f0ffd06d75e741e931d03df669b0929c88cfb Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Mon, 7 Oct 2024 21:33:17 +0200 Subject: [PATCH 1/2] fix(trait): execute GC only in running phases This is required as many resource are not available in the Initialization phase, therefore may be removed even when they are expected to be instead updated (by Deployer trait). Closes #5874 --- pkg/trait/gc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/trait/gc.go b/pkg/trait/gc.go index 91254de68c..be1ba12126 100644 --- a/pkg/trait/gc.go +++ b/pkg/trait/gc.go @@ -113,7 +113,9 @@ func (t *gcTrait) Configure(e *Environment) (bool, *TraitCondition, error) { return false, NewIntegrationConditionUserDisabled("GC"), nil } - return e.IntegrationInPhase(v1.IntegrationPhaseInitialization) || e.IntegrationInRunningPhases(), nil, nil + // We need to execute this trait only when all resources have been created and + // deployed with a new generation if there is was any change during the Integration drift. + return e.IntegrationInRunningPhases(), nil, nil } func (t *gcTrait) Apply(e *Environment) error { From cefcb6b146d99ecd337cc5fd072f962548f8e231 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Wed, 9 Oct 2024 21:31:22 +0200 Subject: [PATCH 2/2] fix(e2e): add test to make sure Deployment is not recreated on update. --- .../ROOT/partials/apis/camel-k-crds.adoc | 2 + docs/modules/traits/pages/service.adoc | 2 + e2e/common/traits/gc_test.go | 13 +++- e2e/knative/gc_test.go | 64 ------------------- e2e/support/test_support.go | 16 ++--- pkg/apis/camel/v1/trait/service.go | 2 + 6 files changed, 23 insertions(+), 76 deletions(-) delete mode 100644 e2e/knative/gc_test.go diff --git a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc index f8ef11254d..98771dac8d 100644 --- a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc +++ b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc @@ -8746,6 +8746,8 @@ List of Services in the form [[apigroup/]version:]kind:[namespace/]name The Service trait exposes the integration with a Service resource so that it can be accessed by other applications (or integrations) in the same namespace. +NOTE: this trait is automatically disabled if the Knative Service trait is enabled. + It's enabled by default if the integration depends on a Camel component that can expose a HTTP endpoint. diff --git a/docs/modules/traits/pages/service.adoc b/docs/modules/traits/pages/service.adoc index 97bb436acf..6208a346ba 100755 --- a/docs/modules/traits/pages/service.adoc +++ b/docs/modules/traits/pages/service.adoc @@ -6,6 +6,8 @@ The Service trait exposes the integration with a Service resource so that it can be accessed by other applications (or integrations) in the same namespace. +NOTE: this trait is automatically disabled if the Knative Service trait is enabled. + It's enabled by default if the integration depends on a Camel component that can expose a HTTP endpoint. diff --git a/e2e/common/traits/gc_test.go b/e2e/common/traits/gc_test.go index 58163e5aae..7720fbb825 100644 --- a/e2e/common/traits/gc_test.go +++ b/e2e/common/traits/gc_test.go @@ -25,6 +25,7 @@ package common import ( "context" "testing" + "time" . "github.com/onsi/gomega" @@ -38,11 +39,19 @@ func TestGarbageCollectorTrait(t *testing.T) { t.Run("Delete outdated resources", func(t *testing.T) { g.Expect(KamelRun(t, ctx, ns, "files/PlatformHttpServer.java").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "platform-http-server"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) - g.Eventually(ServicesByType(t, ctx, ns, corev1.ServiceTypeClusterIP), TestTimeoutLong).ShouldNot(BeEmpty()) + g.Eventually(ServicesByType(t, ctx, ns, corev1.ServiceTypeClusterIP), TestTimeoutShort).ShouldNot(BeEmpty()) + g.Eventually(Deployment(t, ctx, ns, "platform-http-server"), TestTimeoutShort).ShouldNot(BeNil()) + genOneDeploymentUID := DeploymentUID(t, ctx, ns, "platform-http-server")() // Update integration and disable service trait - existing service should be garbage collected g.Expect(KamelRun(t, ctx, ns, "files/PlatformHttpServer.java", "-t", "service.enabled=false").Execute()).To(Succeed()) - g.Eventually(ServicesByType(t, ctx, ns, corev1.ServiceTypeClusterIP), TestTimeoutLong).Should(BeEmpty()) + g.Eventually(ServicesByType(t, ctx, ns, corev1.ServiceTypeClusterIP), TestTimeoutShort).Should(BeEmpty()) + g.Eventually(Deployment(t, ctx, ns, "platform-http-server"), TestTimeoutShort).ShouldNot(BeNil()) + + // IMPORTANT: The Deployment UID must not change, otherwise we won't honor rolling upgrades as we delete and create a + // new Deployment for every Integration change. + g.Consistently(DeploymentUID(t, ctx, ns, "platform-http-server"), 3*time.Second, 1*time.Minute). + Should(Equal(genOneDeploymentUID)) }) }) } diff --git a/e2e/knative/gc_test.go b/e2e/knative/gc_test.go deleted file mode 100644 index 0430d639b7..0000000000 --- a/e2e/knative/gc_test.go +++ /dev/null @@ -1,64 +0,0 @@ -//go:build integration -// +build integration - -// To enable compilation of this file in Goland, go to "Settings -> Go -> Vendoring & Build Tags -> Custom Tags" and add "integration" - -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package knative - -import ( - "context" - "testing" - - v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" - corev1 "k8s.io/api/core/v1" - - . "github.com/apache/camel-k/v2/e2e/support" - . "github.com/onsi/gomega" -) - -func TestGarbageCollectResources(t *testing.T) { - WithNewTestNamespace(t, func(ctx context.Context, g *WithT, ns string) { - integration := "platform-http-server" - g.Expect(KamelRun(t, ctx, ns, "files/PlatformHttpServer.java", "-t", "knative-service.enabled=false").Execute()).To(Succeed()) - g.Eventually(IntegrationPodPhase(t, ctx, ns, integration), TestTimeoutLong).Should(Equal(corev1.PodRunning)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, integration, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) - - g.Eventually(KnativeService(t, ctx, ns, integration), TestTimeoutMedium).Should(BeNil()) - g.Eventually(ServiceType(t, ctx, ns, integration), TestTimeoutMedium).Should(Equal(corev1.ServiceTypeClusterIP)) - - // Update integration and enable knative service trait - existing arbitrary service should be garbage collected - g.Expect(KamelRun(t, ctx, ns, "files/PlatformHttpServer.java").Execute()).To(Succeed()) - - g.Eventually(KnativeService(t, ctx, ns, integration), TestTimeoutShort).ShouldNot(BeNil()) - g.Eventually(ServiceType(t, ctx, ns, integration), TestTimeoutShort).Should(Equal(corev1.ServiceTypeExternalName)) - - g.Eventually(IntegrationPodPhase(t, ctx, ns, integration), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, integration, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) - - // Disable knative service trait again - this time knative service should be garbage collected - g.Expect(KamelRun(t, ctx, ns, "files/PlatformHttpServer.java", "-t", "knative-service.enabled=false").Execute()).To(Succeed()) - - g.Eventually(KnativeService(t, ctx, ns, integration), TestTimeoutMedium).Should(BeNil()) - g.Eventually(ServiceType(t, ctx, ns, integration), TestTimeoutMedium).Should(Equal(corev1.ServiceTypeClusterIP)) - - g.Eventually(IntegrationPodPhase(t, ctx, ns, integration), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, integration, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) - }) -} diff --git a/e2e/support/test_support.go b/e2e/support/test_support.go index a1fceebaf2..54d58fc631 100644 --- a/e2e/support/test_support.go +++ b/e2e/support/test_support.go @@ -839,16 +839,6 @@ func Service(t *testing.T, ctx context.Context, ns string, name string) func() * } } -func ServiceType(t *testing.T, ctx context.Context, ns string, name string) func() corev1.ServiceType { - return func() corev1.ServiceType { - svc := Service(t, ctx, ns, name)() - if svc == nil { - return "" - } - return svc.Spec.Type - } -} - // ServicesByType Find the service in the given namespace with the given type func ServicesByType(t *testing.T, ctx context.Context, ns string, svcType corev1.ServiceType) func() []corev1.Service { return func() []corev1.Service { @@ -1850,6 +1840,12 @@ func DeploymentCondition(t *testing.T, ctx context.Context, ns string, name stri } } +func DeploymentUID(t *testing.T, ctx context.Context, ns string, name string) func() string { + return func() string { + return string(Deployment(t, ctx, ns, name)().GetUID()) + } +} + func Build(t *testing.T, ctx context.Context, ns, name string) func() *v1.Build { return func() *v1.Build { build := v1.NewBuild(ns, name) diff --git a/pkg/apis/camel/v1/trait/service.go b/pkg/apis/camel/v1/trait/service.go index d16baae425..4452576d98 100644 --- a/pkg/apis/camel/v1/trait/service.go +++ b/pkg/apis/camel/v1/trait/service.go @@ -20,6 +20,8 @@ package trait // The Service trait exposes the integration with a Service resource so that it can be accessed by other applications // (or integrations) in the same namespace. // +// NOTE: this trait is automatically disabled if the Knative Service trait is enabled. +// // It's enabled by default if the integration depends on a Camel component that can expose a HTTP endpoint. // // +camel-k:trait=service.