From 1139d687aac5c95ee356af6091fe4b6edecb025a Mon Sep 17 00:00:00 2001 From: odubajDT Date: Mon, 28 Oct 2024 11:29:59 +0100 Subject: [PATCH 1/4] fix(lifecycle-operator): improve Job status checks in KeptnTaskReconciler Signed-off-by: odubajDT --- .../controllers/lifecycle/keptntask/job_utils.go | 14 ++++++++++++-- .../lifecycle/keptntask/job_utils_test.go | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go index 827595afa2..279369d866 100644 --- a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go +++ b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go @@ -72,9 +72,9 @@ func (r *KeptnTaskReconciler) createFunctionJob(ctx context.Context, req ctrl.Re func (r *KeptnTaskReconciler) updateTaskStatus(job *batchv1.Job, task *apilifecycle.KeptnTask) { if len(job.Status.Conditions) > 0 { - if job.Status.Conditions[0].Type == batchv1.JobComplete { + if hasJobCondition(job.Status.Conditions, batchv1.JobComplete) { task.Status.Status = apicommon.StateSucceeded - } else if job.Status.Conditions[0].Type == batchv1.JobFailed { + } else if hasJobCondition(job.Status.Conditions, batchv1.JobFailed) { task.Status.Status = apicommon.StateFailed task.Status.Message = job.Status.Conditions[0].Message task.Status.Reason = job.Status.Conditions[0].Reason @@ -82,6 +82,16 @@ func (r *KeptnTaskReconciler) updateTaskStatus(job *batchv1.Job, task *apilifecy } } +func hasJobCondition(conditions []batchv1.JobCondition, searched batchv1.JobConditionType) bool { + for _, v := range conditions { + if v.Type == searched { + return true + } + } + + return false +} + func (r *KeptnTaskReconciler) getJob(ctx context.Context, jobName string, namespace string) (*batchv1.Job, error) { job := &batchv1.Job{} err := r.Client.Get(ctx, types.NamespacedName{Name: jobName, Namespace: namespace}, job) diff --git a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go index b19b2af03b..c9f9d293bc 100644 --- a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -158,6 +158,7 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { jobStatus := batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ + //todo { Type: batchv1.JobFailed, }, @@ -191,6 +192,7 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { // now, set the job to succeeded job.Status.Conditions = []batchv1.JobCondition{ + //todo { Type: batchv1.JobComplete, }, From be8f75d0e2845de70198cbbbc5552f26ebfa8c75 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 29 Oct 2024 14:20:24 +0100 Subject: [PATCH 2/4] support k8s 31 conditions Signed-off-by: odubajDT --- .../deploy-keptn-on-cluster/action.yml | 4 +- .../lifecycle/keptntask/job_utils.go | 6 ++- .../lifecycle/keptntask/job_utils_test.go | 51 ++++++++++++++++++- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/.github/actions/deploy-keptn-on-cluster/action.yml b/.github/actions/deploy-keptn-on-cluster/action.yml index 2be44c6781..36e7d31d21 100644 --- a/.github/actions/deploy-keptn-on-cluster/action.yml +++ b/.github/actions/deploy-keptn-on-cluster/action.yml @@ -5,12 +5,12 @@ inputs: required: false description: "Version of kind that should be used" # renovate: datasource=github-releases depName=kubernetes-sigs/kind - default: "v0.18.0" + default: "v0.24.0" k8s-version: required: false description: "Kubernetes version that should be used" # renovate: datasource=github-releases depName=kubernetes/kubernetes - default: "v1.27.3" + default: "v1.31.0" runtime_tag: description: "Tag for the runner image" required: true diff --git a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go index 279369d866..baaa9c9cd5 100644 --- a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go +++ b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go @@ -72,9 +72,11 @@ func (r *KeptnTaskReconciler) createFunctionJob(ctx context.Context, req ctrl.Re func (r *KeptnTaskReconciler) updateTaskStatus(job *batchv1.Job, task *apilifecycle.KeptnTask) { if len(job.Status.Conditions) > 0 { - if hasJobCondition(job.Status.Conditions, batchv1.JobComplete) { + if hasJobCondition(job.Status.Conditions, batchv1.JobComplete) || + hasJobCondition(job.Status.Conditions, batchv1.JobSuccessCriteriaMet) { task.Status.Status = apicommon.StateSucceeded - } else if hasJobCondition(job.Status.Conditions, batchv1.JobFailed) { + } else if hasJobCondition(job.Status.Conditions, batchv1.JobFailed) || + hasJobCondition(job.Status.Conditions, batchv1.JobFailureTarget) { task.Status.Status = apicommon.StateFailed task.Status.Message = job.Status.Conditions[0].Message task.Status.Reason = job.Status.Conditions[0].Reason diff --git a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go index c9f9d293bc..912d205fc2 100644 --- a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -158,7 +158,6 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { jobStatus := batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ - //todo { Type: batchv1.JobFailed, }, @@ -192,7 +191,6 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { // now, set the job to succeeded job.Status.Conditions = []batchv1.JobCondition{ - //todo { Type: batchv1.JobComplete, }, @@ -203,6 +201,55 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { require.Equal(t, apicommon.StateSucceeded, task.Status.Status) } +func TestKeptnTaskReconciler_updateTaskStatusK8s31(t *testing.T) { + namespace := "default" + taskDefinitionName := "my-task-definition" + + jobStatus := batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobFailureTarget, + }, + }, + } + + job := makeJob("my.job", namespace, jobStatus) + + fakeClient := fake.NewClientBuilder().WithObjects(job).Build() + + err := apilifecycle.AddToScheme(fakeClient.Scheme()) + require.Nil(t, err) + + r := &KeptnTaskReconciler{ + Client: fakeClient, + EventSender: eventsender.NewK8sSender(record.NewFakeRecorder(100)), + 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) + + task.Status.JobName = job.Name + + r.updateTaskStatus(job, task) + + require.Equal(t, apicommon.StateFailed, task.Status.Status) + + // now, set the job to succeeded + job.Status.Conditions = []batchv1.JobCondition{ + { + Type: batchv1.JobSuccessCriteriaMet, + }, + } + + r.updateTaskStatus(job, task) + + require.Equal(t, apicommon.StateSucceeded, task.Status.Status) +} + func TestKeptnTaskReconciler_generateJob(t *testing.T) { namespace := "default" taskName := "my-task" From 97d321b2b9065aa0aa22dbd11f1cc61b62d29bc6 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 29 Oct 2024 14:25:21 +0100 Subject: [PATCH 3/4] fix lint Signed-off-by: odubajDT --- .../controllers/lifecycle/keptntask/job_utils_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go index 912d205fc2..c00be40eb0 100644 --- a/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/lifecycle-operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -152,6 +152,7 @@ func TestKeptnTaskReconciler_createJob_withTaskDefInDefaultNamespace(t *testing. }, resultingJob.Annotations) } +//nolint:dupl func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { namespace := "default" taskDefinitionName := "my-task-definition" @@ -201,6 +202,7 @@ func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { require.Equal(t, apicommon.StateSucceeded, task.Status.Status) } +//nolint:dupl func TestKeptnTaskReconciler_updateTaskStatusK8s31(t *testing.T) { namespace := "default" taskDefinitionName := "my-task-definition" From 1bd23226e4dfe57c82e2c379487825054a2b5399 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 29 Oct 2024 15:31:25 +0100 Subject: [PATCH 4/4] adapt integration tests Signed-off-by: odubajDT --- .../integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml | 3 --- test/chainsaw/integration/container-runtime/00-assert.yaml | 3 --- test/chainsaw/integration/imagepullsecret/00-assert.yaml | 3 --- .../simple-deployment-python-runtime/00-assert.yaml | 3 --- .../simple-deployment-recursive-task/00-assert.yaml | 2 -- 5 files changed, 14 deletions(-) diff --git a/test/chainsaw/integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml b/test/chainsaw/integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml index 45f9c1d144..4d447b5f41 100644 --- a/test/chainsaw/integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml +++ b/test/chainsaw/integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml @@ -9,7 +9,4 @@ metadata: spec: ttlSecondsAfterFinished: 100 status: - conditions: - - type: Complete - status: 'True' succeeded: 1 diff --git a/test/chainsaw/integration/container-runtime/00-assert.yaml b/test/chainsaw/integration/container-runtime/00-assert.yaml index 05d9dcea17..35ffda6e24 100644 --- a/test/chainsaw/integration/container-runtime/00-assert.yaml +++ b/test/chainsaw/integration/container-runtime/00-assert.yaml @@ -44,7 +44,4 @@ spec: - '-c' - 'sleep 30' status: - conditions: - - type: Complete - status: 'True' succeeded: 1 diff --git a/test/chainsaw/integration/imagepullsecret/00-assert.yaml b/test/chainsaw/integration/imagepullsecret/00-assert.yaml index b44ba3dc96..2813cf85b9 100644 --- a/test/chainsaw/integration/imagepullsecret/00-assert.yaml +++ b/test/chainsaw/integration/imagepullsecret/00-assert.yaml @@ -46,7 +46,4 @@ spec: imagePullSecrets: - name: my-registry-secret status: - conditions: - - type: Complete - status: 'True' succeeded: 1 diff --git a/test/chainsaw/integration/simple-deployment-python-runtime/00-assert.yaml b/test/chainsaw/integration/simple-deployment-python-runtime/00-assert.yaml index 71111c1ddf..151e0a9ba5 100644 --- a/test/chainsaw/integration/simple-deployment-python-runtime/00-assert.yaml +++ b/test/chainsaw/integration/simple-deployment-python-runtime/00-assert.yaml @@ -31,7 +31,4 @@ metadata: keptn.sh/version: '0.4' keptn.sh/workload: waiter-waiter status: - conditions: - - type: Complete - status: 'True' succeeded: 1 diff --git a/test/chainsaw/integration/simple-deployment-recursive-task/00-assert.yaml b/test/chainsaw/integration/simple-deployment-recursive-task/00-assert.yaml index de110ea706..f262a2ca59 100644 --- a/test/chainsaw/integration/simple-deployment-recursive-task/00-assert.yaml +++ b/test/chainsaw/integration/simple-deployment-recursive-task/00-assert.yaml @@ -11,7 +11,6 @@ apiVersion: batch/v1 kind: Job metadata: annotations: - batch.kubernetes.io/job-tracking: "" keptn.sh/app: waiter keptn.sh/version: "0.4" keptn.sh/workload: waiter-waiter @@ -43,7 +42,6 @@ apiVersion: batch/v1 kind: Job metadata: annotations: - batch.kubernetes.io/job-tracking: "" keptn.sh/app: waiter keptn.sh/version: "0.4" keptn.sh/workload: waiter-waiter