From d5ce36b581442be76467e65784ca3bd3b202b7e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvira=20Garc=C3=ADa?= Date: Mon, 8 Apr 2024 18:18:54 +0200 Subject: [PATCH] Revert "Add support for adding multiple networkattachments to the ovncontroller pods" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 5aea578acbfedc1483f2feef64e06213c7963a15. Seems like NetworkAttachments won't be used by Octavia in the end, so reverting the code to align this property with the other openstack-k8s-operator repos. Signed-off-by: Elvira García --- .../ovn.openstack.org_ovncontrollers.yaml | 9 +- api/v1beta1/ovncontroller_types.go | 7 - api/v1beta1/zz_generated.deepcopy.go | 5 - .../ovn.openstack.org_ovncontrollers.yaml | 9 +- controllers/ovncontroller_controller.go | 31 +-- .../ovncontroller_controller_test.go | 233 +----------------- 6 files changed, 11 insertions(+), 283 deletions(-) diff --git a/api/bases/ovn.openstack.org_ovncontrollers.yaml b/api/bases/ovn.openstack.org_ovncontrollers.yaml index 1786cb38..9bcfb750 100644 --- 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 index bc47aac7..f6d66774 100644 --- 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 index 669c76fc..14a6edd6 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -120,11 +120,6 @@ func (in *OVNControllerSpecCore) DeepCopyInto(out *OVNControllerSpecCore) { (*out)[key] = val } } - if in.NetworkAttachments != nil { - in, out := &in.NetworkAttachments, &out.NetworkAttachments - *out = make([]string, len(*in)) - copy(*out, *in) - } in.TLS.DeepCopyInto(&out.TLS) } diff --git a/config/crd/bases/ovn.openstack.org_ovncontrollers.yaml b/config/crd/bases/ovn.openstack.org_ovncontrollers.yaml index 1786cb38..9bcfb750 100644 --- 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 index 59803708..b5922a16 100644 --- 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 index 03e741de..8f59c370 100644 --- 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