Merge pull request #557 from fmount/topology_envtest

Improve topology testing coverage

Author: openshift-merge-bot[bot]

tests/functional/base_test.go

@@ -26,9 +26,11 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
 	keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
 	keystone_base "github.com/openstack-k8s-operators/keystone-operator/pkg/keystone"
 	condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func GetKeystoneAPISpec(fernetMaxKeys int32) map[string]interface{} {
@@ -121,14 +123,14 @@ func CreateKeystoneMessageBusSecret(namespace string, name string) *corev1.Secre
 }
 
 // GetSampleTopologySpec - A sample (and opinionated) Topology Spec used to
-// test Keystone
+// test KeystoneAPI
 // Note this is just an example that should not be used in production for
 // multiple reasons:
 // 1. It uses ScheduleAnyway as strategy, which is something we might
 // want to avoid by default
 // 2. Usually a topologySpreadConstraints is used to take care about
 // multi AZ, which is not applicable in this context
-func GetSampleTopologySpec() map[string]interface{} {
+func GetSampleTopologySpec(label string) (map[string]interface{}, []corev1.TopologySpreadConstraint) {
 	// Build the topology Spec
 	topologySpec := map[string]interface{}{
 		"topologySpreadConstraints": []map[string]interface{}{
@@ -138,17 +140,35 @@ func GetSampleTopologySpec() map[string]interface{} {
 				"whenUnsatisfiable": "ScheduleAnyway",
 				"labelSelector": map[string]interface{}{
 					"matchLabels": map[string]interface{}{
-						"service": keystone_base.ServiceName,
+						"service":   keystone_base.ServiceName,
+						"component": label,
 					},
 				},
 			},
 		},
 	}
-	return topologySpec
+	// Build the topologyObj representation
+	topologySpecObj := []corev1.TopologySpreadConstraint{
+		{
+			MaxSkew:           1,
+			TopologyKey:       corev1.LabelHostname,
+			WhenUnsatisfiable: corev1.ScheduleAnyway,
+			LabelSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"service":   keystone_base.ServiceName,
+					"component": label,
+				},
+			},
+		},
+	}
+	return topologySpec, topologySpecObj
 }
 
 // CreateTopology - Creates a Topology CR based on the spec passed as input
-func CreateTopology(topology types.NamespacedName, spec map[string]interface{}) client.Object {
+func CreateTopology(
+	topology types.NamespacedName,
+	spec map[string]interface{},
+) (client.Object, topologyv1.TopoRef) {
 	raw := map[string]interface{}{
 		"apiVersion": "topology.openstack.org/v1beta1",
 		"kind":       "Topology",
@@ -158,5 +178,20 @@ func CreateTopology(topology types.NamespacedName, spec map[string]interface{})
 		},
 		"spec": spec,
 	}
-	return th.CreateUnstructured(raw)
+	// other than creating the topology based on the raw spec, we return the
+	// TopoRef that can be referenced
+	topologyRef := topologyv1.TopoRef{
+		Name:      topology.Name,
+		Namespace: topology.Namespace,
+	}
+	return th.CreateUnstructured(raw), topologyRef
+}
+
+// GetTopology - Returns the referenced Topology
+func GetTopology(name types.NamespacedName) *topologyv1.Topology {
+	instance := &topologyv1.Topology{}
+	Eventually(func(g Gomega) {
+		g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed())
+	}, timeout, interval).Should(Succeed())
+	return instance
 }

tests/functional/keystoneapi_controller_test.go

@@ -31,6 +31,7 @@ import (
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
 
 	memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
+	topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
 	condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	mariadb_test "github.com/openstack-k8s-operators/mariadb-operator/api/test/helpers"
 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
@@ -1454,16 +1455,28 @@ var _ = Describe("Keystone controller", func() {
 	})
 
 	When("Topology is referenced", func() {
+		var topologyRef, topologyRefAlt *topologyv1.TopoRef
 		BeforeEach(func() {
-			// Build the topology Spec
-			topologySpec := GetSampleTopologySpec()
+
+			// Define the two topology references used in this test
+			topologyRef = &topologyv1.TopoRef{
+				Name:      keystoneAPITopologies[0].Name,
+				Namespace: keystoneAPITopologies[0].Namespace,
+			}
+			topologyRefAlt = &topologyv1.TopoRef{
+				Name:      keystoneAPITopologies[1].Name,
+				Namespace: keystoneAPITopologies[1].Namespace,
+			}
+
 			// Create Test Topologies
 			for _, t := range keystoneAPITopologies {
+				// Build the topology Spec
+				topologySpec, _ := GetSampleTopologySpec(t.Name)
 				CreateTopology(t, topologySpec)
 			}
 			spec := GetDefaultKeystoneAPISpec()
 			spec["topologyRef"] = map[string]interface{}{
-				"name": keystoneAPITopologies[0].Name,
+				"name": topologyRef.Name,
 			}
 			DeferCleanup(
 				k8sClient.Delete, ctx, CreateKeystoneMessageBusSecret(namespace, "rabbitmq-secret"))
@@ -1499,28 +1512,55 @@ var _ = Describe("Keystone controller", func() {
 
 		It("check topology has been applied", func() {
 			Eventually(func(g Gomega) {
+				tp := GetTopology(types.NamespacedName{
+					Name:      topologyRef.Name,
+					Namespace: topologyRef.Namespace,
+				})
+				finalizers := tp.GetFinalizers()
+				g.Expect(finalizers).To(HaveLen(1))
 				keystoneAPI := GetKeystoneAPI(keystoneAPIName)
 				g.Expect(keystoneAPI.Status.LastAppliedTopology).ToNot(BeNil())
-				g.Expect(keystoneAPI.Status.LastAppliedTopology.Name).To(Equal(keystoneAPITopologies[0].Name))
+				g.Expect(keystoneAPI.Status.LastAppliedTopology).To(Equal(topologyRef))
+				g.Expect(finalizers).To(ContainElement(
+					fmt.Sprintf("openstack.org/keystoneapi-%s", keystoneAPIName.Name)))
 			}, timeout, interval).Should(Succeed())
 		})
 		It("sets topology in resource specs", func() {
 			Eventually(func(g Gomega) {
-				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
+				_, topologySpecObj := GetSampleTopologySpec(topologyRef.Name)
 				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.Affinity).To(BeNil())
+				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
+				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(topologySpecObj))
 			}, timeout, interval).Should(Succeed())
 		})
 		It("updates topology when the reference changes", func() {
 			Eventually(func(g Gomega) {
 				keystoneAPI := GetKeystoneAPI(keystoneAPIName)
-				keystoneAPI.Spec.TopologyRef.Name = keystoneAPITopologies[1].Name
+				keystoneAPI.Spec.TopologyRef.Name = topologyRefAlt.Name
 				g.Expect(k8sClient.Update(ctx, keystoneAPI)).To(Succeed())
 			}, timeout, interval).Should(Succeed())
 
 			Eventually(func(g Gomega) {
+				tp := GetTopology(types.NamespacedName{
+					Name:      topologyRefAlt.Name,
+					Namespace: topologyRefAlt.Namespace,
+				})
+				finalizers := tp.GetFinalizers()
+				g.Expect(finalizers).To(HaveLen(1))
+
 				keystoneAPI := GetKeystoneAPI(keystoneAPIName)
 				g.Expect(keystoneAPI.Status.LastAppliedTopology).ToNot(BeNil())
-				g.Expect(keystoneAPI.Status.LastAppliedTopology.Name).To(Equal(keystoneAPITopologies[1].Name))
+				g.Expect(keystoneAPI.Status.LastAppliedTopology).To(Equal(topologyRefAlt))
+				g.Expect(finalizers).To(ContainElement(
+					fmt.Sprintf("openstack.org/keystoneapi-%s", keystoneAPIName.Name)))
+
+				// Verify the previous referenced topology has no finalizers
+				tp = GetTopology(types.NamespacedName{
+					Name:      topologyRef.Name,
+					Namespace: topologyRef.Namespace,
+				})
+				finalizers = tp.GetFinalizers()
+				g.Expect(finalizers).To(BeEmpty())
 			}, timeout, interval).Should(Succeed())
 		})
 		It("removes topologyRef from the spec", func() {
@@ -1540,6 +1580,18 @@ var _ = Describe("Keystone controller", func() {
 				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.TopologySpreadConstraints).To(BeNil())
 				g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.Affinity).ToNot(BeNil())
 			}, timeout, interval).Should(Succeed())
+
+			// Verify the existing topologies have no finalizer anymore
+			Eventually(func(g Gomega) {
+				for _, topology := range keystoneAPITopologies {
+					tp := GetTopology(types.NamespacedName{
+						Name:      topology.Name,
+						Namespace: topology.Namespace,
+					})
+					finalizers := tp.GetFinalizers()
+					g.Expect(finalizers).To(BeEmpty())
+				}
+			}, timeout, interval).Should(Succeed())
 		})
 	})