fix(operator): avoid multiple creations of the same KeptnTask (#1676)

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

operator/controllers/common/taskhandler.go

@@ -91,7 +91,9 @@ func (r TaskHandler) ReconcileTasks(ctx context.Context, phaseCtx context.Contex
 				&taskStatus,
 			)
 			if err != nil {
-				return nil, summary, err
+				// log the error, but continue to proceed with other tasks that may be created
+				r.Log.Error(err, "Could not create task", "task", taskDefinitionName)
+				continue
 			}
 		} else {
 			r.handleTaskExists(

operator/controllers/common/taskhandler_test.go

@@ -83,10 +83,50 @@ func TestTaskHandler(t *testing.T) {
 			},
 			wantStatus:      nil,
 			wantSummary:     apicommon.StatusSummary{Total: 1, Pending: 0},
-			wantErr:         controllererrors.ErrCannotGetKeptnTaskDefinition,
+			wantErr:         nil,
 			getSpanCalls:    0,
 			unbindSpanCalls: 0,
 		},
+		{
+			name: "tasks not started - could not find taskDefinition of one task",
+			object: &v1alpha3.KeptnAppVersion{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "namespace",
+				},
+				Spec: v1alpha3.KeptnAppVersionSpec{
+					KeptnAppSpec: v1alpha3.KeptnAppSpec{
+						PreDeploymentTasks: []string{"task-def", "other-task-def"},
+					},
+				},
+			},
+			taskDef: &v1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: KLTNamespace,
+					Name:      "task-def",
+				},
+			},
+			taskObj: v1alpha3.KeptnTask{},
+			createAttr: CreateTaskAttributes{
+				SpanName: "",
+				Definition: v1alpha3.KeptnTaskDefinition{
+					ObjectMeta: v1.ObjectMeta{
+						Name: "task-def",
+					},
+				},
+				CheckType: apicommon.PreDeploymentCheckType,
+			},
+			wantStatus: []v1alpha3.ItemStatus{
+				{
+					DefinitionName: "task-def",
+					Status:         apicommon.StatePending,
+					Name:           "pre-task-def-",
+				},
+			},
+			wantSummary:     apicommon.StatusSummary{Total: 2, Pending: 1},
+			wantErr:         nil,
+			getSpanCalls:    1,
+			unbindSpanCalls: 0,
+		},
 		{
 			name: "task not started - taskDefinition in default KLT namespace",
 			object: &v1alpha3.KeptnAppVersion{

operator/controllers/lifecycle/keptnappversion/controller.go

@@ -167,7 +167,6 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		span.SetStatus(codes.Error, err.Error())
 		return ctrl.Result{Requeue: true}, err
 	}
-
 	// AppVersion is completed at this place
 
 	return r.finishKeptnAppVersionReconcile(ctx, appVersion, spanAppTrace)

operator/test/component/task/task_test.go

@@ -48,7 +48,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.JobName).To(Not(BeEmpty()))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 
 				createdJob := &batchv1.Job{}
 
@@ -101,7 +101,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.JobName).To(Not(BeEmpty()))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 
 				createdJob := &batchv1.Job{}
 
@@ -129,7 +129,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.Status).To(Equal(apicommon.StateSucceeded))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 			})
 			It("succeed task if taskDefiniton for Container is present in default KLT namespace", func() {
 				By("create default KLT namespace")
@@ -154,7 +154,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.JobName).To(Not(BeEmpty()))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 
 				createdJob := &batchv1.Job{}
 
@@ -182,7 +182,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.Status).To(Equal(apicommon.StateSucceeded))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 			})
 			It("should propagate labels and annotations to the job and job pod", func() {
 				taskDefinition = makeTaskDefinition(taskDefinitionName, namespace)
@@ -197,7 +197,7 @@ var _ = Describe("Task", Ordered, func() {
 					}, task)
 					g.Expect(err).To(BeNil())
 					g.Expect(task.Status.JobName).To(Not(BeEmpty()))
-				}, "10s").Should(Succeed())
+				}, "20s").Should(Succeed())
 
 				createdJob := &batchv1.Job{}
 

test/integration/app-one-taskdefinition-not-found/00-assert.yaml

@@ -0,0 +1,24 @@
+apiVersion: lifecycle.keptn.sh/v1alpha3
+kind: KeptnAppVersion
+metadata:
+  name: podtato-head-1.3-6b86b273
+status:
+  currentPhase: AppPreDeployTasks
+  postDeploymentEvaluationStatus: Pending
+  postDeploymentStatus: Pending
+  preDeploymentEvaluationStatus: Pending
+  preDeploymentStatus: Unknown
+  status: Progressing
+  workloadOverallStatus: Pending
+---
+apiVersion: lifecycle.keptn.sh/v1alpha3
+kind: KeptnTask
+spec:
+  app: podtato-head
+  appVersion: '1.3'
+  checkType: pre
+  retries: 10
+  taskDefinition: pre-task-timeout
+  timeout: 30s
+status:
+  status: Succeeded

test/integration/app-one-taskdefinition-not-found/00-install.yaml

@@ -0,0 +1,53 @@
+apiVersion: lifecycle.keptn.sh/v1alpha3
+kind: KeptnApp
+metadata:
+  name: podtato-head
+spec:
+  version: "1.3"
+  workloads:
+    - name: podtato-head-entry
+      version: 0.1.0
+  preDeploymentTasks:
+    - pre-task-timeout
+    - pre-task-notfound
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: podtato-head-entry
+  labels:
+    app: podtato-head
+spec:
+  selector:
+    matchLabels:
+      component: podtato-head-entry
+  template:
+    metadata:
+      labels:
+        component: podtato-head-entry
+      annotations:
+        keptn.sh/app: podtato-head
+        keptn.sh/workload: podtato-head-entry
+        keptn.sh/version: 0.1.0
+    spec:
+      terminationGracePeriodSeconds: 5
+      containers:
+        - name: server
+          image: ghcr.io/podtato-head/entry:latest
+          imagePullPolicy: Always
+          ports:
+            - containerPort: 9000
+          env:
+            - name: PODTATO_PORT
+              value: "9000"
+---
+apiVersion: lifecycle.keptn.sh/v1alpha3
+kind: KeptnTaskDefinition
+metadata:
+  name: pre-task-timeout
+spec:
+  function:
+    inline:
+      code: |
+        console.log('foo')
+  timeout: "30s"

test/integration/app-one-taskdefinition-not-found/00-teststep.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1
+kind: TestStep
+commands:
+  - script: kubectl annotate ns $NAMESPACE keptn.sh/lifecycle-toolkit='enabled'

test/integration/app-one-taskdefinition-not-found/01-teststep-verify-number-of-tasks.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1
+kind: TestStep
+commands:
+  - script: ../common-scripts/verify-number-of-resources.sh "keptntask" "1"

test/integration/common-scripts/verify-number-of-resources.sh

@@ -0,0 +1,26 @@
+#!/bin/bash
+
+ RETRY_COUNT=3
+ SLEEP_TIME=5
+ RESOURCE_TYPE=$1
+ EXPECTED_NUMBER=$2
+
+ for i in $(seq 1 $RETRY_COUNT); do
+     NR_RESOURCES=$(kubectl get ${RESOURCE_TYPE} -n ${NAMESPACE} --no-headers | wc -l)
+
+     if [[ ${NR_RESOURCES} -eq ${EXPECTED_NUMBER} ]]; then
+       echo "Found expected number of ${RESOURCE_TYPE}: ${EXPECTED_NUMBER}"
+       exit 0
+     else
+       echo "Number of ${RESOURCE_TYPE} is not equal to ${EXPECTED_NUMBER}"
+     fi
+
+     if [ "$i" -lt "$RETRY_COUNT" ]; then
+         echo "Sleeping for ${SLEEP_TIME} seconds before retrying..."
+         sleep ${SLEEP_TIME}
+     fi
+ done
+
+ echo "Retried ${RETRY_COUNT} times, but expected number of ${RESOURCE_TYPE} could not be verified. Exiting..."
+ exit 1
+

test/integration/simple-task/01-teststep.yaml

@@ -1,4 +1,4 @@
 apiVersion: kuttl.dev/v1
 kind: TestStep
 commands:
-  - script: ./verify-tasks.sh
+  - script: ../common-scripts/verify-number-of-resources.sh "job" "1"