Merge pull request #262 from elvgarrui/revert_networkattachments

Revert "Add support for adding multiple networkattachments to the ovn…
openstack-k8s-operators · Apr 9, 2024 · 4d443b8 · 4d443b8
2 parents 765fc25 + d5ce36b
commit 4d443b8
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 283 deletions.
diff --git a/api/bases/ovn.openstack.org_ovncontrollers.yaml b/api/bases/ovn.openstack.org_ovncontrollers.yaml
@@ -73,15 +73,10 @@ spec:
                     type: string
                 type: object
               networkAttachment:
-                description: 'NetworkAttachment is a NetworkAttachment resource name
+                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 OvnEncapIP. Deprecated: superseded
-                  by NetworkAttachments'
+                  address of this network is used as the OvnEncapIP.
                 type: string
-              networkAttachments:
-                items:
-                  type: string
-                type: array
               nicMappings:
                 additionalProperties:
                   type: string

diff --git a/api/v1beta1/ovncontroller_types.go b/api/v1beta1/ovncontroller_types.go
@@ -77,15 +77,8 @@ type OVNControllerSpecCore struct {
 	// +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 OvnEncapIP.
-	// Deprecated: superseded by NetworkAttachments
 	NetworkAttachment string `json:"networkAttachment"`
 
-	// +kubebuilder:validation:Optional
-	// NetworkAttachments are NetworkAttachment resources used to expose the service to the specified networks.
-	// If present, the IP of the attachment named "tenant", will be used as the OvnEncapIP.
-
-	NetworkAttachments []string `json:"networkAttachments,omitempty"`
-
 	// +kubebuilder:validation:Optional
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// TLS - Parameters related to TLS

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/ovn.openstack.org_ovncontrollers.yaml b/config/crd/bases/ovn.openstack.org_ovncontrollers.yaml
@@ -73,15 +73,10 @@ spec:
                     type: string
                 type: object
               networkAttachment:
-                description: 'NetworkAttachment is a NetworkAttachment resource name
+                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 OvnEncapIP. Deprecated: superseded
-                  by NetworkAttachments'
+                  address of this network is used as the OvnEncapIP.
                 type: string
-              networkAttachments:
-                items:
-                  type: string
-                type: array
               nicMappings:
                 additionalProperties:
                   type: string

diff --git a/controllers/ovncontroller_controller.go b/controllers/ovncontroller_controller.go
@@ -59,23 +59,6 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 )
 
-const (
-	// TunnelNetworkAttachment is the name of the network attachment
-	// that will be used for overlay tenant networking. If present in
-	// the list of attachments, OvnEncapIP is set to this interface's
-	// IP.
-	TunnelNetworkAttachmentName = "tenant"
-)
-
-func contains(s []string, str string) bool {
-	for _, v := range s {
-		if v == str {
-			return true
-		}
-	}
-	return false
-}
-
 // getlog returns a logger object with a prefix of "conroller.name" and aditional controller context fields
 func (r *OVNControllerReconciler) GetLogger(ctx context.Context) logr.Logger {
 	return log.FromContext(ctx).WithName("Controllers").WithName("OVNController")
@@ -466,14 +449,12 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
 		return ctrl.Result{}, err
 	}
 
+	// network to attach to
 	networkAttachmentsNoPhysNet := []string{}
-	if instance.Spec.NetworkAttachments != nil && len(instance.Spec.NetworkAttachments) > 0 {
-		networkAttachmentsNoPhysNet = append(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachments...)
-	}
-	if instance.Spec.NetworkAttachment != "" && !contains(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachment) {
+	if instance.Spec.NetworkAttachment != "" {
+		networkAttachments = append(networkAttachments, instance.Spec.NetworkAttachment)
 		networkAttachmentsNoPhysNet = append(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachment)
 	}
-	networkAttachments = append(networkAttachments, networkAttachmentsNoPhysNet...)
 	sort.Strings(networkAttachments)
 
 	for _, netAtt := range networkAttachments {
@@ -591,7 +572,7 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
 	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: %v", networkAttachmentsNoPhysNet)
+		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,
@@ -710,9 +691,7 @@ func (r *OVNControllerReconciler) generateServiceConfigMaps(
 	cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(ovnv1.ServiceNameOvnController), map[string]string{})
 
 	templateParameters := make(map[string]interface{})
-	if contains(instance.Spec.NetworkAttachments, TunnelNetworkAttachmentName) {
-		templateParameters["OvnEncapNIC"] = nad.GetNetworkIFName(TunnelNetworkAttachmentName)
-	} else if instance.Spec.NetworkAttachment != "" {
+	if instance.Spec.NetworkAttachment != "" {
 		templateParameters["OvnEncapNIC"] = nad.GetNetworkIFName(instance.Spec.NetworkAttachment)
 	} else {
 		templateParameters["OvnEncapNIC"] = "eth0"

diff --git a/tests/functional/ovncontroller_controller_test.go b/tests/functional/ovncontroller_controller_test.go
@@ -362,7 +362,7 @@ var _ = Describe("OVNController controller", func() {
 				corev1.ConditionFalse,
 				condition.ErrorReason,
 				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]",
+					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
 			)
 		})
 		It("reports that an IP is missing", func() {
@@ -408,7 +408,7 @@ var _ = Describe("OVNController controller", func() {
 				corev1.ConditionFalse,
 				condition.ErrorReason,
 				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]",
+					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
 			)
 		})
 		It("reports NetworkAttachmentsReady if the Pods got the proper annotations", func() {
@@ -731,235 +731,6 @@ var _ = Describe("OVNController controller", func() {
 		})
 	})
 
-	When("OVNController is created with networkAttachments and nic configs", func() {
-		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
-			DeferCleanup(DeleteOVNDBClusters, dbs)
-			spec := GetDefaultOVNControllerSpec()
-			spec.NetworkAttachments = []string{"internalapi"}
-			spec.NicMappings = map[string]string{
-				"physnet1": "enp2s0.100",
-			}
-			instance := CreateOVNController(namespace, spec)
-			DeferCleanup(th.DeleteInstance, instance)
-		})
-
-		It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			daemonSetName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller",
-			}
-			daemonSetNameOVS := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller-ovs",
-			}
-
-			ds := GetDaemonSet(daemonSetName)
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
-
-			ds = GetDaemonSet(daemonSetNameOVS)
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					},
-					{
-						Name:             "physnet1",
-						Namespace:        namespace,
-						InterfaceRequest: "physnet1",
-					},
-				})
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-		})
-	})
-
-	When("OVNController is created with old networkAttachment and new networkAttachments and nic configs", func() {
-		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
-			DeferCleanup(DeleteOVNDBClusters, dbs)
-			spec := GetDefaultOVNControllerSpec()
-			spec.NetworkAttachment = "tenant"
-			spec.NetworkAttachments = []string{"internalapi"}
-			spec.NicMappings = map[string]string{
-				"physnet1": "enp2s0.100",
-			}
-			instance := CreateOVNController(namespace, spec)
-			DeferCleanup(th.DeleteInstance, instance)
-		})
-
-		It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-			tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
-			nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			daemonSetName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller",
-			}
-			daemonSetNameOVS := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller-ovs",
-			}
-			ds := GetDaemonSet(daemonSetName)
-
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
-			ds = GetDaemonSet(daemonSetNameOVS)
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					},
-					{
-						Name:             "physnet1",
-						Namespace:        namespace,
-						InterfaceRequest: "physnet1",
-					},
-					{
-						Name:             "tenant",
-						Namespace:        namespace,
-						InterfaceRequest: "tenant",
-					},
-				})
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-		})
-	})
-
-	When("OVNController is created with networkAttachments and nic configs", func() {
-		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
-			DeferCleanup(DeleteOVNDBClusters, dbs)
-			spec := GetDefaultOVNControllerSpec()
-			spec.NetworkAttachments = []string{"internalapi", "tenant"}
-			spec.NicMappings = map[string]string{
-				"physnet1": "enp2s0.100",
-			}
-			instance := CreateOVNController(namespace, spec)
-			DeferCleanup(th.DeleteInstance, instance)
-		})
-
-		It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-			tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
-			nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			daemonSetName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller",
-			}
-			daemonSetNameOVS := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller-ovs",
-			}
-			ds := GetDaemonSet(daemonSetName)
-
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					},
-					{
-						Name:             "physnet1",
-						Namespace:        namespace,
-						InterfaceRequest: "physnet1",
-					},
-					{
-						Name:             "tenant",
-						Namespace:        namespace,
-						InterfaceRequest: "tenant",
-					},
-				})
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
-
-			ds = GetDaemonSet(daemonSetNameOVS)
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-		})
-	})
-
-	When("OVNController is created with old networkAttachment and new networkAttachments (shared value) and nic configs", func() {
-		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
-			DeferCleanup(DeleteOVNDBClusters, dbs)
-			spec := GetDefaultOVNControllerSpec()
-			spec.NetworkAttachment = "tenant"
-			spec.NetworkAttachments = []string{"internalapi", "tenant"}
-			spec.NicMappings = map[string]string{
-				"physnet1": "enp2s0.100",
-			}
-			instance := CreateOVNController(namespace, spec)
-			DeferCleanup(th.DeleteInstance, instance)
-		})
-
-		It("reports that daemonset has annotations for both Networkattachment  and nic-configs", func() {
-			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
-			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
-			DeferCleanup(th.DeleteInstance, nad)
-			tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
-			nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
-			DeferCleanup(th.DeleteInstance, nad)
-
-			daemonSetName := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller",
-			}
-			daemonSetNameOVS := types.NamespacedName{
-				Namespace: namespace,
-				Name:      "ovn-controller-ovs",
-			}
-			ds := GetDaemonSet(daemonSetName)
-
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
-			expectedAnnotation, err := json.Marshal(
-				[]networkv1.NetworkSelectionElement{
-					{
-						Name:             "internalapi",
-						Namespace:        namespace,
-						InterfaceRequest: "internalapi",
-					},
-					{
-						Name:             "physnet1",
-						Namespace:        namespace,
-						InterfaceRequest: "physnet1",
-					},
-					{
-						Name:             "tenant",
-						Namespace:        namespace,
-						InterfaceRequest: "tenant",
-					},
-				})
-			Expect(err).ShouldNot(HaveOccurred())
-			ds = GetDaemonSet(daemonSetNameOVS)
-			Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
-				HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
-			)
-
-		})
-	})
-
 	When("OVNController is created with empty spec", func() {
 		var ovnControllerName types.NamespacedName