Skip to content

Refactor common controller functions #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2025
+106 −162
Merged

Refactor common controller functions #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/ansibletest_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
podName := r.GetPodName(instance, nextWorkflowStep)
envVars := r.PrepareAnsibleEnv(instance)
logsPVCName := r.GetPVCLogsName(instance, 0)
containerImage, err := r.GetContainerImage(ctx, instance.Spec.ContainerImage, instance)
containerImage, err := r.GetContainerImage(ctx, instance)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
245 changes: 88 additions & 157 deletions controllers/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"reflect"
"strconv"
"strings"
"time"

"crypto/sha256"
Expand All @@ -15,7 +16,6 @@ import (
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
"github.com/openstack-k8s-operators/lib-common/modules/common/pvc"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
v1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
"gopkg.in/yaml.v3"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
Expand All @@ -30,15 +30,13 @@ import (
)

const (
podNameStepInfix = "-s"
envVarsConfigMapInfix = "-env-vars-s"
customDataConfigMapInfix = "-custom-data-s"
workflowStepNumInvalid = -1
workflowStepNameInvalid = "no-name"
workflowStepLabel = "workflowStep"
instanceNameLabel = "instanceName"
operatorNameLabel = "operator"

podNameStepInfix = "-s"
envVarsConfigMapInfix = "-env-vars-s"
customDataConfigMapInfix = "-custom-data-s"
workflowStepNameInvalid = "no-name"
workflowStepLabel = "workflowStep"
instanceNameLabel = "instanceName"
operatorNameLabel = "operator"
testOperatorLockName = "test-operator-lock"
testOperatorLockOwnerField = "owner"
)
Expand Down Expand Up @@ -82,6 +80,15 @@ var (

// ErrLockFieldMissing indicates that a required field is missing in the lock config map.
ErrLockFieldMissing = errors.New("field is missing in the config map")

// ErrFieldExpectedStruct indicates attempting to access a field on a non-struct type.
ErrFieldExpectedStruct = errors.New("field cannot be accessed: expected struct")

// ErrFieldNilPointer indicates attempting to dereference a nil pointer.
ErrFieldNilPointer = errors.New("field cannot be accessed: nil pointer")

// ErrFieldNotFound indicates a field name does not exist on the struct.
ErrFieldNotFound = errors.New("field not found")
)

// Reconciler provides common functionality for all test framework reconcilers
Expand Down Expand Up @@ -253,164 +260,68 @@ func (r *Reconciler) GetLastPod(
return maxPod, nil
}

// GetEnvVarsConfigMapName returns the name of the environment variables ConfigMap for the given instance and workflow step
func GetEnvVarsConfigMapName(instance interface{}, workflowStepNum int) string {
if _, ok := instance.(*v1beta1.Tobiko); ok {
return "not-implemented"
} else if typedInstance, ok := instance.(*v1beta1.Tempest); ok {
return typedInstance.Name + envVarsConfigMapInfix + strconv.Itoa(workflowStepNum)
}

return "not-implemented"
}

// GetCustomDataConfigMapName returns the name of the custom data ConfigMap for the given instance and workflow step
func GetCustomDataConfigMapName(instance interface{}, workflowStepNum int) string {
if _, ok := instance.(*v1beta1.Tobiko); ok {
return "not-implemented"
} else if typedInstance, ok := instance.(*v1beta1.Tempest); ok {
return typedInstance.Name + customDataConfigMapInfix + strconv.Itoa(workflowStepNum)
}

return "not-implemented"
}

// GetContainerImage returns the container image to use for the given instance, either from the provided parameter or from configuration
func (r *Reconciler) GetContainerImage(
ctx context.Context,
containerImage string,
instance interface{},
) (string, error) {
cm := &corev1.ConfigMap{}
testOperatorConfigMapName := "test-operator-config"
if typedInstance, ok := instance.(*v1beta1.Tempest); ok {
if len(containerImage) > 0 {
return containerImage, nil
}

objectKey := client.ObjectKey{Namespace: typedInstance.Namespace, Name: testOperatorConfigMapName}
err := r.Client.Get(ctx, objectKey, cm)
if err != nil {
return "", err
}

if cm.Data == nil {
return util.GetEnvVar("RELATED_IMAGE_TEST_TEMPEST_IMAGE_URL_DEFAULT", ""), nil

}

if cmImage, exists := cm.Data["tempest-image"]; exists {
return cmImage, nil
}

return util.GetEnvVar("RELATED_IMAGE_TEST_TEMPEST_IMAGE_URL_DEFAULT", ""), nil
} else if typedInstance, ok := instance.(*v1beta1.Tobiko); ok {
if len(containerImage) > 0 {
return containerImage, nil
}

objectKey := client.ObjectKey{Namespace: typedInstance.Namespace, Name: testOperatorConfigMapName}
err := r.Client.Get(ctx, objectKey, cm)
if err != nil {
return "", err
}

if cm.Data == nil {
return util.GetEnvVar("RELATED_IMAGE_TEST_TOBIKO_IMAGE_URL_DEFAULT", ""), nil

}

if cmImage, exists := cm.Data["tobiko-image"]; exists {
return cmImage, nil
}

return util.GetEnvVar("RELATED_IMAGE_TEST_TOBIKO_IMAGE_URL_DEFAULT", ""), nil
} else if typedInstance, ok := instance.(*v1beta1.HorizonTest); ok {
if len(containerImage) > 0 {
return containerImage, nil
}

objectKey := client.ObjectKey{Namespace: typedInstance.Namespace, Name: testOperatorConfigMapName}
err := r.Client.Get(ctx, objectKey, cm)
if err != nil {
return "", err
}

if cm.Data == nil {
return util.GetEnvVar("RELATED_IMAGE_TEST_HORIZONTEST_IMAGE_URL_DEFAULT", ""), nil

}
v := reflect.ValueOf(instance)

if cmImage, exists := cm.Data["horizontest-image"]; exists {
return cmImage, nil
}
spec, err := SafetyCheck(v, "Spec")
if err != nil {
return "", err
}

return util.GetEnvVar("RELATED_IMAGE_TEST_HORIZONTEST_IMAGE_URL_DEFAULT", ""), nil
} else if typedInstance, ok := instance.(*v1beta1.AnsibleTest); ok {
if len(containerImage) > 0 {
return containerImage, nil
}
containerImage := GetStringField(spec, "ContainerImage")
if containerImage != "" {
return containerImage, nil
}

objectKey := client.ObjectKey{Namespace: typedInstance.Namespace, Name: testOperatorConfigMapName}
err := r.Client.Get(ctx, objectKey, cm)
if err != nil {
return "", err
}
namespace := GetStringField(v, "Namespace")
kind := GetStringField(v, "Kind")

if cm.Data == nil {
return util.GetEnvVar("RELATED_IMAGE_TEST_ANSIBLETEST_IMAGE_URL_DEFAULT", ""), nil
}
cm := &corev1.ConfigMap{}
objectKey := client.ObjectKey{Namespace: namespace, Name: "test-operator-config"}
if err := r.Client.Get(ctx, objectKey, cm); err != nil {
return "", err
}

if cmImage, exists := cm.Data["ansibletest-image"]; exists {
return cmImage, nil
imageKey := strings.ToLower(kind) + "-image"
if cm.Data != nil {
if image, exists := cm.Data[imageKey]; exists && image != "" {
return image, nil
}

return util.GetEnvVar("RELATED_IMAGE_TEST_ANSIBLETEST_IMAGE_URL_DEFAULT", ""), nil
}

return "", nil
relatedImage := "RELATED_IMAGE_TEST_" + strings.ToUpper(kind) + "_IMAGE_URL_DEFAULT"
return util.GetEnvVar(relatedImage, ""), nil
}

// GetPodName returns the name of the pod for the given instance and workflow step
func (r *Reconciler) GetPodName(instance interface{}, workflowStepNum int) string {
if typedInstance, ok := instance.(*v1beta1.Tobiko); ok {
if len(typedInstance.Spec.Workflow) == 0 || workflowStepNum == workflowStepNumInvalid {
return typedInstance.Name
}

workflowStepName := workflowStepNameInvalid
if workflowStepNum < len(typedInstance.Spec.Workflow) {
workflowStepName = typedInstance.Spec.Workflow[workflowStepNum].StepName
}

return typedInstance.Name + podNameStepInfix + fmt.Sprintf("%02d", workflowStepNum) + "-" + workflowStepName
} else if typedInstance, ok := instance.(*v1beta1.Tempest); ok {
if len(typedInstance.Spec.Workflow) == 0 || workflowStepNum == workflowStepNumInvalid {
return typedInstance.Name
}
func (r *Reconciler) GetPodName(instance interface{}, stepNum int) string {
v := reflect.ValueOf(instance)

workflowStepName := workflowStepNameInvalid
if workflowStepNum < len(typedInstance.Spec.Workflow) {
workflowStepName = typedInstance.Spec.Workflow[workflowStepNum].StepName
}
name := GetStringField(v, "Name")
spec, err := SafetyCheck(v, "Spec")
if err != nil {
return name
}

return typedInstance.Name + podNameStepInfix + fmt.Sprintf("%02d", workflowStepNum) + "-" + workflowStepName
} else if typedInstance, ok := instance.(*v1beta1.HorizonTest); ok {
return typedInstance.Name
} else if typedInstance, ok := instance.(*v1beta1.AnsibleTest); ok {
if len(typedInstance.Spec.Workflow) == 0 || workflowStepNum == workflowStepNumInvalid {
return typedInstance.Name
}
workflow, err := SafetyCheck(spec, "Workflow")
if err != nil || workflow.Len() == 0 {
return name
}

workflowStepName := workflowStepNameInvalid
if workflowStepNum < len(typedInstance.Spec.Workflow) {
workflowStepName = typedInstance.Spec.Workflow[workflowStepNum].StepName
// Get workflow step name
stepName := workflowStepNameInvalid
if stepNum >= 0 && stepNum < workflow.Len() {
stepName = GetStringField(workflow.Index(stepNum), "StepName")
if stepName == "" {
stepName = workflowStepNameInvalid
}

return typedInstance.Name + podNameStepInfix + fmt.Sprintf("%02d", workflowStepNum) + "-" + workflowStepName
}

return workflowStepNameInvalid
return name + podNameStepInfix + fmt.Sprintf("%02d", stepNum) + "-" + stepName
}

// GetPVCLogsName returns the name of the PVC for logs for the given instance and workflow step
Expand Down Expand Up @@ -493,11 +404,6 @@ func (r *Reconciler) EnsureLogsPVCExists(
return ctrlResult, nil
}

// GetClient returns the Kubernetes client
func (r *Reconciler) GetClient() client.Client {
return r.Client
}

// GetLogger returns the logger instance
func (r *Reconciler) GetLogger() logr.Logger {
return r.Log
Expand Down Expand Up @@ -624,12 +530,6 @@ func (r *Reconciler) PodExists(ctx context.Context, instance client.Object, work
return true
}

func (r *Reconciler) setConfigOverwrite(customData map[string]string, configOverwrite map[string]string) {
for key, data := range configOverwrite {
customData[key] = data
}
}

// GetCommonRbacRules returns the common RBAC rules for test operations, with optional privileged permissions
func GetCommonRbacRules(privileged bool) []rbacv1.PolicyRule {
rbacPolicyRule := rbacv1.PolicyRule{
Expand Down Expand Up @@ -717,6 +617,37 @@ func EnsureCloudsConfigMapExists(
return ctrl.Result{}, nil
}

// GetStringField returns reflect string field safely
func GetStringField(v reflect.Value, fieldName string) string {
field, err := SafetyCheck(v, fieldName)
if err != nil || field.Kind() != reflect.String {
return ""
}

return field.String()
}

// SafetyCheck returns reflect value after checking its validity
func SafetyCheck(v reflect.Value, fieldName string) (reflect.Value, error) {
if v.Kind() == reflect.Ptr {
if v.IsNil() {
return reflect.Value{}, fmt.Errorf("%s: %w", fieldName, ErrFieldNilPointer)
}
v = v.Elem()
}

if v.Kind() != reflect.Struct {
return reflect.Value{}, fmt.Errorf("%s: %w, got %s", fieldName, ErrFieldExpectedStruct, v.Kind())
}

field := v.FieldByName(fieldName)
if !field.IsValid() {
return reflect.Value{}, fmt.Errorf("%s: %w", fieldName, ErrFieldNotFound)
}

return field, nil
}

// IsEmpty checks if the provided value is empty based on its type
func IsEmpty(value interface{}) bool {
if v, ok := value.(reflect.Value); ok {
Expand Down
2 changes: 1 addition & 1 deletion controllers/horizontest_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
envVars := r.PrepareHorizonTestEnvVars(instance)
podName := r.GetPodName(instance, 0)
logsPVCName := r.GetPVCLogsName(instance, 0)
containerImage, err := r.GetContainerImage(ctx, instance.Spec.ContainerImage, instance)
containerImage, err := r.GetContainerImage(ctx, instance)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
17 changes: 15 additions & 2 deletions controllers/tempest_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
EnvVarsConfigMapName := GetEnvVarsConfigMapName(instance, nextWorkflowStep)
podName := r.GetPodName(instance, nextWorkflowStep)
logsPVCName := r.GetPVCLogsName(instance, workflowStepNum)
containerImage, err := r.GetContainerImage(ctx, instance.Spec.ContainerImage, instance)
containerImage, err := r.GetContainerImage(ctx, instance)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -589,7 +589,10 @@ func (r *TempestReconciler) generateServiceConfigMaps(

r.setTempestConfigVars(envVars, customData, instance, workflowStepNum)
r.setTempestconfConfigVars(envVars, customData, instance)
r.setConfigOverwrite(customData, instance.Spec.ConfigOverwrite)

for key, data := range instance.Spec.ConfigOverwrite {
customData[key] = data
}

envVars["TEMPEST_DEBUG_MODE"] = strconv.FormatBool(instance.Spec.Debug)
envVars["TEMPEST_CLEANUP"] = strconv.FormatBool(instance.Spec.Cleanup)
Expand Down Expand Up @@ -620,3 +623,13 @@ func (r *TempestReconciler) generateServiceConfigMaps(

return configmap.EnsureConfigMaps(ctx, h, instance, cms, nil)
}

// GetEnvVarsConfigMapName returns the name of the environment variables ConfigMap for the given workflow step
func GetEnvVarsConfigMapName(instance *testv1beta1.Tempest, workflowStepNum int) string {
return instance.Name + envVarsConfigMapInfix + strconv.Itoa(workflowStepNum)
}

// GetCustomDataConfigMapName returns the name of the custom data ConfigMap for the given workflow step
func GetCustomDataConfigMapName(instance *testv1beta1.Tempest, workflowStepNum int) string {
return instance.Name + customDataConfigMapInfix + strconv.Itoa(workflowStepNum)
}
2 changes: 1 addition & 1 deletion controllers/tobiko_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
envVars := r.PrepareTobikoEnvVars(ctx, serviceLabels, instance, helper, nextWorkflowStep)
podName := r.GetPodName(instance, nextWorkflowStep)
logsPVCName := r.GetPVCLogsName(instance, workflowStepNum)
containerImage, err := r.GetContainerImage(ctx, instance.Spec.ContainerImage, instance)
containerImage, err := r.GetContainerImage(ctx, instance)
if err != nil {
return ctrl.Result{}, err
}
Expand Down