fix(operator): detect Job failure and set Task to failed (#424)

Closes #380

operator/controllers/keptntask/controller.go

@@ -85,7 +85,6 @@ func (r *KeptnTaskReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		if err != nil {
 			r.Log.Error(err, "could not update status")
 		}
-
 	}(task)
 
 	jobExists, err := r.JobExists(ctx, *task, req.Namespace)

operator/controllers/keptntask/job_utils.go

@@ -117,10 +117,8 @@ func (r *KeptnTaskReconciler) updateJob(ctx context.Context, req ctrl.Request, t
 	}
 	if job.Status.Succeeded > 0 {
 		task.Status.Status = common.StateSucceeded
-		err = r.Client.Status().Update(ctx, task)
-		if err != nil {
-			r.Log.Error(err, "could not update job status for: "+task.Name)
-		}
+	} else if job.Status.Failed > 0 {
+		task.Status.Status = common.StateFailed
 	}
 	return nil
 }

operator/controllers/keptntask/job_utils_test.go

@@ -0,0 +1,191 @@
+package keptntask
+
+import (
+	"context"
+	klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
+	"github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common"
+	"github.com/stretchr/testify/require"
+	batchv1 "k8s.io/api/batch/v1"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"testing"
+)
+
+func TestKeptnTaskReconciler_createJob(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 := klcv1alpha1.AddToScheme(fakeClient.Scheme())
+	require.Nil(t, err)
+
+	taskDefinition := makeTaskDefinitionWithConfigmapRef(taskDefinitionName, namespace, 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)
+}
+
+func TestKeptnTaskReconciler_updateJob(t *testing.T) {
+	namespace := "default"
+	taskDefinitionName := "my-task-definition"
+
+	job := makeJob("my.job", namespace)
+
+	fakeClient := fake.NewClientBuilder().WithObjects(job).Build()
+
+	fakeRecorder := &record.FakeRecorder{}
+
+	err := klcv1alpha1.AddToScheme(fakeClient.Scheme())
+	require.Nil(t, err)
+
+	job.Status.Failed = 1
+
+	err = fakeClient.Status().Update(context.TODO(), job)
+	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,
+		},
+	}
+
+	task.Status.JobName = job.Name
+
+	err = r.updateJob(context.TODO(), req, task)
+	require.Nil(t, err)
+
+	require.Equal(t, common.StateFailed, task.Status.Status)
+
+	// now, set the job to succeeded
+	job.Status.Succeeded = 1
+	job.Status.Failed = 0
+
+	err = fakeClient.Status().Update(context.TODO(), job)
+	require.Nil(t, err)
+
+	err = r.updateJob(context.TODO(), req, task)
+	require.Nil(t, err)
+
+	require.Equal(t, common.StateSucceeded, task.Status.Status)
+}
+
+func makeJob(name, namespace string) *batchv1.Job {
+	return &batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: batchv1.JobSpec{},
+	}
+}
+
+func makeTask(name, namespace string, taskDefinitionName string) *klcv1alpha1.KeptnTask {
+	return &klcv1alpha1.KeptnTask{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: klcv1alpha1.KeptnTaskSpec{
+			Workload:       "my-workload",
+			AppName:        "my-app",
+			AppVersion:     "0.1.0",
+			TaskDefinition: taskDefinitionName,
+		},
+	}
+}
+
+func makeTaskDefinitionWithConfigmapRef(name, namespace, configMapName string) *klcv1alpha1.KeptnTaskDefinition {
+	return &klcv1alpha1.KeptnTaskDefinition{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: klcv1alpha1.KeptnTaskDefinitionSpec{
+			Function: klcv1alpha1.FunctionSpec{
+				ConfigMapReference: klcv1alpha1.ConfigMapReference{
+					Name: configMapName,
+				},
+				Parameters:       klcv1alpha1.TaskParameters{Inline: map[string]string{"foo": "bar"}},
+				SecureParameters: klcv1alpha1.SecureParameters{Secret: "my-secret"},
+			},
+		},
+	}
+}
+
+func makeConfigMap(name, namespace string) *v1.ConfigMap {
+	return &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Data: map[string]string{
+			"code": "console.log('hello');",
+		},
+	}
+}

operator/test/component/taskcontroller_test.go

@@ -0,0 +1,172 @@
+package component
+
+import (
+	"context"
+	klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
+	keptncommon "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common"
+	keptncontroller "github.com/keptn/lifecycle-toolkit/operator/controllers/common"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/keptntask"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	otelsdk "go.opentelemetry.io/otel/sdk/trace"
+	sdktest "go.opentelemetry.io/otel/sdk/trace/tracetest"
+	batchv1 "k8s.io/api/batch/v1"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"os"
+)
+
+var _ = Describe("KeptnTaskController", Ordered, func() {
+	var (
+		name               string
+		taskDefinitionName string
+		namespace          string
+		spanRecorder       *sdktest.SpanRecorder
+		tracer             *otelsdk.TracerProvider
+	)
+
+	BeforeAll(func() {
+		// setup once
+		By("Waiting for Manager")
+		Eventually(func() bool {
+			return k8sManager != nil
+		}).Should(Equal(true))
+
+		By("Creating the Controller")
+		_ = os.Setenv("FUNCTION_RUNNER_IMAGE", "my-image")
+
+		spanRecorder = sdktest.NewSpanRecorder()
+		tracer = otelsdk.NewTracerProvider(otelsdk.WithSpanProcessor(spanRecorder))
+
+		////setup controllers here
+		controllers := []keptncontroller.Controller{&keptntask.KeptnTaskReconciler{
+			Client:   k8sManager.GetClient(),
+			Scheme:   k8sManager.GetScheme(),
+			Recorder: k8sManager.GetEventRecorderFor("test-task-controller"),
+			Log:      GinkgoLogr,
+			Meters:   initKeptnMeters(),
+			Tracer:   tracer.Tracer("test-task-tracer"),
+		}}
+		setupManager(controllers) // we can register multiple time the same controller
+	})
+
+	BeforeEach(func() { // list var here they will be copied for every spec
+		name = "test-task"
+		taskDefinitionName = "my-taskdefinition"
+		namespace = "default" // namespaces are not deleted in the api so be careful
+	})
+
+	Describe("Creation of a Task", func() {
+		var (
+			taskDefinition *klcv1alpha1.KeptnTaskDefinition
+			task           *klcv1alpha1.KeptnTask
+		)
+		Context("with an existing TaskDefinition", func() {
+			BeforeEach(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) {
+					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 failed")
+				createdJob.Status.Failed = 1
+
+				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(keptncommon.StateFailed))
+				}, "10s").Should(Succeed())
+			})
+			AfterEach(func() {
+				k8sClient.Delete(context.TODO(), taskDefinition)
+				k8sClient.Delete(context.TODO(), task)
+			})
+		})
+	})
+})
+
+func makeTask(name string, namespace, taskDefinitionName string) *klcv1alpha1.KeptnTask {
+	task := &klcv1alpha1.KeptnTask{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: klcv1alpha1.KeptnTaskSpec{
+			Workload:       "my-workload",
+			AppName:        "my-app",
+			AppVersion:     "0.1.0",
+			TaskDefinition: taskDefinitionName,
+		},
+	}
+
+	err := k8sClient.Create(ctx, task)
+	Expect(err).To(BeNil())
+
+	return task
+}
+
+func makeTaskDefinition(taskDefinitionName, namespace string) *klcv1alpha1.KeptnTaskDefinition {
+	cmName := "my-cm"
+	cm := &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cmName,
+			Namespace: namespace,
+		},
+		Data: map[string]string{
+			"code": "console.log('hello');",
+		},
+	}
+	err := k8sClient.Create(context.TODO(), cm)
+
+	Expect(err).To(BeNil())
+
+	taskDefinition := &klcv1alpha1.KeptnTaskDefinition{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      taskDefinitionName,
+			Namespace: namespace,
+		},
+		Spec: klcv1alpha1.KeptnTaskDefinitionSpec{
+			Function: klcv1alpha1.FunctionSpec{
+				ConfigMapReference: klcv1alpha1.ConfigMapReference{
+					Name: cmName,
+				},
+			},
+		},
+	}
+
+	err = k8sClient.Create(context.TODO(), taskDefinition)
+	Expect(err).To(BeNil())
+
+	taskDefinition.Status.Function.ConfigMap = cmName
+	err = k8sClient.Status().Update(context.TODO(), taskDefinition)
+	Expect(err).To(BeNil())
+
+	return taskDefinition
+}