From 1bec453824d18cd1250183f925a4f9b61398c100 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Wed, 4 Sep 2024 12:20:25 -0700 Subject: [PATCH] Move to VerifySecret when checking the ctlplane secret Instead of recording a hash of the entire ctlplane secret, a new function is introduced that uses lib-common's VerifySecret() to verify and return the hash of only the cinder service's portion of the secret. This avoids restarting all cinder services when an unrelated portion of the ctlplane secret changes. This change follows a similar manila PR [1]. [1] https://github.com/openstack-k8s-operators/manila-operator/pull/324 Jira: OSPRH-8192 --- controllers/cinder_common.go | 71 +++++++++++++++++++++++ controllers/cinder_controller.go | 36 +++++------- controllers/cinderapi_controller.go | 15 ++++- controllers/cinderbackup_controller.go | 15 ++++- controllers/cinderscheduler_controller.go | 15 ++++- controllers/cindervolume_controller.go | 15 ++++- 6 files changed, 143 insertions(+), 24 deletions(-) create mode 100644 controllers/cinder_common.go diff --git a/controllers/cinder_common.go b/controllers/cinder_common.go new file mode 100644 index 00000000..dd43d96e --- /dev/null +++ b/controllers/cinder_common.go @@ -0,0 +1,71 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "fmt" + "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + "github.com/openstack-k8s-operators/lib-common/modules/common/secret" + "k8s.io/apimachinery/pkg/types" + "time" + + "github.com/openstack-k8s-operators/lib-common/modules/common/env" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +type conditionUpdater interface { + Set(c *condition.Condition) + MarkTrue(t condition.Type, messageFormat string, messageArgs ...interface{}) +} + +// verifyServiceSecret - ensures that the Secret object exists and the expected +// fields are in the Secret. It also sets a hash of the values of the expected +// fields passed as input. +func verifyServiceSecret( + ctx context.Context, + secretName types.NamespacedName, + expectedFields []string, + reader client.Reader, + conditionUpdater conditionUpdater, + requeueTimeout time.Duration, + envVars *map[string]env.Setter, +) (ctrl.Result, error) { + + hash, res, err := secret.VerifySecret(ctx, secretName, expectedFields, reader, requeueTimeout) + if err != nil { + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) + return res, err + } else if (res != ctrl.Result{}) { + log.FromContext(ctx).Info(fmt.Sprintf("OpenStack secret %s not found", secretName)) + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.InputReadyWaitingMessage)) + return res, nil + } + (*envVars)[secretName.Name] = env.SetValue(hash) + return ctrl.Result{}, nil +} diff --git a/controllers/cinder_controller.go b/controllers/cinder_controller.go index 0bde0302..5dfe21b6 100644 --- a/controllers/cinder_controller.go +++ b/controllers/cinder_controller.go @@ -23,6 +23,7 @@ import ( k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -540,28 +541,23 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - ospSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace) + + result, err := verifyServiceSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + }, + helper.GetClient(), + &instance.Status.Conditions, + cinder.NormalDuration, + &configVars, + ) if err != nil { - if k8s_errors.IsNotFound(err) { - Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.InputReadyWaitingMessage)) - return cinder.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err + return result, err + } else if (result != ctrl.Result{}) { + return result, nil } - // Add a prefix to the var name to avoid accidental collision with other non-secret vars. - configVars["secret-"+ospSecret.Name] = env.SetValue(hash) - instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) // run check OpenStack secret - end diff --git a/controllers/cinderapi_controller.go b/controllers/cinderapi_controller.go index ca4f28d2..9c23e190 100644 --- a/controllers/cinderapi_controller.go +++ b/controllers/cinderapi_controller.go @@ -641,9 +641,22 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Secret, &configVars) + + ctrlResult, err := verifyServiceSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + }, + helper.GetClient(), + &instance.Status.Conditions, + cinder.NormalDuration, + &configVars, + ) if err != nil { return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil } // diff --git a/controllers/cinderbackup_controller.go b/controllers/cinderbackup_controller.go index e289d4d0..209e172b 100644 --- a/controllers/cinderbackup_controller.go +++ b/controllers/cinderbackup_controller.go @@ -342,9 +342,22 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance * // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Secret, &configVars) + + ctrlResult, err := verifyServiceSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + }, + helper.GetClient(), + &instance.Status.Conditions, + cinder.NormalDuration, + &configVars, + ) if err != nil { return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil } // diff --git a/controllers/cinderscheduler_controller.go b/controllers/cinderscheduler_controller.go index 89bdfa17..8bdcc576 100644 --- a/controllers/cinderscheduler_controller.go +++ b/controllers/cinderscheduler_controller.go @@ -341,9 +341,22 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Secret, &configVars) + + ctrlResult, err := verifyServiceSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + }, + helper.GetClient(), + &instance.Status.Conditions, + cinder.NormalDuration, + &configVars, + ) if err != nil { return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil } // diff --git a/controllers/cindervolume_controller.go b/controllers/cindervolume_controller.go index ff3a2eb9..da5ca3fb 100644 --- a/controllers/cindervolume_controller.go +++ b/controllers/cindervolume_controller.go @@ -343,9 +343,22 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance * // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Secret, &configVars) + + ctrlResult, err := verifyServiceSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + }, + helper.GetClient(), + &instance.Status.Conditions, + cinder.NormalDuration, + &configVars, + ) if err != nil { return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil } //