From cfae93ef74816f39d69e202575b66648461ff271 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Wed, 26 Apr 2023 09:53:16 +0200 Subject: [PATCH 1/3] feat(operator): propagate KeptnTaskDefinition labels and annotations to Job Pods Signed-off-by: odubajDT --- .../apis/lifecycle/v1alpha3/common/common.go | 11 ++++ .../lifecycle/v1alpha3/common/common_test.go | 57 +++++++++++++++++++ .../v1alpha3/keptnappversion_test.go | 14 +++++ .../v1alpha3/keptnappversion_types.go | 6 +- .../apis/lifecycle/v1alpha3/keptntask_test.go | 14 +++++ .../lifecycle/v1alpha3/keptntask_types.go | 8 +-- .../v1alpha3/keptnworkloadinstance_test.go | 14 +++++ .../v1alpha3/keptnworkloadinstance_types.go | 6 +- .../lifecycle/keptntask/function_utils.go | 11 +++- .../lifecycle/keptntask/job_utils_test.go | 22 +++++++ 10 files changed, 152 insertions(+), 11 deletions(-) diff --git a/operator/apis/lifecycle/v1alpha3/common/common.go b/operator/apis/lifecycle/v1alpha3/common/common.go index 2d7c978d72..b199f07686 100644 --- a/operator/apis/lifecycle/v1alpha3/common/common.go +++ b/operator/apis/lifecycle/v1alpha3/common/common.go @@ -185,3 +185,14 @@ func GenerateEvaluationName(checkType CheckType, evalName string) string { randomId := rand.Intn(99_999-10_000) + 10000 return fmt.Sprintf("%s-%s-%d", checkType, TruncateString(evalName, 27), randomId) } + +func MergeMaps(m1 map[string]string, m2 map[string]string) map[string]string { + merged := make(map[string]string, len(m1)+len(m2)) + for k, v := range m1 { + merged[k] = v + } + for key, value := range m2 { + merged[key] = value + } + return merged +} diff --git a/operator/apis/lifecycle/v1alpha3/common/common_test.go b/operator/apis/lifecycle/v1alpha3/common/common_test.go index 035fc63e26..9f40c417e4 100644 --- a/operator/apis/lifecycle/v1alpha3/common/common_test.go +++ b/operator/apis/lifecycle/v1alpha3/common/common_test.go @@ -331,3 +331,60 @@ func Test_GenerateEvaluationName(t *testing.T) { }) } } + +func Test_MergeMaps(t *testing.T) { + tests := []struct { + In1 map[string]string + In2 map[string]string + Want map[string]string + }{ + { + In1: nil, + In2: nil, + Want: map[string]string{}, + }, + { + In1: nil, + In2: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + }, + Want: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + }, + }, + { + In1: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + }, + In2: nil, + Want: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + }, + }, + { + In1: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + }, + In2: map[string]string{ + "ll5": "ll6", + "ll7": "ll8", + }, + Want: map[string]string{ + "ll1": "ll2", + "ll3": "ll4", + "ll5": "ll6", + "ll7": "ll8", + }, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, MergeMaps(tt.In1, tt.In2), tt.Want) + }) + } +} diff --git a/operator/apis/lifecycle/v1alpha3/keptnappversion_test.go b/operator/apis/lifecycle/v1alpha3/keptnappversion_test.go index c537f6f5a9..e718dd9a16 100644 --- a/operator/apis/lifecycle/v1alpha3/keptnappversion_test.go +++ b/operator/apis/lifecycle/v1alpha3/keptnappversion_test.go @@ -182,6 +182,12 @@ func TestKeptnAppVersion(t *testing.T) { task := app.GenerateTask(KeptnTaskDefinition{ ObjectMeta: v1.ObjectMeta{ Name: "task-def", + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: KeptnTaskDefinitionSpec{ Timeout: v1.Duration{ @@ -203,6 +209,14 @@ func TestKeptnAppVersion(t *testing.T) { Retries: &retries, }, task.Spec) + require.Equal(t, map[string]string{ + "label1": "label2", + }, task.Labels) + + require.Equal(t, map[string]string{ + "annotation1": "annotation2", + }, task.Annotations) + evaluation := app.GenerateEvaluation(KeptnEvaluationDefinition{ ObjectMeta: v1.ObjectMeta{ Name: "eval-def", diff --git a/operator/apis/lifecycle/v1alpha3/keptnappversion_types.go b/operator/apis/lifecycle/v1alpha3/keptnappversion_types.go index 1b4bc38733..bdbfeec829 100644 --- a/operator/apis/lifecycle/v1alpha3/keptnappversion_types.go +++ b/operator/apis/lifecycle/v1alpha3/keptnappversion_types.go @@ -313,8 +313,10 @@ func (a KeptnAppVersion) GetVersion() string { func (a KeptnAppVersion) GenerateTask(taskDefinition KeptnTaskDefinition, checkType common.CheckType) KeptnTask { return KeptnTask{ ObjectMeta: metav1.ObjectMeta{ - Name: common.GenerateTaskName(checkType, taskDefinition.Name), - Namespace: a.Namespace, + Name: common.GenerateTaskName(checkType, taskDefinition.Name), + Namespace: a.Namespace, + Labels: taskDefinition.Labels, + Annotations: taskDefinition.Annotations, }, Spec: KeptnTaskSpec{ AppVersion: a.GetVersion(), diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_test.go b/operator/apis/lifecycle/v1alpha3/keptntask_test.go index 724b710ff3..df541ca84f 100644 --- a/operator/apis/lifecycle/v1alpha3/keptntask_test.go +++ b/operator/apis/lifecycle/v1alpha3/keptntask_test.go @@ -14,6 +14,12 @@ func TestKeptnTask(t *testing.T) { task := &KeptnTask{ ObjectMeta: metav1.ObjectMeta{ Name: "task", + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: KeptnTaskSpec{ AppName: "app", @@ -33,6 +39,12 @@ func TestKeptnTask(t *testing.T) { require.Equal(t, KeptnTask{ ObjectMeta: metav1.ObjectMeta{ Name: "task", + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: KeptnTaskSpec{ AppName: "app", @@ -83,6 +95,7 @@ func TestKeptnTask(t *testing.T) { "keptn.sh/app": "app", "keptn.sh/task-name": "task", "keptn.sh/version": "appversion", + "label1": "label2", }, task.CreateKeptnLabels()) task.Spec.Workload = "workload" @@ -93,6 +106,7 @@ func TestKeptnTask(t *testing.T) { "keptn.sh/workload": "workload", "keptn.sh/task-name": "task", "keptn.sh/version": "workloadversion", + "label1": "label2", }, task.CreateKeptnLabels()) require.Equal(t, []attribute.KeyValue{ diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_types.go b/operator/apis/lifecycle/v1alpha3/keptntask_types.go index c8cebf8409..15cce49204 100644 --- a/operator/apis/lifecycle/v1alpha3/keptntask_types.go +++ b/operator/apis/lifecycle/v1alpha3/keptntask_types.go @@ -168,18 +168,18 @@ func (t KeptnTask) SetSpanAttributes(span trace.Span) { func (t KeptnTask) CreateKeptnLabels() map[string]string { if t.Spec.Workload != "" { - return map[string]string{ + return common.MergeMaps(map[string]string{ common.AppAnnotation: t.Spec.AppName, common.WorkloadAnnotation: t.Spec.Workload, common.VersionAnnotation: t.Spec.WorkloadVersion, common.TaskNameAnnotation: t.Name, - } + }, t.Labels) } - return map[string]string{ + return common.MergeMaps(map[string]string{ common.AppAnnotation: t.Spec.AppName, common.VersionAnnotation: t.Spec.AppVersion, common.TaskNameAnnotation: t.Name, - } + }, t.Labels) } func (t KeptnTask) GetSpanAttributes() []attribute.KeyValue { diff --git a/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_test.go b/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_test.go index e9b440a5ff..551519c230 100644 --- a/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_test.go +++ b/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_test.go @@ -186,6 +186,12 @@ func TestKeptnWorkloadInstance(t *testing.T) { task := workload.GenerateTask(KeptnTaskDefinition{ ObjectMeta: v1.ObjectMeta{ Name: "task-def", + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: KeptnTaskDefinitionSpec{ Timeout: v1.Duration{ @@ -208,6 +214,14 @@ func TestKeptnWorkloadInstance(t *testing.T) { Retries: &retries, }, task.Spec) + require.Equal(t, map[string]string{ + "label1": "label2", + }, task.Labels) + + require.Equal(t, map[string]string{ + "annotation1": "annotation2", + }, task.Annotations) + evaluation := workload.GenerateEvaluation(KeptnEvaluationDefinition{ ObjectMeta: v1.ObjectMeta{ Name: "eval-def", diff --git a/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_types.go b/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_types.go index bfc48fb32b..3b03b64d9f 100644 --- a/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_types.go +++ b/operator/apis/lifecycle/v1alpha3/keptnworkloadinstance_types.go @@ -321,8 +321,10 @@ func (w KeptnWorkloadInstance) GetVersion() string { func (w KeptnWorkloadInstance) GenerateTask(taskDefinition KeptnTaskDefinition, checkType common.CheckType) KeptnTask { return KeptnTask{ ObjectMeta: metav1.ObjectMeta{ - Name: common.GenerateTaskName(checkType, taskDefinition.Name), - Namespace: w.Namespace, + Name: common.GenerateTaskName(checkType, taskDefinition.Name), + Namespace: w.Namespace, + Labels: taskDefinition.Labels, + Annotations: taskDefinition.Annotations, }, Spec: KeptnTaskSpec{ AppName: w.GetAppName(), diff --git a/operator/controllers/lifecycle/keptntask/function_utils.go b/operator/controllers/lifecycle/keptntask/function_utils.go index b71321dfc8..877d0467a3 100644 --- a/operator/controllers/lifecycle/keptntask/function_utils.go +++ b/operator/controllers/lifecycle/keptntask/function_utils.go @@ -28,12 +28,17 @@ func (r *KeptnTaskReconciler) generateFunctionJob(task *klcv1alpha3.KeptnTask, p jobId := fmt.Sprintf("klc-%s-%d", apicommon.TruncateString(task.Name, apicommon.MaxTaskNameLength), randomId) job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Name: jobId, - Namespace: task.Namespace, - Labels: task.CreateKeptnLabels(), + Name: jobId, + Namespace: task.Namespace, + Labels: task.CreateKeptnLabels(), + Annotations: task.Annotations, }, Spec: batchv1.JobSpec{ Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: task.Labels, + Annotations: task.Annotations, + }, Spec: corev1.PodSpec{ RestartPolicy: "OnFailure", }, diff --git a/operator/controllers/lifecycle/keptntask/job_utils_test.go b/operator/controllers/lifecycle/keptntask/job_utils_test.go index 0ca7216ac7..8ebd6ba9aa 100644 --- a/operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -78,6 +78,16 @@ func TestKeptnTaskReconciler_createJob(t *testing.T) { require.NotEmpty(t, resultingJob.OwnerReferences) 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", + "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) } func TestKeptnTaskReconciler_updateJob(t *testing.T) { @@ -158,6 +168,12 @@ func makeTask(name, namespace string, taskDefinitionName string) *klcv1alpha3.Ke ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: klcv1alpha3.KeptnTaskSpec{ Workload: "my-workload", @@ -173,6 +189,12 @@ func makeTaskDefinitionWithConfigmapRef(name, namespace, configMapName string) * ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: klcv1alpha3.KeptnTaskDefinitionSpec{ Function: klcv1alpha3.FunctionSpec{ From 7dfc8afd0ba24b2f3344d12eefd7ca6457326f7f Mon Sep 17 00:00:00 2001 From: odubajDT Date: Wed, 26 Apr 2023 10:43:50 +0200 Subject: [PATCH 2/3] introduce component test Signed-off-by: odubajDT --- operator/test/component/task/task_test.go | 52 +++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/operator/test/component/task/task_test.go b/operator/test/component/task/task_test.go index 877a21561b..6e1b435fb3 100644 --- a/operator/test/component/task/task_test.go +++ b/operator/test/component/task/task_test.go @@ -79,11 +79,57 @@ var _ = Describe("Task", Ordered, func() { g.Expect(task.Status.Status).To(Equal(apicommon.StateFailed)) }, "10s").Should(Succeed()) }) + It("should propagate labels and annotations to the job and job pod", func() { + By("Verifying that a job has been created") + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: task.Name, + }, task) + g.Expect(err).To(BeNil()) + g.Expect(task.Status.JobName).To(Not(BeEmpty())) + }, "10s").Should(Succeed()) + + createdJob := &batchv1.Job{} + + err := k8sClient.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: task.Status.JobName, + }, createdJob) + + Expect(err).To(BeNil()) + + Expect(createdJob.Annotations).To(Equal(map[string]string{ + "annotation1": "annotation2", + })) + + Expect(createdJob.Labels).To(Equal(map[string]string{ + "keptn.sh/task-name": task.Name, + "keptn.sh/version": "", + "keptn.sh/workload": "my-workload", + "label1": "label2", + "keptn.sh/app": "my-app", + })) + + val, ok := createdJob.Spec.Template.Labels["label1"] + Expect(ok && val == "label2").To(BeTrue()) + + val, ok = createdJob.Spec.Template.Annotations["annotation1"] + Expect(ok && val == "annotation2").To(BeTrue()) + }) AfterEach(func() { err := k8sClient.Delete(context.TODO(), taskDefinition) common.LogErrorIfPresent(err) err = k8sClient.Delete(context.TODO(), task) common.LogErrorIfPresent(err) + err = k8sClient.Delete(context.TODO(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: taskDefinition.Status.Function.ConfigMap, + Namespace: namespace, + }, + }) + common.LogErrorIfPresent(err) }) }) }) @@ -94,6 +140,12 @@ func makeTask(name string, namespace, taskDefinitionName string) *klcv1alpha3.Ke ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, + Labels: map[string]string{ + "label1": "label2", + }, + Annotations: map[string]string{ + "annotation1": "annotation2", + }, }, Spec: klcv1alpha3.KeptnTaskSpec{ Workload: "my-workload", From c18d0b47576a0c123c906a4644b86388227a03b9 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Wed, 26 Apr 2023 13:15:54 +0200 Subject: [PATCH 3/3] pr review Signed-off-by: odubajDT --- operator/apis/lifecycle/v1alpha3/common/common.go | 6 ++++-- operator/apis/lifecycle/v1alpha3/keptntask_types.go | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/operator/apis/lifecycle/v1alpha3/common/common.go b/operator/apis/lifecycle/v1alpha3/common/common.go index b199f07686..fe40028763 100644 --- a/operator/apis/lifecycle/v1alpha3/common/common.go +++ b/operator/apis/lifecycle/v1alpha3/common/common.go @@ -186,10 +186,12 @@ func GenerateEvaluationName(checkType CheckType, evalName string) string { return fmt.Sprintf("%s-%s-%d", checkType, TruncateString(evalName, 27), randomId) } +// MergeMaps merges two maps into a new map. If a key exists in both maps, the +// value of the second map is picked. func MergeMaps(m1 map[string]string, m2 map[string]string) map[string]string { merged := make(map[string]string, len(m1)+len(m2)) - for k, v := range m1 { - merged[k] = v + for key, value := range m1 { + merged[key] = value } for key, value := range m2 { merged[key] = value diff --git a/operator/apis/lifecycle/v1alpha3/keptntask_types.go b/operator/apis/lifecycle/v1alpha3/keptntask_types.go index 15cce49204..98221eb0b5 100644 --- a/operator/apis/lifecycle/v1alpha3/keptntask_types.go +++ b/operator/apis/lifecycle/v1alpha3/keptntask_types.go @@ -168,18 +168,18 @@ func (t KeptnTask) SetSpanAttributes(span trace.Span) { func (t KeptnTask) CreateKeptnLabels() map[string]string { if t.Spec.Workload != "" { - return common.MergeMaps(map[string]string{ + return common.MergeMaps(t.Labels, map[string]string{ common.AppAnnotation: t.Spec.AppName, common.WorkloadAnnotation: t.Spec.Workload, common.VersionAnnotation: t.Spec.WorkloadVersion, common.TaskNameAnnotation: t.Name, - }, t.Labels) + }) } - return common.MergeMaps(map[string]string{ + return common.MergeMaps(t.Labels, map[string]string{ common.AppAnnotation: t.Spec.AppName, common.VersionAnnotation: t.Spec.AppVersion, common.TaskNameAnnotation: t.Name, - }, t.Labels) + }) } func (t KeptnTask) GetSpanAttributes() []attribute.KeyValue {