diff --git a/controllers/common.go b/controllers/common.go index c31d3e56..e4d4a3a8 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -12,8 +12,11 @@ import ( "crypto/sha256" "github.com/go-logr/logr" + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/configmap" "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/lib-common/modules/common/pvc" "github.com/openstack-k8s-operators/lib-common/modules/common/util" "gopkg.in/yaml.v3" @@ -548,6 +551,101 @@ func GetCommonRbacRules(privileged bool) []rbacv1.PolicyRule { return []rbacv1.PolicyRule{rbacPolicyRule} } +// EnsureNetworkAttachments fetches NetworkAttachmentDefinitions and creates annotations +func (r *Reconciler) EnsureNetworkAttachments( + ctx context.Context, + log logr.Logger, + helper *helper.Helper, + networkAttachments []string, + namespace string, + conditions *condition.Conditions, +) (map[string]string, ctrl.Result, error) { + nadList := []networkv1.NetworkAttachmentDefinition{} + for _, netAtt := range networkAttachments { + netAttachDef, err := nad.GetNADWithName(ctx, helper, netAtt, namespace) + if err != nil { + if k8s_errors.IsNotFound(err) { + // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, + // we treat this as a warning because it means that the service will not be able to start. + log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) + conditions.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyWaitingMessage, + netAtt)) + return nil, ctrl.Result{RequeueAfter: time.Second * 10}, nil + } + conditions.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err.Error())) + return nil, ctrl.Result{}, err + } + + if netAttachDef != nil { + nadList = append(nadList, *netAttachDef) + } + } + + serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList) + if err != nil { + return nil, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", + networkAttachments, err) + } + + return serviceAnnotations, ctrl.Result{}, nil +} + +// VerifyNetworkAttachments verifies network status on the pod and updates conditions +func (r *Reconciler) VerifyNetworkAttachments( + ctx context.Context, + helper *helper.Helper, + instance client.Object, + networkAttachments []string, + serviceLabels map[string]string, + nextWorkflowStep int, + conditions *condition.Conditions, + networkAttachmentStatus *map[string][]string, +) (ctrl.Result, error) { + if !r.PodExists(ctx, instance, nextWorkflowStep) { + return ctrl.Result{}, nil + } + + networkReady, status, err := nad.VerifyNetworkStatusFromAnnotation( + ctx, + helper, + networkAttachments, + serviceLabels, + 1, + ) + if err != nil { + return ctrl.Result{}, err + } + + *networkAttachmentStatus = status + + if networkReady { + conditions.MarkTrue( + condition.NetworkAttachmentsReadyCondition, + condition.NetworkAttachmentsReadyMessage) + } else { + err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, networkAttachments) + conditions.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err.Error())) + + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil +} + // EnsureCloudsConfigMapExists ensures that frameworks like Tobiko and Horizon have password values // present in clouds.yaml. This code ensures that we set a default value of // 12345678 when password value is missing in the clouds.yaml diff --git a/controllers/tempest_controller.go b/controllers/tempest_controller.go index bb88ab50..8f14dd40 100644 --- a/controllers/tempest_controller.go +++ b/controllers/tempest_controller.go @@ -20,16 +20,13 @@ import ( "context" "fmt" "strconv" - "time" "github.com/go-logr/logr" - networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" "github.com/openstack-k8s-operators/lib-common/modules/common" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/configmap" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" "github.com/openstack-k8s-operators/lib-common/modules/common/labels" - nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" "github.com/openstack-k8s-operators/lib-common/modules/common/util" testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1" "github.com/openstack-k8s-operators/test-operator/pkg/tempest" @@ -248,40 +245,16 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) // Generate ConfigMaps - end - nadList := []networkv1.NetworkAttachmentDefinition{} - for _, netAtt := range instance.Spec.NetworkAttachments { - nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, - // we treat this as a warning because it means that the service will not be able to start. - Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - - if nad != nil { - nadList = append(nadList, *nad) - } - } - - serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", - instance.Spec.NetworkAttachments, err) + serviceAnnotations, ctrlResult, err := r.EnsureNetworkAttachments( + ctx, + Log, + helper, + instance.Spec.NetworkAttachments, + instance.Namespace, + &instance.Status.Conditions, + ) + if err != nil || (ctrlResult != ctrl.Result{}) { + return ctrlResult, err } // Create a new pod @@ -335,37 +308,19 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // Create a new pod - end // NetworkAttachments - if r.PodExists(ctx, instance, nextWorkflowStep) { - networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation( - ctx, - helper, - instance.Spec.NetworkAttachments, - serviceLabels, - 1, - ) - if err != nil { - return ctrl.Result{}, err - } - - instance.Status.NetworkAttachments = networkAttachmentStatus - - if networkReady { - instance.Status.Conditions.MarkTrue( - condition.NetworkAttachmentsReadyCondition, - condition.NetworkAttachmentsReadyMessage) - } else { - err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, instance.Spec.NetworkAttachments) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - - return ctrl.Result{}, err - } + ctrlResult, err = r.VerifyNetworkAttachments( + ctx, + helper, + instance, + instance.Spec.NetworkAttachments, + serviceLabels, + nextWorkflowStep, + &instance.Status.Conditions, + &instance.Status.NetworkAttachments, + ) + if err != nil || (ctrlResult != ctrl.Result{}) { + return ctrlResult, err } - // NetworkAttachments - end return ctrl.Result{}, nil } diff --git a/controllers/tobiko_controller.go b/controllers/tobiko_controller.go index 88a3b5e9..95e2af9e 100644 --- a/controllers/tobiko_controller.go +++ b/controllers/tobiko_controller.go @@ -20,16 +20,13 @@ import ( "context" "fmt" "strconv" - "time" "github.com/go-logr/logr" - networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" "github.com/openstack-k8s-operators/lib-common/modules/common" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/configmap" "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/lib-common/modules/common/util" testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1" "github.com/openstack-k8s-operators/test-operator/pkg/tobiko" @@ -221,74 +218,32 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } // Create PersistentVolumeClaim - end - nadList := []networkv1.NetworkAttachmentDefinition{} - for _, netAtt := range instance.Spec.NetworkAttachments { - nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, - // we treat this as a warning because it means that the service will not be able to start. - Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - - if nad != nil { - nadList = append(nadList, *nad) - } - } - - serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", - instance.Spec.NetworkAttachments, err) + serviceAnnotations, ctrlResult, err := r.EnsureNetworkAttachments( + ctx, + Log, + helper, + instance.Spec.NetworkAttachments, + instance.Namespace, + &instance.Status.Conditions, + ) + if err != nil || (ctrlResult != ctrl.Result{}) { + return ctrlResult, err } // NetworkAttachments - if r.PodExists(ctx, instance, nextWorkflowStep) { - networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation( - ctx, - helper, - instance.Spec.NetworkAttachments, - serviceLabels, - 1, - ) - if err != nil { - return ctrl.Result{}, err - } - - instance.Status.NetworkAttachments = networkAttachmentStatus - - if networkReady { - instance.Status.Conditions.MarkTrue( - condition.NetworkAttachmentsReadyCondition, - condition.NetworkAttachmentsReadyMessage) - } else { - err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, instance.Spec.NetworkAttachments) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - - return ctrl.Result{}, err - } + ctrlResult, err = r.VerifyNetworkAttachments( + ctx, + helper, + instance, + instance.Spec.NetworkAttachments, + serviceLabels, + nextWorkflowStep, + &instance.Status.Conditions, + &instance.Status.NetworkAttachments, + ) + if err != nil || (ctrlResult != ctrl.Result{}) { + return ctrlResult, err } - // NetworkAttachments - end // Create Job mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle")