From 28dd6b77c4cacd038539e30ac8275d6f63d39155 Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Tue, 23 May 2023 13:03:56 +0200 Subject: [PATCH] chore(operator): make use of status.jobName when searching for job in KeptnTask controller (#1436) Signed-off-by: odubajDT --- .../apis/lifecycle/v1alpha3/keptntask_test.go | 8 ++--- .../lifecycle/v1alpha3/keptntask_types.go | 6 ++-- .../lifecycle/keptntask/controller.go | 31 ++----------------- .../lifecycle/keptntask/function_utils.go | 4 +-- .../lifecycle/keptntask/job_utils.go | 2 +- .../lifecycle/keptntask/job_utils_test.go | 16 +++++----- operator/test/component/task/task_test.go | 10 +++--- 7 files changed, 26 insertions(+), 51 deletions(-) diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_test.go b/operator/apis/lifecycle/v1alpha3/keptntask_test.go index df541ca84f..ff3865c8a3 100644 --- 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 index 5112b468bc..182cd4512b 100644 --- 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 index 9a0482197d..c6100ccc17 100644 --- 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 index 877d0467a3..66682f44c1 100644 --- 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 index a57874927b..c9847c7a47 100644 --- 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 index 3ed1e2c4b5..ec242e07e1 100644 --- 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 index 9954f9f7d2..b86997c559 100644 --- 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())