From cbbc2ef8fed09d67510eeda411e93ae1e4d4a7ef Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Fri, 27 Sep 2024 11:27:35 +0200 Subject: [PATCH] Reapply "[ovn-controller] Don't create ovn-controller if nicMappings empty" This reverts commit ba9d53e572e55f3f342a270d9cd480376ec2b1b8. This commit was reverted the first time as it broke the adoption gates since a config map needed by edpm deployment was not being generated by ovn-controller. This is fixed on ovn-operator PR#352, hence this commit can be reverted. Depends-on: https://github.com/openstack-k8s-operators/ovn-operator/pull/352 --- .../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, 469 insertions(+), 1 deletion(-) create mode 100644 config/samples/nad_datacentre.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml create mode 120000 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml create mode 100644 tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml create 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 f2e59375d..d738a67a1 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 new file mode 100644 index 000000000..cab45e980 --- /dev/null +++ b/config/samples/nad_datacentre.yaml @@ -0,0 +1,17 @@ +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 77c5fc279..25997af7a 100644 --- a/pkg/openstack/ovn.go +++ b/pkg/openstack/ovn.go @@ -314,6 +314,13 @@ 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 6bad34358..166e657a3 100644 --- a/tests/functional/ctlplane/openstackoperator_controller_test.go +++ b/tests/functional/ctlplane/openstackoperator_controller_test.go @@ -1620,6 +1620,11 @@ var _ = Describe("OpenStackOperator controller", func() { "dbType": "SB", }, }, + "ovnController": map[string]interface{}{ + "nicMappings": map[string]interface{}{ + "datacentre": "ospbr", + }, + }, }, } DeferCleanup( @@ -1670,6 +1675,37 @@ 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 5536b881c..1b5545554 100644 --- a/tests/functional/ctlplane/openstackversion_controller_test.go +++ b/tests/functional/ctlplane/openstackversion_controller_test.go @@ -239,6 +239,11 @@ 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 new file mode 100644 index 000000000..aef2ee26f --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/01-create-nic-mappings.yaml @@ -0,0 +1,5 @@ +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 new file mode 120000 index 000000000..762a8cf31 --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-assert-deploy-openstack.yaml @@ -0,0 +1 @@ +../../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 new file mode 100644 index 000000000..6c9d0887d --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/02-deploy-openstack.yaml @@ -0,0 +1,5 @@ +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 new file mode 100644 index 000000000..ae2e618e9 --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-add-ovn-nic-mappings.yaml @@ -0,0 +1,10 @@ +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 new file mode 100644 index 000000000..b15044e0f --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/03-assert.yaml @@ -0,0 +1,15 @@ +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 new file mode 100644 index 000000000..93835505e --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-assert.yaml @@ -0,0 +1,11 @@ +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 new file mode 100644 index 000000000..49882f502 --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/04-remove-ovn-nic-mappings.yaml @@ -0,0 +1,9 @@ +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 new file mode 100644 index 000000000..6b4992512 --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-cleanup.yaml @@ -0,0 +1,13 @@ +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 new file mode 100644 index 000000000..8b9f6e6f9 --- /dev/null +++ b/tests/kuttl/tests/ctlplane-basic-deployment-with-nicMappings/05-errors-cleanup.yaml @@ -0,0 +1,334 @@ +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