chore(operator): make use of status.jobName when searching for job in…

… KeptnTask controller (keptn#1436) Signed-off-by: odubajDT <[email protected]>
StackScribe · May 23, 2023 · 28dd6b7 · 28dd6b7
1 parent 6035e55
commit 28dd6b7
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 51 deletions.
diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_test.go b/operator/apis/lifecycle/v1alpha3/keptntask_test.go
@@ -95,8 +95,8 @@ func TestKeptnTask(t *testing.T) {
 		"keptn.sh/app":       "app",
 		"keptn.sh/task-name": "task",
 		"keptn.sh/version":   "appversion",
-		"label1":             "label2",
-	}, task.CreateKeptnLabels())
+		"annotation1":        "annotation2",
+	}, task.CreateKeptnAnnotations())
 
 	task.Spec.Workload = "workload"
 	task.Spec.WorkloadVersion = "workloadversion"
@@ -106,8 +106,8 @@ func TestKeptnTask(t *testing.T) {
 		"keptn.sh/workload":  "workload",
 		"keptn.sh/task-name": "task",
 		"keptn.sh/version":   "workloadversion",
-		"label1":             "label2",
-	}, task.CreateKeptnLabels())
+		"annotation1":        "annotation2",
+	}, task.CreateKeptnAnnotations())
 
 	require.Equal(t, []attribute.KeyValue{
 		common.AppName.String("app"),

diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_types.go b/operator/apis/lifecycle/v1alpha3/keptntask_types.go
@@ -206,16 +206,16 @@ func (t KeptnTask) SetSpanAttributes(span trace.Span) {
 	span.SetAttributes(t.GetSpanAttributes()...)
 }
 
-func (t KeptnTask) CreateKeptnLabels() map[string]string {
+func (t KeptnTask) CreateKeptnAnnotations() map[string]string {
 	if t.Spec.Workload != "" {
-		return common.MergeMaps(t.Labels, map[string]string{
+		return common.MergeMaps(t.Annotations, map[string]string{
 			common.AppAnnotation:      t.Spec.AppName,
 			common.WorkloadAnnotation: t.Spec.Workload,
 			common.VersionAnnotation:  t.Spec.WorkloadVersion,
 			common.TaskNameAnnotation: t.Name,
 		})
 	}
-	return common.MergeMaps(t.Labels, map[string]string{
+	return common.MergeMaps(t.Annotations, map[string]string{
 		common.AppAnnotation:      t.Spec.AppName,
 		common.VersionAnnotation:  t.Spec.AppVersion,
 		common.TaskNameAnnotation: t.Name,

diff --git a/operator/controllers/lifecycle/keptntask/controller.go b/operator/controllers/lifecycle/keptntask/controller.go
@@ -18,14 +18,12 @@ package keptntask
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
 	apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3/common"
 	controllercommon "github.com/keptn/lifecycle-toolkit/operator/controllers/common"
-	controllererrors "github.com/keptn/lifecycle-toolkit/operator/controllers/errors"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/metric"
@@ -91,14 +89,14 @@ func (r *KeptnTaskReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 	}(task)
 
-	jobExists, err := r.JobExists(ctx, *task, req.Namespace)
-	if err != nil {
+	job, err := r.getJob(ctx, task.Status.JobName, req.Namespace)
+	if err != nil && !errors.IsNotFound(err) {
 		r.Log.Error(err, "Could not check if job is running")
 		span.SetStatus(codes.Error, err.Error())
 		return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
 	}
 
-	if !jobExists {
+	if job == nil {
 		err = r.createJob(ctx, req, task)
 		if err != nil {
 			span.SetStatus(codes.Error, err.Error())
@@ -146,29 +144,6 @@ func (r *KeptnTaskReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *KeptnTaskReconciler) JobExists(ctx context.Context, task klcv1alpha3.KeptnTask, namespace string) (bool, error) {
-	jobList := &batchv1.JobList{}
-
-	jobLabels := client.MatchingLabels{}
-	for k, v := range task.CreateKeptnLabels() {
-		jobLabels[k] = v
-	}
-
-	if len(jobLabels) == 0 {
-		return false, fmt.Errorf(controllererrors.ErrNoLabelsFoundTask, task.Name)
-	}
-
-	if err := r.Client.List(ctx, jobList, client.InNamespace(namespace), jobLabels); err != nil {
-		return false, err
-	}
-
-	if len(jobList.Items) > 0 {
-		return true, nil
-	}
-
-	return false, nil
-}
-
 func (r *KeptnTaskReconciler) getTracer() controllercommon.ITracer {
 	return r.TracerFactory.GetTracer(traceComponentName)
 }
diff --git a/operator/controllers/lifecycle/keptntask/function_utils.go b/operator/controllers/lifecycle/keptntask/function_utils.go
@@ -30,8 +30,8 @@ func (r *KeptnTaskReconciler) generateFunctionJob(task *klcv1alpha3.KeptnTask, p
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        jobId,
 			Namespace:   task.Namespace,
-			Labels:      task.CreateKeptnLabels(),
-			Annotations: task.Annotations,
+			Labels:      task.Labels,
+			Annotations: task.CreateKeptnAnnotations(),
 		},
 		Spec: batchv1.JobSpec{
 			Template: corev1.PodTemplateSpec{

diff --git a/operator/controllers/lifecycle/keptntask/job_utils.go b/operator/controllers/lifecycle/keptntask/job_utils.go
@@ -105,7 +105,7 @@ func (r *KeptnTaskReconciler) getJob(ctx context.Context, jobName string, namesp
 	job := &batchv1.Job{}
 	err := r.Client.Get(ctx, types.NamespacedName{Name: jobName, Namespace: namespace}, job)
 	if err != nil {
-		return job, err
+		return nil, err
 	}
 	return job, nil
 }

diff --git a/operator/controllers/lifecycle/keptntask/job_utils_test.go b/operator/controllers/lifecycle/keptntask/job_utils_test.go
@@ -80,14 +80,14 @@ func TestKeptnTaskReconciler_createJob(t *testing.T) {
 	require.Len(t, resultingJob.Spec.Template.Spec.Containers, 1)
 	require.Len(t, resultingJob.Spec.Template.Spec.Containers[0].Env, 4)
 	require.Equal(t, map[string]string{
-		"label1":             "label2",
+		"label1": "label2",
+	}, resultingJob.Labels)
+	require.Equal(t, map[string]string{
+		"annotation1":        "annotation2",
 		"keptn.sh/app":       "my-app",
 		"keptn.sh/task-name": "my-task",
 		"keptn.sh/version":   "",
 		"keptn.sh/workload":  "my-workload",
-	}, resultingJob.Labels)
-	require.Equal(t, map[string]string{
-		"annotation1": "annotation2",
 	}, resultingJob.Annotations)
 }
 
@@ -154,14 +154,14 @@ func TestKeptnTaskReconciler_createJob_withTaskDefInDefaultNamespace(t *testing.
 	require.Len(t, resultingJob.Spec.Template.Spec.Containers, 1)
 	require.Len(t, resultingJob.Spec.Template.Spec.Containers[0].Env, 4)
 	require.Equal(t, map[string]string{
-		"label1":             "label2",
+		"label1": "label2",
+	}, resultingJob.Labels)
+	require.Equal(t, map[string]string{
+		"annotation1":        "annotation2",
 		"keptn.sh/app":       "my-app",
 		"keptn.sh/task-name": "my-task",
 		"keptn.sh/version":   "",
 		"keptn.sh/workload":  "my-workload",
-	}, resultingJob.Labels)
-	require.Equal(t, map[string]string{
-		"annotation1": "annotation2",
 	}, resultingJob.Annotations)
 }
 

diff --git a/operator/test/component/task/task_test.go b/operator/test/component/task/task_test.go
@@ -156,17 +156,17 @@ var _ = Describe("Task", Ordered, func() {
 				Expect(err).To(BeNil())
 
 				Expect(createdJob.Annotations).To(Equal(map[string]string{
-					"annotation1": "annotation2",
-				}))
-
-				Expect(createdJob.Labels).To(Equal(map[string]string{
+					"annotation1":        "annotation2",
 					"keptn.sh/task-name": task.Name,
 					"keptn.sh/version":   "",
 					"keptn.sh/workload":  "my-workload",
-					"label1":             "label2",
 					"keptn.sh/app":       "my-app",
 				}))
 
+				Expect(createdJob.Labels).To(Equal(map[string]string{
+					"label1": "label2",
+				}))
+
 				val, ok := createdJob.Spec.Template.Labels["label1"]
 				Expect(ok && val == "label2").To(BeTrue())