This is an automated email from the ASF dual-hosted git repository. cdeppisch pushed a commit to branch release-2.3.x in repository https://gitbox.apache.org/repos/asf/camel-k.git
The following commit(s) were added to refs/heads/release-2.3.x by this push: new 88f8545ab fix: Fix garbage collection trait 88f8545ab is described below commit 88f8545abf09df6b4c1cc928668e9aacef48409e Author: Christoph Deppisch <cdeppi...@redhat.com> AuthorDate: Fri Apr 12 17:03:46 2024 +0200 fix: Fix garbage collection trait - Run garbage collection trait also in Integration initialization phase - Fallback to minimal set of deletable types when auto discovery fails (e.g. in OpenShift) - Add some unit and E2E test --- e2e/common/config/config_test.go | 42 +++-- e2e/common/traits/gc_test.go | 61 +++++++ e2e/knative/files/PlatformHttpServer.java | 26 +++ e2e/knative/gc_test.go | 65 ++++++++ pkg/apis/camel/v1/integration_types_support.go | 3 + pkg/trait/gc.go | 105 ++++++++++-- pkg/trait/gc_test.go | 218 +++++++++++++++++++++++++ pkg/trait/trait.go | 7 +- pkg/util/test/client.go | 25 +++ 9 files changed, 524 insertions(+), 28 deletions(-) diff --git a/e2e/common/config/config_test.go b/e2e/common/config/config_test.go index c60267d75..58a3578a9 100644 --- a/e2e/common/config/config_test.go +++ b/e2e/common/config/config_test.go @@ -73,7 +73,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Property from ConfigMap", func(t *testing.T) { var cmData = make(map[string]string) cmData["my.message"] = "my-configmap-property-value" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-property", cmData) + err := CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-property", cmData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/property-route.groovy", "-p", "configmap:my-cm-test-property").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "property-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -85,7 +86,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Property from ConfigMap as property file", func(t *testing.T) { var cmData = make(map[string]string) cmData["my.properties"] = "my.message=my-configmap-property-entry" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-properties", cmData) + err := CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-properties", cmData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/property-route.groovy", "-p", "configmap:my-cm-test-properties").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "property-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -97,7 +99,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Property from Secret", func(t *testing.T) { var secData = make(map[string]string) secData["my.message"] = "my-secret-property-value" - CreatePlainTextSecret(t, ctx, ns, "my-sec-test-property", secData) + err := CreatePlainTextSecret(t, ctx, ns, "my-sec-test-property", secData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/property-route.groovy", "-p", "secret:my-sec-test-property").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "property-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -109,7 +112,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Property from Secret as property file", func(t *testing.T) { var secData = make(map[string]string) secData["my.properties"] = "my.message=my-secret-property-entry" - CreatePlainTextSecret(t, ctx, ns, "my-sec-test-properties", secData) + err := CreatePlainTextSecret(t, ctx, ns, "my-sec-test-properties", secData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/property-route.groovy", "--name", "property-route-secret", "-p", "secret:my-sec-test-properties").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "property-route-secret"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -120,7 +124,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Property from Secret inlined", func(t *testing.T) { var secData = make(map[string]string) secData["my-message"] = "my-secret-external-value" - CreatePlainTextSecret(t, ctx, ns, "my-sec-inlined", secData) + err := CreatePlainTextSecret(t, ctx, ns, "my-sec-inlined", secData) + g.Expect(err).To(BeNil()) // TODO: remove jvm.options trait as soon as CAMEL-20054 gets fixed g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/property-secret-route.groovy", "-t", "mount.configs=secret:my-sec-inlined", "-t", "jvm.options=-Dcamel.k.mount-path.secrets=/etc/camel/conf.d/_secrets").Execute()).To(Succeed()) @@ -145,13 +150,15 @@ func TestRunConfigExamples(t *testing.T) { // Store a configmap on the cluster var cmData = make(map[string]string) cmData["my-configmap-key"] = "my-configmap-content" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm", cmData) + err := CreatePlainTextConfigmap(t, ctx, ns, "my-cm", cmData) + g.Expect(err).To(BeNil()) // Store a configmap with multiple values var cmDataMulti = make(map[string]string) cmDataMulti["my-configmap-key"] = "should-not-see-it" cmDataMulti["my-configmap-key-2"] = "my-configmap-content-2" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-multi", cmDataMulti) + err = CreatePlainTextConfigmap(t, ctx, ns, "my-cm-multi", cmDataMulti) + g.Expect(err).To(BeNil()) t.Run("Config configmap", func(t *testing.T) { g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/config-configmap-route.groovy", "--config", "configmap:my-cm").Execute()).To(Succeed()) @@ -192,7 +199,8 @@ func TestRunConfigExamples(t *testing.T) { // Store a configmap as property file var cmDataProps = make(map[string]string) cmDataProps["my.properties"] = "my.key.1=hello\nmy.key.2=world" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-properties", cmDataProps) + err = CreatePlainTextConfigmap(t, ctx, ns, "my-cm-properties", cmDataProps) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/config-configmap-properties-route.groovy", "--config", "configmap:my-cm-properties").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "config-configmap-properties-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -205,13 +213,15 @@ func TestRunConfigExamples(t *testing.T) { // Store a secret on the cluster var secData = make(map[string]string) secData["my-secret-key"] = "very top secret" - CreatePlainTextSecret(t, ctx, ns, "my-sec", secData) + err = CreatePlainTextSecret(t, ctx, ns, "my-sec", secData) + g.Expect(err).To(BeNil()) // Store a secret with multi values var secDataMulti = make(map[string]string) secDataMulti["my-secret-key"] = "very top secret" secDataMulti["my-secret-key-2"] = "even more secret" - CreatePlainTextSecret(t, ctx, ns, "my-sec-multi", secDataMulti) + err = CreatePlainTextSecret(t, ctx, ns, "my-sec-multi", secDataMulti) + g.Expect(err).To(BeNil()) t.Run("Config secret", func(t *testing.T) { g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/config-secret-route.groovy", "--config", "secret:my-sec").Execute()).To(Succeed()) @@ -277,7 +287,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Build time property from ConfigMap", func(t *testing.T) { var cmData = make(map[string]string) cmData["quarkus.application.name"] = "my-cool-application" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-build-property", cmData) + err = CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-build-property", cmData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/build-property-file-route.groovy", "--build-property", "configmap:my-cm-test-build-property").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "build-property-file-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -289,7 +300,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Build time property from ConfigMap as property file", func(t *testing.T) { var cmData = make(map[string]string) cmData["my.properties"] = "quarkus.application.name=my-super-cool-application" - CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-build-properties", cmData) + err = CreatePlainTextConfigmap(t, ctx, ns, "my-cm-test-build-properties", cmData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/build-property-file-route.groovy", "--name", "build-property-file-route-cm", "--build-property", "configmap:my-cm-test-build-properties").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "build-property-file-route-cm"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -301,7 +313,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Build time property from Secret", func(t *testing.T) { var secData = make(map[string]string) secData["quarkus.application.name"] = "my-great-application" - CreatePlainTextSecret(t, ctx, ns, "my-sec-test-build-property", secData) + err = CreatePlainTextSecret(t, ctx, ns, "my-sec-test-build-property", secData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/build-property-file-route.groovy", "--build-property", "secret:my-sec-test-build-property").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "build-property-file-route"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -313,7 +326,8 @@ func TestRunConfigExamples(t *testing.T) { t.Run("Build time property from Secret as property file", func(t *testing.T) { var secData = make(map[string]string) secData["my.properties"] = "quarkus.application.name=my-awsome-application" - CreatePlainTextSecret(t, ctx, ns, "my-sec-test-build-properties", secData) + err = CreatePlainTextSecret(t, ctx, ns, "my-sec-test-build-properties", secData) + g.Expect(err).To(BeNil()) g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "./files/build-property-file-route.groovy", "--name", "build-property-file-route-secret", "--build-property", "secret:my-sec-test-build-properties").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, "build-property-file-route-secret"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) diff --git a/e2e/common/traits/gc_test.go b/e2e/common/traits/gc_test.go new file mode 100644 index 000000000..6709181f1 --- /dev/null +++ b/e2e/common/traits/gc_test.go @@ -0,0 +1,61 @@ +//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 traits + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + + . "github.com/apache/camel-k/v2/e2e/support" + v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + corev1 "k8s.io/api/core/v1" +) + +func TestGarbageCollectorTrait(t *testing.T) { + t.Parallel() + + WithNewTestNamespace(t, func(ctx context.Context, g *WithT, ns string) { + operatorID := "camel-k-traits-gc" + g.Expect(CopyCamelCatalog(t, ctx, ns, operatorID)).To(Succeed()) + g.Expect(CopyIntegrationKits(t, ctx, ns, operatorID)).To(Succeed()) + g.Expect(KamelInstallWithID(t, ctx, operatorID, ns)).To(Succeed()) + + g.Eventually(SelectedPlatformPhase(t, ctx, ns, operatorID), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady)) + + t.Run("Delete outdated resources", func(t *testing.T) { + g.Expect(KamelRunWithID(t, ctx, operatorID, 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()) + + // Update integration and disable service trait - existing service should be garbage collected + g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/PlatformHttpServer.java", "-t", "service.enabled=false").Execute()).To(Succeed()) + + g.Eventually(ServicesByType(t, ctx, ns, corev1.ServiceTypeClusterIP), TestTimeoutLong).Should(BeEmpty()) + + g.Expect(Kamel(t, ctx, "delete", "--all", "-n", ns).Execute()).To(Succeed()) + }) + }) +} diff --git a/e2e/knative/files/PlatformHttpServer.java b/e2e/knative/files/PlatformHttpServer.java new file mode 100644 index 000000000..31d8d19f3 --- /dev/null +++ b/e2e/knative/files/PlatformHttpServer.java @@ -0,0 +1,26 @@ +/* + * 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. + */ + +import org.apache.camel.builder.RouteBuilder; + +public class PlatformHttpServer extends RouteBuilder { + @Override + public void configure() throws Exception { + from("platform-http:/hello?httpMethodRestrict=GET") + .setBody(simple("Hello ${header.name}")); + } +} diff --git a/e2e/knative/gc_test.go b/e2e/knative/gc_test.go new file mode 100644 index 000000000..b5df6dc3c --- /dev/null +++ b/e2e/knative/gc_test.go @@ -0,0 +1,65 @@ +//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 ( + v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + corev1 "k8s.io/api/core/v1" + "testing" + + . "github.com/apache/camel-k/v2/e2e/support" + . "github.com/onsi/gomega" +) + +func TestGarbageCollectResources(t *testing.T) { + ctx := TestContext() + g := NewWithT(t) + + integration := "platform-http-server" + g.Expect(KamelRunWithID(t, ctx, operatorID, 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(KamelRunWithID(t, ctx, operatorID, 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(KamelRunWithID(t, ctx, operatorID, 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)) + + g.Expect(Kamel(t, ctx, "delete", "--all", "-n", ns).Execute()).To(Succeed()) +} diff --git a/pkg/apis/camel/v1/integration_types_support.go b/pkg/apis/camel/v1/integration_types_support.go index 3705c5a05..ca0de0626 100644 --- a/pkg/apis/camel/v1/integration_types_support.go +++ b/pkg/apis/camel/v1/integration_types_support.go @@ -28,6 +28,9 @@ import ( // IntegrationLabel is used to tag k8s object created by a given Integration. const IntegrationLabel = "camel.apache.org/integration" +// IntegrationGenerationLabel is used to check on outdated integration resources that can be removed by garbage collection. +const IntegrationGenerationLabel = "camel.apache.org/generation" + // IntegrationSyntheticLabel is used to tag k8s synthetic Integrations. const IntegrationSyntheticLabel = "camel.apache.org/is-synthetic" diff --git a/pkg/trait/gc.go b/pkg/trait/gc.go index 5598c1c70..e3e5e79e8 100644 --- a/pkg/trait/gc.go +++ b/pkg/trait/gc.go @@ -20,6 +20,7 @@ package trait import ( "context" "fmt" + "maps" "strconv" "strings" "sync" @@ -27,7 +28,11 @@ import ( "golang.org/x/time/rate" + appsv1 "k8s.io/api/apps/v1" authorization "k8s.io/api/authorization/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -42,12 +47,59 @@ import ( v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" traitv1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1/trait" "github.com/apache/camel-k/v2/pkg/util" + "github.com/apache/camel-k/v2/pkg/util/log" ) var ( - lock sync.Mutex - rateLimiter = rate.NewLimiter(rate.Every(time.Minute), 1) - collectableGVKs = make(map[schema.GroupVersionKind]struct{}) + lock sync.Mutex + rateLimiter = rate.NewLimiter(rate.Every(time.Minute), 1) + collectableGVKs = make(map[schema.GroupVersionKind]struct{}) + defaultDeletableTypes = map[schema.GroupVersionKind]struct{}{ + { + Kind: "ConfigMap", + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + }: {}, + { + Kind: "Deployment", + Group: appsv1.SchemeGroupVersion.Group, + Version: appsv1.SchemeGroupVersion.Version, + }: {}, + { + Kind: "Secret", + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + }: {}, + { + Kind: "Service", + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + }: {}, + { + Kind: "CronJob", + Group: batchv1.SchemeGroupVersion.Group, + Version: batchv1.SchemeGroupVersion.Version, + }: {}, + { + Kind: "Job", + Group: batchv1.SchemeGroupVersion.Group, + Version: batchv1.SchemeGroupVersion.Version, + }: {}, + } + deletableTypesByProfile = map[v1.TraitProfile]map[schema.GroupVersionKind]struct{}{ + v1.TraitProfileKnative: { + schema.GroupVersionKind{ + Kind: "Service", + Group: "serving.knative.dev", + Version: "v1", + }: {}, + schema.GroupVersionKind{ + Kind: "Trigger", + Group: "eventing.knative.dev", + Version: "v1", + }: {}, + }, + } ) type gcTrait struct { @@ -73,7 +125,7 @@ func (t *gcTrait) Configure(e *Environment) (bool, *TraitCondition, error) { } func (t *gcTrait) Apply(e *Environment) error { - if e.IntegrationInRunningPhases() && e.Integration.GetGeneration() > 1 { + if e.Integration.GetGeneration() > 1 { // Register a post action that deletes the existing resources that are labelled // with the previous integration generation(s). // We make the assumption generation is a monotonically increasing strictly positive integer, @@ -88,12 +140,12 @@ func (t *gcTrait) Apply(e *Environment) error { e.PostProcessors = append(e.PostProcessors, func(env *Environment) error { generation := strconv.FormatInt(env.Integration.GetGeneration(), 10) env.Resources.VisitMetaObject(func(resource metav1.Object) { - labels := resource.GetLabels() + resourceLabels := resource.GetLabels() // Label the resource with the current integration generation - labels["camel.apache.org/generation"] = generation + resourceLabels[v1.IntegrationGenerationLabel] = generation // Make sure the integration label is set - labels[v1.IntegrationLabel] = env.Integration.Name - resource.SetLabels(labels) + resourceLabels[v1.IntegrationLabel] = env.Integration.Name + resource.SetLabels(resourceLabels) }) return nil }) @@ -107,8 +159,18 @@ func (t *gcTrait) garbageCollectResources(e *Environment) error { return fmt.Errorf("cannot discover GVK types: %w", err) } + profile := e.DetermineProfile() + if profileDeletableTypes, ok := deletableTypesByProfile[profile]; ok { + // copy profile related deletable types if not already present + for key, value := range profileDeletableTypes { + if _, found := deletableGVKs[key]; !found { + deletableGVKs[key] = value + } + } + } + integration, _ := labels.NewRequirement(v1.IntegrationLabel, selection.Equals, []string{e.Integration.Name}) - generation, err := labels.NewRequirement("camel.apache.org/generation", selection.LessThan, []string{strconv.FormatInt(e.Integration.GetGeneration(), 10)}) + generation, err := labels.NewRequirement(v1.IntegrationGenerationLabel, selection.LessThan, []string{strconv.FormatInt(e.Integration.GetGeneration(), 10)}) if err != nil { return fmt.Errorf("cannot determine generation requirement: %w", err) } @@ -172,10 +234,14 @@ func (t *gcTrait) getDeletableTypes(e *Environment) (map[schema.GroupVersionKind lock.Lock() defer lock.Unlock() + // Return a fresh map even when returning cached collectables + GVKs := make(map[schema.GroupVersionKind]struct{}) + // Rate limit to avoid Discovery and SelfSubjectRulesReview requests at every reconciliation. if !rateLimiter.Allow() { // Return the cached set of garbage collectable GVKs. - return collectableGVKs, nil + maps.Copy(GVKs, collectableGVKs) + return GVKs, nil } // We rely on the discovery API to retrieve all the resources GVK, @@ -204,7 +270,6 @@ func (t *gcTrait) getDeletableTypes(e *Environment) (map[schema.GroupVersionKind return nil, err } - GVKs := make(map[schema.GroupVersionKind]struct{}) for _, APIResourceList := range APIResourceLists { for _, resource := range APIResourceList.APIResources { resourceGroup := resource.Group @@ -233,7 +298,21 @@ func (t *gcTrait) getDeletableTypes(e *Environment) (map[schema.GroupVersionKind } } } - collectableGVKs = GVKs - return collectableGVKs, nil + if len(GVKs) == 0 { + // Auto discovery of deletable types has no results (probably an error) + // Make sure to at least use a minimal set of deletable types for garbage collection + t.L.ForIntegration(e.Integration).Debugf("Auto discovery of deletable types returned no results. " + + "Using default minimal set of deletable types for garbage collection") + maps.Copy(GVKs, defaultDeletableTypes) + } + + collectableGVKs = make(map[schema.GroupVersionKind]struct{}) + maps.Copy(collectableGVKs, GVKs) + + for gvk := range GVKs { + log.Debugf("Found deletable type: %s", gvk.String()) + } + + return GVKs, nil } diff --git a/pkg/trait/gc_test.go b/pkg/trait/gc_test.go index 406b00e45..fa21a8531 100644 --- a/pkg/trait/gc_test.go +++ b/pkg/trait/gc_test.go @@ -18,15 +18,23 @@ limitations under the License. package trait import ( + "context" + "strconv" "testing" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + "github.com/apache/camel-k/v2/pkg/util/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" + ctrl "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" ) func TestConfigureGCTraitDoesSucceed(t *testing.T) { @@ -89,6 +97,215 @@ func TestApplyGCTraitDuringInitializationPhaseSkipPostActions(t *testing.T) { assert.Len(t, environment.PostActions, 0) } +func TestGetDefaultMinimalGarbageCollectableTypes(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + + gcTrait.Client, _ = test.NewFakeClient() + environment.Client = gcTrait.Client + + deletableTypes, err := gcTrait.getDeletableTypes(environment) + + require.NoError(t, err) + assert.Len(t, deletableTypes, 6) +} + +func TestGarbageCollectResources(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + + deployment := getIntegrationDeployment(environment.Integration) + deployment.Labels[v1.IntegrationGenerationLabel] = "1" + gcTrait.Client, _ = test.NewFakeClient(deployment) + + environment.Client = gcTrait.Client + + resourceDeleted := false + fakeClient := gcTrait.Client.(*test.FakeClient) //nolint + fakeClient.Intercept(&interceptor.Funcs{ + Delete: func(ctx context.Context, client ctrl.WithWatch, obj ctrl.Object, opts ...ctrl.DeleteOption) error { + assert.Equal(t, environment.Integration.Name, obj.GetName()) + assert.Equal(t, "Deployment", obj.GetObjectKind().GroupVersionKind().Kind) + resourceDeleted = true + return nil + }, + }) + err := gcTrait.garbageCollectResources(environment) + + require.NoError(t, err) + assert.True(t, resourceDeleted) +} + +func TestGarbageCollectPreserveResourcesWithSameGeneration(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + + deployment := getIntegrationDeployment(environment.Integration) + gcTrait.Client, _ = test.NewFakeClient(deployment) + + environment.Client = gcTrait.Client + + resourceDeleted := false + fakeClient := gcTrait.Client.(*test.FakeClient) //nolint + fakeClient.Intercept(&interceptor.Funcs{ + Delete: func(ctx context.Context, client ctrl.WithWatch, obj ctrl.Object, opts ...ctrl.DeleteOption) error { + resourceDeleted = true + return nil + }, + }) + err := gcTrait.garbageCollectResources(environment) + + require.NoError(t, err) + assert.False(t, resourceDeleted) +} + +func TestGarbageCollectPreserveResourcesOwnerReferenceMismatch(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + + deployment := getIntegrationDeployment(environment.Integration) + deployment.Labels[v1.IntegrationGenerationLabel] = "1" + deployment.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: v1.SchemeGroupVersion.String(), + Kind: "Integration", + Name: "other-integration-owner", + }, + } + gcTrait.Client, _ = test.NewFakeClient(deployment) + + environment.Client = gcTrait.Client + + resourceDeleted := false + fakeClient := gcTrait.Client.(*test.FakeClient) //nolint + fakeClient.Intercept(&interceptor.Funcs{ + Delete: func(ctx context.Context, client ctrl.WithWatch, obj ctrl.Object, opts ...ctrl.DeleteOption) error { + resourceDeleted = true + return nil + }, + }) + err := gcTrait.garbageCollectResources(environment) + + require.NoError(t, err) + assert.False(t, resourceDeleted) +} + +func TestGarbageCollectKnativeServiceResources(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + environment.Integration.Spec.Profile = v1.TraitProfileKnative + + gcTrait.Client, _ = test.NewFakeClient(&servingv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: environment.Integration.Name, + Namespace: environment.Integration.Namespace, + Labels: map[string]string{ + v1.IntegrationLabel: environment.Integration.Name, + v1.IntegrationGenerationLabel: "1", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1.SchemeGroupVersion.String(), + Kind: "Integration", + Name: environment.Integration.Name, + }, + }, + }, + }) + + environment.Client = gcTrait.Client + + resourceDeleted := false + fakeClient := gcTrait.Client.(*test.FakeClient) //nolint + fakeClient.Intercept(&interceptor.Funcs{ + Delete: func(ctx context.Context, client ctrl.WithWatch, obj ctrl.Object, opts ...ctrl.DeleteOption) error { + assert.Equal(t, environment.Integration.Name, obj.GetName()) + assert.Equal(t, "Service", obj.GetObjectKind().GroupVersionKind().Kind) + assert.Equal(t, servingv1.SchemeGroupVersion, obj.GetObjectKind().GroupVersionKind().GroupVersion()) + resourceDeleted = true + return nil + }, + }) + err := gcTrait.garbageCollectResources(environment) + + require.NoError(t, err) + assert.True(t, resourceDeleted) +} + +func TestGarbageCollectKnativeTriggerResources(t *testing.T) { + gcTrait, environment := createNominalGCTest() + environment.Integration.Generation = 2 + environment.Integration.Spec.Profile = v1.TraitProfileKnative + + gcTrait.Client, _ = test.NewFakeClient(&eventingv1.Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: environment.Integration.Name, + Namespace: environment.Integration.Namespace, + Labels: map[string]string{ + v1.IntegrationLabel: environment.Integration.Name, + v1.IntegrationGenerationLabel: "1", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1.SchemeGroupVersion.String(), + Kind: "Integration", + Name: environment.Integration.Name, + }, + }, + }, + }) + + environment.Client = gcTrait.Client + + resourceDeleted := false + fakeClient := gcTrait.Client.(*test.FakeClient) //nolint + fakeClient.Intercept(&interceptor.Funcs{ + Delete: func(ctx context.Context, client ctrl.WithWatch, obj ctrl.Object, opts ...ctrl.DeleteOption) error { + assert.Equal(t, environment.Integration.Name, obj.GetName()) + assert.Equal(t, "Trigger", obj.GetObjectKind().GroupVersionKind().Kind) + assert.Equal(t, eventingv1.SchemeGroupVersion, obj.GetObjectKind().GroupVersionKind().GroupVersion()) + resourceDeleted = true + return nil + }, + }) + err := gcTrait.garbageCollectResources(environment) + + require.NoError(t, err) + assert.True(t, resourceDeleted) +} + +func getIntegrationDeployment(integration *v1.Integration) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: integration.Name, + Namespace: integration.Namespace, + Labels: map[string]string{ + v1.IntegrationLabel: integration.Name, + v1.IntegrationGenerationLabel: strconv.FormatInt(integration.Generation, 10), + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1.SchemeGroupVersion.String(), + Kind: "Integration", + Name: integration.Name, + }, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: new(int32), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: defaultContainerName, + }, + }, + }, + }, + }, + } +} + func createNominalGCTest() (*gcTrait, *Environment) { trait, _ := newGCTrait().(*gcTrait) trait.Enabled = pointer.Bool(true) @@ -98,6 +315,7 @@ func createNominalGCTest() (*gcTrait, *Environment) { Integration: &v1.Integration{ ObjectMeta: metav1.ObjectMeta{ Name: "integration-name", + Namespace: "namespace", Generation: 1, }, Status: v1.IntegrationStatus{ diff --git a/pkg/trait/trait.go b/pkg/trait/trait.go index 48e45b4d0..5d9076c13 100644 --- a/pkg/trait/trait.go +++ b/pkg/trait/trait.go @@ -74,14 +74,19 @@ func Apply(ctx context.Context, c client.Client, integration *v1.Integration, ki return nil, fmt.Errorf("error during trait customization: %w", err) } + postActionErrors := make([]error, 0) // execute post actions registered by traits for _, postAction := range environment.PostActions { err := postAction(environment) if err != nil { - return nil, fmt.Errorf("error executing post actions: %w", err) + postActionErrors = append(postActionErrors, err) } } + if len(postActionErrors) > 0 { + return nil, fmt.Errorf("error executing post actions - %d/%d failed: %s", len(postActionErrors), len(environment.PostActions), postActionErrors) + } + switch { case integration != nil: ilog.Debug("Applied traits to Integration", "integration", integration.Name, "namespace", integration.Namespace) diff --git a/pkg/util/test/client.go b/pkg/util/test/client.go index 6c45fc0e1..a821bf81d 100644 --- a/pkg/util/test/client.go +++ b/pkg/util/test/client.go @@ -39,12 +39,14 @@ import ( "k8s.io/client-go/kubernetes" fakeclientset "k8s.io/client-go/kubernetes/fake" clientscheme "k8s.io/client-go/kubernetes/scheme" + authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1" "k8s.io/client-go/rest" "k8s.io/client-go/scale" fakescale "k8s.io/client-go/scale/fake" "k8s.io/client-go/testing" controller "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" ) // NewFakeClient ---. @@ -142,6 +144,11 @@ type FakeClient struct { enabledOpenshift bool } +func (c *FakeClient) Intercept(intercept *interceptor.Funcs) { + cw := c.Client.(controller.WithWatch) // nolint: forcetypeassert + c.Client = interceptor.NewClient(cw, *intercept) +} + func (c *FakeClient) AddReactor(verb, resource string, reaction testing.ReactionFunc) { c.camel.AddReactor(verb, resource, reaction) } @@ -184,6 +191,14 @@ func (c *FakeClient) EnableOpenshiftDiscovery() { c.enabledOpenshift = true } +func (c *FakeClient) AuthorizationV1() authorizationv1.AuthorizationV1Interface { + return &FakeAuthorization{ + AuthorizationV1Interface: c.Interface.AuthorizationV1(), + disabledGroups: c.disabledGroups, + enabledOpenshift: c.enabledOpenshift, + } +} + func (c *FakeClient) Discovery() discovery.DiscoveryInterface { return &FakeDiscovery{ DiscoveryInterface: c.Interface.Discovery(), @@ -202,6 +217,16 @@ func (c *FakeClient) ScalesClient() (scale.ScalesGetter, error) { return c.scales, nil } +type FakeAuthorization struct { + authorizationv1.AuthorizationV1Interface + disabledGroups []string + enabledOpenshift bool +} + +func (f *FakeAuthorization) SelfSubjectRulesReviews() authorizationv1.SelfSubjectRulesReviewInterface { + return f.AuthorizationV1Interface.SelfSubjectRulesReviews() +} + type FakeDiscovery struct { discovery.DiscoveryInterface disabledGroups []string