Remove networkAttachment for ovnNorthd

ovnNorthd does not require additional interface via networkAttachment, this patch removes it from the API and controller code. Patches removing it from the docs and sample files: * openstack-k8s-operators/openstack-operator#759 * openstack-k8s-operators/data-plane-adoption#408 * openstack-k8s-operators/architecture#186 Closes: OSPRH-659 Signed-off-by: Lucas Alvares Gomes <[email protected]>
umago · Apr 19, 2024 · 066def4 · 066def4
1 parent 056d38f
commit 066def4
Show file tree

Hide file tree

Showing 5 changed files with 0 additions and 237 deletions.
diff --git a/api/bases/ovn.openstack.org_ovnnorthds.yaml b/api/bases/ovn.openstack.org_ovnnorthds.yaml
@@ -16,10 +16,6 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - description: NetworkAttachments
-      jsonPath: .status.networkAttachments
-      name: NetworkAttachments
-      type: string
     - description: Status
       jsonPath: .status.conditions[0].status
       name: Status
@@ -62,11 +58,6 @@ spec:
                   flows
                 format: int32
                 type: integer
-              networkAttachment:
-                description: NetworkAttachment is a NetworkAttachment resource name
-                  to expose the service to the given network. If specified the IP
-                  address of this network is used as the dbAddress connection.
-                type: string
               nodeSelector:
                 additionalProperties:
                   type: string
@@ -189,13 +180,6 @@ spec:
                   - type
                   type: object
                 type: array
-              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/ovnnorthd_types.go b/api/v1beta1/ovnnorthd_types.go
@@ -66,11 +66,6 @@ type OVNNorthdSpecCore struct {
 	// https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
 	Resources corev1.ResourceRequirements `json:"resources,omitempty"`
 
-	// +kubebuilder:validation:Optional
-	// NetworkAttachment is a NetworkAttachment resource name to expose the service to the given network.
-	// If specified the IP address of this network is used as the dbAddress connection.
-	NetworkAttachment string `json:"networkAttachment"`
-
 	// +kubebuilder:validation:Optional
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// TLS - Parameters related to TLS
@@ -90,16 +85,12 @@ type OVNNorthdStatus struct {
 	// Conditions
 	Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"`
 
-	// NetworkAttachments status of the deployment pods
-	NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
-
 	//ObservedGeneration - the most recent generation observed for this service. If the observed generation is less than the spec generation, then the controller has not processed the latest changes.
 	ObservedGeneration int64 `json:"observedGeneration,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/config/crd/bases/ovn.openstack.org_ovnnorthds.yaml b/config/crd/bases/ovn.openstack.org_ovnnorthds.yaml
@@ -16,10 +16,6 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - description: NetworkAttachments
-      jsonPath: .status.networkAttachments
-      name: NetworkAttachments
-      type: string
     - description: Status
       jsonPath: .status.conditions[0].status
       name: Status
@@ -62,11 +58,6 @@ spec:
                   flows
                 format: int32
                 type: integer
-              networkAttachment:
-                description: NetworkAttachment is a NetworkAttachment resource name
-                  to expose the service to the given network. If specified the IP
-                  address of this network is used as the dbAddress connection.
-                type: string
               nodeSelector:
                 additionalProperties:
                   type: string
@@ -189,13 +180,6 @@ spec:
                   - type
                   type: object
                 type: array
-              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/controllers/ovnnorthd_controller.go b/controllers/ovnnorthd_controller.go
@@ -40,7 +40,6 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/deployment"
 	"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"
 	common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
 	ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
@@ -82,7 +81,6 @@ func (r *OVNNorthdReconciler) GetLogger(ctx context.Context) logr.Logger {
 //+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete;
 //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;patch;update;delete;
 //+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;
-//+kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch
 
 // service account, role, rolebinding
 // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update
@@ -136,7 +134,6 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	// initialize conditions used later as Status=Unknown
 	cl := condition.CreateList(
 		condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
-		condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage),
 		condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage),
 		condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage),
 		condition.UnknownCondition(condition.RoleReadyCondition, condition.InitReason, condition.RoleReadyInitMessage),
@@ -147,10 +144,6 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	instance.Status.Conditions.Init(&cl)
 	instance.Status.ObservedGeneration = instance.Generation
 
-	if instance.Status.NetworkAttachments == nil {
-		instance.Status.NetworkAttachments = map[string][]string{}
-	}
-
 	// Always patch the instance status when exiting this function so we can persist any changes.
 	defer func() {
 		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
@@ -328,39 +321,6 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
 		common.AppSelector: ovnv1.ServiceNameOvnNorthd,
 	}
 
-	// network to attach to
-	networkAttachments := []string{}
-	if instance.Spec.NetworkAttachment != "" {
-		networkAttachments = append(networkAttachments, instance.Spec.NetworkAttachment)
-
-		_, err = nad.GetNADWithName(ctx, helper, instance.Spec.NetworkAttachment, instance.Namespace)
-		if err != nil {
-			if k8s_errors.IsNotFound(err) {
-				instance.Status.Conditions.Set(condition.FalseCondition(
-					condition.NetworkAttachmentsReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
-					condition.NetworkAttachmentsReadyWaitingMessage,
-					instance.Spec.NetworkAttachment))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", instance.Spec.NetworkAttachment)
-			}
-			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, networkAttachments)
-	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
-			instance.Spec.NetworkAttachment, err)
-	}
-
 	// Handle service update
 	ctrlResult, err := r.reconcileUpdate(ctx, instance, helper)
 	if err != nil {
@@ -463,27 +423,6 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
 
 	instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
 
-	// verify if network attachment matches expectations
-	networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, networkAttachments, serviceLabels, instance.Status.ReadyCount)
-	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("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachment)
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.NetworkAttachmentsReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.NetworkAttachmentsReadyErrorMessage,
-			err.Error()))
-
-		return ctrlResult, nil
-	}
-
 	if instance.Status.ReadyCount > 0 {
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 	} else if *instance.Spec.Replicas == 0 {

diff --git a/tests/functional/ovnnorthd_controller_test.go b/tests/functional/ovnnorthd_controller_test.go
@@ -20,7 +20,6 @@ import (
 	"encoding/json"
 	"fmt"
 
-	networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
@@ -129,150 +128,16 @@ var _ = Describe("OVNNorthd controller", func() {
 		})
 	})
 
-	When("OVNNorthd is created with networkAttachments", func() {
-		var ovnNorthdName types.NamespacedName
-
-		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
-			DeferCleanup(DeleteOVNDBClusters, dbs)
-			spec := GetDefaultOVNNorthdSpec()
-			spec.NetworkAttachment = "internalapi"
-			ovnNorthdName = ovn.CreateOVNNorthd(namespace, spec)
-			DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName)
-		})
-
-		It("reports that the definition is missing", func() {
-			th.ExpectConditionWithDetails(
-				ovnNorthdName,
-				ConditionGetterFunc(OVNNorthdConditionGetter),
-				condition.NetworkAttachmentsReadyCondition,
-				corev1.ConditionFalse,
-				condition.RequestedReason,
-				"NetworkAttachment resources missing: internalapi",
-			)
-		})
-		It("reports that network attachment is missing", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			deplName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-northd",
-			}
-
-			depl := th.GetDeployment(deplName)
-
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					}})
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(depl.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-
-			// We don't add network attachment status annotations to the Pods
-			// to simulate that the network attachments are missing.
-			//th.SimulateDeploymentReadyWithPods(deplName, map[string][]string{})
-
-			th.ExpectConditionWithDetails(
-				ovnNorthdName,
-				ConditionGetterFunc(OVNNorthdConditionGetter),
-				condition.NetworkAttachmentsReadyCondition,
-				corev1.ConditionFalse,
-				condition.ErrorReason,
-				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
-			)
-		})
-		It("reports that an IP is missing", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			deplName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-northd",
-			}
-
-			depl := th.GetDeployment(deplName)
-
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					}})
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(depl.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-
-			// We simulate that there is no IP associated with the internalapi
-			// network attachment
-			th.SimulateDeploymentReadyWithPods(
-				deplName,
-				map[string][]string{namespace + "/internalapi": {}},
-			)
-
-			th.ExpectConditionWithDetails(
-				ovnNorthdName,
-				ConditionGetterFunc(OVNNorthdConditionGetter),
-				condition.NetworkAttachmentsReadyCondition,
-				corev1.ConditionFalse,
-				condition.ErrorReason,
-				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
-			)
-		})
-		It("reports NetworkAttachmentsReady if the Pods got the proper annotations", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			deplName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-northd",
-			}
-
-			th.SimulateDeploymentReadyWithPods(
-				deplName,
-				map[string][]string{namespace + "/internalapi": {"10.0.0.1"}},
-			)
-
-			th.ExpectCondition(
-				ovnNorthdName,
-				ConditionGetterFunc(OVNNorthdConditionGetter),
-				condition.NetworkAttachmentsReadyCondition,
-				corev1.ConditionTrue,
-			)
-
-			Eventually(func(g Gomega) {
-				ovnNorthd := GetOVNNorthd(ovnNorthdName)
-				g.Expect(ovnNorthd.Status.NetworkAttachments).To(
-					Equal(map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}))
-
-			}, timeout, interval).Should(Succeed())
-		})
-	})
-
 	When("OVNNorthd is created with TLS", func() {
 		var ovnNorthdName types.NamespacedName
 
 		BeforeEach(func() {
 			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
 			DeferCleanup(DeleteOVNDBClusters, dbs)
 			spec := GetTLSOVNNorthdSpec()
-			spec.NetworkAttachment = "internalapi"
 			ovnNorthdName = ovn.CreateOVNNorthd(namespace, spec)
 			DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName)
 			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
 			DeferCleanup(th.DeleteInstance, nad)
 		})