From 94c4df196cbffead74df9f786c0afacaa37396c0 Mon Sep 17 00:00:00 2001 From: Katarina Strenkova Date: Mon, 12 Aug 2024 05:44:48 -0400 Subject: [PATCH] Move merging of workflow section into webhook Currently, the merging of workflow section is done in controllers of related CRs. We want to move the merging logic into webhooks to make the process cleaner. --- api/v1beta1/ansibletest_webhook.go | 12 ++- api/v1beta1/common_webhook.go | 78 ++++++++++++++++++ api/v1beta1/tempest_webhook.go | 6 ++ api/v1beta1/tobiko_webhook.go | 11 ++- controllers/ansibletest_controller.go | 96 +++------------------- controllers/common.go | 110 +++++++++++++++++--------- controllers/tempest_controller.go | 107 +++++++++---------------- controllers/tobiko_controller.go | 53 +++---------- 8 files changed, 238 insertions(+), 235 deletions(-) diff --git a/api/v1beta1/ansibletest_webhook.go b/api/v1beta1/ansibletest_webhook.go index 267a3258..e28ea71c 100644 --- a/api/v1beta1/ansibletest_webhook.go +++ b/api/v1beta1/ansibletest_webhook.go @@ -49,7 +49,17 @@ var _ webhook.Defaulter = &AnsibleTest{} func (r *AnsibleTest) Default() { ansibletestlog.Info("default", "name", r.Name) - // TODO(user): fill in your defaulting logic. + r.Spec.Default() +} + +// Default - set defaults for this AnsibleTest spec. +func (spec *AnsibleTestSpec) Default() { + if len(spec.Workflow) > 0 { + for i := range spec.Workflow { + mergeSectionIntoWorkflow(*spec, &spec.Workflow[i]) + } + + } } // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. diff --git a/api/v1beta1/common_webhook.go b/api/v1beta1/common_webhook.go index 7dd95bb0..0b2ea5b9 100644 --- a/api/v1beta1/common_webhook.go +++ b/api/v1beta1/common_webhook.go @@ -1,5 +1,10 @@ package v1beta1 +import ( + "fmt" + "reflect" +) + const ( // ErrPrivilegedModeRequired ErrPrivilegedModeRequired = "%s.Spec.Privileged is requied in order to successfully " + @@ -28,3 +33,76 @@ const ( "ensures that the copying of the logs to the PV is completed without any " + "complications." ) + +func isEmpty(value interface{}) bool { + if v, ok := value.(reflect.Value); ok { + switch v.Kind() { + case reflect.String, reflect.Map: + return v.Len() == 0 + case reflect.Ptr, reflect.Interface, reflect.Slice: + return v.IsNil() + } + } + return false +} + +// merge non-workflow section into workflow +func mergeSectionIntoWorkflow(main interface{}, workflow interface{}) { + mReflect := reflect.ValueOf(main) + wReflect := reflect.ValueOf(workflow).Elem() + + for i := 0; i < mReflect.NumField(); i++ { + name := mReflect.Type().Field(i).Name + mValue := mReflect.Field(i) + wValue := wReflect.FieldByName(name) + + fmt.Println("Name: ", name) + //fmt.Println("M Value: ", mValue) + //fmt.Println("W Value: ", wValue) + //fmt.Println("M Kind: ", mValue.Kind()) + //fmt.Println("W Kind: ", wValue.Kind()) + //fmt.Println("M Empty: ", isEmpty(mValue)) + //fmt.Println("W Empty: ", isEmpty(wValue)) + + if mValue.Kind() == reflect.Struct { + switch name { + case "CommonOptions": + wValue := wReflect.FieldByName("WorkflowCommonParameters") + mergeSectionIntoWorkflow(mValue.Interface(), wValue.Addr().Interface()) + case "TempestRun", "TempestconfRun": + mergeSectionIntoWorkflow(mValue.Interface(), wValue.Addr().Interface()) + case "Resources": + mergeSectionIntoWorkflow(mValue.Interface(), wValue.Interface()) + + } + continue + } + + if !wValue.IsValid() { + continue + } + + if isEmpty(wValue) && !isEmpty(mValue) { + if mValue.Kind() == reflect.Map { + mapCopy := reflect.MakeMap(mValue.Type()) + for _, key := range mValue.MapKeys() { + value := mValue.MapIndex(key) + mapCopy.SetMapIndex(key, value) + } + wValue = reflect.New(wValue.Type().Elem()).Elem() + wValue.Set(mapCopy) + } else if mValue.Kind() == reflect.Slice { + sliceCopy := reflect.MakeSlice(mValue.Type(), mValue.Len(), mValue.Cap()) + reflect.Copy(sliceCopy, mValue) + wValue = reflect.New(wValue.Type().Elem()).Elem() + wValue.Set(sliceCopy) + } else if wValue.Kind() == reflect.Ptr { + mPtr := reflect.New(mValue.Type()) + mPtr.Elem().Set(mValue) + wValue.Set(mPtr) + } else { + wValue.Set(mValue) + } + } + } +} diff --git a/api/v1beta1/tempest_webhook.go b/api/v1beta1/tempest_webhook.go index 13cb7c56..ed6cdc77 100644 --- a/api/v1beta1/tempest_webhook.go +++ b/api/v1beta1/tempest_webhook.go @@ -73,6 +73,12 @@ func (spec *TempestSpec) Default() { if spec.TempestconfRun == (TempestconfRunSpec{}) { spec.TempestconfRun.Create = true } + + if len(spec.Workflow) > 0 { + for i := range spec.Workflow { + mergeSectionIntoWorkflow(*spec, &spec.Workflow[i]) + } + } } func (r *Tempest) PrivilegedRequired() bool { diff --git a/api/v1beta1/tobiko_webhook.go b/api/v1beta1/tobiko_webhook.go index d6d08679..6a0139b4 100644 --- a/api/v1beta1/tobiko_webhook.go +++ b/api/v1beta1/tobiko_webhook.go @@ -52,7 +52,16 @@ var _ webhook.Defaulter = &Tobiko{} func (r *Tobiko) Default() { tobikolog.Info("default", "name", r.Name) - // TODO(user): fill in your defaulting logic. + r.Spec.Default() +} + +// Default - set defaults for this Tobiko spec. +func (spec *TobikoSpec) Default() { + if len(spec.Workflow) > 0 { + for i := range spec.Workflow { + mergeSectionIntoWorkflow(*spec, &spec.Workflow[i]) + } + } } // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. diff --git a/controllers/ansibletest_controller.go b/controllers/ansibletest_controller.go index b80d5849..10d4326b 100644 --- a/controllers/ansibletest_controller.go +++ b/controllers/ansibletest_controller.go @@ -23,8 +23,6 @@ import ( "strconv" "time" - "reflect" - "github.com/go-logr/logr" "github.com/openstack-k8s-operators/lib-common/modules/common" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" @@ -32,7 +30,6 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/helper" "github.com/openstack-k8s-operators/lib-common/modules/common/job" common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" - "github.com/openstack-k8s-operators/test-operator/api/v1beta1" testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1" "github.com/openstack-k8s-operators/test-operator/pkg/ansibletest" batchv1 "k8s.io/api/batch/v1" @@ -137,6 +134,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) workflowLength := len(instance.Spec.Workflow) nextAction, nextWorkflowStep, err := r.NextAction(ctx, instance, workflowLength) + instance.Spec = getMergedCR(instance, nextWorkflowStep).(testv1beta1.AnsibleTestSpec) switch nextAction { case Failure: @@ -214,32 +212,14 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Create a new job mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle") jobName := r.GetJobName(instance, nextWorkflowStep) - envVars, workflowOverrideParams := r.PrepareAnsibleEnv(instance, nextWorkflowStep) + envVars, workflowOverrideParams := r.PrepareAnsibleEnv(instance) logsPVCName := r.GetPVCLogsName(instance, 0) containerImage, err := r.GetContainerImage(ctx, workflowOverrideParams["ContainerImage"], instance) - privileged := r.OverwriteAnsibleWithWorkflow(instance.Spec, "Privileged", "pbool", nextWorkflowStep).(bool) + privileged := instance.Spec.Privileged if err != nil { return ctrl.Result{}, err } - if nextWorkflowStep < len(instance.Spec.Workflow) { - if instance.Spec.Workflow[nextWorkflowStep].NodeSelector != nil { - instance.Spec.NodeSelector = *instance.Spec.Workflow[nextWorkflowStep].NodeSelector - } - - if instance.Spec.Workflow[nextWorkflowStep].Tolerations != nil { - instance.Spec.Tolerations = *instance.Spec.Workflow[nextWorkflowStep].Tolerations - } - - if instance.Spec.Workflow[nextWorkflowStep].SELinuxLevel != nil { - instance.Spec.SELinuxLevel = *instance.Spec.Workflow[nextWorkflowStep].SELinuxLevel - } - - if instance.Spec.Workflow[nextWorkflowStep].Resources != nil { - instance.Spec.Resources = *instance.Spec.Workflow[nextWorkflowStep].Resources - } - } - // Service account, role, binding rbacRules := GetCommonRbacRules(privileged) rbacResult, err := common_rbac.ReconcileRbac(ctx, helper, instance, rbacRules) @@ -309,82 +289,32 @@ func (r *AnsibleTestReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func (r *Reconciler) OverwriteAnsibleWithWorkflow( - instance v1beta1.AnsibleTestSpec, - sectionName string, - workflowValueType string, - workflowStepNum int, -) interface{} { - if len(instance.Workflow)-1 < workflowStepNum { - reflected := reflect.ValueOf(instance) - fieldValue := reflected.FieldByName(sectionName) - return fieldValue.Interface() - } - - reflected := reflect.ValueOf(instance) - SpecValue := reflected.FieldByName(sectionName).Interface() - - reflected = reflect.ValueOf(instance.Workflow[workflowStepNum]) - WorkflowValue := reflected.FieldByName(sectionName).Interface() - - if workflowValueType == "pbool" { - if val, ok := WorkflowValue.(*bool); ok && val != nil { - return *(WorkflowValue.(*bool)) - } - return SpecValue.(bool) - } else if workflowValueType == "puint8" { - if val, ok := WorkflowValue.(*uint8); ok && val != nil { - return *(WorkflowValue.(*uint8)) - } - return SpecValue - } else if workflowValueType == "string" { - if val, ok := WorkflowValue.(string); ok && val != "" { - return WorkflowValue - } - return SpecValue - } - - return nil -} - // This function prepares env variables for a single workflow step. func (r *AnsibleTestReconciler) PrepareAnsibleEnv( instance *testv1beta1.AnsibleTest, - step int, ) (map[string]env.Setter, map[string]string) { // Prepare env vars envVars := make(map[string]env.Setter) workflowOverrideParams := make(map[string]string) // volumes workflow override - workflowOverrideParams["WorkloadSSHKeySecretName"] = r.OverwriteAnsibleWithWorkflow(instance.Spec, "WorkloadSSHKeySecretName", "string", step).(string) - workflowOverrideParams["ComputesSSHKeySecretName"] = r.OverwriteAnsibleWithWorkflow(instance.Spec, "ComputesSSHKeySecretName", "string", step).(string) - workflowOverrideParams["ContainerImage"] = r.OverwriteAnsibleWithWorkflow(instance.Spec, "ContainerImage", "string", step).(string) + workflowOverrideParams["WorkloadSSHKeySecretName"] = instance.Spec.WorkloadSSHKeySecretName + workflowOverrideParams["ComputesSSHKeySecretName"] = instance.Spec.ComputesSSHKeySecretName + workflowOverrideParams["ContainerImage"] = instance.Spec.ContainerImage // bool - debug := r.OverwriteAnsibleWithWorkflow(instance.Spec, "Debug", "pbool", step).(bool) + debug := instance.Spec.Debug if debug { envVars["POD_DEBUG"] = env.SetValue("true") } // strings - extraVars := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsibleExtraVars", "string", step).(string) - envVars["POD_ANSIBLE_EXTRA_VARS"] = env.SetValue(extraVars) - - extraVarsFile := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsibleVarFiles", "string", step).(string) - envVars["POD_ANSIBLE_FILE_EXTRA_VARS"] = env.SetValue(extraVarsFile) - - inventory := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsibleInventory", "string", step).(string) - envVars["POD_ANSIBLE_INVENTORY"] = env.SetValue(inventory) - - gitRepo := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsibleGitRepo", "string", step).(string) - envVars["POD_ANSIBLE_GIT_REPO"] = env.SetValue(gitRepo) - - playbookPath := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsiblePlaybookPath", "string", step).(string) - envVars["POD_ANSIBLE_PLAYBOOK"] = env.SetValue(playbookPath) - - ansibleCollections := r.OverwriteAnsibleWithWorkflow(instance.Spec, "AnsibleCollections", "string", step).(string) - envVars["POD_INSTALL_COLLECTIONS"] = env.SetValue(ansibleCollections) + envVars["POD_ANSIBLE_EXTRA_VARS"] = env.SetValue(instance.Spec.AnsibleExtraVars) + envVars["POD_ANSIBLE_FILE_EXTRA_VARS"] = env.SetValue(instance.Spec.AnsibleVarFiles) + envVars["POD_ANSIBLE_INVENTORY"] = env.SetValue(instance.Spec.AnsibleInventory) + envVars["POD_ANSIBLE_GIT_REPO"] = env.SetValue(instance.Spec.AnsibleGitRepo) + envVars["POD_ANSIBLE_PLAYBOOK"] = env.SetValue(instance.Spec.AnsiblePlaybookPath) + envVars["POD_INSTALL_COLLECTIONS"] = env.SetValue(instance.Spec.AnsibleCollections) return envVars, workflowOverrideParams } diff --git a/controllers/common.go b/controllers/common.go index 4abed53b..2ff0fc6c 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -570,44 +570,6 @@ func (r *Reconciler) setConfigOverwrite(customData map[string]string, configOver } } -func (r *Reconciler) OverwriteValueWithWorkflow( - instance v1beta1.TobikoSpec, - sectionName string, - workflowValueType string, - workflowStepNum int, -) interface{} { - if len(instance.Workflow)-1 < workflowStepNum { - reflected := reflect.ValueOf(instance) - fieldValue := reflected.FieldByName(sectionName) - return fieldValue.Interface() - } - - reflected := reflect.ValueOf(instance) - tobikoSpecValue := reflected.FieldByName(sectionName).Interface() - - reflected = reflect.ValueOf(instance.Workflow[workflowStepNum]) - tobikoWorkflowValue := reflected.FieldByName(sectionName).Interface() - - if workflowValueType == "pbool" { - if val, ok := tobikoWorkflowValue.(*bool); ok && val != nil { - return *(tobikoWorkflowValue.(*bool)) - } - return tobikoSpecValue.(bool) - } else if workflowValueType == "puint8" { - if val, ok := tobikoWorkflowValue.(*uint8); ok && val != nil { - return *(tobikoWorkflowValue.(*uint8)) - } - return tobikoSpecValue - } else if workflowValueType == "string" { - if val, ok := tobikoWorkflowValue.(string); ok && val != "" { - return tobikoWorkflowValue - } - return tobikoSpecValue - } - - return nil -} - func GetCommonRbacRules(privileged bool) []rbacv1.PolicyRule { rbacPolicyRule := rbacv1.PolicyRule{ APIGroups: []string{"security.openshift.io"}, @@ -693,3 +655,75 @@ func EnsureCloudsConfigMapExists( return ctrl.Result{}, nil } + +// TODO remove repetitive code +func getMergedCR(instance interface{}, workflowStepNum int) interface{} { + switch typedInstance := instance.(type) { + case *v1beta1.Tempest: + if workflowStepNum < len(typedInstance.Spec.Workflow) { + workflow := typedInstance.Spec.Workflow[workflowStepNum] + mergeSections(&typedInstance.Spec, workflow) + } + return typedInstance.Spec + case *v1beta1.Tobiko: + if workflowStepNum < len(typedInstance.Spec.Workflow) { + workflow := typedInstance.Spec.Workflow[workflowStepNum] + mergeSections(&typedInstance.Spec, workflow) + } + return typedInstance.Spec + case *v1beta1.AnsibleTest: + if workflowStepNum < len(typedInstance.Spec.Workflow) { + workflow := typedInstance.Spec.Workflow[workflowStepNum] + mergeSections(&typedInstance.Spec, workflow) + } + return typedInstance.Spec + default: + return nil + } +} + +func mergeSections(main interface{}, workflow interface{}) { + mReflect := reflect.ValueOf(main).Elem() + wReflect := reflect.ValueOf(workflow) + + for i := 0; i < mReflect.NumField(); i++ { + name := mReflect.Type().Field(i).Name + mValue := mReflect.Field(i) + wValue := wReflect.FieldByName(name) + + //fmt.Println("MERGE SECTIONS") + //fmt.Println("Name", name) + //fmt.Println("M value", mValue) + //fmt.Println("M Kind", mValue.Kind()) + //fmt.Println("W value", wValue) + //fmt.Println("W Kind", wValue.Kind()) + + if mValue.Kind() == reflect.Struct { + switch name { + case "CommonOptions": + wValue := wReflect.FieldByName("WorkflowCommonParameters") + mergeSections(mValue.Addr().Interface(), wValue.Interface()) + case "TempestRun", "TempestconfRun": + mergeSections(mValue.Addr().Interface(), wValue.Interface()) + case "Resources": + if !wValue.IsNil() { + mValue.Set(wValue.Elem()) + } + } + continue + } + + if !wValue.IsValid() { + continue + } + + if wValue.Kind() == reflect.Ptr && mValue.Kind() != reflect.Ptr { + if wValue.IsNil() { + continue + } + wValue = wValue.Elem() + } + mValue.Set(wValue) + + } +} diff --git a/controllers/tempest_controller.go b/controllers/tempest_controller.go index 512e6a0f..477935c8 100644 --- a/controllers/tempest_controller.go +++ b/controllers/tempest_controller.go @@ -154,6 +154,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re workflowLength := len(instance.Spec.Workflow) nextAction, nextWorkflowStep, err := r.NextAction(ctx, instance, workflowLength) + instance.Spec = getMergedCR(instance, nextWorkflowStep).(testv1beta1.TempestSpec) switch nextAction { case Failure: @@ -340,26 +341,6 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re } // Service account, role, binding - end - // Note(lpiwowar): Remove all the workflow merge code to webhook once it is done. - // It will simplify the logic and duplicite code (Tempest vs Tobiko) - if nextWorkflowStep < len(instance.Spec.Workflow) { - if instance.Spec.Workflow[nextWorkflowStep].NodeSelector != nil { - instance.Spec.NodeSelector = *instance.Spec.Workflow[nextWorkflowStep].NodeSelector - } - - if instance.Spec.Workflow[nextWorkflowStep].Tolerations != nil { - instance.Spec.Tolerations = *instance.Spec.Workflow[nextWorkflowStep].Tolerations - } - - if instance.Spec.Workflow[nextWorkflowStep].SELinuxLevel != nil { - instance.Spec.SELinuxLevel = *instance.Spec.Workflow[nextWorkflowStep].SELinuxLevel - } - - if instance.Spec.Workflow[nextWorkflowStep].Resources != nil { - instance.Spec.Resources = *instance.Spec.Workflow[nextWorkflowStep].Resources - } - } - jobDef := tempest.Job( instance, serviceLabels, @@ -441,36 +422,32 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, workflowStepNum int, ) { tRun := instance.Spec.TempestRun - wtRun := testv1beta1.WorkflowTempestRunSpec{} - if workflowStepNum < len(instance.Spec.Workflow) { - wtRun = instance.Spec.Workflow[workflowStepNum].TempestRun - } testOperatorDir := "/etc/test_operator/" // Files - value := mergeWithWorkflow(tRun.WorkerFile, wtRun.WorkerFile) + value := tRun.WorkerFile if len(value) != 0 { workerFile := "worker_file.yaml" customData[workerFile] = value envVars["TEMPEST_WORKER_FILE"] = testOperatorDir + workerFile } - value = mergeWithWorkflow(tRun.IncludeList, wtRun.IncludeList) + value = tRun.IncludeList if len(value) != 0 { includeListFile := "include.txt" customData[includeListFile] = value envVars["TEMPEST_INCLUDE_LIST"] = testOperatorDir + includeListFile } - value = mergeWithWorkflow(tRun.ExcludeList, wtRun.ExcludeList) + value = tRun.ExcludeList if len(value) != 0 { excludeListFile := "exclude.txt" customData[excludeListFile] = value envVars["TEMPEST_EXCLUDE_LIST"] = testOperatorDir + excludeListFile } - value = mergeWithWorkflow(tRun.ExpectedFailuresList, wtRun.ExpectedFailuresList) + value = tRun.ExpectedFailuresList if len(value) != 0 { expectedFailuresListFile := "expected_failures.txt" customData[expectedFailuresListFile] = value @@ -479,9 +456,9 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, // Bool tempestBoolEnvVars := map[string]bool{ - "TEMPEST_SERIAL": mergeWithWorkflow(tRun.Serial, wtRun.Serial), - "TEMPEST_PARALLEL": mergeWithWorkflow(tRun.Parallel, wtRun.Parallel), - "TEMPEST_SMOKE": mergeWithWorkflow(tRun.Smoke, wtRun.Smoke), + "TEMPEST_SERIAL": tRun.Serial, + "TEMPEST_PARALLEL": tRun.Parallel, + "TEMPEST_SMOKE": tRun.Smoke, "USE_EXTERNAL_FILES": true, } @@ -490,11 +467,11 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, } // Int - numValue := mergeWithWorkflow(tRun.Concurrency, wtRun.Concurrency) + numValue := tRun.Concurrency envVars["TEMPEST_CONCURRENCY"] = r.GetDefaultInt(numValue) // Dictionary - dictValue := mergeWithWorkflow(tRun.ExternalPlugin, wtRun.ExternalPlugin) + dictValue := tRun.ExternalPlugin for _, externalPluginDictionary := range dictValue { envVars["TEMPEST_EXTERNAL_PLUGIN_GIT_URL"] += externalPluginDictionary.Repository + "," @@ -510,7 +487,7 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, envVars["TEMPEST_WORKFLOW_STEP_DIR_NAME"] = r.GetJobName(instance, workflowStepNum) - extraImages := mergeWithWorkflow(tRun.ExtraImages, wtRun.ExtraImages) + extraImages := tRun.ExtraImages for _, extraImageDict := range extraImages { envVars["TEMPEST_EXTRA_IMAGES_URL"] += extraImageDict.URL + "," envVars["TEMPEST_EXTRA_IMAGES_OS_CLOUD"] += extraImageDict.OsCloud + "," @@ -528,20 +505,12 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, envVars["TEMPEST_EXTRA_IMAGES_FLAVOR_VCPUS"] += r.GetDefaultInt(extraImageDict.Flavor.Vcpus, "-") + "," } - extraRPMs := mergeWithWorkflow(tRun.ExtraRPMs, wtRun.ExtraRPMs) + extraRPMs := tRun.ExtraRPMs for _, extraRPMURL := range extraRPMs { envVars["TEMPEST_EXTRA_RPMS"] += extraRPMURL + "," } } -func mergeWithWorkflow[T any](value T, workflowValue *T) T { - if workflowValue == nil { - return value - } - - return *workflowValue -} - func (r *TempestReconciler) setTempestconfConfigVars( envVars map[string]string, customData map[string]string, @@ -549,27 +518,23 @@ func (r *TempestReconciler) setTempestconfConfigVars( workflowStepNum int, ) { tcRun := instance.Spec.TempestconfRun - wtcRun := testv1beta1.WorkflowTempestconfRunSpec{} - if workflowStepNum < len(instance.Spec.Workflow) { - wtcRun = instance.Spec.Workflow[workflowStepNum].TempestconfRun - } testOperatorDir := "/etc/test_operator/" - value := mergeWithWorkflow(tcRun.DeployerInput, wtcRun.DeployerInput) + value := tcRun.DeployerInput if len(value) != 0 { deployerInputFile := "deployer_input.ini" customData[deployerInputFile] = value envVars["TEMPESTCONF_DEPLOYER_INPUT"] = testOperatorDir + deployerInputFile } - value = mergeWithWorkflow(tcRun.TestAccounts, wtcRun.TestAccounts) + value = tcRun.TestAccounts if len(value) != 0 { accountsFile := "accounts.yaml" customData[accountsFile] = value envVars["TEMPESTCONF_TEST_ACCOUNTS"] = testOperatorDir + accountsFile } - value = mergeWithWorkflow(tcRun.Profile, wtcRun.Profile) + value = tcRun.Profile if len(value) != 0 { profileFile := "profile.yaml" customData[profileFile] = value @@ -578,15 +543,15 @@ func (r *TempestReconciler) setTempestconfConfigVars( // Bool tempestconfBoolEnvVars := map[string]bool{ - "TEMPESTCONF_CREATE": mergeWithWorkflow(tcRun.Create, wtcRun.Create), - "TEMPESTCONF_COLLECT_TIMING": mergeWithWorkflow(tcRun.CollectTiming, wtcRun.CollectTiming), - "TEMPESTCONF_INSECURE": mergeWithWorkflow(tcRun.Insecure, wtcRun.Insecure), - "TEMPESTCONF_NO_DEFAULT_DEPLOYER": mergeWithWorkflow(tcRun.NoDefaultDeployer, wtcRun.NoDefaultDeployer), - "TEMPESTCONF_DEBUG": mergeWithWorkflow(tcRun.Debug, wtcRun.Debug), - "TEMPESTCONF_VERBOSE": mergeWithWorkflow(tcRun.Verbose, wtcRun.Verbose), - "TEMPESTCONF_NON_ADMIN": mergeWithWorkflow(tcRun.NonAdmin, wtcRun.NonAdmin), - "TEMPESTCONF_RETRY_IMAGE": mergeWithWorkflow(tcRun.RetryImage, wtcRun.RetryImage), - "TEMPESTCONF_CONVERT_TO_RAW": mergeWithWorkflow(tcRun.ConvertToRaw, wtcRun.ConvertToRaw), + "TEMPESTCONF_CREATE": tcRun.Create, + "TEMPESTCONF_COLLECT_TIMING": tcRun.CollectTiming, + "TEMPESTCONF_INSECURE": tcRun.Insecure, + "TEMPESTCONF_NO_DEFAULT_DEPLOYER": tcRun.NoDefaultDeployer, + "TEMPESTCONF_DEBUG": tcRun.Debug, + "TEMPESTCONF_VERBOSE": tcRun.Verbose, + "TEMPESTCONF_NON_ADMIN": tcRun.NonAdmin, + "TEMPESTCONF_RETRY_IMAGE": tcRun.RetryImage, + "TEMPESTCONF_CONVERT_TO_RAW": tcRun.ConvertToRaw, } for key, value := range tempestconfBoolEnvVars { @@ -594,9 +559,9 @@ func (r *TempestReconciler) setTempestconfConfigVars( } tempestconfIntEnvVars := map[string]int64{ - "TEMPESTCONF_TIMEOUT": mergeWithWorkflow(tcRun.Timeout, wtcRun.Timeout), - "TEMPESTCONF_FLAVOR_MIN_MEM": mergeWithWorkflow(tcRun.FlavorMinMem, wtcRun.FlavorMinMem), - "TEMPESTCONF_FLAVOR_MIN_DISK": mergeWithWorkflow(tcRun.FlavorMinDisk, wtcRun.FlavorMinDisk), + "TEMPESTCONF_TIMEOUT": tcRun.Timeout, + "TEMPESTCONF_FLAVOR_MIN_MEM": tcRun.FlavorMinMem, + "TEMPESTCONF_FLAVOR_MIN_DISK": tcRun.FlavorMinDisk, } for key, value := range tempestconfIntEnvVars { @@ -604,31 +569,31 @@ func (r *TempestReconciler) setTempestconfConfigVars( } // String - mValue := mergeWithWorkflow(tcRun.Out, wtcRun.Out) + mValue := tcRun.Out envVars["TEMPESTCONF_OUT"] = mValue - mValue = mergeWithWorkflow(tcRun.CreateAccountsFile, wtcRun.CreateAccountsFile) + mValue = tcRun.CreateAccountsFile envVars["TEMPESTCONF_CREATE_ACCOUNTS_FILE"] = mValue - mValue = mergeWithWorkflow(tcRun.GenerateProfile, wtcRun.GenerateProfile) + mValue = tcRun.GenerateProfile envVars["TEMPESTCONF_GENERATE_PROFILE"] = mValue - mValue = mergeWithWorkflow(tcRun.ImageDiskFormat, wtcRun.ImageDiskFormat) + mValue = tcRun.ImageDiskFormat envVars["TEMPESTCONF_IMAGE_DISK_FORMAT"] = mValue - mValue = mergeWithWorkflow(tcRun.Image, wtcRun.Image) + mValue = tcRun.Image envVars["TEMPESTCONF_IMAGE"] = mValue - mValue = mergeWithWorkflow(tcRun.NetworkID, wtcRun.NetworkID) + mValue = tcRun.NetworkID envVars["TEMPESTCONF_NETWORK_ID"] = mValue - mValue = mergeWithWorkflow(tcRun.Append, wtcRun.Append) + mValue = tcRun.Append envVars["TEMPESTCONF_APPEND"] = mValue - mValue = mergeWithWorkflow(tcRun.Remove, wtcRun.Remove) + mValue = tcRun.Remove envVars["TEMPESTCONF_REMOVE"] = mValue - mValue = mergeWithWorkflow(tcRun.Overrides, wtcRun.Overrides) + mValue = tcRun.Overrides envVars["TEMPESTCONF_OVERRIDES"] = mValue } diff --git a/controllers/tobiko_controller.go b/controllers/tobiko_controller.go index 7d1e9e46..8605b485 100644 --- a/controllers/tobiko_controller.go +++ b/controllers/tobiko_controller.go @@ -141,6 +141,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res workflowLength := len(instance.Spec.Workflow) nextAction, nextWorkflowStep, err := r.NextAction(ctx, instance, workflowLength) + instance.Spec = getMergedCR(instance, nextWorkflowStep).(testv1beta1.TobikoSpec) switch nextAction { case Failure: @@ -313,7 +314,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res jobName := r.GetJobName(instance, nextWorkflowStep) logsPVCName := r.GetPVCLogsName(instance, workflowStepNum) containerImage, err := r.GetContainerImage(ctx, instance.Spec.ContainerImage, instance) - privileged := r.OverwriteValueWithWorkflow(instance.Spec, "Privileged", "pbool", nextWorkflowStep).(bool) + privileged := instance.Spec.Privileged if err != nil { return ctrl.Result{}, err } @@ -387,50 +388,23 @@ func (r *TobikoReconciler) PrepareTobikoEnvVars( labels map[string]string, instance *testv1beta1.Tobiko, helper *helper.Helper, - step int, + workflowStepNum int, ) map[string]env.Setter { - - // NOTE(lpiwowar): Move all the merge code to the webhook once it is completed. - // It will clean up the workflow code and remove the duplicit code - // (Tempest vs Tobiko) - if step < len(instance.Spec.Workflow) { - if instance.Spec.Workflow[step].NodeSelector != nil { - instance.Spec.NodeSelector = *instance.Spec.Workflow[step].NodeSelector - } - - if instance.Spec.Workflow[step].Tolerations != nil { - instance.Spec.Tolerations = *instance.Spec.Workflow[step].Tolerations - } - - if instance.Spec.Workflow[step].SELinuxLevel != nil { - instance.Spec.SELinuxLevel = *instance.Spec.Workflow[step].SELinuxLevel - } - - if instance.Spec.Workflow[step].Resources != nil { - instance.Spec.Resources = *instance.Spec.Workflow[step].Resources - } - } - // Prepare env vars envVars := make(map[string]env.Setter) envVars["USE_EXTERNAL_FILES"] = env.SetValue("True") - envVars["TOBIKO_LOGS_DIR_NAME"] = env.SetValue(r.GetJobName(instance, step)) - - testenv := r.OverwriteValueWithWorkflow(instance.Spec, "Testenv", "string", step).(string) - envVars["TOBIKO_TESTENV"] = env.SetValue(testenv) - - version := r.OverwriteValueWithWorkflow(instance.Spec, "Version", "string", step).(string) - envVars["TOBIKO_VERSION"] = env.SetValue(version) + envVars["TOBIKO_LOGS_DIR_NAME"] = env.SetValue(r.GetJobName(instance, workflowStepNum)) - pytestAddopts := r.OverwriteValueWithWorkflow(instance.Spec, "PytestAddopts", "string", step).(string) - envVars["TOBIKO_PYTEST_ADDOPTS"] = env.SetValue(pytestAddopts) + envVars["TOBIKO_TESTENV"] = env.SetValue(instance.Spec.Testenv) + envVars["TOBIKO_VERSION"] = env.SetValue(instance.Spec.Version) + envVars["TOBIKO_PYTEST_ADDOPTS"] = env.SetValue(instance.Spec.PytestAddopts) - preventCreate := r.OverwriteValueWithWorkflow(instance.Spec, "PreventCreate", "pbool", step).(bool) + preventCreate := instance.Spec.PreventCreate if preventCreate { envVars["TOBIKO_PREVENT_CREATE"] = env.SetValue("True") } - numProcesses := r.OverwriteValueWithWorkflow(instance.Spec, "NumProcesses", "puint8", step).(uint8) + numProcesses := instance.Spec.NumProcesses if numProcesses > 0 { envVars["TOX_NUM_PROCESSES"] = env.SetValue(strconv.Itoa(int(numProcesses))) } @@ -441,16 +415,13 @@ func (r *TobikoReconciler) PrepareTobikoEnvVars( // Prepare custom data customData := make(map[string]string) - tobikoConf := r.OverwriteValueWithWorkflow(instance.Spec, "Config", "string", step).(string) - customData["tobiko.conf"] = tobikoConf + customData["tobiko.conf"] = instance.Spec.Config privateKeyData := make(map[string]string) - privateKey := r.OverwriteValueWithWorkflow(instance.Spec, "PrivateKey", "string", step).(string) - privateKeyData["id_ecdsa"] = privateKey + privateKeyData["id_ecdsa"] = instance.Spec.PrivateKey publicKeyData := make(map[string]string) - publicKey := r.OverwriteValueWithWorkflow(instance.Spec, "PublicKey", "string", step).(string) - publicKeyData["id_ecdsa.pub"] = publicKey + publicKeyData["id_ecdsa.pub"] = instance.Spec.PublicKey cms := []util.Template{ {