Skip to content

Commit

Permalink
Add ensureNAD common function
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@fmount
fmount committed Sep 24, 2024
1 parent 95ffca3 commit 182a0f8
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 161 deletions.
4 changes: 2 additions & 2 deletions api/v1beta1/manila_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
51 changes: 51 additions & 0 deletions controllers/manila_common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down
52 changes: 7 additions & 45 deletions controllers/manila_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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"
Expand Down Expand Up @@ -510,7 +509,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{
Expand All @@ -521,10 +520,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
Expand Down Expand Up @@ -591,51 +588,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
}
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{}) {
Expand Down
39 changes: 3 additions & 36 deletions controllers/manilaapi_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,43 +786,10 @@ 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
}

Expand Down
43 changes: 4 additions & 39 deletions controllers/manilascheduler_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//
Expand Down Expand Up @@ -453,43 +451,10 @@ 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
}

Expand Down
43 changes: 4 additions & 39 deletions controllers/manilashare_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//
Expand Down Expand Up @@ -450,43 +448,10 @@ 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
}

Expand Down

0 comments on commit 182a0f8

Please sign in to comment.