From ba9d53e572e55f3f342a270d9cd480376ec2b1b8 Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Thu, 26 Sep 2024 16:36:21 +0200 Subject: [PATCH] Revert "[ovn-controller] Don't create ovn-controller if nicMappings empty" This reverts commit a02a1596b972a599d77158d9dc746849f8075b23. This change created a bug on the adoption where all the gates reaches TIME_OUT, reverting this patch to unblock prod chain while fixing it. The problem that this patch was introducing was that edpm deployment expects a config map created by ovn-operator (ovncontroller) called ovncontroller-config. With this patch the ovncontroller CR was not created and the ovn-operator was not reconciling this object, which was needed to create the said config map. Related: OSPRH-7463 Related: OSPCIX-504 --- .../core_v1beta1_openstackcontrolplane.yaml | 2 +- config/samples/nad_datacentre.yaml | 17 - pkg/openstack/ovn.go | 7 - .../openstackoperator_controller_test.go | 36 -- .../openstackversion_controller_test.go | 5 - .../01-create-nic-mappings.yaml | 5 - .../02-assert-deploy-openstack.yaml | 1 - .../02-deploy-openstack.yaml | 5 - .../03-add-ovn-nic-mappings.yaml | 10 - .../03-assert.yaml | 15 - .../04-assert.yaml | 11 - .../04-remove-ovn-nic-mappings.yaml | 9 - .../05-cleanup.yaml | 13 - .../05-errors-cleanup.yaml | 334 ------------------ 14 files changed, 1 insertion(+), 469 deletions(-) delete mode 100644 config/samples/nad_datacentre.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml delete mode 120000 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml delete mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-errors-cleanup.yaml diff --git a/config/samples/base/openstackcontrolplane/core_v1beta1_openstackcontrolplane.yaml b/config/samples/base/openstackcontrolplane/core_v1beta1_openstackcontrolplane.yaml index d738a67a1..f2e59375d 100644 --- a/config/samples/base/openstackcontrolplane/core_v1beta1_openstackcontrolplane.yaml +++ b/config/samples/base/openstackcontrolplane/core_v1beta1_openstackcontrolplane.yaml @@ -97,9 +97,9 @@ spec: replicas: 1 dbType: SB storageRequest: 10G - ovnController: {} ovnNorthd: replicas: 1 + ovnController: {} neutron: template: databaseInstance: openstack diff --git a/config/samples/nad_datacentre.yaml b/config/samples/nad_datacentre.yaml deleted file mode 100644 index cab45e980..000000000 --- a/config/samples/nad_datacentre.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: k8s.cni.cncf.io/v1 -kind: NetworkAttachmentDefinition -metadata: - annotations: - labels: - osp/net: data-centre - service: ovn-controller - name: datacentre -spec: - config: | - { - "cniVersion": "0.3.1", - "name": "datacentre", - "type": "bridge", - "bridge": "ospbr", - "ipam": {} - } diff --git a/pkg/openstack/ovn.go b/pkg/openstack/ovn.go index 25997af7a..77c5fc279 100644 --- a/pkg/openstack/ovn.go +++ b/pkg/openstack/ovn.go @@ -314,13 +314,6 @@ func ReconcileOVNController(ctx context.Context, instance *corev1beta1.OpenStack return false, err } return false, nil - } else if len(instance.Spec.Ovn.Template.OVNController.NicMappings) == 0 { - // If nicMappings is not configured there's no need to start ovn-controller - Log.Info("OVN Controller has no nicMappings configured, forcing ready condition to true.") - if _, err := EnsureDeleted(ctx, helper, OVNController); err != nil { - return false, err - } - return true, nil } ovnControllerSpec := &instance.Spec.Ovn.Template.OVNController diff --git a/tests/functional/ctlplane/openstackoperator_controller_test.go b/tests/functional/ctlplane/openstackoperator_controller_test.go index 166e657a3..6bad34358 100644 --- a/tests/functional/ctlplane/openstackoperator_controller_test.go +++ b/tests/functional/ctlplane/openstackoperator_controller_test.go @@ -1620,11 +1620,6 @@ var _ = Describe("OpenStackOperator controller", func() { "dbType": "SB", }, }, - "ovnController": map[string]interface{}{ - "nicMappings": map[string]interface{}{ - "datacentre": "ospbr", - }, - }, }, } DeferCleanup( @@ -1675,37 +1670,6 @@ var _ = Describe("OpenStackOperator controller", func() { }, timeout, interval).Should(Succeed()) }) - It("should remove ovn-controller if nicMappings are removed", func() { - // Update spec - Eventually(func(g Gomega) { - OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) - OSCtlplane.Spec.Ovn.Template.OVNController.NicMappings = nil - g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed()) - }, timeout, interval).Should(Succeed()) - - // ovn services exist - Eventually(func(g Gomega) { - ovnNorthd := ovn.GetOVNNorthd(names.OVNNorthdName) - g.Expect(ovnNorthd).Should(Not(BeNil())) - }, timeout, interval).Should(Succeed()) - - // If nicMappings are not configured, ovnController shouldn't spawn - Eventually(func(g Gomega) { - instance := &ovnv1.OVNController{} - g.Expect(th.K8sClient.Get(th.Ctx, names.OVNControllerName, instance)).Should(Not(Succeed())) - }, timeout, interval).Should(Succeed()) - - Eventually(func(g Gomega) { - ovnDbServerNB := ovn.GetOVNDBCluster(names.OVNDbServerNBName) - g.Expect(ovnDbServerNB).Should(Not(BeNil())) - }, timeout, interval).Should(Succeed()) - - Eventually(func(g Gomega) { - ovnDbServerSB := ovn.GetOVNDBCluster(names.OVNDbServerSBName) - g.Expect(ovnDbServerSB).Should(Not(BeNil())) - }, timeout, interval).Should(Succeed()) - }) - It("should remove OVN resources on disable", func() { Eventually(func(g Gomega) { OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) diff --git a/tests/functional/ctlplane/openstackversion_controller_test.go b/tests/functional/ctlplane/openstackversion_controller_test.go index 1b5545554..5536b881c 100644 --- a/tests/functional/ctlplane/openstackversion_controller_test.go +++ b/tests/functional/ctlplane/openstackversion_controller_test.go @@ -239,11 +239,6 @@ var _ = Describe("OpenStackOperator controller", func() { "dbType": "SB", }, }, - "ovnController": map[string]interface{}{ - "nicMappings": map[string]interface{}{ - "datacentre": "ospbr", - }, - }, }, } DeferCleanup( diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml deleted file mode 100644 index aef2ee26f..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml +++ /dev/null @@ -1,5 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -commands: - - script: | - oc apply -n $NAMESPACE -f ../../../../config/samples/nad_datacentre.yaml diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml deleted file mode 120000 index 762a8cf31..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml +++ /dev/null @@ -1 +0,0 @@ -../../common/assert-sample-deployment.yaml \ No newline at end of file diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml deleted file mode 100644 index 6c9d0887d..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml +++ /dev/null @@ -1,5 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -commands: - - script: | - oc kustomize ../../../../config/samples/base/openstackcontrolplane | oc apply -n $NAMESPACE -f - diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml deleted file mode 100644 index ae2e618e9..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -timeout: 60 -commands: - - script: | - oc patch openstackcontrolplane -n $NAMESPACE openstack --type='json' -p='[{ - "op": "replace", - "path": "/spec/ovn/template/ovnController/nicMappings", - "value":{"datacentre":"ospbr"} - }]' diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml deleted file mode 100644 index b15044e0f..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml +++ /dev/null @@ -1,15 +0,0 @@ -apiVersion: v1 -kind: Pod -metadata: - labels: - service: ovn-controller -status: - phase: Running ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - service: ovn-controller-ovs -status: - phase: Running diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml deleted file mode 100644 index 93835505e..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestAssert -timeout: 300 -commands: - - script: | - ovs_controller_pod_count=$(oc get pod -n $NAMESPACE -l service=ovn-controller-ovs --no-headers=true --ignore-not-found=true | wc -l) - ovn_controller_pod_count=$(oc get pod -n $NAMESPACE -l service=ovn-controller --no-headers=true --ignore-not-found=true | wc -l) - if [ $ovs_controller_pod_count -eq 0 ] && [ $ovn_controller_pod_count -eq 0 ]; then - exit 0 - fi - exit 1 diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml deleted file mode 100644 index 49882f502..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -timeout: 60 -commands: - - script: | - oc patch OpenStackControlPlane -n $NAMESPACE openstack --type='json' -p='[{ - "op": "remove", - "path": "/spec/ovn/template/ovnController/nicMappings", - }]' diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml deleted file mode 100644 index 6b4992512..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -delete: -- apiVersion: core.openstack.org/v1beta1 - kind: OpenStackControlPlane - name: openstack -commands: -- script: | - oc delete --ignore-not-found=true -n $NAMESPACE pvc \ - srv-swift-storage-0 - oc delete secret --ignore-not-found=true combined-ca-bundle -n $NAMESPACE - oc delete secret -l service-cert -n $NAMESPACE - oc delete secret -l ca-cert -n $NAMESPACE diff --git a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-errors-cleanup.yaml b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-errors-cleanup.yaml deleted file mode 100644 index 8b9f6e6f9..000000000 --- a/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-errors-cleanup.yaml +++ /dev/null @@ -1,334 +0,0 @@ -apiVersion: v1 -kind: Pod -metadata: - labels: - app: mariadb - cr: mariadb-openstack - owner: mariadb-operator - name: mariadb-openstack ---- -apiVersion: mariadb.openstack.org/v1beta1 -kind: Galera -metadata: - name: openstack ---- -apiVersion: apps/v1 -kind: StatefulSet -metadata: - name: openstack-galera ---- -apiVersion: v1 -kind: Pod -metadata: - name: openstack-galera-0 ---- -apiVersion: v1 -kind: Pod -metadata: - name: openstack-galera-1 ---- -apiVersion: v1 -kind: Pod -metadata: - name: openstack-galera-2 ---- -apiVersion: v1 -kind: Service -metadata: - name: openstack-galera ---- -apiVersion: v1 -kind: Endpoints -metadata: - name: openstack-galera ---- -apiVersion: v1 -kind: ConfigMap -metadata: - name: openstack-config-data ---- -apiVersion: ovn.openstack.org/v1beta1 -kind: OVNNorthd -metadata: - finalizers: - - openstack.org/ovnnorthd - name: ovnnorthd-sample ---- -apiVersion: ovn.openstack.org/v1beta1 -kind: OVNDBCluster -metadata: - finalizers: - - openstack.org/ovndbcluster - name: ovndbcluster-nb-sample ---- -apiVersion: ovn.openstack.org/v1beta1 -kind: OVNDBCluster -metadata: - finalizers: - - openstack.org/ovndbcluster - name: ovndbcluster-sb-sample ---- -apiVersion: ovn.openstack.org/v1beta1 -kind: OVNController -metadata: - finalizers: - - openstack.org/ovncontroller - name: ovncontroller-sample ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: ovn-northd ---- -apiVersion: apps/v1 -kind: DaemonSet -metadata: - name: ovncontroller ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - service: ovn-northd ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - service: ovsdbserver-nb - name: ovsdbserver-nb-0 ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - service: ovsdbserver-sb - name: ovsdbserver-sb-0 ---- -apiVersion: v1 -kind: Pod -metadata: - annotations: - openshift.io/scc: privileged - generateName: ovn-controller- - labels: - service: ovn-controller ---- -apiVersion: v1 -kind: Service -metadata: - labels: - service: ovsdbserver-nb - name: ovsdbserver-nb ---- -apiVersion: v1 -kind: Service -metadata: - labels: - service: ovsdbserver-nb - statefulset.kubernetes.io/pod-name: ovsdbserver-nb-0 - name: ovsdbserver-nb-0 ---- -apiVersion: v1 -kind: Service -metadata: - labels: - service: ovsdbserver-sb - name: ovsdbserver-sb ---- -apiVersion: v1 -kind: Service -metadata: - labels: - service: ovsdbserver-sb - statefulset.kubernetes.io/pod-name: ovsdbserver-sb-0 - name: ovsdbserver-sb-0 ---- -apiVersion: v1 -kind: Pod -metadata: - name: nova-api-0 ---- -apiVersion: v1 -kind: Pod -metadata: - name: nova-cell1-novncproxy-0 ---- -apiVersion: v1 -kind: Pod -metadata: - name: nova-metadata-0 ---- -apiVersion: v1 -kind: Pod -metadata: - labels: - app.kubernetes.io/name: SwiftProxy ---- -apiVersion: v1 -kind: Pod -metadata: - name: swift-storage-0 ---- -apiVersion: v1 -kind: Pod -metadata: - annotations: - openshift.io/scc: anyuid - labels: - service: neutron ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: cinder-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: glance-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: barbican-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: keystone-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: neutron-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: nova-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: nova-novncproxy-cell1-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: placement-public ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: swift-public ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: barbican-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: cinder-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: barbican-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: cinder-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: glance-default-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: glance-default-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: keystone-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: keystone-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: neutron-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: neutron-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: nova-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: nova-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: nova-novncproxy-cell1-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: nova-novncproxy-cell1-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: placement-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: placement-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: swift-public-route ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: swift-public-svc ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: rootca-internal ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: rootca-libvirt ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: rootca-ovn ---- -apiVersion: cert-manager.io/v1 -kind: Certificate -metadata: - name: rootca-public