From ed85fc972cd676ca0be05dae10aabf90e969d503 Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Thu, 15 Jun 2023 08:49:19 +0200 Subject: [PATCH] chore(operator): refactor KeptnTask controller logic (#1536) Signed-off-by: odubajDT Signed-off-by: odubajDT <93584209+odubajDT@users.noreply.github.com> Co-authored-by: Florian Bacher --- .../lifecycle/keptntask/container_builder.go | 26 ++-- .../keptntask/container_builder_test.go | 116 +++++++++--------- .../lifecycle/keptntask/controller.go | 8 +- .../lifecycle/keptntask/job_runner_builder.go | 9 +- .../keptntask/job_runner_builder_test.go | 10 +- .../lifecycle/keptntask/job_utils.go | 39 +++--- .../lifecycle/keptntask/job_utils_test.go | 15 +-- ...function_builder.go => runtime_builder.go} | 74 ++++++----- ...uilder_test.go => runtime_builder_test.go} | 36 +++--- .../01-assert.yaml | 6 +- 10 files changed, 175 insertions(+), 164 deletions(-) rename operator/controllers/lifecycle/keptntask/{function_builder.go => runtime_builder.go} (82%) rename operator/controllers/lifecycle/keptntask/{function_builder_test.go => runtime_builder_test.go} (91%) diff --git a/operator/controllers/lifecycle/keptntask/container_builder.go b/operator/controllers/lifecycle/keptntask/container_builder.go index b9dda67487..d68f61b584 100644 --- a/operator/controllers/lifecycle/keptntask/container_builder.go +++ b/operator/controllers/lifecycle/keptntask/container_builder.go @@ -13,14 +13,18 @@ type ContainerBuilder struct { spec *klcv1alpha3.ContainerSpec } -func NewContainerBuilder(spec *klcv1alpha3.ContainerSpec) *ContainerBuilder { +func NewContainerBuilder(options BuilderOptions) *ContainerBuilder { return &ContainerBuilder{ - spec: spec, + spec: options.containerSpec, } } -func (c *ContainerBuilder) CreateContainerWithVolumes(ctx context.Context) (*corev1.Container, []corev1.Volume, error) { - return c.spec.Container, c.generateVolumes(), nil +func (c *ContainerBuilder) CreateContainer(ctx context.Context) (*corev1.Container, error) { + return c.spec.Container, nil +} + +func (c *ContainerBuilder) CreateVolume(ctx context.Context) (*corev1.Volume, error) { + return c.generateVolume(), nil } func (c *ContainerBuilder) getVolumeSource() *corev1.EmptyDirVolumeSource { @@ -39,16 +43,14 @@ func (c *ContainerBuilder) getVolumeSource() *corev1.EmptyDirVolumeSource { } } -func (c *ContainerBuilder) generateVolumes() []corev1.Volume { +func (c *ContainerBuilder) generateVolume() *corev1.Volume { if !common.IsVolumeMountPresent(c.spec) { - return []corev1.Volume{} + return nil } - return []corev1.Volume{ - { - Name: c.spec.VolumeMounts[0].Name, - VolumeSource: corev1.VolumeSource{ - EmptyDir: c.getVolumeSource(), - }, + return &corev1.Volume{ + Name: c.spec.VolumeMounts[0].Name, + VolumeSource: corev1.VolumeSource{ + EmptyDir: c.getVolumeSource(), }, } } diff --git a/operator/controllers/lifecycle/keptntask/container_builder_test.go b/operator/controllers/lifecycle/keptntask/container_builder_test.go index 167d09e9d2..beb805b19d 100644 --- a/operator/controllers/lifecycle/keptntask/container_builder_test.go +++ b/operator/controllers/lifecycle/keptntask/container_builder_test.go @@ -15,10 +15,9 @@ func TestContainerBuilder_CreateContainerWithVolumes(t *testing.T) { name string builder ContainerBuilder wantContainer *v1.Container - wantVolumes []v1.Volume }{ { - name: "defined without volumes", + name: "defined", builder: ContainerBuilder{ spec: &v1alpha3.ContainerSpec{ Container: &v1.Container{ @@ -29,7 +28,41 @@ func TestContainerBuilder_CreateContainerWithVolumes(t *testing.T) { wantContainer: &v1.Container{ Image: "image", }, - wantVolumes: []v1.Volume{}, + }, + { + name: "nil", + builder: ContainerBuilder{ + spec: &v1alpha3.ContainerSpec{ + Container: nil, + }, + }, + wantContainer: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + container, _ := tt.builder.CreateContainer(context.TODO()) + require.Equal(t, tt.wantContainer, container) + }) + } +} + +func TestContainerBuilder_CreateVolume(t *testing.T) { + tests := []struct { + name string + builder ContainerBuilder + wantVolume *v1.Volume + }{ + { + name: "defined without volume", + builder: ContainerBuilder{ + spec: &v1alpha3.ContainerSpec{ + Container: &v1.Container{ + Image: "image", + }, + }, + }, + wantVolume: nil, }, { name: "defined with volume", @@ -46,23 +79,12 @@ func TestContainerBuilder_CreateContainerWithVolumes(t *testing.T) { }, }, }, - wantContainer: &v1.Container{ - Image: "image", - VolumeMounts: []v1.VolumeMount{ - { - Name: "test-volume", - MountPath: "path", - }, - }, - }, - wantVolumes: []v1.Volume{ - { - Name: "test-volume", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{ - SizeLimit: resource.NewQuantity(1, resource.Format("Gi")), - Medium: v1.StorageMedium("Memory"), - }, + wantVolume: &v1.Volume{ + Name: "test-volume", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{ + SizeLimit: resource.NewQuantity(1, resource.Format("Gi")), + Medium: v1.StorageMedium("Memory"), }, }, }, @@ -87,29 +109,12 @@ func TestContainerBuilder_CreateContainerWithVolumes(t *testing.T) { }, }, }, - - wantContainer: &v1.Container{ - Image: "image", - VolumeMounts: []v1.VolumeMount{ - { - Name: "test-volume", - MountPath: "path", - }, - }, - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{ - "memory": *resource.NewQuantity(100, resource.Format("Mi")), - }, - }, - }, - wantVolumes: []v1.Volume{ - { - Name: "test-volume", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{ - SizeLimit: resource.NewQuantity(100, resource.Format("Mi")), - Medium: v1.StorageMedium("Memory"), - }, + wantVolume: &v1.Volume{ + Name: "test-volume", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{ + SizeLimit: resource.NewQuantity(100, resource.Format("Mi")), + Medium: v1.StorageMedium("Memory"), }, }, }, @@ -117,9 +122,8 @@ func TestContainerBuilder_CreateContainerWithVolumes(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - container, volumes, _ := tt.builder.CreateContainerWithVolumes(context.TODO()) - require.Equal(t, tt.wantContainer, container) - require.Equal(t, tt.wantVolumes, volumes) + volume, _ := tt.builder.CreateVolume(context.TODO()) + require.Equal(t, tt.wantVolume, volume) }) } } @@ -128,7 +132,7 @@ func Test_GenerateVolumes(t *testing.T) { tests := []struct { name string spec *v1alpha3.ContainerSpec - want []v1.Volume + want *v1.Volume }{ { name: "defined", @@ -143,14 +147,12 @@ func Test_GenerateVolumes(t *testing.T) { }, }, }, - want: []v1.Volume{ - { - Name: "name", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{ - SizeLimit: resource.NewQuantity(1, resource.Format("Gi")), - Medium: v1.StorageMedium("Memory"), - }, + want: &v1.Volume{ + Name: "name", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{ + SizeLimit: resource.NewQuantity(1, resource.Format("Gi")), + Medium: v1.StorageMedium("Memory"), }, }, }, @@ -158,7 +160,7 @@ func Test_GenerateVolumes(t *testing.T) { { name: "empty", spec: &v1alpha3.ContainerSpec{}, - want: []v1.Volume{}, + want: nil, }, } for _, tt := range tests { @@ -166,7 +168,7 @@ func Test_GenerateVolumes(t *testing.T) { spec: tt.spec, } t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.want, builder.generateVolumes()) + require.Equal(t, tt.want, builder.generateVolume()) }) } } diff --git a/operator/controllers/lifecycle/keptntask/controller.go b/operator/controllers/lifecycle/keptntask/controller.go index 2be95b7465..6891831d97 100644 --- a/operator/controllers/lifecycle/keptntask/controller.go +++ b/operator/controllers/lifecycle/keptntask/controller.go @@ -85,7 +85,7 @@ func (r *KeptnTaskReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( defer func() { err := r.Client.Status().Update(ctx, task) if err != nil { - r.Log.Error(err, "could not update status") + r.Log.Error(err, "could not update KeptnTask status reference for: "+task.Name) } }() @@ -108,11 +108,7 @@ func (r *KeptnTaskReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if !task.Status.Status.IsCompleted() { - err := r.updateJob(ctx, req, task) - if err != nil { - span.SetStatus(codes.Error, err.Error()) - return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err - } + r.updateTaskStatus(job, task) return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil } diff --git a/operator/controllers/lifecycle/keptntask/job_runner_builder.go b/operator/controllers/lifecycle/keptntask/job_runner_builder.go index 41be24e3c5..a7ee0025b6 100644 --- a/operator/controllers/lifecycle/keptntask/job_runner_builder.go +++ b/operator/controllers/lifecycle/keptntask/job_runner_builder.go @@ -13,7 +13,8 @@ import ( // JobRunnerBuilder is the interface that describes the operations needed to help build job specs of a task type JobRunnerBuilder interface { // CreateContainerWithVolumes returns a job container and volumes based on the task definition spec - CreateContainerWithVolumes(ctx context.Context) (*corev1.Container, []corev1.Volume, error) + CreateContainer(ctx context.Context) (*corev1.Container, error) + CreateVolume(ctx context.Context) (*corev1.Volume, error) } // BuilderOptions contains everything needed to build the current job @@ -30,12 +31,12 @@ type BuilderOptions struct { ConfigMap string } -func getJobRunnerBuilder(options BuilderOptions) JobRunnerBuilder { +func NewJobRunnerBuilder(options BuilderOptions) JobRunnerBuilder { if options.funcSpec != nil { - return NewFunctionBuilder(options) + return NewRuntimeBuilder(options) } if options.containerSpec != nil { - return NewContainerBuilder(options.containerSpec) + return NewContainerBuilder(options) } return nil } diff --git a/operator/controllers/lifecycle/keptntask/job_runner_builder_test.go b/operator/controllers/lifecycle/keptntask/job_runner_builder_test.go index 33b36e1ad6..3bfd781a08 100644 --- a/operator/controllers/lifecycle/keptntask/job_runner_builder_test.go +++ b/operator/controllers/lifecycle/keptntask/job_runner_builder_test.go @@ -9,7 +9,7 @@ import ( ) func Test_getJobRunnerBuilder(t *testing.T) { - functionBuilderOptions := BuilderOptions{ + runtimeBuilderOptions := BuilderOptions{ funcSpec: &v1alpha3.RuntimeSpec{ Inline: v1alpha3.Inline{ Code: "some code", @@ -30,13 +30,13 @@ func Test_getJobRunnerBuilder(t *testing.T) { }{ { name: "js builder", - options: functionBuilderOptions, - want: NewFunctionBuilder(functionBuilderOptions), + options: runtimeBuilderOptions, + want: NewRuntimeBuilder(runtimeBuilderOptions), }, { name: "container builder", options: containerBuilderOptions, - want: NewContainerBuilder(containerBuilderOptions.containerSpec), + want: NewContainerBuilder(containerBuilderOptions), }, { name: "invalid builder", @@ -46,7 +46,7 @@ func Test_getJobRunnerBuilder(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.want, getJobRunnerBuilder(tt.options)) + require.Equal(t, tt.want, NewJobRunnerBuilder(tt.options)) }) } } diff --git a/operator/controllers/lifecycle/keptntask/job_utils.go b/operator/controllers/lifecycle/keptntask/job_utils.go index 3523aaad2a..76cf0f016e 100644 --- a/operator/controllers/lifecycle/keptntask/job_utils.go +++ b/operator/controllers/lifecycle/keptntask/job_utils.go @@ -33,11 +33,7 @@ func (r *KeptnTaskReconciler) createJob(ctx context.Context, req ctrl.Request, t task.Status.JobName = jobName task.Status.Status = apicommon.StatePending - err = r.Client.Status().Update(ctx, task) - if err != nil { - r.Log.Error(err, "could not update KeptnTask status reference for: "+task.Name) - } - r.Log.Info("updated configmap status reference for: " + definition.Name) + return nil } @@ -58,17 +54,7 @@ func (r *KeptnTaskReconciler) createFunctionJob(ctx context.Context, req ctrl.Re return job.Name, nil } -func (r *KeptnTaskReconciler) updateJob(ctx context.Context, req ctrl.Request, task *klcv1alpha3.KeptnTask) error { - job, err := r.getJob(ctx, task.Status.JobName, req.Namespace) - if err != nil { - task.Status.JobName = "" - controllercommon.RecordEvent(r.Recorder, apicommon.PhaseReconcileTask, "Warning", task, "JobReferenceRemoved", "removed Job Reference as Job could not be found", "") - err = r.Client.Status().Update(ctx, task) - if err != nil { - r.Log.Error(err, "could not remove job reference for: "+task.Name) - } - return err - } +func (r *KeptnTaskReconciler) updateTaskStatus(job *batchv1.Job, task *klcv1alpha3.KeptnTask) { if len(job.Status.Conditions) > 0 { if job.Status.Conditions[0].Type == batchv1.JobComplete { task.Status.Status = apicommon.StateSucceeded @@ -78,8 +64,8 @@ func (r *KeptnTaskReconciler) updateJob(ctx context.Context, req ctrl.Request, t task.Status.Reason = job.Status.Conditions[0].Reason } } - return nil } + 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) @@ -101,6 +87,7 @@ func setupTaskContext(task *klcv1alpha3.KeptnTask) klcv1alpha3.TaskContext { taskContext.ObjectType = "Application" taskContext.AppVersion = task.Spec.AppVersion } + taskContext.TaskType = string(task.Spec.Type) taskContext.AppName = task.Spec.AppName return taskContext @@ -146,18 +133,26 @@ func (r *KeptnTaskReconciler) generateJob(ctx context.Context, task *klcv1alpha3 ConfigMap: definition.Status.Function.ConfigMap, } - builder := getJobRunnerBuilder(builderOpt) + builder := NewJobRunnerBuilder(builderOpt) if builder == nil { return nil, controllererrors.ErrNoTaskDefinitionSpec } - container, volumes, err := builder.CreateContainerWithVolumes(ctx) + container, err := builder.CreateContainer(ctx) + if err != nil { + return nil, fmt.Errorf("could not create container for Job: %w", err) + } + + volume, err := builder.CreateVolume(ctx) if err != nil { - r.Log.Error(err, "could not create Job") - return nil, controllererrors.ErrCannotMarshalParams + return nil, fmt.Errorf("could not create volume for Job: %w", err) + } + + if volume != nil { + job.Spec.Template.Spec.Volumes = []corev1.Volume{*volume} } job.Spec.Template.Spec.Containers = []corev1.Container{*container} - job.Spec.Template.Spec.Volumes = volumes + return job, nil } diff --git a/operator/controllers/lifecycle/keptntask/job_utils_test.go b/operator/controllers/lifecycle/keptntask/job_utils_test.go index e547c0809f..1e5010b98b 100644 --- a/operator/controllers/lifecycle/keptntask/job_utils_test.go +++ b/operator/controllers/lifecycle/keptntask/job_utils_test.go @@ -165,7 +165,7 @@ func TestKeptnTaskReconciler_createJob_withTaskDefInDefaultNamespace(t *testing. }, resultingJob.Annotations) } -func TestKeptnTaskReconciler_updateJob(t *testing.T) { +func TestKeptnTaskReconciler_updateTaskStatus(t *testing.T) { namespace := "default" taskDefinitionName := "my-task-definition" @@ -199,16 +199,9 @@ func TestKeptnTaskReconciler_updateJob(t *testing.T) { 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) + r.updateTaskStatus(job, task) require.Equal(t, apicommon.StateFailed, task.Status.Status) @@ -222,8 +215,7 @@ func TestKeptnTaskReconciler_updateJob(t *testing.T) { err = fakeClient.Status().Update(context.TODO(), job) require.Nil(t, err) - err = r.updateJob(context.TODO(), req, task) - require.Nil(t, err) + r.updateTaskStatus(job, task) require.Equal(t, apicommon.StateSucceeded, task.Status.Status) } @@ -255,6 +247,7 @@ func makeTask(name, namespace string, taskDefinitionName string) *klcv1alpha3.Ke AppName: "my-app", AppVersion: "0.1.0", TaskDefinition: taskDefinitionName, + Type: apicommon.PostDeploymentCheckType, }, } } diff --git a/operator/controllers/lifecycle/keptntask/function_builder.go b/operator/controllers/lifecycle/keptntask/runtime_builder.go similarity index 82% rename from operator/controllers/lifecycle/keptntask/function_builder.go rename to operator/controllers/lifecycle/keptntask/runtime_builder.go index cb3738c3bc..df562573c6 100644 --- a/operator/controllers/lifecycle/keptntask/function_builder.go +++ b/operator/controllers/lifecycle/keptntask/runtime_builder.go @@ -13,20 +13,20 @@ import ( corev1 "k8s.io/api/core/v1" ) -// FunctionBuilder implements container builder interface for javascript deno -type FunctionBuilder struct { +// RuntimeBuilder implements container builder interface for Deno/Python +type RuntimeBuilder struct { options BuilderOptions } -func NewFunctionBuilder(options BuilderOptions) *FunctionBuilder { +func NewRuntimeBuilder(options BuilderOptions) *RuntimeBuilder { - return &FunctionBuilder{ + return &RuntimeBuilder{ options: options, } } -// FunctionExecutionParams stores parameters related to js deno container creation -type FunctionExecutionParams struct { +// RuntimeExecutionParams stores parameters related to Deno/Python container creation +type RuntimeExecutionParams struct { ConfigMap string Parameters map[string]string SecureParameters string @@ -46,25 +46,25 @@ const ( FunctionMountName = "function-mount" ) -func (fb *FunctionBuilder) CreateContainerWithVolumes(ctx context.Context) (*corev1.Container, []corev1.Volume, error) { +func (fb *RuntimeBuilder) CreateContainer(ctx context.Context) (*corev1.Container, error) { var envVars []corev1.EnvVar params, err := fb.getParams(ctx) if err != nil { - return nil, nil, err + return nil, err } if len(params.Parameters) > 0 { jsonParams, err := json.Marshal(params.Parameters) if err != nil { - return nil, nil, err + return nil, err } envVars = append(envVars, corev1.EnvVar{Name: Data, Value: string(jsonParams)}) } jsonParams, err := json.Marshal(params.Context) if err != nil { - return nil, nil, err + return nil, err } envVars = append(envVars, corev1.EnvVar{Name: Context, Value: string(jsonParams)}) envVars = append(envVars, corev1.EnvVar{Name: CmdArgs, Value: params.CmdParameters}) @@ -79,10 +79,9 @@ func (fb *FunctionBuilder) CreateContainerWithVolumes(ctx context.Context) (*cor }, }) } - var jobVolumes []corev1.Volume + // Mount the function code if a ConfigMap is provided // The ConfigMap might be provided manually or created by the TaskDefinition controller - container := corev1.Container{ ImagePullPolicy: corev1.PullIfNotPresent, Name: "keptn-function-runner", @@ -92,17 +91,6 @@ func (fb *FunctionBuilder) CreateContainerWithVolumes(ctx context.Context) (*cor if params.ConfigMap != "" { envVars = append(envVars, corev1.EnvVar{Name: Script, Value: params.MountPath}) - jobVolumes = append(jobVolumes, corev1.Volume{ - Name: FunctionMountName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: params.ConfigMap, - }, - }, - }, - }) - container.VolumeMounts = []corev1.VolumeMount{ { Name: FunctionMountName, @@ -116,12 +104,36 @@ func (fb *FunctionBuilder) CreateContainerWithVolumes(ctx context.Context) (*cor } container.Env = envVars - return &container, jobVolumes, nil + return &container, nil + +} + +//nolint:nilnil +func (fb *RuntimeBuilder) CreateVolume(ctx context.Context) (*corev1.Volume, error) { + params, err := fb.getParams(ctx) + if err != nil { + return nil, err + } + + if params.ConfigMap != "" { + return &corev1.Volume{ + Name: FunctionMountName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: params.ConfigMap, + }, + }, + }, + }, nil + } + + return nil, nil } -func (fb *FunctionBuilder) getParams(ctx context.Context) (*FunctionExecutionParams, error) { - params, hasParent, err := fb.parseFunctionTaskDefinition( +func (fb *RuntimeBuilder) getParams(ctx context.Context) (*RuntimeExecutionParams, error) { + params, hasParent, err := fb.parseRuntimeTaskDefinition( fb.options.funcSpec, fb.options.task.Spec.TaskDefinition, fb.options.task.Namespace, @@ -156,8 +168,8 @@ func (fb *FunctionBuilder) getParams(ctx context.Context) (*FunctionExecutionPar return ¶ms, nil } -func (fb *FunctionBuilder) parseFunctionTaskDefinition(spec *klcv1alpha3.RuntimeSpec, name string, namespace string, configMap string) (FunctionExecutionParams, bool, error) { - params := FunctionExecutionParams{} +func (fb *RuntimeBuilder) parseRuntimeTaskDefinition(spec *klcv1alpha3.RuntimeSpec, name string, namespace string, configMap string) (RuntimeExecutionParams, bool, error) { + params := RuntimeExecutionParams{} // Firstly check if this task definition has a parent object hasParent := false @@ -195,8 +207,8 @@ func (fb *FunctionBuilder) parseFunctionTaskDefinition(spec *klcv1alpha3.Runtime return params, hasParent, nil } -func (fb *FunctionBuilder) handleParent(ctx context.Context, params *FunctionExecutionParams) error { - var parentJobParams FunctionExecutionParams +func (fb *RuntimeBuilder) handleParent(ctx context.Context, params *RuntimeExecutionParams) error { + var parentJobParams RuntimeExecutionParams parentDefinition, err := controllercommon.GetTaskDefinition(fb.options.Client, fb.options.Log, ctx, fb.options.funcSpec.FunctionReference.Name, fb.options.req.Namespace) if err != nil { controllercommon.RecordEvent(fb.options.recorder, apicommon.PhaseCreateTask, "Warning", fb.options.task, "TaskDefinitionNotFound", fmt.Sprintf("could not find KeptnTaskDefinition: %s ", fb.options.task.Spec.TaskDefinition), "") @@ -204,7 +216,7 @@ func (fb *FunctionBuilder) handleParent(ctx context.Context, params *FunctionExe } parSpec := controllercommon.GetRuntimeSpec(parentDefinition) // if the parent has also another parent, the data from the grandparent are alredy copied to the parent and therefore parent can copy it's data to the child - parentJobParams, _, err = fb.parseFunctionTaskDefinition(parSpec, parentDefinition.Name, parentDefinition.Namespace, parentDefinition.Status.Function.ConfigMap) + parentJobParams, _, err = fb.parseRuntimeTaskDefinition(parSpec, parentDefinition.Name, parentDefinition.Namespace, parentDefinition.Status.Function.ConfigMap) if err != nil { return err } diff --git a/operator/controllers/lifecycle/keptntask/function_builder_test.go b/operator/controllers/lifecycle/keptntask/runtime_builder_test.go similarity index 91% rename from operator/controllers/lifecycle/keptntask/function_builder_test.go rename to operator/controllers/lifecycle/keptntask/runtime_builder_test.go index d536e79985..055c21939d 100644 --- a/operator/controllers/lifecycle/keptntask/function_builder_test.go +++ b/operator/controllers/lifecycle/keptntask/runtime_builder_test.go @@ -5,6 +5,7 @@ import ( "github.com/go-logr/logr/testr" 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/keptn/lifecycle-toolkit/operator/controllers/common/fake" "github.com/stretchr/testify/require" @@ -50,7 +51,7 @@ func TestJSBuilder_handleParent(t *testing.T) { tests := []struct { name string options BuilderOptions - params FunctionExecutionParams + params RuntimeExecutionParams wantErr bool err string }{ @@ -66,7 +67,7 @@ func TestJSBuilder_handleParent(t *testing.T) { funcSpec: common.GetRuntimeSpec(def), task: makeTask("myt", "default", def.Name), }, - params: FunctionExecutionParams{}, + params: RuntimeExecutionParams{}, wantErr: true, err: "not found", }, @@ -82,7 +83,7 @@ func TestJSBuilder_handleParent(t *testing.T) { funcSpec: common.GetRuntimeSpec(def), task: makeTask("myt2", "default", def.Name), }, - params: FunctionExecutionParams{}, + params: RuntimeExecutionParams{}, wantErr: false, }, { @@ -97,13 +98,13 @@ func TestJSBuilder_handleParent(t *testing.T) { funcSpec: common.GetRuntimeSpec(paramDef), task: makeTask("myt3", "default", paramDef.Name), }, - params: FunctionExecutionParams{}, + params: RuntimeExecutionParams{}, wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - js := &FunctionBuilder{ + js := &RuntimeBuilder{ options: tt.options, } err := js.handleParent(context.TODO(), &tt.params) @@ -134,7 +135,8 @@ func TestJSBuilder_getParams(t *testing.T) { SecureParameters: klcv1alpha3.SecureParameters{ Secret: "parent_secret", }, - }}, + }, + }, Status: klcv1alpha3.KeptnTaskDefinitionStatus{ Function: klcv1alpha3.FunctionStatus{ ConfigMap: "mymap", @@ -196,7 +198,7 @@ func TestJSBuilder_getParams(t *testing.T) { tests := []struct { name string options BuilderOptions - params *FunctionExecutionParams + params *RuntimeExecutionParams wantErr bool err string }{ @@ -215,7 +217,7 @@ func TestJSBuilder_getParams(t *testing.T) { MountPath: common.FunctionScriptMountPath, ConfigMap: def.Status.Function.ConfigMap, }, - params: &FunctionExecutionParams{ + params: &RuntimeExecutionParams{ ConfigMap: def.Status.Function.ConfigMap, Parameters: def.Spec.Function.Parameters.Inline, SecureParameters: def.Spec.Function.SecureParameters.Secret, @@ -223,7 +225,9 @@ func TestJSBuilder_getParams(t *testing.T) { Context: klcv1alpha3.TaskContext{ WorkloadName: "my-workload", AppName: "my-app", - ObjectType: "Workload"}, + ObjectType: "Workload", + TaskType: string(apicommon.PostDeploymentCheckType), + }, Image: "js", MountPath: common.FunctionScriptMountPath, }, @@ -242,7 +246,7 @@ func TestJSBuilder_getParams(t *testing.T) { task: makeTask("myt3", "default", paramDef.Name), ConfigMap: def.Status.Function.ConfigMap, }, - params: &FunctionExecutionParams{ + params: &RuntimeExecutionParams{ ConfigMap: def.Status.Function.ConfigMap, Parameters: map[string]string{ //maps should be merged "DATA2": "parent_data", @@ -253,7 +257,9 @@ func TestJSBuilder_getParams(t *testing.T) { Context: klcv1alpha3.TaskContext{ WorkloadName: "my-workload", AppName: "my-app", - ObjectType: "Workload"}, + ObjectType: "Workload", + TaskType: string(apicommon.PostDeploymentCheckType), + }, Image: "js", MountPath: common.FunctionScriptMountPath, }, @@ -272,13 +278,15 @@ func TestJSBuilder_getParams(t *testing.T) { task: makeTask("myt4", "default", defJS.Name), ConfigMap: defJS.Status.Function.ConfigMap, }, - params: &FunctionExecutionParams{ + params: &RuntimeExecutionParams{ ConfigMap: parentPy.Status.Function.ConfigMap, URL: parentPy.Spec.Python.HttpReference.Url, //we support a single URL so the original should be taken not the parent one Context: klcv1alpha3.TaskContext{ WorkloadName: "my-workload", AppName: "my-app", - ObjectType: "Workload"}, + ObjectType: "Workload", + TaskType: string(apicommon.PostDeploymentCheckType), + }, Image: "python", MountPath: common.PythonScriptMountPath, }, @@ -287,7 +295,7 @@ func TestJSBuilder_getParams(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - js := &FunctionBuilder{ + js := &RuntimeBuilder{ options: tt.options, } params, err := js.getParams(context.TODO()) diff --git a/test/integration/simple-deployment-recursive-task/01-assert.yaml b/test/integration/simple-deployment-recursive-task/01-assert.yaml index d3c9d5cc38..576ecdf9aa 100644 --- a/test/integration/simple-deployment-recursive-task/01-assert.yaml +++ b/test/integration/simple-deployment-recursive-task/01-assert.yaml @@ -54,7 +54,8 @@ spec: - name: DATA value: '{"data":"myotherdata","other":"data","user":"myotheruser"}' - name: CONTEXT - value: '{"workloadName":"waiter-waiter","appName":"waiter","appVersion":"","workloadVersion":"0.4","taskType":"","objectType":"Workload"}' + value: >- + {"workloadName":"waiter-waiter","appName":"waiter","appVersion":"","workloadVersion":"0.4","taskType":"post","objectType":"Workload"} - name: CMD_ARGS - name: SCRIPT value: /var/data/function.ts @@ -89,7 +90,8 @@ spec: - name: DATA value: '{"data":"mydata","other":"data","user":"myuser"}' - name: CONTEXT - value: '{"workloadName":"waiter-waiter","appName":"waiter","appVersion":"","workloadVersion":"0.4","taskType":"","objectType":"Workload"}' + value: >- + {"workloadName":"waiter-waiter","appName":"waiter","appVersion":"","workloadVersion":"0.4","taskType":"pre","objectType":"Workload"} - name: CMD_ARGS - name: SECURE_DATA valueFrom: