Skip to content

Commit

Permalink
chore(operator): make use of status.jobName when searching for job in…
Browse files Browse the repository at this point in the history
… KeptnTask controller (keptn#1436)

Signed-off-by: odubajDT <[email protected]>
  • Loading branch information
odubajDT authored and StackScribe committed May 24, 2023
1 parent 909b3e8 commit 1c501fb
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 51 deletions.
8 changes: 4 additions & 4 deletions operator/apis/lifecycle/v1alpha3/keptntask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"),
Expand Down
6 changes: 3 additions & 3 deletions operator/apis/lifecycle/v1alpha3/keptntask_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
31 changes: 3 additions & 28 deletions operator/controllers/lifecycle/keptntask/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions operator/controllers/lifecycle/keptntask/function_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion operator/controllers/lifecycle/keptntask/job_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions operator/controllers/lifecycle/keptntask/job_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
10 changes: 5 additions & 5 deletions operator/test/component/task/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down

0 comments on commit 1c501fb

Please sign in to comment.