From 8509ddcabcee143d2393e24802587fa62d029f2c Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Wed, 4 Sep 2024 12:55:32 -0700 Subject: [PATCH] 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] https://github.com/openstack-k8s-operators/manila-operator/pull/324 --- controllers/cinder_common.go | 43 ++++++++++++ controllers/cinderapi_controller.go | 80 ++++++----------------- controllers/cinderbackup_controller.go | 80 ++++++----------------- controllers/cinderscheduler_controller.go | 80 ++++++----------------- controllers/cindervolume_controller.go | 80 ++++++----------------- 5 files changed, 119 insertions(+), 244 deletions(-) diff --git a/controllers/cinder_common.go b/controllers/cinder_common.go index dd43d96e..ea8446ee 100644 --- 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 index 9c23e190..55130abe 100644 --- 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 index 209e172b..5aba831a 100644 --- 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 index 8bdcc576..55cf79f9 100644 --- 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, diff --git a/controllers/cindervolume_controller.go b/controllers/cindervolume_controller.go index da5ca3fb..bf31bb49 100644 --- a/controllers/cindervolume_controller.go +++ b/controllers/cindervolume_controller.go @@ -362,37 +362,30 @@ func (r *CinderVolumeReconciler) 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) @@ -683,41 +676,6 @@ func (r *CinderVolumeReconciler) reconcileUpgrade(ctx context.Context, instance return ctrl.Result{}, nil } -// getSecret - get the specified secret, and add its hash to envVars -func (r *CinderVolumeReconciler) getSecret( - ctx context.Context, - h *helper.Helper, - instance *cinderv1beta1.CinderVolume, - 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 and check if it's using LVM func (r *CinderVolumeReconciler) generateServiceConfigs( ctx context.Context,