Skip to content

Commit 223f7d6

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #233 from fmount/topology_envtest
Improve topology testing coverage
2 parents 1b72b65 + 252ec44 commit 223f7d6

File tree

2 files changed

+247
-37
lines changed

2 files changed

+247
-37
lines changed

‎tests/functional/barbican_controller_test.go‎

Lines changed: 204 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
barbicanv1beta1 "github.com/openstack-k8s-operators/barbican-operator/api/v1beta1"
1515
controllers "github.com/openstack-k8s-operators/barbican-operator/controllers"
1616
"github.com/openstack-k8s-operators/barbican-operator/pkg/barbican"
17+
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
1718
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
1819
mariadb_test "github.com/openstack-k8s-operators/mariadb-operator/api/test/helpers"
1920
corev1 "k8s.io/api/core/v1"
@@ -397,16 +398,37 @@ var _ = Describe("Barbican controller", func() {
397398
})
398399

399400
When("Barbican is created with topologyRef", func() {
401+
var topologyRef, topologyRefAlt *topologyv1.TopoRef
400402
BeforeEach(func() {
401-
// Build the topology Spec
402-
topologySpec := GetSampleTopologySpec()
403-
// Create Test Topologies
403+
// Create Test Topologies based on the entries defined in the
404+
// BarbicanTopologies array. The spec is generated using GetSampleTopologySpec
405+
// function, and t.Name is used as label to customize the default
406+
// selector. By doing this we are able to have well defined and distinct
407+
// topologies that are applied to BarbicanAPI, BarbicanKeystoneListener
408+
// and BarbicanWorker.
404409
for _, t := range barbicanTest.BarbicanTopologies {
410+
// Build the topology Spec
411+
topologySpec, _ := GetSampleTopologySpec(t.Name)
405412
infra.CreateTopology(t, topologySpec)
406413
}
414+
415+
// Define the two topology references used in this test: topologyRef
416+
// represents the global-topology, while topologyRefAlt represents
417+
// an alternative topologyRef that is used by the "update" envTest
418+
// to override the global topology referenced here.
419+
topologyRef = &topologyv1.TopoRef{
420+
Name: barbicanTest.BarbicanTopologies[0].Name,
421+
Namespace: barbicanTest.BarbicanTopologies[0].Namespace,
422+
}
423+
topologyRefAlt = &topologyv1.TopoRef{
424+
Name: barbicanTest.BarbicanTopologies[1].Name,
425+
Namespace: barbicanTest.BarbicanTopologies[1].Namespace,
426+
}
407427
spec := GetDefaultBarbicanSpec()
428+
// Update the spec and add a top-level topologyRef that points to
429+
// gloabal-topology
408430
spec["topologyRef"] = map[string]interface{}{
409-
"name": barbicanTest.BarbicanTopologies[0].Name,
431+
"name": topologyRef.Name,
410432
}
411433
DeferCleanup(k8sClient.Delete, ctx, CreateBarbicanMessageBusSecret(barbicanTest.Instance.Namespace, "rabbitmq-secret"))
412434
DeferCleanup(th.DeleteInstance, CreateBarbican(barbicanTest.Instance, spec))
@@ -429,79 +451,225 @@ var _ = Describe("Barbican controller", func() {
429451
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
430452
})
431453
It("sets topology in CR status", func() {
454+
// expectedTopology points to topologyRef, the global-topology
455+
// defined in BeforeEach
456+
expectedTopology := &topologyv1.TopoRef{
457+
Name: topologyRef.Name,
458+
Namespace: topologyRef.Namespace,
459+
}
432460
Eventually(func(g Gomega) {
461+
// retrieve the created topology in the current namespace
462+
tp := infra.GetTopology(types.NamespacedName{
463+
Name: expectedTopology.Name,
464+
Namespace: expectedTopology.Namespace,
465+
})
466+
// We expect that finalizer has length eq to 3, because the same
467+
// global-topology is applied at top-level and inherited by all
468+
// the barbican subCRs
469+
g.Expect(tp.GetFinalizers()).To(HaveLen(3))
470+
finalizers := tp.GetFinalizers()
433471
barbicanAPI := GetBarbicanAPI(barbicanTest.BarbicanAPI)
434472
g.Expect(barbicanAPI.Status.LastAppliedTopology).ToNot(BeNil())
435-
g.Expect(barbicanAPI.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[0].Name))
473+
// verify the .Status.LastAppliedTopology is updated and points
474+
// to the referenced topologyRef
475+
g.Expect(barbicanAPI.Status.LastAppliedTopology).To(Equal(topologyRef))
476+
// verify the finalizer has been applied
477+
g.Expect(finalizers).To(ContainElement(
478+
fmt.Sprintf("openstack.org/barbicanapi-%s", barbicanAPI.Name)))
479+
436480
barbicanWorker := GetBarbicanWorker(barbicanTest.BarbicanWorker)
437481
g.Expect(barbicanWorker.Status.LastAppliedTopology).ToNot(BeNil())
438-
g.Expect(barbicanWorker.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[0].Name))
482+
// verify the .Status.LastAppliedTopology is updated and points
483+
// to the referenced topologyRef
484+
g.Expect(barbicanWorker.Status.LastAppliedTopology).To(Equal(topologyRef))
485+
// verify the finalizer has been applied
486+
g.Expect(finalizers).To(ContainElement(
487+
fmt.Sprintf("openstack.org/barbicanworker-%s", barbicanWorker.Name)))
488+
439489
barbicanKeystoneListener := GetBarbicanKeystoneListener(barbicanTest.BarbicanKeystoneListener)
440490
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).ToNot(BeNil())
441-
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[0].Name))
491+
// verify the .Status.LastAppliedTopology is updated and points
492+
// to the referenced topologyRef
493+
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).To(Equal(topologyRef))
494+
// verify the finalizer has been applied
495+
g.Expect(finalizers).To(ContainElement(
496+
fmt.Sprintf("openstack.org/barbicankeystonelistener-%s", barbicanKeystoneListener.Name)))
442497
}, timeout, interval).Should(Succeed())
443498
})
444499

445500
It("sets topology in the resulting deployments", func() {
446501
Eventually(func(g Gomega) {
447-
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
502+
// retrieve the topologySpec associated to the currently referenced
503+
// topology
504+
_, expectedTopologySpecObj := GetSampleTopologySpec(topologyRef.Name)
505+
// verify that TopologySpreadConstraints is not nil (it proves that the Topology has been injected in the PodSpec)
506+
// and that is matches with what has been referenced at top-level and inherited by the current service. The same
507+
// comment applies to API, Worker and KeystoneListener.
508+
// NOTE: this tests also the logic defined in the deployment definition, where Affinity is not set by default when
509+
// a Topology is referenced. By checking that .Spec.Affinity is nil, we prove that we get the expected result
448510
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.Affinity).To(BeNil())
511+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
512+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
449513
g.Expect(th.GetDeployment(barbicanTest.BarbicanWorkerDeployment).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
514+
g.Expect(th.GetDeployment(barbicanTest.BarbicanWorkerDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
450515
g.Expect(th.GetDeployment(barbicanTest.BarbicanWorkerDeployment).Spec.Template.Spec.Affinity).To(BeNil())
451516
g.Expect(th.GetDeployment(barbicanTest.BarbicanKeystoneListenerDeployment).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
452517
g.Expect(th.GetDeployment(barbicanTest.BarbicanKeystoneListenerDeployment).Spec.Template.Spec.Affinity).To(BeNil())
518+
g.Expect(th.GetDeployment(barbicanTest.BarbicanKeystoneListenerDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
453519
}, timeout, interval).Should(Succeed())
454520
})
455521
It("updates topology when the reference changes", func() {
522+
// expectedTopology points to topologyRefAlt, the alternative topology
523+
// that is defined in BeforeEach and is suppposed to override the existing
524+
// referenced topology. This test ensures that we are able to properly
525+
// perform day2 operations and manage the Topology lifecycle.
526+
expectedTopology := &topologyv1.TopoRef{
527+
Name: topologyRefAlt.Name,
528+
Namespace: topologyRefAlt.Namespace,
529+
}
530+
var finalizers []string
456531
Eventually(func(g Gomega) {
457532
barbican := GetBarbican(barbicanTest.Instance)
458-
barbican.Spec.TopologyRef.Name = barbicanTest.BarbicanTopologies[1].Name
533+
// Update the top-level TopologyRef and point to the alternative
534+
// Topology that will be propagated to the underlying components
535+
barbican.Spec.TopologyRef.Name = topologyRefAlt.Name
459536
g.Expect(k8sClient.Update(ctx, barbican)).To(Succeed())
460537
}, timeout, interval).Should(Succeed())
461538

462539
Eventually(func(g Gomega) {
540+
tp := infra.GetTopology(types.NamespacedName{
541+
Name: expectedTopology.Name,
542+
Namespace: expectedTopology.Namespace,
543+
})
544+
finalizers = tp.GetFinalizers()
545+
// We expect that finalizer has length eq to 3, because the same
546+
// alternative topology is applied at top-level and inherited by
547+
// all the barbican subCRs
548+
g.Expect(finalizers).To(HaveLen(3))
463549
keystone.SimulateKeystoneEndpointReady(barbicanTest.BarbicanKeystoneEndpoint)
464550
barbicanAPI := GetBarbicanAPI(barbicanTest.BarbicanAPI)
465551
g.Expect(barbicanAPI.Status.LastAppliedTopology).ToNot(BeNil())
466-
g.Expect(barbicanAPI.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[1].Name))
552+
g.Expect(barbicanAPI.Status.LastAppliedTopology).To(Equal(expectedTopology))
553+
// verify the finalizer has been applied to the alternative topology
554+
g.Expect(finalizers).To(ContainElement(
555+
fmt.Sprintf("openstack.org/barbicanapi-%s", barbicanAPI.Name)))
467556
barbicanWorker := GetBarbicanWorker(barbicanTest.BarbicanWorker)
468557
g.Expect(barbicanWorker.Status.LastAppliedTopology).ToNot(BeNil())
469-
g.Expect(barbicanWorker.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[1].Name))
558+
g.Expect(barbicanWorker.Status.LastAppliedTopology).To(Equal(expectedTopology))
559+
// verify the finalizer has been applied to the alternative topology
560+
g.Expect(finalizers).To(ContainElement(
561+
fmt.Sprintf("openstack.org/barbicanworker-%s", barbicanWorker.Name)))
470562
barbicanKeystoneListener := GetBarbicanKeystoneListener(barbicanTest.BarbicanKeystoneListener)
471563
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).ToNot(BeNil())
472-
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[1].Name))
564+
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).To(Equal(expectedTopology))
565+
// verify the finalizer has been applied to the alternative topology
566+
g.Expect(finalizers).To(ContainElement(
567+
fmt.Sprintf("openstack.org/barbicankeystonelistener-%s", barbicanKeystoneListener.Name)))
568+
}, timeout, interval).Should(Succeed())
569+
})
570+
571+
It("verifies the previous topology reference has no finalizers anymore", func() {
572+
Eventually(func(g Gomega) {
573+
// Get the previous (global) topology and verify that the finalizers
574+
// have been removed
575+
tp := infra.GetTopology(types.NamespacedName{
576+
Name: topologyRef.Name,
577+
Namespace: topologyRef.Namespace,
578+
})
579+
g.Expect(tp.GetFinalizers()).To(BeEmpty())
473580
}, timeout, interval).Should(Succeed())
474581
})
475582

476583
It("overrides topology when the reference changes", func() {
584+
// Each sub-component defines its own topologyRef: with this test
585+
// we prove that they do not inherit the top-level topology anymore,
586+
// but they honor the reference that has been passed
477587
Eventually(func(g Gomega) {
478588
barbican := GetBarbican(barbicanTest.Instance)
479589
//Patch BarbicanAPI Spec
480590
newAPI := GetBarbicanAPISpec(barbicanTest.BarbicanAPI)
591+
// update barbicanAPI.spec.topologyRef to point to api-topology
481592
newAPI.TopologyRef.Name = barbicanTest.BarbicanTopologies[1].Name
482593
barbican.Spec.BarbicanAPI = newAPI
483594
//Patch BarbicanKeystoneListener Spec
484595
newKl := GetBarbicanKeystoneListenerSpec(barbicanTest.BarbicanKeystoneListener)
596+
// update barbicanKeystoneListener.spec.topologyRef to point to
597+
// klistener-topology
485598
newKl.TopologyRef.Name = barbicanTest.BarbicanTopologies[2].Name
486599
barbican.Spec.BarbicanKeystoneListener = newKl
487600
//Patch BarbicanWorker Spec
488601
newWorker := GetBarbicanWorkerSpec(barbicanTest.BarbicanWorker)
602+
// update worker.spec.TopologyRef to point to worker-topology
489603
newWorker.TopologyRef.Name = barbicanTest.BarbicanTopologies[3].Name
490604
barbican.Spec.BarbicanWorker = newWorker
491605
g.Expect(k8sClient.Update(ctx, barbican)).To(Succeed())
492606
}, timeout, interval).Should(Succeed())
493607

608+
// For each subcomponent, as we did in the previous test, verify
609+
// that the topologyRef passed at the sub-level has been applied,
610+
// no inheritance happened
494611
Eventually(func(g Gomega) {
495-
612+
expectedTopology := &topologyv1.TopoRef{
613+
Name: barbicanTest.BarbicanTopologies[1].Name,
614+
Namespace: barbicanTest.BarbicanTopologies[1].Namespace,
615+
}
616+
// retrieve the referenced topology using the k8s client
617+
tp := infra.GetTopology(types.NamespacedName{
618+
Name: expectedTopology.Name,
619+
Namespace: expectedTopology.Namespace,
620+
})
621+
finalizers := tp.GetFinalizers()
622+
// api-topology has BarbicanAPI finalizer only, hence we expect
623+
// length to be 1
624+
g.Expect(finalizers).To(HaveLen(1))
496625
barbicanAPI := GetBarbicanAPI(barbicanTest.BarbicanAPI)
497626
g.Expect(barbicanAPI.Status.LastAppliedTopology).ToNot(BeNil())
498-
g.Expect(barbicanAPI.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[1].Name))
627+
g.Expect(barbicanAPI.Status.LastAppliedTopology).To(Equal(expectedTopology))
628+
// verify the finalizer has been applied
629+
g.Expect(finalizers).To(ContainElement(
630+
fmt.Sprintf("openstack.org/barbicanapi-%s", barbicanAPI.Name)))
631+
}, timeout, interval).Should(Succeed())
632+
633+
Eventually(func(g Gomega) {
634+
expectedTopology := &topologyv1.TopoRef{
635+
Name: barbicanTest.BarbicanTopologies[2].Name,
636+
Namespace: barbicanTest.BarbicanTopologies[2].Namespace,
637+
}
638+
tp := infra.GetTopology(types.NamespacedName{
639+
Name: expectedTopology.Name,
640+
Namespace: expectedTopology.Namespace,
641+
})
642+
finalizers := tp.GetFinalizers()
643+
// klistener-topology has BarbicanKeystoneListener finalizer
644+
// only, hence we expect length to be 1
645+
g.Expect(finalizers).To(HaveLen(1))
499646
barbicanKeystoneListener := GetBarbicanKeystoneListener(barbicanTest.BarbicanKeystoneListener)
500647
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).ToNot(BeNil())
501-
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[2].Name))
648+
g.Expect(barbicanKeystoneListener.Status.LastAppliedTopology).To(Equal(expectedTopology))
649+
// verify the finalizer has been applied
650+
g.Expect(finalizers).To(ContainElement(
651+
fmt.Sprintf("openstack.org/barbicankeystonelistener-%s", barbicanKeystoneListener.Name)))
652+
}, timeout, interval).Should(Succeed())
653+
654+
Eventually(func(g Gomega) {
655+
expectedTopology := &topologyv1.TopoRef{
656+
Name: barbicanTest.BarbicanTopologies[3].Name,
657+
Namespace: barbicanTest.BarbicanTopologies[3].Namespace,
658+
}
659+
tp := infra.GetTopology(types.NamespacedName{
660+
Name: expectedTopology.Name,
661+
Namespace: expectedTopology.Namespace,
662+
})
663+
finalizers := tp.GetFinalizers()
664+
// worker-topology has BarbicanWorker finalizer
665+
// only, hence we expect length to be 1
666+
g.Expect(finalizers).To(HaveLen(1))
502667
barbicanWorker := GetBarbicanWorker(barbicanTest.BarbicanWorker)
503668
g.Expect(barbicanWorker.Status.LastAppliedTopology).ToNot(BeNil())
504-
g.Expect(barbicanWorker.Status.LastAppliedTopology.Name).To(Equal(barbicanTest.BarbicanTopologies[3].Name))
669+
g.Expect(barbicanWorker.Status.LastAppliedTopology).To(Equal(expectedTopology))
670+
// verify the finalizer has been applied
671+
g.Expect(finalizers).To(ContainElement(
672+
fmt.Sprintf("openstack.org/barbicanworker-%s", barbicanWorker.Name)))
505673
}, timeout, interval).Should(Succeed())
506674
})
507675
It("removes topologyRef from the spec", func() {
@@ -513,6 +681,9 @@ var _ = Describe("Barbican controller", func() {
513681
}, timeout, interval).Should(Succeed())
514682

515683
Eventually(func(g Gomega) {
684+
// When TopologyRef is removed, LastAppliedTopology topology
685+
// status field is nil, and not present anymore in the yaml
686+
// definition
516687
barbicanAPI := GetBarbicanAPI(barbicanTest.BarbicanAPI)
517688
g.Expect(barbicanAPI.Status.LastAppliedTopology).Should(BeNil())
518689
barbicanWorker := GetBarbicanWorker(barbicanTest.BarbicanWorker)
@@ -522,13 +693,30 @@ var _ = Describe("Barbican controller", func() {
522693
}, timeout, interval).Should(Succeed())
523694

524695
Eventually(func(g Gomega) {
696+
// When TopologyRef is removed, LastAppliedTopology topology
697+
// status field is nil, and the resulting deployment has no
698+
// TopologySpreadConstraints. In this case the default AntiAffinity
699+
// using the function provided by lib-common is applied
525700
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(BeNil())
526701
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPIDeployment).Spec.Template.Spec.Affinity).ToNot(BeNil())
527702
g.Expect(th.GetDeployment(barbicanTest.BarbicanWorkerDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(BeNil())
528703
g.Expect(th.GetDeployment(barbicanTest.BarbicanWorkerDeployment).Spec.Template.Spec.Affinity).ToNot(BeNil())
529704
g.Expect(th.GetDeployment(barbicanTest.BarbicanKeystoneListenerDeployment).Spec.Template.Spec.TopologySpreadConstraints).To(BeNil())
530705
g.Expect(th.GetDeployment(barbicanTest.BarbicanKeystoneListenerDeployment).Spec.Template.Spec.Affinity).ToNot(BeNil())
531706
}, timeout, interval).Should(Succeed())
707+
708+
// Verify that all the finalizers in all the existing Topologies
709+
// have been removed
710+
Eventually(func(g Gomega) {
711+
for _, topology := range barbicanTest.BarbicanTopologies {
712+
// Get the current topology and verify there are no finalizers
713+
tp := infra.GetTopology(types.NamespacedName{
714+
Name: topology.Name,
715+
Namespace: topology.Namespace,
716+
})
717+
g.Expect(tp.GetFinalizers()).To(BeEmpty())
718+
}
719+
}, timeout, interval).Should(Succeed())
532720
})
533721
})
534722

0 commit comments

Comments
 (0)