diff --git a/operator/controllers/common/helperfunctions.go b/operator/controllers/common/helperfunctions.go index e4a5f2e2e7..053e8c63e1 100644 --- a/operator/controllers/common/helperfunctions.go +++ b/operator/controllers/common/helperfunctions.go @@ -1,16 +1,21 @@ package common import ( + "context" "fmt" + "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" "github.com/keptn/lifecycle-toolkit/operator/controllers/lifecycle/interfaces" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" ) +const KLTNamespace = "keptn-lifecycle-toolkit-system" + // GetItemStatus retrieves the state of the task/evaluation, if it does not exists, it creates a default one func GetItemStatus(name string, instanceStatus []klcv1alpha3.ItemStatus) klcv1alpha3.ItemStatus { for _, status := range instanceStatus { @@ -81,3 +86,20 @@ func copyMap[M1 ~map[K]V, M2 ~map[K]V, K comparable, V any](dst M1, src M2) { dst[k] = v } } + +func GetTaskDefinition(k8sclient client.Client, log logr.Logger, ctx context.Context, definitionName string, namespace string) (*klcv1alpha3.KeptnTaskDefinition, error) { + definition := &klcv1alpha3.KeptnTaskDefinition{} + err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: namespace}, definition) + if err != nil { + log.Error(err, "Failed to get KeptnTaskDefinition from application namespace") + if k8serrors.IsNotFound(err) { + if err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: KLTNamespace}, definition); err != nil { + log.Error(err, "Failed to get KeptnTaskDefinition from default KLT namespace") + return nil, err + } + return definition, nil + } + return nil, err + } + return definition, nil +} diff --git a/operator/controllers/common/helperfunctions_test.go b/operator/controllers/common/helperfunctions_test.go index 6ab884271c..3ac3c8bba2 100644 --- a/operator/controllers/common/helperfunctions_test.go +++ b/operator/controllers/common/helperfunctions_test.go @@ -1,6 +1,7 @@ package common import ( + "context" "testing" klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3" @@ -8,7 +9,10 @@ import ( "github.com/stretchr/testify/require" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func Test_GetItemStatus(t *testing.T) { @@ -402,3 +406,84 @@ func Test_setAnnotations(t *testing.T) { }) } } + +func Test_GetTaskDefinition(t *testing.T) { + tests := []struct { + name string + taskDef *klcv1alpha3.KeptnTaskDefinition + taskDefName string + taskDefNamespace string + out *klcv1alpha3.KeptnTaskDefinition + wantError bool + }{ + { + name: "taskDef not found", + taskDef: &klcv1alpha3.KeptnTaskDefinition{ + ObjectMeta: v1.ObjectMeta{ + Name: "taskDef", + Namespace: "some-other-namespace", + }, + }, + taskDefName: "taskDef", + taskDefNamespace: "some-namespace", + out: nil, + wantError: true, + }, + { + name: "taskDef found", + taskDef: &klcv1alpha3.KeptnTaskDefinition{ + ObjectMeta: v1.ObjectMeta{ + Name: "taskDef", + Namespace: "some-namespace", + }, + }, + taskDefName: "taskDef", + taskDefNamespace: "some-namespace", + out: &klcv1alpha3.KeptnTaskDefinition{ + ObjectMeta: v1.ObjectMeta{ + Name: "taskDef", + Namespace: "some-namespace", + }, + }, + wantError: false, + }, + { + name: "taskDef found in default KLT namespace", + taskDef: &klcv1alpha3.KeptnTaskDefinition{ + ObjectMeta: v1.ObjectMeta{ + Name: "taskDef", + Namespace: KLTNamespace, + }, + }, + taskDefName: "taskDef", + taskDefNamespace: "some-namespace", + out: &klcv1alpha3.KeptnTaskDefinition{ + ObjectMeta: v1.ObjectMeta{ + Name: "taskDef", + Namespace: KLTNamespace, + }, + }, + wantError: false, + }, + } + + err := klcv1alpha3.AddToScheme(scheme.Scheme) + require.Nil(t, err) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := fake.NewClientBuilder().WithObjects(tt.taskDef).Build() + d, err := GetTaskDefinition(client, ctrl.Log.WithName("testytest"), context.TODO(), tt.taskDefName, tt.taskDefNamespace) + if tt.out != nil && d != nil { + require.Equal(t, tt.out.Name, d.Name) + require.Equal(t, tt.out.Namespace, d.Namespace) + } else if tt.out != d { + t.Errorf("want: %v, got: %v", tt.out, d) + } + if tt.wantError != (err != nil) { + t.Errorf("want error: %t, got: %v", tt.wantError, err) + } + + }) + } +} diff --git a/operator/controllers/common/providers/keptnmetric/keptnmetric.go b/operator/controllers/common/providers/keptnmetric/keptnmetric.go index 0593c7c497..fd08879a7b 100644 --- a/operator/controllers/common/providers/keptnmetric/keptnmetric.go +++ b/operator/controllers/common/providers/keptnmetric/keptnmetric.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2" klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3" + "github.com/keptn/lifecycle-toolkit/operator/controllers/common" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -16,8 +17,6 @@ type KeptnMetricProvider struct { K8sClient client.Client } -const KLTNamespace = "keptn-lifecycle-toolkit-system" - // FetchData fetches the SLI values from KeptnMetric resource func (p *KeptnMetricProvider) FetchData(ctx context.Context, objective klcv1alpha3.Objective, namespace string) (string, []byte, error) { metric, err := p.GetKeptnMetric(ctx, objective, namespace) @@ -45,8 +44,8 @@ func (p *KeptnMetricProvider) GetKeptnMetric(ctx context.Context, objective klcv } else { if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: namespace}, metric); err != nil { p.Log.Error(err, "Failed to get KeptnMetric from KeptnEvaluation resource namespace") - if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: KLTNamespace}, metric); err != nil { - p.Log.Error(err, "Failed to get KeptnMetric from "+KLTNamespace+" namespace") + if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: common.KLTNamespace}, metric); err != nil { + p.Log.Error(err, "Failed to get KeptnMetric from "+common.KLTNamespace+" namespace") return nil, err } } diff --git a/operator/controllers/common/providers/keptnmetric/keptnmetric_test.go b/operator/controllers/common/providers/keptnmetric/keptnmetric_test.go index 713f9c3b97..e05cb5919a 100644 --- a/operator/controllers/common/providers/keptnmetric/keptnmetric_test.go +++ b/operator/controllers/common/providers/keptnmetric/keptnmetric_test.go @@ -6,6 +6,7 @@ import ( metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2" klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3" + "github.com/keptn/lifecycle-toolkit/operator/controllers/common" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" @@ -170,14 +171,14 @@ func Test_Getkeptnmetric(t *testing.T) { metric: &metricsapi.KeptnMetric{ ObjectMeta: metav1.ObjectMeta{ Name: "metric", - Namespace: KLTNamespace, + Namespace: common.KLTNamespace, }, }, namespace: "my-other-namespace", out: &metricsapi.KeptnMetric{ ObjectMeta: metav1.ObjectMeta{ Name: "metric", - Namespace: KLTNamespace, + Namespace: common.KLTNamespace, }, }, wantError: false, diff --git a/operator/controllers/common/taskhandler.go b/operator/controllers/common/taskhandler.go index 16e90e7c5a..c34641dff5 100644 --- a/operator/controllers/common/taskhandler.go +++ b/operator/controllers/common/taskhandler.go @@ -159,8 +159,7 @@ func (r TaskHandler) setupTasks(taskCreateAttributes CreateTaskAttributes, piWra } func (r TaskHandler) handleTaskNotExists(ctx context.Context, phaseCtx context.Context, taskCreateAttributes CreateTaskAttributes, taskName string, piWrapper *interfaces.PhaseItemWrapper, reconcileObject client.Object, task *klcv1alpha3.KeptnTask, taskStatus *klcv1alpha3.ItemStatus) error { - definition := &klcv1alpha3.KeptnTaskDefinition{} - err := r.Client.Get(ctx, types.NamespacedName{Name: taskName, Namespace: piWrapper.GetNamespace()}, definition) + definition, err := GetTaskDefinition(r.Client, r.Log, ctx, taskName, piWrapper.GetNamespace()) if err != nil { r.Log.Error(err, "could not find KeptnTaskDefinition") return controllererrors.ErrCannotGetKeptnTaskDefinition diff --git a/operator/controllers/common/taskhandler_test.go b/operator/controllers/common/taskhandler_test.go index 1ee331f39e..6c319d9bef 100644 --- a/operator/controllers/common/taskhandler_test.go +++ b/operator/controllers/common/taskhandler_test.go @@ -87,6 +87,46 @@ func TestTaskHandler(t *testing.T) { getSpanCalls: 0, unbindSpanCalls: 0, }, + { + name: "task not started - taskDefinition in default KLT namespace", + object: &v1alpha3.KeptnAppVersion{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "namespace", + }, + Spec: v1alpha3.KeptnAppVersionSpec{ + KeptnAppSpec: v1alpha3.KeptnAppSpec{ + PreDeploymentTasks: []string{"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: 1, Pending: 1}, + wantErr: nil, + getSpanCalls: 1, + unbindSpanCalls: 0, + }, { name: "task not started", object: &v1alpha3.KeptnAppVersion{ diff --git a/operator/controllers/lifecycle/keptntask/job_utils.go b/operator/controllers/lifecycle/keptntask/job_utils.go index 47563bf82b..a57874927b 100644 --- a/operator/controllers/lifecycle/keptntask/job_utils.go +++ b/operator/controllers/lifecycle/keptntask/job_utils.go @@ -16,7 +16,7 @@ import ( func (r *KeptnTaskReconciler) createJob(ctx context.Context, req ctrl.Request, task *klcv1alpha3.KeptnTask) error { jobName := "" - definition, err := r.getTaskDefinition(ctx, task.Spec.TaskDefinition, req.Namespace) + definition, err := controllercommon.GetTaskDefinition(r.Client, r.Log, ctx, task.Spec.TaskDefinition, req.Namespace) if err != nil { controllercommon.RecordEvent(r.Recorder, apicommon.PhaseCreateTask, "Warning", task, "TaskDefinitionNotFound", fmt.Sprintf("could not find KeptnTaskDefinition: %s ", task.Spec.TaskDefinition), "") return err @@ -129,7 +129,7 @@ func setupTaskContext(task *klcv1alpha3.KeptnTask) klcv1alpha3.TaskContext { func (r *KeptnTaskReconciler) handleParent(ctx context.Context, req ctrl.Request, task *klcv1alpha3.KeptnTask, definition *klcv1alpha3.KeptnTaskDefinition, params FunctionExecutionParams) error { var parentJobParams FunctionExecutionParams - parentDefinition, err := r.getTaskDefinition(ctx, definition.Spec.Function.FunctionReference.Name, req.Namespace) + parentDefinition, err := controllercommon.GetTaskDefinition(r.Client, r.Log, ctx, definition.Spec.Function.FunctionReference.Name, req.Namespace) if err != nil { controllercommon.RecordEvent(r.Recorder, apicommon.PhaseCreateTask, "Warning", task, "TaskDefinitionNotFound", fmt.Sprintf("could not find KeptnTaskDefinition: %s ", task.Spec.TaskDefinition), "") return err diff --git a/operator/controllers/lifecycle/keptntask/job_utils_test.go b/operator/controllers/lifecycle/keptntask/job_utils_test.go index 8ebd6ba9aa..3ed1e2c4b5 100644 --- a/operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -6,6 +6,7 @@ import ( klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3" apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3/common" + "github.com/keptn/lifecycle-toolkit/operator/controllers/common" "github.com/stretchr/testify/require" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" @@ -90,6 +91,80 @@ func TestKeptnTaskReconciler_createJob(t *testing.T) { }, resultingJob.Annotations) } +func TestKeptnTaskReconciler_createJob_withTaskDefInDefaultNamespace(t *testing.T) { + namespace := "default" + cmName := "my-cmd" + taskDefinitionName := "my-task-definition" + + cm := makeConfigMap(cmName, namespace) + + fakeClient := fake.NewClientBuilder().WithObjects(cm).Build() + + fakeRecorder := &record.FakeRecorder{} + + err := klcv1alpha3.AddToScheme(fakeClient.Scheme()) + require.Nil(t, err) + + taskDefinition := makeTaskDefinitionWithConfigmapRef(taskDefinitionName, common.KLTNamespace, cmName) + + err = fakeClient.Create(context.TODO(), taskDefinition) + require.Nil(t, err) + + taskDefinition.Status.Function.ConfigMap = cmName + err = fakeClient.Status().Update(context.TODO(), taskDefinition) + require.Nil(t, err) + + r := &KeptnTaskReconciler{ + Client: fakeClient, + Recorder: fakeRecorder, + Log: ctrl.Log.WithName("task-controller"), + Scheme: fakeClient.Scheme(), + } + + task := makeTask("my-task", namespace, taskDefinitionName) + + err = fakeClient.Create(context.TODO(), task) + require.Nil(t, err) + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: namespace, + }, + } + + // retrieve the task again to verify its status + err = fakeClient.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: task.Name, + }, task) + + require.Nil(t, err) + + err = r.createJob(context.TODO(), req, task) + require.Nil(t, err) + + require.NotEmpty(t, task.Status.JobName) + + resultingJob := &batchv1.Job{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: task.Status.JobName}, resultingJob) + require.Nil(t, err) + + require.Equal(t, namespace, resultingJob.Namespace) + 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) { namespace := "default" taskDefinitionName := "my-task-definition" diff --git a/operator/controllers/lifecycle/keptntask/task_utils.go b/operator/controllers/lifecycle/keptntask/task_utils.go deleted file mode 100644 index 08e6c4b623..0000000000 --- a/operator/controllers/lifecycle/keptntask/task_utils.go +++ /dev/null @@ -1,17 +0,0 @@ -package keptntask - -import ( - "context" - - klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3" - "k8s.io/apimachinery/pkg/types" -) - -func (r *KeptnTaskReconciler) getTaskDefinition(ctx context.Context, definitionName string, namespace string) (*klcv1alpha3.KeptnTaskDefinition, error) { - definition := &klcv1alpha3.KeptnTaskDefinition{} - err := r.Client.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: namespace}, definition) - if err != nil { - return definition, err - } - return definition, nil -} diff --git a/operator/test/component/task/task_test.go b/operator/test/component/task/task_test.go index 6e1b435fb3..9954f9f7d2 100644 --- a/operator/test/component/task/task_test.go +++ b/operator/test/component/task/task_test.go @@ -5,6 +5,7 @@ import ( 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" "github.com/keptn/lifecycle-toolkit/operator/test/component/common" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -34,12 +35,10 @@ var _ = Describe("Task", Ordered, func() { task *klcv1alpha3.KeptnTask ) Context("with an existing TaskDefinition", func() { - BeforeEach(func() { + It("should end up in a failed state if the created job fails", func() { taskDefinition = makeTaskDefinition(taskDefinitionName, namespace) task = makeTask(name, namespace, taskDefinition.Name) - }) - It("should end up in a failed state if the created job fails", func() { By("Verifying that a job has been created") Eventually(func(g Gomega) { @@ -79,7 +78,63 @@ var _ = Describe("Task", Ordered, func() { g.Expect(task.Status.Status).To(Equal(apicommon.StateFailed)) }, "10s").Should(Succeed()) }) + It("succeed task if taskDefiniton is present in default KLT namespace", func() { + By("create default KLT namespace") + + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: controllercommon.KLTNamespace, + }, + } + err := k8sClient.Create(context.TODO(), ns) + Expect(err).To(BeNil()) + + taskDefinition = makeTaskDefinition(taskDefinitionName, controllercommon.KLTNamespace) + task = makeTask(name, namespace, taskDefinition.Name) + + 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()) + + By("Setting the Job Status to complete") + createdJob.Status.Conditions = []batchv1.JobCondition{ + { + Type: batchv1.JobComplete, + }, + } + + err = k8sClient.Status().Update(context.TODO(), createdJob) + Expect(err).To(BeNil()) + + 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.Status).To(Equal(apicommon.StateSucceeded)) + }, "10s").Should(Succeed()) + }) It("should propagate labels and annotations to the job and job pod", func() { + taskDefinition = makeTaskDefinition(taskDefinitionName, namespace) + task = makeTask(name, namespace, taskDefinition.Name) + By("Verifying that a job has been created") Eventually(func(g Gomega) { @@ -126,7 +181,7 @@ var _ = Describe("Task", Ordered, func() { err = k8sClient.Delete(context.TODO(), &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: taskDefinition.Status.Function.ConfigMap, - Namespace: namespace, + Namespace: taskDefinition.Namespace, }, }) common.LogErrorIfPresent(err)