From 7f2d5f2e6853bd29eb687e29599a9d08ff4b246a Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Tue, 24 Sep 2024 10:03:42 +0200 Subject: [PATCH] Add ensureNAD common function This patch improves the work started with verifyServiceSecret and introduces an ensureNAD function that is common to all the Manila controllers. In addition, the compound form is extended to all the verifyServiceSecret function calls. Signed-off-by: Francesco Pantano --- api/v1beta1/manila_webhook.go | 4 +- controllers/manila_common.go | 51 +++++++++++++++++++++ controllers/manila_controller.go | 54 ++++------------------- controllers/manilaapi_controller.go | 41 ++--------------- controllers/manilascheduler_controller.go | 45 +++---------------- controllers/manilashare_controller.go | 45 +++---------------- 6 files changed, 75 insertions(+), 165 deletions(-) diff --git a/api/v1beta1/manila_webhook.go b/api/v1beta1/manila_webhook.go index 6af18cd1..445923ab 100644 --- a/api/v1beta1/manila_webhook.go +++ b/api/v1beta1/manila_webhook.go @@ -248,12 +248,12 @@ func (spec *ManilaSpecCore) SetDefaultRouteAnnotations(annotations map[string]st valHAProxy, okHAProxy := annotations[haProxyAnno] // Human operator set the HAProxy timeout manually - if (!okManila && okHAProxy) { + if !okManila && okHAProxy { return } // Human operator modified the HAProxy timeout manually without removing the Manila flag - if (okManila && okHAProxy && valManila != valHAProxy) { + if okManila && okHAProxy && valManila != valHAProxy { delete(annotations, manilaAnno) return } diff --git a/controllers/manila_common.go b/controllers/manila_common.go index 409dba29..5f3032dd 100644 --- a/controllers/manila_common.go +++ b/controllers/manila_common.go @@ -26,6 +26,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/env" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" "github.com/openstack-k8s-operators/manila-operator/pkg/manila" k8s_errors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" @@ -38,6 +39,56 @@ type conditionUpdater interface { MarkTrue(t condition.Type, messageFormat string, messageArgs ...interface{}) } +// ensureNAD - common function called in the Manila controllers that GetNAD based +// on the string[] passed as input and produces the required Annotation for each +// Manila component +func ensureNAD( + ctx context.Context, + conditionUpdater conditionUpdater, + nAttach []string, + helper *helper.Helper, +) (map[string]string, ctrl.Result, error) { + + var serviceAnnotations map[string]string + var err error + // Iterate over the []networkattachment, get the corresponding NAD and create + // the required annotation + for _, netAtt := range nAttach { + _, err = nad.GetNADWithName(ctx, helper, netAtt, helper.GetBeforeObject().GetNamespace()) + if err != nil { + if k8s_errors.IsNotFound(err) { + log.FromContext(ctx).Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) + conditionUpdater.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.NetworkAttachmentsReadyWaitingMessage, + netAtt)) + return serviceAnnotations, manila.ResultRequeue, nil + } + conditionUpdater.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err.Error())) + return serviceAnnotations, manila.ResultRequeue, nil + } + } + // Create NetworkAnnotations + serviceAnnotations, err = nad.CreateNetworksAnnotation(helper.GetBeforeObject().GetNamespace(), nAttach) + if err != nil { + err := fmt.Errorf("failed create network annotation from %s: %w", nAttach, err) + conditionUpdater.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err.Error())) + } + return serviceAnnotations, ctrl.Result{}, err +} + // 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. diff --git a/controllers/manila_controller.go b/controllers/manila_controller.go index 761dcfde..d5205eca 100644 --- a/controllers/manila_controller.go +++ b/controllers/manila_controller.go @@ -34,7 +34,6 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/helper" "github.com/openstack-k8s-operators/lib-common/modules/common/job" "github.com/openstack-k8s-operators/lib-common/modules/common/labels" - nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" @@ -511,7 +510,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - result, err := verifyServiceSecret( + ctrlResult, err := verifyServiceSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ @@ -522,10 +521,8 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila manila.NormalDuration, &configVars, ) - if err != nil { - return result, err - } else if (result != ctrl.Result{}) { - return result, nil + if (err != nil || ctrlResult != ctrl.Result{}) { + return ctrlResult, nil } instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) // run check OpenStack secret - end @@ -592,51 +589,16 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - // - // TODO check when/if Init, Update, or Upgrade should/could be skipped - // - - // Check networks that the DBSync job will use in reconcileInit. The ones from the API service are always enough, - // it doesn't need the storage specific ones that manila-share may have. - for _, netAtt := range instance.Spec.ManilaAPI.NetworkAttachments { - _, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - r.Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return manila.ResultRequeue, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - } - - serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.ManilaAPI.NetworkAttachments) + var serviceAnnotations map[string]string + // Ensure NAD annotations are generated + serviceAnnotations, ctrlResult, err = ensureNAD(ctx, &instance.Status.Conditions, instance.Spec.ManilaAPI.NetworkAttachments, helper) if err != nil { - err := fmt.Errorf("failed create network annotation from %s: %w", - instance.Spec.ManilaAPI.NetworkAttachments, err) - instance.Status.Conditions.MarkFalse( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error()) - return ctrl.Result{}, err + return ctrlResult, err } instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) // Handle service init - ctrlResult, err := r.reconcileInit(ctx, instance, helper, serviceLabels, serviceAnnotations) + ctrlResult, err = r.reconcileInit(ctx, instance, helper, serviceLabels, serviceAnnotations) if err != nil { return ctrlResult, err } else if (ctrlResult != ctrl.Result{}) { diff --git a/controllers/manilaapi_controller.go b/controllers/manilaapi_controller.go index a54ee917..3c12b1a9 100644 --- a/controllers/manilaapi_controller.go +++ b/controllers/manilaapi_controller.go @@ -786,44 +786,11 @@ func (r *ManilaAPIReconciler) reconcileNormal(ctx context.Context, instance *man } instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - // - // TODO check when/if Init, Update, or Upgrade should/could be skipped - // - - // networks to attach to - for _, netAtt := range instance.Spec.NetworkAttachments { - _, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - r.Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return manila.ResultRequeue, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - } - - serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.NetworkAttachments) + var serviceAnnotations map[string]string + // Ensure NAD annotations are generated + serviceAnnotations, ctrlResult, err = ensureNAD(ctx, &instance.Status.Conditions, instance.Spec.NetworkAttachments, helper) if err != nil { - err := fmt.Errorf("failed create network annotation from %s: %w", instance.Spec.NetworkAttachments, err) - instance.Status.Conditions.MarkFalse( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err) - return ctrl.Result{}, err + return ctrlResult, err } // Handle service init diff --git a/controllers/manilascheduler_controller.go b/controllers/manilascheduler_controller.go index 25cc8eb5..aea7fc2f 100644 --- a/controllers/manilascheduler_controller.go +++ b/controllers/manilascheduler_controller.go @@ -335,10 +335,8 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc manila.NormalDuration, &configVars, ) - if err != nil { + if (err != nil || ctrlResult != ctrl.Result{}) { return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil } // @@ -453,44 +451,11 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc } instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - // - // TODO check when/if Init, Update, or Upgrade should/could be skipped - // - - // networks to attach to - for _, netAtt := range instance.Spec.NetworkAttachments { - _, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - r.Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return manila.ResultRequeue, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - } - - serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.NetworkAttachments) + var serviceAnnotations map[string]string + // Ensure NAD annotations are generated + serviceAnnotations, ctrlResult, err = ensureNAD(ctx, &instance.Status.Conditions, instance.Spec.NetworkAttachments, helper) if err != nil { - err := fmt.Errorf("failed create network annotation from %s: %w", instance.Spec.NetworkAttachments, err) - instance.Status.Conditions.MarkFalse( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err) - return ctrl.Result{}, err + return ctrlResult, err } // diff --git a/controllers/manilashare_controller.go b/controllers/manilashare_controller.go index 1e14430e..5d7de08e 100644 --- a/controllers/manilashare_controller.go +++ b/controllers/manilashare_controller.go @@ -334,10 +334,8 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m manila.NormalDuration, &configVars, ) - if err != nil { + if (err != nil || ctrlResult != ctrl.Result{}) { return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil } // @@ -450,44 +448,11 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m } instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - // - // TODO check when/if Init, Update, or Upgrade should/could be skipped - // - - // networks to attach to - for _, netAtt := range instance.Spec.NetworkAttachments { - _, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - r.Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return manila.ResultRequeue, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - } - - serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.NetworkAttachments) + var serviceAnnotations map[string]string + // Ensure NAD annotations are generated + serviceAnnotations, ctrlResult, err = ensureNAD(ctx, &instance.Status.Conditions, instance.Spec.NetworkAttachments, helper) if err != nil { - err := fmt.Errorf("failed create network annotation from %s: %w", instance.Spec.NetworkAttachments, err) - instance.Status.Conditions.MarkFalse( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err) - return ctrl.Result{}, err + return ctrlResult, err } //