From 0287c9ab24319f83365c74653b978983d1f91f42 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/common_webhook.go | 60 +++++++++++++++++++++++++++++++ api/v1beta1/tempest_webhook.go | 6 ++++ controllers/common.go | 35 ++++++++++++++++++ controllers/tempest_controller.go | 28 +++++++-------- 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/api/v1beta1/common_webhook.go b/api/v1beta1/common_webhook.go index 4c3f3280..da2a5ebc 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,58 @@ const ( "ensures that the copying of the logs to the PV is completed without any " + "complications." ) + +// merge non-workflow section into workflow +func mergeSectionIntoWorkflow(instance interface{}, workflowStepNum int) { + // TODO add other resources + switch spec := instance.(type) { + case *TempestSpec: + fmt.Println("Instance is type *TempestSpec") + + tRun := spec.TempestRun + wtRun := &spec.Workflow[workflowStepNum].TempestRun + + tRunReflect := reflect.ValueOf(tRun) + wtRunReflect := reflect.ValueOf(wtRun).Elem() + + setNonZeroValues(tRunReflect, wtRunReflect, false) + default: + fmt.Println("Error, instance of unknown type.") + } +} + +func isEmpty(value interface{}) bool { + v := reflect.ValueOf(value) + + switch v.Kind() { + case reflect.String: + return v.Len() == 0 + case reflect.Ptr, reflect.Interface, reflect.Slice: + return v.IsNil() + default: + zero := reflect.Zero(v.Type()).Interface() + return reflect.DeepEqual(value, zero) + } +} + +func setNonZeroValues(src reflect.Value, dest reflect.Value, isStruct bool) { + for i := 0; i < src.NumField(); i++ { + tRunName := src.Type().Field(i).Name + tRunValue := src.Field(i) + wtRunValue := dest.FieldByName(tRunName) + + if isEmpty(wtRunValue) && !isEmpty(tRunValue.IsNil()) { + if tRunValue.Kind() == reflect.Struct { + setNonZeroValues(tRunValue, wtRunValue, true) + } else { + if isStruct { + wtRunValue.Set(tRunValue) + } else { + tRunPtr := reflect.New(tRunValue.Type()) + tRunPtr.Elem().Set(tRunValue) + wtRunValue.Set(tRunPtr) + } + } + } + } +} diff --git a/api/v1beta1/tempest_webhook.go b/api/v1beta1/tempest_webhook.go index 13cb7c56..53020e83 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 key, _ := range spec.Workflow { + mergeSectionIntoWorkflow(spec, key) + } + } } func (r *Tempest) PrivilegedRequired() bool { diff --git a/controllers/common.go b/controllers/common.go index 4abed53b..6caa7262 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -693,3 +693,38 @@ func EnsureCloudsConfigMapExists( return ctrl.Result{}, nil } + +// TODO make general for all resources +func getResourceRun(instance *v1beta1.Tempest, workflowStepNum int) v1beta1.TempestRunSpec { + if workflowStepNum < len(instance.Spec.Workflow) { + newInstance := changeType(instance, workflowStepNum).(*v1beta1.Tempest) + return newInstance.Spec.TempestRun + } + return instance.Spec.TempestRun +} + +func changeType(instance interface{}, workflowStepNum int) interface{} { + // TODO other types; else if typedInstance, ok := instance.(*v1beta1.HorizonTest); + + typedInstance, _ := instance.(*v1beta1.Tempest) + wtRun := typedInstance.Spec.Workflow[workflowStepNum].TempestRun + + var tRun v1beta1.TempestRunSpec + + wtReflected := reflect.ValueOf(wtRun) + tReflected := reflect.ValueOf(&tRun).Elem() + + for i := 0; i < wtReflected.NumField(); i++ { + tName := tReflected.Type().Field(i).Name + tValue := tReflected.Field(i) + + wtValue := wtReflected.FieldByName(tName) + if !wtValue.IsNil() { + wtValue = wtValue.Elem() + tValue.Set(wtValue) + } + } + typedInstance.Spec.TempestRun = tRun + + return typedInstance +} diff --git a/controllers/tempest_controller.go b/controllers/tempest_controller.go index d82ff242..35aafdcc 100644 --- a/controllers/tempest_controller.go +++ b/controllers/tempest_controller.go @@ -436,37 +436,33 @@ func (r *TempestReconciler) setTempestConfigVars(envVars map[string]string, instance *testv1beta1.Tempest, workflowStepNum int, ) { - tRun := instance.Spec.TempestRun - wtRun := testv1beta1.WorkflowTempestRunSpec{} - if workflowStepNum < len(instance.Spec.Workflow) { - wtRun = instance.Spec.Workflow[workflowStepNum].TempestRun - } + tRun := getResourceRun(instance, workflowStepNum) 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 @@ -475,9 +471,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, } @@ -486,11 +482,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 + "," @@ -506,7 +502,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 + "," @@ -524,7 +520,7 @@ 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 + "," }