diff --git a/internal/controller/ansibletest_controller.go b/internal/controller/ansibletest_controller.go index 908799fe..4b6a9aa1 100644 --- a/internal/controller/ansibletest_controller.go +++ b/internal/controller/ansibletest_controller.go @@ -174,7 +174,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) // another instance. This is considered to be an error state. lockAcquired, err := r.AcquireLock(ctx, instance, helper, false) if !lockAcquired { - Log.Error(err, ErrConfirmLockOwnership, testOperatorLockName) + Log.Error(err, fmt.Sprintf(ErrConfirmLockOwnership, testOperatorLockName)) return ctrl.Result{RequeueAfter: RequeueAfterValue}, err } diff --git a/internal/controller/common.go b/internal/controller/common.go index a282c01c..57aa5688 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -48,7 +48,7 @@ const ( const ( // ErrConfirmLockOwnership is the error message for lock ownership confirmation failures - ErrConfirmLockOwnership = "can not confirm ownership of %s lock" + ErrConfirmLockOwnership = "Can not confirm ownership of %s lock." ) const ( @@ -727,7 +727,7 @@ func EnsureCloudsConfigMapExists( helper *helper.Helper, labels map[string]string, openstackConfigMapName string, -) (ctrl.Result, error) { +) error { const testOperatorCloudsConfigMapName = "test-operator-clouds-config" cm, _, _ := configmap.GetConfigMap( @@ -738,7 +738,7 @@ func EnsureCloudsConfigMapExists( time.Second*10, ) if cm.Name == testOperatorCloudsConfigMapName { - return ctrl.Result{}, nil + return nil } cm, _, _ = configmap.GetConfigMap( @@ -753,7 +753,7 @@ func EnsureCloudsConfigMapExists( err := yaml.Unmarshal([]byte(cm.Data["clouds.yaml"]), &result) if err != nil { - return ctrl.Result{}, err + return err } clouds := result["clouds"].(map[string]interface{}) @@ -766,7 +766,7 @@ func EnsureCloudsConfigMapExists( yamlString, err := yaml.Marshal(result) if err != nil { - return ctrl.Result{}, err + return err } cms := []util.Template{ @@ -781,10 +781,10 @@ func EnsureCloudsConfigMapExists( } err = configmap.EnsureConfigMaps(ctx, helper, instance, cms, nil) if err != nil { - return ctrl.Result{}, err + return err } - return ctrl.Result{}, nil + return nil } // Int64OrPlaceholder converts int64 to string, returns placeholder if 0 diff --git a/internal/controller/horizontest_controller.go b/internal/controller/horizontest_controller.go index 24ff2518..483e5556 100644 --- a/internal/controller/horizontest_controller.go +++ b/internal/controller/horizontest_controller.go @@ -167,7 +167,7 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) // another instance. This is considered to be an error state. lockAcquired, err := r.AcquireLock(ctx, instance, helper, instance.Spec.Parallel) if !lockAcquired { - Log.Error(err, ErrConfirmLockOwnership, testOperatorLockName) + Log.Error(err, fmt.Sprintf(ErrConfirmLockOwnership, testOperatorLockName)) return ctrl.Result{RequeueAfter: RequeueAfterValue}, err } @@ -212,13 +212,8 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) mountKubeconfig := len(instance.Spec.KubeconfigSecretName) != 0 instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - yamlResult, err := EnsureCloudsConfigMapExists( - ctx, - instance, - helper, - serviceLabels, - instance.Spec.OpenStackConfigMap, - ) + // Generate ConfigMaps + err = r.generateServiceConfigMaps(ctx, helper, serviceLabels, instance) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, @@ -226,9 +221,10 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) condition.SeverityWarning, condition.ServiceConfigReadyErrorMessage, err.Error())) - return yamlResult, err + return ctrl.Result{}, err } instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) + // Generate ConfigMaps - end // Create PersistentVolumeClaim ctrlResult, err := r.EnsureLogsPVCExists( @@ -306,6 +302,22 @@ func (r *HorizonTestReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +func (r *HorizonTestReconciler) generateServiceConfigMaps( + ctx context.Context, + h *helper.Helper, + labels map[string]string, + instance *testv1beta1.HorizonTest, +) error { + err := EnsureCloudsConfigMapExists( + ctx, + instance, + h, + labels, + instance.Spec.OpenStackConfigMap, + ) + return err +} + // PrepareHorizonTestEnvVars prepares environment variables for HorizonTest execution func (r *HorizonTestReconciler) PrepareHorizonTestEnvVars( instance *testv1beta1.HorizonTest, diff --git a/internal/controller/tempest_controller.go b/internal/controller/tempest_controller.go index e56ac1d0..65c1a551 100644 --- a/internal/controller/tempest_controller.go +++ b/internal/controller/tempest_controller.go @@ -193,7 +193,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // another instance. This is considered to be an error state. lockAcquired, err := r.AcquireLock(ctx, instance, helper, instance.Spec.Parallel) if !lockAcquired { - Log.Error(err, ErrConfirmLockOwnership, testOperatorLockName) + Log.Error(err, fmt.Sprintf(ErrConfirmLockOwnership, testOperatorLockName)) return ctrl.Result{RequeueAfter: RequeueAfterValue}, err } @@ -287,7 +287,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // Create a new pod mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle") customDataConfigMapName := GetCustomDataConfigMapName(instance, workflowStepIndex) - EnvVarsConfigMapName := GetEnvVarsConfigMapName(instance, workflowStepIndex) + envVarsConfigMapName := GetEnvVarsConfigMapName(instance, workflowStepIndex) podName := r.GetPodName(instance, workflowStepIndex) logsPVCName := r.GetPVCLogsName(instance, pvcIndex) containerImage, err := r.GetContainerImage(ctx, instance) @@ -300,7 +300,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re serviceLabels, serviceAnnotations, podName, - EnvVarsConfigMapName, + envVarsConfigMapName, customDataConfigMapName, logsPVCName, mountCerts, @@ -313,7 +313,7 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // Creation of the tempest pod was not successful. // Release the lock and allow other controllers to spawn // a pod. - if lockReleased, lockErr := r.ReleaseLock(ctx, instance); lockReleased { + if lockReleased, lockErr := r.ReleaseLock(ctx, instance); !lockReleased { return ctrl.Result{RequeueAfter: RequeueAfterValue}, lockErr } diff --git a/internal/controller/tobiko_controller.go b/internal/controller/tobiko_controller.go index 68215d53..d9703226 100644 --- a/internal/controller/tobiko_controller.go +++ b/internal/controller/tobiko_controller.go @@ -179,7 +179,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res // got claimed by another instance. lockAcquired, err := r.AcquireLock(ctx, instance, helper, instance.Spec.Parallel) if !lockAcquired { - Log.Error(err, ErrConfirmLockOwnership, testOperatorLockName) + Log.Error(err, fmt.Sprintf(ErrConfirmLockOwnership, testOperatorLockName)) return ctrl.Result{RequeueAfter: RequeueAfterValue}, err } @@ -207,13 +207,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{RequeueAfter: RequeueAfterValue}, err } - yamlResult, err := EnsureCloudsConfigMapExists( - ctx, - instance, - helper, - serviceLabels, - instance.Spec.OpenStackConfigMap, - ) + err = r.ValidateSecretWithKeys(ctx, instance, instance.Spec.KubeconfigSecretName, []string{}) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, @@ -221,25 +215,27 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res condition.SeverityWarning, condition.InputReadyErrorMessage, err.Error())) - return yamlResult, err + return ctrl.Result{}, err } + mountKubeconfig := len(instance.Spec.KubeconfigSecretName) != 0 - err = r.ValidateSecretWithKeys(ctx, instance, instance.Spec.KubeconfigSecretName, []string{}) + instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) + + // Generate ConfigMaps + err = r.generateServiceConfigMaps(ctx, helper, serviceLabels, instance, workflowStepIndex) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, + condition.ServiceConfigReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.InputReadyErrorMessage, + condition.ServiceConfigReadyErrorMessage, err.Error())) return ctrl.Result{}, err } - mountKubeconfig := len(instance.Spec.KubeconfigSecretName) != 0 - - instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) + instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) + // Generate ConfigMaps - end pvcIndex := 0 - // Create multiple PVCs for parallel execution if instance.Spec.Parallel && workflowStepIndex < len(instance.Spec.Workflow) { pvcIndex = workflowStepIndex @@ -290,16 +286,13 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res // Create Pod mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle") - - mountKeys := false - if (len(instance.Spec.PublicKey) == 0) || (len(instance.Spec.PrivateKey) == 0) { - Log.Info("Both values privateKey and publicKey need to be specified. Keys not mounted.") - } else { - mountKeys = true + mountKeys := len(instance.Spec.PublicKey) > 0 && len(instance.Spec.PrivateKey) > 0 + if !mountKeys { + Log.Info("Both values 'privateKey' and 'publicKey' need to be specified. Keys not mounted.") } // Prepare Tobiko env vars - envVars := r.PrepareTobikoEnvVars(ctx, serviceLabels, instance, helper, workflowStepIndex) + envVars := r.PrepareTobikoEnvVars(instance, workflowStepIndex) podName := r.GetPodName(instance, workflowStepIndex) logsPVCName := r.GetPVCLogsName(instance, pvcIndex) containerImage, err := r.GetContainerImage(ctx, instance) @@ -358,12 +351,51 @@ func (r *TobikoReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// PrepareTobikoEnvVars prepares environment variables for a single workflow step -func (r *TobikoReconciler) PrepareTobikoEnvVars( +func (r *TobikoReconciler) generateServiceConfigMaps( ctx context.Context, + h *helper.Helper, labels map[string]string, instance *testv1beta1.Tobiko, - helper *helper.Helper, + workflowStepIndex int, +) error { + err := EnsureCloudsConfigMapExists( + ctx, + instance, + h, + labels, + instance.Spec.OpenStackConfigMap, + ) + if err != nil { + return err + } + + templateSpecs := []struct { + infix string + key string + value string + }{ + {tobiko.ConfigMapInfixConfig, tobiko.ConfigFileName, instance.Spec.Config}, + {tobiko.ConfigMapInfixPrivateKey, tobiko.PrivateKeyFileName, instance.Spec.PrivateKey}, + {tobiko.ConfigMapInfixPublicKey, tobiko.PublicKeyFileName, instance.Spec.PublicKey}, + } + + cms := make([]util.Template, 0, len(templateSpecs)) + for _, spec := range templateSpecs { + cms = append(cms, util.Template{ + Name: tobiko.GetConfigMapName(instance, spec.infix, workflowStepIndex), + Namespace: instance.Namespace, + InstanceType: instance.Kind, + Labels: labels, + CustomData: map[string]string{spec.key: spec.value}, + }) + } + + return configmap.EnsureConfigMaps(ctx, h, instance, cms, nil) +} + +// PrepareTobikoEnvVars prepares environment variables for a single workflow step +func (r *TobikoReconciler) PrepareTobikoEnvVars( + instance *testv1beta1.Tobiko, workflowStepIndex int, ) map[string]env.Setter { // Prepare env vars @@ -404,39 +436,5 @@ func (r *TobikoReconciler) PrepareTobikoEnvVars( }) } - // Prepare custom data - templateSpecs := []struct { - infix string - key string - value string - }{ - {tobiko.ConfigMapInfixConfig, tobiko.ConfigFileName, instance.Spec.Config}, - {tobiko.ConfigMapInfixPrivateKey, tobiko.PrivateKeyFileName, instance.Spec.PrivateKey}, - {tobiko.ConfigMapInfixPublicKey, tobiko.PublicKeyFileName, instance.Spec.PublicKey}, - } - - cms := make([]util.Template, 0, len(templateSpecs)) - for _, spec := range templateSpecs { - cms = append(cms, util.Template{ - Name: tobiko.GetConfigMapName(instance, spec.infix, workflowStepIndex), - Namespace: instance.Namespace, - InstanceType: instance.Kind, - Labels: labels, - CustomData: map[string]string{spec.key: spec.value}, - }) - } - - err := configmap.EnsureConfigMaps(ctx, helper, instance, cms, nil) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return map[string]env.Setter{} - } - instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - return envVars }