Repository: apache/camel-k
Pull request #3327 (2022-06-02 08:09:29)

feat(gc): Skip GC for first integration generation
The GC trait relies on the generation metadata field of the integration, by propagating it to owned resources in a label, and listing the laters with label selection using the `<` selector operator. Assuming the generation field is a monotonically increasing strictly positive integer, it's safe to skip garbage collection on the first integration generation, i.e., when the generation field is less or equal to one, to reduce the overhead on the API server.

**Release Note**
```release-note
feat(gc): Skip GC for first integration generation
```



Patch:
diff --git a/pkg/trait/gc.go b/pkg/trait/gc.go
index a088367fb..ef20ae7b1 100644
--- a/pkg/trait/gc.go
+++ b/pkg/trait/gc.go
@@ -93,34 +93,30 @@ func (t *garbageCollectorTrait) Configure(e *Environment) (bool, error) {
 }
 
 func (t *garbageCollectorTrait) Apply(e *Environment) error {
-	switch e.Integration.Status.Phase {
-
-	case v1.IntegrationPhaseDeploying, v1.IntegrationPhaseRunning, v1.IntegrationPhaseError:
+	if e.IntegrationInRunningPhases() && e.Integration.GetGeneration() > 1 {
 		// Register a post action that deletes the existing resources that are labelled
-		// with the previous integration generations.
-		// TODO: this should be refined so that it's run when all the replicas for the newer generation
-		// are ready.
+		// with the previous integration generation(s).
+		// We make the assumption generation is a monotonically increasing strictly positive integer,
+		// in which case we can skip garbage collection on the first generation.
+		// TODO: this should be refined so that it's run when all the replicas for the newer generation are ready.
 		e.PostActions = append(e.PostActions, func(env *Environment) error {
 			return t.garbageCollectResources(env)
 		})
+	}
 
-		fallthrough
-
-	default:
-		// Register a post processor that adds the required labels to the new resources
-		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()
-				// Label the resource with the current integration generation
-				labels["camel.apache.org/generation"] = generation
-				// Make sure the integration label is set
-				labels[v1.IntegrationLabel] = env.Integration.Name
-				resource.SetLabels(labels)
-			})
-			return nil
+	// Register a post processor that adds the required labels to the new resources
+	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()
+			// Label the resource with the current integration generation
+			labels["camel.apache.org/generation"] = generation
+			// Make sure the integration label is set
+			labels[v1.IntegrationLabel] = env.Integration.Name
+			resource.SetLabels(labels)
 		})
-	}
+		return nil
+	})
 
 	return nil
 }
@@ -222,12 +218,12 @@ func (t *garbageCollectorTrait) getDeletableTypes(e *Environment) (map[schema.Gr
 
 	// Retrieve the permissions granted to the operator service account.
 	// We assume the operator has only to garbage collect the resources it has created.
-	srr := &authorization.SelfSubjectRulesReview{
+	ssrr := &authorization.SelfSubjectRulesReview{
 		Spec: authorization.SelfSubjectRulesReviewSpec{
 			Namespace: e.Integration.Namespace,
 		},
 	}
-	res, err := e.Client.AuthorizationV1().SelfSubjectRulesReviews().Create(e.Ctx, srr, metav1.CreateOptions{})
+	ssrr, err = e.Client.AuthorizationV1().SelfSubjectRulesReviews().Create(e.Ctx, ssrr, metav1.CreateOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -236,7 +232,7 @@ func (t *garbageCollectorTrait) getDeletableTypes(e *Environment) (map[schema.Gr
 	for _, APIResourceList := range APIResourceLists {
 		for _, resource := range APIResourceList.APIResources {
 		rule:
-			for _, rule := range res.Status.ResourceRules {
+			for _, rule := range ssrr.Status.ResourceRules {
 				if !util.StringSliceContainsAnyOf(rule.Verbs, "delete", "*") {
 					continue
 				}


Test patch:
diff --git a/pkg/trait/gc_test.go b/pkg/trait/gc_test.go
index 9e76d1056..db8322f4e 100644
--- a/pkg/trait/gc_test.go
+++ b/pkg/trait/gc_test.go
@@ -44,11 +44,22 @@ func TestConfigureDisabledGarbageCollectorTraitDoesNotSucceed(t *testing.T) {
 	assert.Nil(t, err)
 }
 
-func TestApplyGarbageCollectorTraitDoesSucceed(t *testing.T) {
+func TestApplyGarbageCollectorTraitFirstGenerationDoesSucceed(t *testing.T) {
 	gcTrait, environment := createNominalGarbageCollectorTest()
 
 	err := gcTrait.Apply(environment)
 
+	assert.Nil(t, err)
+	assert.Len(t, environment.PostProcessors, 1)
+	assert.Len(t, environment.PostActions, 0)
+}
+
+func TestApplyGarbageCollectorTraitNextGenerationDoesSucceed(t *testing.T) {
+	gcTrait, environment := createNominalGarbageCollectorTest()
+	environment.Integration.Generation = 2
+
+	err := gcTrait.Apply(environment)
+
 	assert.Nil(t, err)
 	assert.Len(t, environment.PostProcessors, 1)
 	assert.Len(t, environment.PostActions, 1)
@@ -73,7 +84,8 @@ func createNominalGarbageCollectorTest() (*garbageCollectorTrait, *Environment)
 		Catalog: NewCatalog(nil),
 		Integration: &v1.Integration{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: "integration-name",
+				Name:       "integration-name",
+				Generation: 1,
 			},
 			Status: v1.IntegrationStatus{
 				Phase: v1.IntegrationPhaseRunning,