Consolidate hashing TransportURL and config secrets

Replace multiple calls to collect hashes of the Transport URL secret and other service configuration secrets with a new verifyConfigSecrets() function that can be shared by all cinder services. This also eliminates the need for each service to provide its own getSecret function. This change continues to mimic code in manila's PR [1]. [1] openstack-k8s-operators/manila-operator#324
openstack-k8s-operators · Sep 4, 2024 · 8509ddc · 8509ddc
1 parent 1bec453
commit 8509ddc
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 244 deletions.
diff --git a/controllers/cinder_common.go b/controllers/cinder_common.go
@@ -24,7 +24,10 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"time"
 
+	"github.com/openstack-k8s-operators/cinder-operator/pkg/cinder"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
+	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -69,3 +72,43 @@ func verifyServiceSecret(
 	(*envVars)[secretName.Name] = env.SetValue(hash)
 	return ctrl.Result{}, nil
 }
+
+// verifyConfigSecrets - It iterates over the secretNames passed as input and
+// sets the hash of values in the envVars map.
+func verifyConfigSecrets(
+	ctx context.Context,
+	h *helper.Helper,
+	conditionUpdater conditionUpdater,
+	secretNames []string,
+	namespace string,
+	envVars *map[string]env.Setter,
+) (ctrl.Result, error) {
+	var hash string
+	var err error
+	for _, secretName := range secretNames {
+		_, hash, err = secret.GetSecret(ctx, h, secretName, namespace)
+		if err != nil {
+			if k8s_errors.IsNotFound(err) {
+				log.FromContext(ctx).Info(fmt.Sprintf("Secret %s not found", secretName))
+				conditionUpdater.Set(condition.FalseCondition(
+					condition.InputReadyCondition,
+					condition.RequestedReason,
+					condition.SeverityInfo,
+					condition.InputReadyWaitingMessage))
+				return cinder.ResultRequeue, nil
+			}
+			conditionUpdater.Set(condition.FalseCondition(
+				condition.InputReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.InputReadyErrorMessage,
+				err.Error()))
+			return ctrl.Result{}, err
+		}
+		// Add a prefix to the var name to avoid accidental collision with other non-secret
+		// vars. The secret names themselves will be unique.
+		(*envVars)["secret-"+secretName] = env.SetValue(hash)
+	}
+
+	return ctrl.Result{}, nil
+}
diff --git a/controllers/cinderapi_controller.go b/controllers/cinderapi_controller.go
@@ -660,37 +660,30 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
 	}
 
 	//
-	// check for required TransportURL secret holding transport URL string
+	// check for required Transport URL and config secrets
 	//
-	ctrlResult, err = r.getSecret(ctx, helper, instance, instance.Spec.TransportURLSecret, &configVars)
-	if err != nil {
-		return ctrlResult, err
-	}
-
-	//
-	// check for required service secrets
-	//
-	for _, secretName := range instance.Spec.CustomServiceConfigSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, secretName, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
-	}
 
-	//
-	// check for required Cinder secrets that should have been created by parent Cinder CR
-	//
 	parentCinderName := cinder.GetOwningCinderName(instance)
-	parentSecrets := []string{
-		fmt.Sprintf("%s-scripts", parentCinderName),
-		fmt.Sprintf("%s-config-data", parentCinderName),
+	secretNames := []string{
+		instance.Spec.TransportURLSecret,                // TransportURLSecret
+		fmt.Sprintf("%s-scripts", parentCinderName),     // ScriptsSecret
+		fmt.Sprintf("%s-config-data", parentCinderName), // ConfigSecret
 	}
+	// Append CustomServiceConfigSecrets that should be checked
+	secretNames = append(secretNames, instance.Spec.CustomServiceConfigSecrets...)
 
-	for _, parentSecret := range parentSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, parentSecret, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
+	ctrlResult, err = verifyConfigSecrets(
+		ctx,
+		helper,
+		&instance.Status.Conditions,
+		secretNames,
+		instance.Namespace,
+		&configVars,
+	)
+	if err != nil {
+		return ctrlResult, err
+	} else if (ctrlResult != ctrl.Result{}) {
+		return ctrlResult, nil
 	}
 
 	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
@@ -1012,41 +1005,6 @@ func (r *CinderAPIReconciler) reconcileUpgrade(ctx context.Context, instance *ci
 	return ctrl.Result{}, nil
 }
 
-// getSecret - get the specified secret, and add its hash to envVars
-func (r *CinderAPIReconciler) getSecret(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *cinderv1beta1.CinderAPI,
-	secretName string,
-	envVars *map[string]env.Setter,
-) (ctrl.Result, error) {
-	secret, hash, err := secret.GetSecret(ctx, h, secretName, instance.Namespace)
-	if err != nil {
-		if k8s_errors.IsNotFound(err) {
-			h.GetLogger().Info(fmt.Sprintf("Secret %s not found", secretName))
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.InputReadyCondition,
-				condition.RequestedReason,
-				condition.SeverityInfo,
-				condition.InputReadyWaitingMessage))
-			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
-		}
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.InputReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.InputReadyErrorMessage,
-			err.Error()))
-		return ctrl.Result{}, err
-	}
-
-	// Add a prefix to the var name to avoid accidental collision with other non-secret
-	// vars. The secret names themselves will be unique.
-	(*envVars)["secret-"+secret.Name] = env.SetValue(hash)
-
-	return ctrl.Result{}, nil
-}
-
 // generateServiceConfigs - create Secret which holds the service configuration
 func (r *CinderAPIReconciler) generateServiceConfigs(
 	ctx context.Context,

diff --git a/controllers/cinderbackup_controller.go b/controllers/cinderbackup_controller.go
@@ -361,37 +361,30 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance *
 	}
 
 	//
-	// check for required TransportURL secret holding transport URL string
+	// check for required Transport URL and config secrets
 	//
-	ctrlResult, err = r.getSecret(ctx, helper, instance, instance.Spec.TransportURLSecret, &configVars)
-	if err != nil {
-		return ctrlResult, err
-	}
-
-	//
-	// check for required service secrets
-	//
-	for _, secretName := range instance.Spec.CustomServiceConfigSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, secretName, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
-	}
 
-	//
-	// check for required Cinder secrets that should have been created by parent Cinder CR
-	//
 	parentCinderName := cinder.GetOwningCinderName(instance)
-	parentSecrets := []string{
-		fmt.Sprintf("%s-scripts", parentCinderName),
-		fmt.Sprintf("%s-config-data", parentCinderName),
+	secretNames := []string{
+		instance.Spec.TransportURLSecret,                // TransportURLSecret
+		fmt.Sprintf("%s-scripts", parentCinderName),     // ScriptsSecret
+		fmt.Sprintf("%s-config-data", parentCinderName), // ConfigSecret
 	}
+	// Append CustomServiceConfigSecrets that should be checked
+	secretNames = append(secretNames, instance.Spec.CustomServiceConfigSecrets...)
 
-	for _, parentSecret := range parentSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, parentSecret, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
+	ctrlResult, err = verifyConfigSecrets(
+		ctx,
+		helper,
+		&instance.Status.Conditions,
+		secretNames,
+		instance.Namespace,
+		&configVars,
+	)
+	if err != nil {
+		return ctrlResult, err
+	} else if (ctrlResult != ctrl.Result{}) {
+		return ctrlResult, nil
 	}
 
 	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
@@ -681,41 +674,6 @@ func (r *CinderBackupReconciler) reconcileUpgrade(ctx context.Context, instance
 	return ctrl.Result{}, nil
 }
 
-// getSecret - get the specified secret, and add its hash to envVars
-func (r *CinderBackupReconciler) getSecret(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *cinderv1beta1.CinderBackup,
-	secretName string,
-	envVars *map[string]env.Setter,
-) (ctrl.Result, error) {
-	secret, hash, err := secret.GetSecret(ctx, h, secretName, instance.Namespace)
-	if err != nil {
-		if k8s_errors.IsNotFound(err) {
-			h.GetLogger().Info(fmt.Sprintf("Secret %s not found", secretName))
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.InputReadyCondition,
-				condition.RequestedReason,
-				condition.SeverityInfo,
-				condition.InputReadyWaitingMessage))
-			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
-		}
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.InputReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.InputReadyErrorMessage,
-			err.Error()))
-		return ctrl.Result{}, err
-	}
-
-	// Add a prefix to the var name to avoid accidental collision with other non-secret
-	// vars. The secret names themselves will be unique.
-	(*envVars)["secret-"+secret.Name] = env.SetValue(hash)
-
-	return ctrl.Result{}, nil
-}
-
 // generateServiceConfigs - create Secret which holds the service configuration
 func (r *CinderBackupReconciler) generateServiceConfigs(
 	ctx context.Context,

diff --git a/controllers/cinderscheduler_controller.go b/controllers/cinderscheduler_controller.go
@@ -360,37 +360,30 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc
 	}
 
 	//
-	// check for required TransportURL secret holding transport URL string
+	// check for required Transport URL and config secrets
 	//
-	ctrlResult, err = r.getSecret(ctx, helper, instance, instance.Spec.TransportURLSecret, &configVars)
-	if err != nil {
-		return ctrlResult, err
-	}
-
-	//
-	// check for required service secrets
-	//
-	for _, secretName := range instance.Spec.CustomServiceConfigSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, secretName, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
-	}
 
-	//
-	// check for required Cinder secrets that should have been created by parent Cinder CR
-	//
 	parentCinderName := cinder.GetOwningCinderName(instance)
-	parentSecrets := []string{
-		fmt.Sprintf("%s-scripts", parentCinderName),
-		fmt.Sprintf("%s-config-data", parentCinderName),
+	secretNames := []string{
+		instance.Spec.TransportURLSecret,                // TransportURLSecret
+		fmt.Sprintf("%s-scripts", parentCinderName),     // ScriptsSecret
+		fmt.Sprintf("%s-config-data", parentCinderName), // ConfigSecret
 	}
+	// Append CustomServiceConfigSecrets that should be checked
+	secretNames = append(secretNames, instance.Spec.CustomServiceConfigSecrets...)
 
-	for _, parentSecret := range parentSecrets {
-		ctrlResult, err = r.getSecret(ctx, helper, instance, parentSecret, &configVars)
-		if err != nil {
-			return ctrlResult, err
-		}
+	ctrlResult, err = verifyConfigSecrets(
+		ctx,
+		helper,
+		&instance.Status.Conditions,
+		secretNames,
+		instance.Namespace,
+		&configVars,
+	)
+	if err != nil {
+		return ctrlResult, err
+	} else if (ctrlResult != ctrl.Result{}) {
+		return ctrlResult, nil
 	}
 
 	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
@@ -680,41 +673,6 @@ func (r *CinderSchedulerReconciler) reconcileUpgrade(ctx context.Context, instan
 	return ctrl.Result{}, nil
 }
 
-// getSecret - get the specified secret, and add its hash to envVars
-func (r *CinderSchedulerReconciler) getSecret(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *cinderv1beta1.CinderScheduler,
-	secretName string,
-	envVars *map[string]env.Setter,
-) (ctrl.Result, error) {
-	secret, hash, err := secret.GetSecret(ctx, h, secretName, instance.Namespace)
-	if err != nil {
-		if k8s_errors.IsNotFound(err) {
-			h.GetLogger().Info(fmt.Sprintf("Secret %s not found", secretName))
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.InputReadyCondition,
-				condition.RequestedReason,
-				condition.SeverityInfo,
-				condition.InputReadyWaitingMessage))
-			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
-		}
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.InputReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.InputReadyErrorMessage,
-			err.Error()))
-		return ctrl.Result{}, err
-	}
-
-	// Add a prefix to the var name to avoid accidental collision with other non-secret
-	// vars. The secret names themselves will be unique.
-	(*envVars)["secret-"+secret.Name] = env.SetValue(hash)
-
-	return ctrl.Result{}, nil
-}
-
 // generateServiceConfigs - create Secret which holds the service configuration
 func (r *CinderSchedulerReconciler) generateServiceConfigs(
 	ctx context.Context,