Add network-attachment-definition support

It might be required for the horizon Pods to be able to reach external services on a different network. For this reason this patch introduces the NetworkAttachment interface to the horizon API and implements the same logic already present in other operators. Jira: https://issues.redhat.com/browse/OSPRH-9285 Signed-off-by: Francesco Pantano <[email protected]>
openstack-k8s-operators · Aug 28, 2024 · 8a3e5c6 · 8a3e5c6
1 parent b73449a
commit 8a3e5c6
Show file tree

Hide file tree

Showing 11 changed files with 255 additions and 7 deletions.
diff --git a/api/bases/horizon.openstack.org_horizons.yaml b/api/bases/horizon.openstack.org_horizons.yaml
@@ -16,6 +16,10 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - description: NetworkAttachments
+      jsonPath: .status.networkAttachments
+      name: NetworkAttachments
+      type: string
     - description: Status
       jsonPath: .status.conditions[0].status
       name: Status
@@ -1926,6 +1930,12 @@ spec:
                 default: memcached
                 description: Memcached instance name.
                 type: string
+              networkAttachments:
+                description: NetworkAttachments is a list of NetworkAttachment resource
+                  names to expose the services to the given network
+                items:
+                  type: string
+                type: array
               nodeSelector:
                 additionalProperties:
                   type: string
@@ -2233,6 +2243,13 @@ spec:
                   type: string
                 description: Map of hashes to track e.g. job status
                 type: object
+              networkAttachments:
+                additionalProperties:
+                  items:
+                    type: string
+                  type: array
+                description: NetworkAttachments status of the deployment pods
+                type: object
               observedGeneration:
                 description: ObservedGeneration - the most recent generation observed
                   for this service. If the observed generation is less than the spec

diff --git a/api/v1beta1/horizon_types.go b/api/v1beta1/horizon_types.go
@@ -100,6 +100,10 @@ type HorizonSpecCore struct {
 	// ExtraMounts containing conf files
 	// +kubebuilder:default={}
 	ExtraMounts []HorizonExtraVolMounts `json:"extraMounts,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	// NetworkAttachments is a list of NetworkAttachment resource names to expose the services to the given network
+	NetworkAttachments []string `json:"networkAttachments,omitempty"`
 }
 
 // HorizionOverrideSpec to override the generated manifest of several child resources.
@@ -127,10 +131,14 @@ type HorizonStatus struct {
 	// then the controller has not processed the latest changes injected by
 	// the opentack-operator in the top-level CR (e.g. the ContainerImage)
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
+	// NetworkAttachments status of the deployment pods
+	NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
 }
 
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
+//+kubebuilder:printcolumn:name="NetworkAttachments",type="string",JSONPath=".status.networkAttachments",description="NetworkAttachments"
 //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[0].status",description="Status"
 //+kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.conditions[0].message",description="Message"
 

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/horizon.openstack.org_horizons.yaml b/config/crd/bases/horizon.openstack.org_horizons.yaml
@@ -16,6 +16,10 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - description: NetworkAttachments
+      jsonPath: .status.networkAttachments
+      name: NetworkAttachments
+      type: string
     - description: Status
       jsonPath: .status.conditions[0].status
       name: Status
@@ -1926,6 +1930,12 @@ spec:
                 default: memcached
                 description: Memcached instance name.
                 type: string
+              networkAttachments:
+                description: NetworkAttachments is a list of NetworkAttachment resource
+                  names to expose the services to the given network
+                items:
+                  type: string
+                type: array
               nodeSelector:
                 additionalProperties:
                   type: string
@@ -2233,6 +2243,13 @@ spec:
                   type: string
                 description: Map of hashes to track e.g. job status
                 type: object
+              networkAttachments:
+                additionalProperties:
+                  items:
+                    type: string
+                  type: array
+                description: NetworkAttachments status of the deployment pods
+                type: object
               observedGeneration:
                 description: ObservedGeneration - the most recent generation observed
                   for this service. If the observed generation is less than the spec

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -103,6 +103,14 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - k8s.cni.cncf.io
+  resources:
+  - network-attachment-definitions
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - keystone.openstack.org
   resources:

diff --git a/controllers/horizon_controller.go b/controllers/horizon_controller.go
@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	horizonv1beta1 "github.com/openstack-k8s-operators/horizon-operator/api/v1beta1"
 	horizon "github.com/openstack-k8s-operators/horizon-operator/pkg/horizon"
 	memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
@@ -35,17 +36,17 @@ import (
 	env "github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	labels "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"
 	oko_secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
 	util "github.com/openstack-k8s-operators/lib-common/modules/common/util"
-	"k8s.io/apimachinery/pkg/fields"
-
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
@@ -106,6 +107,8 @@ type HorizonReconciler struct {
 // service account permissions that are needed to grant permission to the above
 // +kubebuilder:rbac:groups="security.openshift.io",resourceNames=anyuid,resources=securitycontextconstraints,verbs=use
 // +kubebuilder:rbac:groups="",resources=pods,verbs=create;delete;get;list;patch;update;watch
+// +kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch
+// service account, role, rolebinding
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -280,6 +283,35 @@ func (r *HorizonReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		return nil
 	}
 
+	// Watch for changes to NADs
+	nadFn := func(ctx context.Context, o client.Object) []reconcile.Request {
+		result := []reconcile.Request{}
+
+		// get all GlanceAPI CRs
+		glanceAPIs := &horizonv1beta1.HorizonList{}
+		listOpts := []client.ListOption{
+			client.InNamespace(o.GetNamespace()),
+		}
+		if err := r.Client.List(context.Background(), glanceAPIs, listOpts...); err != nil {
+			logger.Error(err, "Unable to retrieve Horizon CRs %w")
+			return nil
+		}
+		for _, cr := range glanceAPIs.Items {
+			if util.StringInSlice(o.GetName(), cr.Spec.NetworkAttachments) {
+				name := client.ObjectKey{
+					Namespace: cr.GetNamespace(),
+					Name:      cr.GetName(),
+				}
+				logger.Info(fmt.Sprintf("NAD %s is used by Horizon CR %s", o.GetName(), cr.GetName()))
+				result = append(result, reconcile.Request{NamespacedName: name})
+			}
+		}
+		if len(result) > 0 {
+			return result
+		}
+		return nil
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&horizonv1beta1.Horizon{}).
 		Owns(&corev1.Service{}).
@@ -291,6 +323,8 @@ func (r *HorizonReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&rbacv1.RoleBinding{}).
 		Watches(&memcachedv1.Memcached{},
 			handler.EnqueueRequestsFromMapFunc(memcachedFn)).
+		Watches(&networkv1.NetworkAttachmentDefinition{},
+			handler.EnqueueRequestsFromMapFunc(nadFn)).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -668,6 +702,22 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
 		return ctrlResult, err
 	}
 
+	var serviceAnnotations map[string]string
+	// networks to attach to
+	serviceAnnotations, ctrlResult, err = r.ensureNAD(ctx, instance, instance.Spec.NetworkAttachments, helper)
+	if err != nil {
+		instance.Status.Conditions.MarkFalse(
+			condition.NetworkAttachmentsReadyCondition,
+			condition.ErrorReason,
+			condition.SeverityWarning,
+			condition.NetworkAttachmentsReadyErrorMessage,
+			err,
+		)
+		return ctrlResult, err
+	}
+
+	instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage)
+
 	// Handle service update
 	ctrlResult, err = r.reconcileUpdate(ctx)
 	if err != nil || (ctrlResult != ctrl.Result{}) {
@@ -685,7 +735,7 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
 	//
 
 	// Define a new Deployment object
-	deplDef, err := horizon.Deployment(instance, inputHash, serviceLabels)
+	deplDef, err := horizon.Deployment(instance, inputHash, serviceLabels, serviceAnnotations)
 	if err != nil {
 		Log.Error(err, "Deployment failed")
 		instance.Status.Conditions.Set(condition.FalseCondition(
@@ -721,6 +771,42 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
 		return ctrlResult, nil
 	}
 	instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
+
+	networkReady := false
+	var networkAttachmentStatus map[string][]string
+	// verify if network attachment matches expectations
+	if *instance.Spec.Replicas > 0 {
+		networkReady, networkAttachmentStatus, err = nad.VerifyNetworkStatusFromAnnotation(
+			ctx,
+			helper,
+			instance.Spec.NetworkAttachments,
+			serviceLabels,
+			instance.Status.ReadyCount,
+		)
+		if err != nil {
+			err = fmt.Errorf("verifying API NetworkAttachments (%s) %w", instance.Spec.NetworkAttachments, err)
+			instance.Status.Conditions.MarkFalse(
+				condition.NetworkAttachmentsReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.NetworkAttachmentsReadyErrorMessage,
+				err)
+			return ctrl.Result{}, err
+		}
+		instance.Status.NetworkAttachments = networkAttachmentStatus
+		if networkReady {
+			instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage)
+		} else {
+			err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments)
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.NetworkAttachmentsReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.NetworkAttachmentsReadyErrorMessage,
+				err.Error()))
+			return ctrl.Result{}, err
+		}
+	}
 	// Mark the Deployment as Ready only if the number of Replicas is equals
 	// to the Deployed instances (ReadyCount), but mark it as True is Replicas
 	// is zero. In addition, make sure the controller sees the last Generation
@@ -748,7 +834,6 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
 }
 
 // generateServiceConfigMaps - create configmaps which hold scripts and service configuration
-// TODO add DefaultConfigOverwrite
 func (r *HorizonReconciler) generateServiceConfigMaps(
 	ctx context.Context,
 	instance *horizonv1beta1.Horizon,
@@ -903,3 +988,48 @@ func configureHorizonRbac(ctx context.Context, helper *helper.Helper, instance *
 	}
 	return common_rbac.ReconcileRbac(ctx, helper, instance, rbacRules)
 }
+
+// ensureNAD - this function gets NADs based on the string[] passed as input
+// and produces the required Annotation for the Horizon component
+func (r *HorizonReconciler) ensureNAD(
+	ctx context.Context,
+	instance *horizonv1beta1.Horizon,
+	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) {
+				helper.GetLogger().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 serviceAnnotations, ctrl.Result{RequeueAfter: time.Second * 10}, nil
+			}
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.NetworkAttachmentsReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.NetworkAttachmentsReadyErrorMessage,
+				err.Error()))
+			return serviceAnnotations, ctrl.Result{RequeueAfter: time.Second * 10}, nil
+		}
+	}
+	// Create NetworkAnnotations
+	serviceAnnotations, err = nad.CreateNetworksAnnotation(helper.GetBeforeObject().GetNamespace(), nAttach)
+	if err != nil {
+		return serviceAnnotations, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
+			nAttach, err)
+	}
+	return serviceAnnotations, ctrl.Result{}, err
+}
diff --git a/go.mod b/go.mod
@@ -7,6 +7,7 @@ replace github.com/openstack-k8s-operators/horizon-operator/api => ./api
 require (
 	github.com/go-logr/logr v1.4.2
 	github.com/google/uuid v1.6.0
+	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
 	github.com/onsi/ginkgo/v2 v2.19.1
 	github.com/onsi/gomega v1.34.1
 	github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240214134444-c675e5f69043
@@ -45,7 +46,6 @@ require (
 	github.com/imdario/mergo v0.3.16 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
-	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect

diff --git a/main.go b/main.go
@@ -38,6 +38,7 @@ import (
 
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
+	networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
 	keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
 
@@ -54,6 +55,7 @@ var (
 func init() {
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 
+	utilruntime.Must(networkv1.AddToScheme(scheme))
 	utilruntime.Must(horizonv1beta1.AddToScheme(scheme))
 	utilruntime.Must(keystonev1.AddToScheme(scheme))
 	utilruntime.Must(memcachedv1.AddToScheme(scheme))