diff --git a/Makefile b/Makefile index 6dfdf12d..614d0107 100644 --- a/Makefile +++ b/Makefile @@ -54,6 +54,7 @@ export KUBEBUILDER_ASSETS = $(LOCAL_TESTBIN)/k8s/$(ENVTEST_K8S_VERSION)-$(platfo .PHONY: kubebuilder-assets kubebuilder-assets: $(KUBEBUILDER_ASSETS) + @echo "export KUBEBUILDER_ASSETS = $(LOCAL_TESTBIN)/k8s/$(ENVTEST_K8S_VERSION)-$(platform)-$(ARCHITECTURE)" $(KUBEBUILDER_ASSETS): setup-envtest --os $(platform) --arch $(ARCHITECTURE) --bin-dir $(LOCAL_TESTBIN) use $(ENVTEST_K8S_VERSION) @@ -73,6 +74,8 @@ $(YTT): | $(LOCAL_BIN) ############## #### Tests ### ############## +GINKGO := go run github.com/onsi/ginkgo/v2/ginkgo + .PHONY: unit-tests unit-tests::install-tools ## Run unit tests unit-tests::$(KUBEBUILDER_ASSETS) @@ -84,7 +87,7 @@ unit-tests::just-unit-tests .PHONY: just-unit-tests just-unit-tests: - ginkgo -r --randomize-all api/ internal/ rabbitmqclient/ + $(GINKGO) -r --randomize-all api/ internal/ rabbitmqclient/ .PHONY: integration-tests integration-tests::install-tools ## Run integration tests. Use GINKGO_EXTRA="-some-arg" to append arguments to 'ginkgo run' @@ -96,14 +99,14 @@ integration-tests::manifests integration-tests::just-integration-tests just-integration-tests: $(KUBEBUILDER_ASSETS) - ginkgo --randomize-all -r -p $(GINKGO_EXTRA) controllers/ + $(GINKGO) --randomize-all -r -p $(GINKGO_EXTRA) controllers/ .PHONY: local-tests local-tests: unit-tests integration-tests ## Run all local tests (unit & integration) .PHONY: system-tests system-tests: ## Run E2E tests using current context in ~/.kube/config. Expects cluster operator and topology operator to be installed in the cluster - NAMESPACE="rabbitmq-system" ginkgo --randomize-all -r $(GINKGO_EXTRA) system_tests/ + NAMESPACE="rabbitmq-system" $(GINKGO) --randomize-all -r $(GINKGO_EXTRA) system_tests/ ################### diff --git a/controllers/binding_controller_test.go b/controllers/binding_controller_test.go index d2c993b6..9b8af892 100644 --- a/controllers/binding_controller_test.go +++ b/controllers/binding_controller_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/controllers" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -34,14 +37,28 @@ var _ = Describe("bindingController", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error bindingMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{bindingNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{bindingNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -66,21 +83,9 @@ var _ = Describe("bindingController", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.BindingReconciler{}, }).SetupWithManager(bindingMgr)).To(Succeed()) - }) + } - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) - - JustBeforeEach(func() { + initialiseBinding := func() { binding = topology.Binding{ ObjectMeta: metav1.ObjectMeta{ Name: bindingName, @@ -92,6 +97,18 @@ var _ = Describe("bindingController", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) When("creating a binding", func() { @@ -106,6 +123,9 @@ var _ = Describe("bindingController", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("some HTTP error")) + initialiseBinding() + binding.Labels = map[string]string{"test": "test-binding-http-error"} + initialiseManager("test", "test-binding-http-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -131,6 +151,9 @@ var _ = Describe("bindingController", func() { BeforeEach(func() { bindingName = "test-binding-go-error" fakeRabbitMQClient.DeclareBindingReturns(nil, errors.New("hit a exception")) + initialiseBinding() + binding.Labels = map[string]string{"test": "test-binding-go-error"} + initialiseManager("test", "test-binding-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -155,6 +178,8 @@ var _ = Describe("bindingController", func() { When("Deleting a binding", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.DeclareBindingReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -183,6 +208,9 @@ var _ = Describe("bindingController", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseBinding() + binding.Labels = map[string]string{"test": "delete-binding-http-error"} + initialiseManager("test", "delete-binding-http-error") }) It("raises an event to indicate a failure to delete", func() { @@ -199,6 +227,9 @@ var _ = Describe("bindingController", func() { BeforeEach(func() { bindingName = "delete-binding-go-error" fakeRabbitMQClient.DeleteBindingReturns(nil, errors.New("some error")) + initialiseBinding() + binding.Labels = map[string]string{"test": "delete-binding-go-error"} + initialiseManager("test", "delete-binding-go-error") }) It("raises an event to indicate a failure to delete", func() { diff --git a/controllers/exchange_controller_test.go b/controllers/exchange_controller_test.go index 1d9ea72a..d2126299 100644 --- a/controllers/exchange_controller_test.go +++ b/controllers/exchange_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -35,14 +38,28 @@ var _ = Describe("exchange-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error exchangeMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{exchangeNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{exchangeNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -67,22 +84,9 @@ var _ = Describe("exchange-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.ExchangeReconciler{}, }).SetupWithManager(exchangeMgr)).To(Succeed()) - }) + } - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) - - JustBeforeEach(func() { - // this will be executed after all BeforeEach have run + initialiseExchange := func() { exchange = topology.Exchange{ ObjectMeta: metav1.ObjectMeta{ Name: exchangeName, @@ -94,6 +98,18 @@ var _ = Describe("exchange-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) Context("creation", func() { @@ -108,6 +124,9 @@ var _ = Describe("exchange-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialiseExchange() + exchange.Labels = map[string]string{"test": "test-http-error"} + initialiseManager("test", "test-http-error") }) It("sets the status condition", func() { @@ -133,6 +152,9 @@ var _ = Describe("exchange-controller", func() { BeforeEach(func() { exchangeName = "test-go-error" fakeRabbitMQClient.DeclareExchangeReturns(nil, errors.New("a go failure")) + initialiseExchange() + exchange.Labels = map[string]string{"test": "test-go-error"} + initialiseManager("test", "test-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -162,8 +184,12 @@ var _ = Describe("exchange-controller", func() { Status: "201 Created", StatusCode: http.StatusCreated, }, nil) + initialiseExchange() + exchange.Labels = map[string]string{"test": "test-last-transition-time"} + initialiseManager("test", "test-last-transition-time") }) + // TODO maybe this is a problem because the delete function does not have a fakeClient prepared to return OK for Delete requests AfterEach(func() { Expect(k8sClient.Delete(ctx, &exchange)).To(Succeed()) }) @@ -190,7 +216,7 @@ var _ = Describe("exchange-controller", func() { Status: "204 No Content", StatusCode: http.StatusNoContent, }, nil) - exchange.Labels = map[string]string{"k1": "v1"} + exchange.Labels["k1"] = "v1" Expect(k8sClient.Update(ctx, &exchange)).To(Succeed()) ConsistentlyWithOffset(1, func() []topology.Condition { _ = k8sClient.Get( @@ -210,7 +236,7 @@ var _ = Describe("exchange-controller", func() { Status: "500 Internal Server Error", StatusCode: http.StatusInternalServerError, }, errors.New("something went wrong")) - exchange.Labels = map[string]string{"k1": "v2"} + exchange.Labels["k1"] = "v2" Expect(k8sClient.Update(ctx, &exchange)).To(Succeed()) EventuallyWithOffset(1, func() []topology.Condition { _ = k8sClient.Get( @@ -231,6 +257,8 @@ var _ = Describe("exchange-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.DeclareExchangeReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -259,6 +287,9 @@ var _ = Describe("exchange-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseExchange() + exchange.Labels = map[string]string{"test": "delete-exchange-http-error"} + initialiseManager("test", "delete-exchange-http-error") }) It("publishes a 'warning' event", func() { @@ -275,6 +306,9 @@ var _ = Describe("exchange-controller", func() { BeforeEach(func() { exchangeName = "delete-go-error" fakeRabbitMQClient.DeleteExchangeReturns(nil, errors.New("some error")) + initialiseExchange() + exchange.Labels = map[string]string{"test": "delete-go-error"} + initialiseManager("test", "delete-go-error") }) It("publishes a 'warning' event", func() { diff --git a/controllers/federation_controller_test.go b/controllers/federation_controller_test.go index 6bb6a399..58e3bc60 100644 --- a/controllers/federation_controller_test.go +++ b/controllers/federation_controller_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/controllers" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -34,14 +37,29 @@ var _ = Describe("federation-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error federationMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{federationNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{federationNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + // Not sure why, but restricting the NS to the test-ns fails the tests :shrug: + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -66,21 +84,9 @@ var _ = Describe("federation-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.FederationReconciler{Client: federationMgr.GetClient()}, }).SetupWithManager(federationMgr)).To(Succeed()) - }) - - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) + } - JustBeforeEach(func() { + initialiseFederation := func() { federation = topology.Federation{ ObjectMeta: metav1.ObjectMeta{ Name: federationName, @@ -95,6 +101,18 @@ var _ = Describe("federation-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) When("creation", func() { @@ -109,6 +127,9 @@ var _ = Describe("federation-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("some HTTP error")) + initialiseFederation() + federation.Labels = map[string]string{"test": "test-federation-http-error"} + initialiseManager("test", "test-federation-http-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -137,6 +158,9 @@ var _ = Describe("federation-controller", func() { BeforeEach(func() { federationName = "test-federation-go-error" fakeRabbitMQClient.PutFederationUpstreamReturns(nil, errors.New("some go failure here")) + initialiseFederation() + federation.Labels = map[string]string{"test": "test-federation-go-error"} + initialiseManager("test", "test-federation-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -164,6 +188,8 @@ var _ = Describe("federation-controller", func() { When("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.PutFederationUpstreamReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -195,6 +221,9 @@ var _ = Describe("federation-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseFederation() + federation.Labels = map[string]string{"test": "delete-federation-http-error"} + initialiseManager("test", "delete-federation-http-error") }) It("raises an event to indicate a failure to delete", func() { @@ -214,6 +243,9 @@ var _ = Describe("federation-controller", func() { BeforeEach(func() { federationName = "delete-federation-go-error" fakeRabbitMQClient.DeleteFederationUpstreamReturns(nil, errors.New("some error")) + initialiseFederation() + federation.Labels = map[string]string{"test": "delete-federation-go-error"} + initialiseManager("test", "delete-federation-go-error") }) It("publishes a 'warning' event", func() { @@ -233,26 +265,34 @@ var _ = Describe("federation-controller", func() { When("the Federation has DeletionPolicy set to retain", func() { BeforeEach(func() { federationName = "federation-with-retain-policy" - federation.Spec.DeletionPolicy = "retain" fakeRabbitMQClient.DeleteFederationUpstreamReturns(&http.Response{ Status: "200 OK", StatusCode: http.StatusOK, }, nil) + fakeRabbitMQClient.PutFederationUpstreamReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil) + initialiseFederation() + federation.Labels = map[string]string{"test": "federation-with-retain-policy"} + initialiseManager("test", "federation-with-retain-policy") }) It("deletes the k8s resource but preserves the federation in RabbitMQ server", func() { + federation.Spec.DeletionPolicy = "retain" Expect(k8sClient.Create(ctx, &federation)).To(Succeed()) - Expect(k8sClient.Delete(ctx, &federation)).To(Succeed()) + Eventually(fakeRabbitMQClient.PutFederationUpstreamCallCount). + WithPolling(time.Second). + Within(time.Second*3). + Should(BeNumerically(">=", 1), "Expected to call RMQ API to create federation") + Expect(k8sClient.Delete(ctx, &federation)).To(Succeed()) Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: federation.Name, Namespace: federation.Namespace}, &federation) return apierrors.IsNotFound(err) }). Within(statusEventsUpdateTimeout). WithPolling(time.Second). - Should(BeTrue()) + Should(BeTrue(), "Federation should not be found") - Expect(fakeRabbitMQClient.DeleteFederationUpstreamCallCount()).To(Equal(0)) + Expect(fakeRabbitMQClient.DeleteFederationUpstreamCallCount()).To(Equal(0), "Expected Federation to be deleted and no calls to RMQ API") }) }) }) diff --git a/controllers/operatorpolicy_controller_test.go b/controllers/operatorpolicy_controller_test.go index 1a27f191..b05817eb 100644 --- a/controllers/operatorpolicy_controller_test.go +++ b/controllers/operatorpolicy_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -37,14 +40,28 @@ var _ = Describe("operatorpolicy-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error policyMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{policyNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{policyNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -69,21 +86,9 @@ var _ = Describe("operatorpolicy-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.OperatorPolicyReconciler{}, }).SetupWithManager(policyMgr)).To(Succeed()) - }) + } - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) - - JustBeforeEach(func() { + initialisePolicy := func() { policy = topology.OperatorPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: policyName, @@ -98,6 +103,18 @@ var _ = Describe("operatorpolicy-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) Context("creation", func() { @@ -112,6 +129,9 @@ var _ = Describe("operatorpolicy-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialisePolicy() + policy.Labels = map[string]string{"test": "test-http-error"} + initialiseManager("test", "test-http-error") }) It("sets the status condition", func() { @@ -140,6 +160,9 @@ var _ = Describe("operatorpolicy-controller", func() { BeforeEach(func() { policyName = "test-go-error" fakeRabbitMQClient.PutOperatorPolicyReturns(nil, errors.New("a go failure")) + initialisePolicy() + policy.Labels = map[string]string{"test": "test-go-error"} + initialiseManager("test", "test-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -167,11 +190,13 @@ var _ = Describe("operatorpolicy-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.PutOperatorPolicyReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, }, nil) - Expect(k8sClient.Create(ctx, &policy)).To(Succeed()) + Expect(k8sClient.Create(context.TODO(), &policy)).To(Succeed()) Eventually(func() []topology.Condition { _ = k8sClient.Get( ctx, @@ -198,6 +223,9 @@ var _ = Describe("operatorpolicy-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialisePolicy() + policy.Labels = map[string]string{"test": "delete-policy-http-error"} + initialiseManager("test", "delete-policy-http-error") }) It("publishes a 'warning' event", func() { @@ -216,6 +244,9 @@ var _ = Describe("operatorpolicy-controller", func() { BeforeEach(func() { policyName = "delete-go-error" fakeRabbitMQClient.DeleteOperatorPolicyReturns(nil, errors.New("some error")) + initialisePolicy() + policy.Labels = map[string]string{"test": policyName} + initialiseManager("test", policyName) }) It("publishes a 'warning' event", func() { diff --git a/controllers/permission_controller_test.go b/controllers/permission_controller_test.go index 24f43cf1..bfd5bec1 100644 --- a/controllers/permission_controller_test.go +++ b/controllers/permission_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -37,14 +40,28 @@ var _ = Describe("permission-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error permissionMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{permissionNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{permissionNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -69,7 +86,23 @@ var _ = Describe("permission-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.PermissionReconciler{Client: permissionMgr.GetClient(), Scheme: permissionMgr.GetScheme()}, }).SetupWithManager(permissionMgr)).To(Succeed()) - }) + } + + initialisePermission := func() { + permission = topology.Permission{ + ObjectMeta: metav1.ObjectMeta{ + Name: permissionName, + Namespace: permissionNamespace, + }, + Spec: topology.PermissionSpec{ + RabbitmqClusterReference: topology.RabbitmqClusterReference{ + Name: "example-rabbit", + }, + User: "example", + Vhost: "example", + }, + } + } AfterEach(func() { managerCancel() @@ -84,22 +117,6 @@ var _ = Describe("permission-controller", func() { }) When("validating RabbitMQ Client failures with username", func() { - JustBeforeEach(func() { - permission = topology.Permission{ - ObjectMeta: metav1.ObjectMeta{ - Name: permissionName, - Namespace: permissionNamespace, - }, - Spec: topology.PermissionSpec{ - RabbitmqClusterReference: topology.RabbitmqClusterReference{ - Name: "example-rabbit", - }, - User: "example", - Vhost: "example", - }, - } - }) - Context("creation", func() { AfterEach(func() { Expect(k8sClient.Delete(ctx, &permission)).To(Succeed()) @@ -112,6 +129,9 @@ var _ = Describe("permission-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialisePermission() + permission.Labels = map[string]string{"test": "test-with-username-http-error"} + initialiseManager("test", "test-with-username-http-error") }) It("sets the status condition", func() { @@ -137,6 +157,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-username-go-error" fakeRabbitMQClient.UpdatePermissionsInReturns(nil, errors.New("a go failure")) + initialisePermission() + permission.Labels = map[string]string{"test": "test-with-username-go-error"} + initialiseManager("test", "test-with-username-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -161,6 +184,8 @@ var _ = Describe("permission-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.UpdatePermissionsInReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -189,6 +214,9 @@ var _ = Describe("permission-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialisePermission() + permission.Labels = map[string]string{"test": "delete-with-username-permission-http-error"} + initialiseManager("test", "delete-with-username-permission-http-error") }) It("publishes a 'warning' event", func() { @@ -205,6 +233,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "delete-with-username-go-error" fakeRabbitMQClient.ClearPermissionsInReturns(nil, errors.New("some error")) + initialisePermission() + permission.Labels = map[string]string{"test": "delete-with-username-go-error"} + initialiseManager("test", "delete-with-username-go-error") }) It("publishes a 'warning' event", func() { @@ -221,10 +252,13 @@ var _ = Describe("permission-controller", func() { When("validating RabbitMQ Client failures with userRef", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed user = topology.User{ ObjectMeta: metav1.ObjectMeta{ Name: userName, Namespace: permissionNamespace, + Labels: map[string]string{"test": permissionName}, }, Spec: topology.UserSpec{ RabbitmqClusterReference: topology.RabbitmqClusterReference{ @@ -237,6 +271,7 @@ var _ = Describe("permission-controller", func() { ObjectMeta: metav1.ObjectMeta{ Name: permissionName, Namespace: permissionNamespace, + Labels: map[string]string{"test": permissionName}, }, Spec: topology.PermissionSpec{ RabbitmqClusterReference: topology.RabbitmqClusterReference{ @@ -284,6 +319,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-userref-create-not-exist" userName = "example-create-not-exist" + initialisePermission() + permission.Labels = map[string]string{"test": "test-with-userref-create-not-exist"} + initialiseManager("test", "test-with-userref-create-not-exist") }) It("sets the status condition 'Ready' to 'true' ", func() { @@ -309,6 +347,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-userref-create-success" userName = "example-create-success" + initialisePermission() + permission.Labels = map[string]string{"test": "test-with-userref-create-success"} + initialiseManager("test", "test-with-userref-create-success") }) It("sets the status condition 'Ready' to 'true' ", func() { @@ -358,6 +399,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-userref-delete-secret" userName = "example-delete-secret-first" + initialisePermission() + permission.Labels = map[string]string{"test": permissionName} + initialiseManager("test", permissionName) }) It("publishes a 'warning' event", func() { @@ -385,6 +429,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-userref-delete-user" userName = "example-delete-user-first" + initialisePermission() + permission.Labels = map[string]string{"test": permissionName} + initialiseManager("test", permissionName) }) It("publishes a 'warning' event", func() { @@ -408,6 +455,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "test-with-userref-delete-success" userName = "example-delete-success" + initialisePermission() + permission.Labels = map[string]string{"test": "test-with-userref-delete-success"} + initialiseManager("test", "test-with-userref-delete-success") }) It("publishes a 'warning' event", func() { @@ -427,6 +477,9 @@ var _ = Describe("permission-controller", func() { BeforeEach(func() { permissionName = "ownerref-with-userref-test" userName = "example-ownerref" + initialisePermission() + permission.Labels = map[string]string{"test": permissionName} + initialiseManager("test", permissionName) }) AfterEach(func() { diff --git a/controllers/policy_controller_test.go b/controllers/policy_controller_test.go index 2cc75262..12658e19 100644 --- a/controllers/policy_controller_test.go +++ b/controllers/policy_controller_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/controllers" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -36,14 +39,28 @@ var _ = Describe("policy-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error policyMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{policyNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{policyNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -68,21 +85,9 @@ var _ = Describe("policy-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.PolicyReconciler{}, }).SetupWithManager(policyMgr)).To(Succeed()) - }) + } - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) - - JustBeforeEach(func() { + initialisePolicy := func() { policy = topology.Policy{ ObjectMeta: metav1.ObjectMeta{ Name: policyName, @@ -97,6 +102,18 @@ var _ = Describe("policy-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) Context("creation", func() { @@ -111,6 +128,9 @@ var _ = Describe("policy-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialisePolicy() + policy.Labels = map[string]string{"test": "test-http-error"} + initialiseManager("test", policyName) }) It("sets the status condition", func() { @@ -139,6 +159,9 @@ var _ = Describe("policy-controller", func() { BeforeEach(func() { policyName = "test-go-error" fakeRabbitMQClient.PutPolicyReturns(nil, errors.New("a go failure")) + initialisePolicy() + policy.Labels = map[string]string{"test": "test-go-error"} + initialiseManager("test", policyName) }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -166,6 +189,8 @@ var _ = Describe("policy-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.PutPolicyReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -197,6 +222,9 @@ var _ = Describe("policy-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialisePolicy() + policy.Labels = map[string]string{"test": "delete-policy-http-error"} + initialiseManager("test", policyName) }) It("publishes a 'warning' event", func() { @@ -215,6 +243,9 @@ var _ = Describe("policy-controller", func() { BeforeEach(func() { policyName = "delete-go-error" fakeRabbitMQClient.DeletePolicyReturns(nil, errors.New("some error")) + initialisePolicy() + policy.Labels = map[string]string{"test": "delete-go-error"} + initialiseManager("test", policyName) }) It("publishes a 'warning' event", func() { diff --git a/controllers/queue_controller_test.go b/controllers/queue_controller_test.go index adb26c4a..f4822dfd 100644 --- a/controllers/queue_controller_test.go +++ b/controllers/queue_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -35,14 +38,28 @@ var _ = Describe("queue-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error queueMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{queueNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{queueNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -67,21 +84,9 @@ var _ = Describe("queue-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.QueueReconciler{}, }).SetupWithManager(queueMgr)).To(Succeed()) - }) - - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) + } - JustBeforeEach(func() { + initialiseQueue := func() { queue = topology.Queue{ ObjectMeta: metav1.ObjectMeta{ Name: queueName, @@ -93,6 +98,18 @@ var _ = Describe("queue-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) Context("creation", func() { @@ -107,6 +124,9 @@ var _ = Describe("queue-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialiseQueue() + queue.Labels = map[string]string{"test": "test-http-error"} + initialiseManager("test", "test-http-error") }) It("sets the status condition", func() { @@ -135,6 +155,9 @@ var _ = Describe("queue-controller", func() { BeforeEach(func() { queueName = "test-go-error" fakeRabbitMQClient.DeclareQueueReturns(nil, errors.New("a go failure")) + initialiseQueue() + queue.Labels = map[string]string{"test": "test-go-error"} + initialiseManager("test", "test-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -162,6 +185,8 @@ var _ = Describe("queue-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.DeclareQueueReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -193,6 +218,9 @@ var _ = Describe("queue-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseQueue() + queue.Labels = map[string]string{"test": "delete-queue-http-error"} + initialiseManager("test", "delete-queue-http-error") }) It("publishes a 'warning' event", func() { @@ -211,6 +239,9 @@ var _ = Describe("queue-controller", func() { BeforeEach(func() { queueName = "delete-go-error" fakeRabbitMQClient.DeleteQueueReturns(nil, errors.New("some error")) + initialiseQueue() + queue.Labels = map[string]string{"test": "delete-go-error"} + initialiseManager("test", "delete-go-error") }) It("publishes a 'warning' event", func() { @@ -229,26 +260,33 @@ var _ = Describe("queue-controller", func() { When("the Queue has DeletionPolicy set to retain", func() { BeforeEach(func() { queueName = "queue-with-retain-policy" - queue.Spec.DeletionPolicy = "retain" fakeRabbitMQClient.DeleteQueueReturns(&http.Response{ Status: "200 OK", StatusCode: http.StatusOK, }, nil) + initialiseQueue() + queue.Labels = map[string]string{"test": "queue-with-retain-policy"} + initialiseManager("test", "queue-with-retain-policy") }) It("deletes the k8s resource but preserves the queue in RabbitMQ server", func() { + queue.Spec.DeletionPolicy = "retain" Expect(k8sClient.Create(ctx, &queue)).To(Succeed()) - Expect(k8sClient.Delete(ctx, &queue)).To(Succeed()) + Eventually(fakeRabbitMQClient.DeclareQueueCallCount). + WithPolling(time.Second). + Within(time.Second*3). + Should(BeNumerically(">=", 1), "Expected to call RMQ API to declare queue") + Expect(k8sClient.Delete(ctx, &queue)).To(Succeed()) Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: queue.Name, Namespace: queue.Namespace}, &queue) return apierrors.IsNotFound(err) }). Within(statusEventsUpdateTimeout). WithPolling(time.Second). - Should(BeTrue()) + Should(BeTrue(), "Queue should not be found") - Expect(fakeRabbitMQClient.DeleteQueueCallCount()).To(Equal(0)) + Expect(fakeRabbitMQClient.DeleteQueueCallCount()).To(Equal(0), "Expected to delete queue and no calls to RMQ API") }) }) }) diff --git a/controllers/shovel_controller_test.go b/controllers/shovel_controller_test.go index 8eeb7d74..672ada34 100644 --- a/controllers/shovel_controller_test.go +++ b/controllers/shovel_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -36,14 +39,28 @@ var _ = Describe("shovel-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error shovelMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{shovelNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{shovelNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -68,21 +85,9 @@ var _ = Describe("shovel-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.ShovelReconciler{Client: shovelMgr.GetClient()}, }).SetupWithManager(shovelMgr)).To(Succeed()) - }) - - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) + } - JustBeforeEach(func() { + initialiseShovel := func() { shovel = topology.Shovel{ ObjectMeta: metav1.ObjectMeta{ Name: shovelName, @@ -97,6 +102,18 @@ var _ = Describe("shovel-controller", func() { }, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) When("creation", func() { @@ -111,6 +128,9 @@ var _ = Describe("shovel-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("some HTTP error")) + initialiseShovel() + shovel.Labels = map[string]string{"test": "test-shovel-http-error"} + initialiseManager("test", "test-shovel-http-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -139,6 +159,9 @@ var _ = Describe("shovel-controller", func() { BeforeEach(func() { shovelName = "test-shovel-go-error" fakeRabbitMQClient.DeclareShovelReturns(nil, errors.New("a go failure")) + initialiseShovel() + shovel.Labels = map[string]string{"test": "test-shovel-go-error"} + initialiseManager("test", "test-shovel-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -170,6 +193,9 @@ var _ = Describe("shovel-controller", func() { Status: "201 Created", StatusCode: http.StatusCreated, }, nil) + initialiseShovel() + shovel.Labels = map[string]string{"test": "test-shovel-success"} + initialiseManager("test", "test-shovel-success") }) It("works", func() { @@ -178,6 +204,7 @@ var _ = Describe("shovel-controller", func() { Eventually(komega.Object(&shovel)).WithTimeout(2 * time.Second).Should(HaveField("ObjectMeta.Finalizers", ConsistOf("deletion.finalizers.shovels.rabbitmq.com"))) By("sets the status condition 'Ready' to 'true'") + // TODO rewrite this using komega.Object Eventually(func() []topology.Condition { _ = k8sClient.Get( ctx, @@ -200,6 +227,8 @@ var _ = Describe("shovel-controller", func() { When("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.DeclareShovelReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -231,6 +260,9 @@ var _ = Describe("shovel-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseShovel() + shovel.Labels = map[string]string{"test": "delete-shovel-http-error"} + initialiseManager("test", "delete-shovel-http-error") }) It("raises an event to indicate a failure to delete", func() { @@ -250,6 +282,9 @@ var _ = Describe("shovel-controller", func() { BeforeEach(func() { shovelName = "delete-shovel-go-error" fakeRabbitMQClient.DeleteShovelReturns(nil, errors.New("some error")) + initialiseShovel() + shovel.Labels = map[string]string{"test": "delete-shovel-go-error"} + initialiseManager("test", "delete-shovel-go-error") }) It("publishes a 'warning' event", func() { @@ -272,6 +307,9 @@ var _ = Describe("shovel-controller", func() { Status: "204 No Content", StatusCode: http.StatusNoContent, }, nil) + initialiseShovel() + shovel.Labels = map[string]string{"test": "delete-shovel-success"} + initialiseManager("test", "delete-shovel-success") }) It("publishes a normal event", func() { @@ -290,18 +328,27 @@ var _ = Describe("shovel-controller", func() { }) }) }) + When("the Shovel has DeletionPolicy set to retain", func() { BeforeEach(func() { shovelName = "shovel-with-retain-policy" - shovel.Spec.DeletionPolicy = "retain" fakeRabbitMQClient.DeleteShovelReturns(&http.Response{ Status: "200 OK", StatusCode: http.StatusOK, }, nil) + fakeRabbitMQClient.DeclareShovelReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil) + initialiseShovel() + shovel.Labels = map[string]string{"test": "shovel-with-retain-policy"} + initialiseManager("test", "shovel-with-retain-policy") }) It("deletes the k8s resource but preserves the shovel in RabbitMQ server", func() { + shovel.Spec.DeletionPolicy = "retain" Expect(k8sClient.Create(ctx, &shovel)).To(Succeed()) + Eventually(fakeRabbitMQClient.DeclareShovelCallCount). + WithPolling(time.Second). + Within(time.Second*3). + Should(BeNumerically(">=", 1), "Expected to call RMQ API to declare shovel") Expect(k8sClient.Delete(ctx, &shovel)).To(Succeed()) Eventually(func() bool { @@ -310,9 +357,9 @@ var _ = Describe("shovel-controller", func() { }). Within(statusEventsUpdateTimeout). WithPolling(time.Second). - Should(BeTrue()) + Should(BeTrue(), "Expected shovel to not be found") - Expect(fakeRabbitMQClient.DeleteShovelCallCount()).To(Equal(0)) + Expect(fakeRabbitMQClient.DeleteShovelCallCount()).To(Equal(0), "Shovel object should have been deleted and no calls to RMQ API") }) }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index fffdcd1f..61c86ece 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -112,7 +112,7 @@ var _ = BeforeSuite(func() { CRDDirectoryPaths: operatorCrds, ErrorIfCRDPathMissing: true, Config: &rest.Config{ - Host: fmt.Sprintf("localhost:818%d", GinkgoParallelProcess()), + Host: fmt.Sprintf("localhost:218%d", GinkgoParallelProcess()), }, } diff --git a/controllers/topicpermission_controller_test.go b/controllers/topicpermission_controller_test.go index fb229a2c..23933764 100644 --- a/controllers/topicpermission_controller_test.go +++ b/controllers/topicpermission_controller_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/controllers" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -36,14 +39,28 @@ var _ = Describe("topicpermission-controller", func() { k8sClient runtimeClient.Client ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error topicPermissionMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{topicPermissionNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{topicPermissionNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -68,7 +85,26 @@ var _ = Describe("topicpermission-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.TopicPermissionReconciler{Client: topicPermissionMgr.GetClient(), Scheme: topicPermissionMgr.GetScheme()}, }).SetupWithManager(topicPermissionMgr)).To(Succeed()) - }) + } + + initialiseTopicPermission := func() { + topicperm = topology.TopicPermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: topicPermissionNamespace, + }, + Spec: topology.TopicPermissionSpec{ + RabbitmqClusterReference: topology.RabbitmqClusterReference{ + Name: "example-rabbit", + }, + User: "example", + Vhost: "example", + Permissions: topology.TopicPermissionConfig{ + Exchange: "some", + }, + }, + } + } AfterEach(func() { managerCancel() @@ -83,25 +119,6 @@ var _ = Describe("topicpermission-controller", func() { }) When("validating RabbitMQ Client failures with username", func() { - JustBeforeEach(func() { - topicperm = topology.TopicPermission{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: topicPermissionNamespace, - }, - Spec: topology.TopicPermissionSpec{ - RabbitmqClusterReference: topology.RabbitmqClusterReference{ - Name: "example-rabbit", - }, - User: "example", - Vhost: "example", - Permissions: topology.TopicPermissionConfig{ - Exchange: "some", - }, - }, - } - }) - Context("creation", func() { AfterEach(func() { Expect(k8sClient.Delete(ctx, &topicperm)).To(Succeed()) @@ -114,6 +131,9 @@ var _ = Describe("topicpermission-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("sets the status condition", func() { @@ -142,6 +162,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-username-go-error" fakeRabbitMQClient.UpdateTopicPermissionsInReturns(nil, errors.New("a go failure")) + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -169,6 +192,8 @@ var _ = Describe("topicpermission-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.UpdateTopicPermissionsInReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -200,6 +225,9 @@ var _ = Describe("topicpermission-controller", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("publishes a 'warning' event", func() { @@ -219,6 +247,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "delete-with-username-go-error" fakeRabbitMQClient.DeleteTopicPermissionsInReturns(nil, errors.New("some error")) + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("publishes a 'warning' event", func() { @@ -242,6 +273,7 @@ var _ = Describe("topicpermission-controller", func() { ObjectMeta: metav1.ObjectMeta{ Name: userName, Namespace: topicPermissionNamespace, + Labels: map[string]string{"test": name}, }, Spec: topology.UserSpec{ RabbitmqClusterReference: topology.RabbitmqClusterReference{ @@ -254,6 +286,7 @@ var _ = Describe("topicpermission-controller", func() { ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: topicPermissionNamespace, + Labels: map[string]string{"test": name}, }, Spec: topology.TopicPermissionSpec{ RabbitmqClusterReference: topology.RabbitmqClusterReference{ @@ -296,6 +329,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-userref-create-not-exist" userName = "topic-perm-example-create-not-exist" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("sets the status condition 'Ready' to 'true' ", func() { @@ -324,6 +360,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-userref-create-success" userName = "topic-perm-example-create-success" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("sets the status condition 'Ready' to 'true' ", func() { @@ -353,6 +392,8 @@ var _ = Describe("topicpermission-controller", func() { Context("deletion", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed Expect(k8sClient.Create(ctx, &user)).To(Succeed()) user.Status.Username = userName Expect(k8sClient.Status().Update(ctx, &user)).To(Succeed()) @@ -379,6 +420,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-userref-delete-secret" userName = "topic-perm-example-delete-secret-first" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("publishes a 'warning' event", func() { @@ -409,6 +453,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-userref-delete-user" userName = "topic-perm-example-delete-user-first" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("publishes a 'warning' event", func() { @@ -440,6 +487,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "test-with-userref-delete-success" userName = "topic-perm-example-delete-success" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) It("publishes a 'warning' event", func() { @@ -463,6 +513,9 @@ var _ = Describe("topicpermission-controller", func() { BeforeEach(func() { name = "ownerref-with-userref-test" userName = "topic-perm-topic-perm-user" + initialiseTopicPermission() + topicperm.Labels = map[string]string{"test": name} + initialiseManager("test", name) }) AfterEach(func() { diff --git a/controllers/user_controller_test.go b/controllers/user_controller_test.go index e97519b9..68385ade 100644 --- a/controllers/user_controller_test.go +++ b/controllers/user_controller_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" "time" @@ -39,14 +42,28 @@ var _ = Describe("UserController", func() { channels int32 ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error userMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{userNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{userNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -71,21 +88,9 @@ var _ = Describe("UserController", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.UserReconciler{Client: userMgr.GetClient(), Scheme: userMgr.GetScheme()}, }).SetupWithManager(userMgr)).To(Succeed()) - }) + } - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) - - JustBeforeEach(func() { + initialiseUser := func() { user = topology.User{ ObjectMeta: metav1.ObjectMeta{ Name: userName, @@ -98,6 +103,18 @@ var _ = Describe("UserController", func() { UserLimits: &userLimits, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) When("creating a user", func() { @@ -112,6 +129,9 @@ var _ = Describe("UserController", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("some HTTP error")) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -140,6 +160,9 @@ var _ = Describe("UserController", func() { BeforeEach(func() { userName = "test-user-go-error" fakeRabbitMQClient.PutUserReturns(nil, errors.New("hit a exception")) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -187,6 +210,9 @@ var _ = Describe("UserController", func() { Message: "Object Not Found", Reason: "Not Found", }) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("should create the user limits", func() { @@ -241,6 +267,9 @@ var _ = Describe("UserController", func() { Status: "204 No Content", StatusCode: http.StatusNoContent, }, nil) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("should update the existing user limit and delete the unused old limit", func() { @@ -277,6 +306,8 @@ var _ = Describe("UserController", func() { When("deleting a user", func() { JustBeforeEach(func() { + // Must use a JustBeforeEach to extract this common behaviour + // JustBeforeEach runs AFTER all BeforeEach have completed fakeRabbitMQClient.PutUserReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -316,6 +347,9 @@ var _ = Describe("UserController", func() { StatusCode: http.StatusBadGateway, Body: io.NopCloser(bytes.NewBufferString("Hello World")), }, nil) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("raises an event to indicate a failure to delete", func() { @@ -335,6 +369,9 @@ var _ = Describe("UserController", func() { BeforeEach(func() { userName = "delete-user-go-error" fakeRabbitMQClient.DeleteUserReturns(nil, errors.New("some error")) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("raises an event to indicate a failure to delete", func() { @@ -357,6 +394,9 @@ var _ = Describe("UserController", func() { Status: "204 No Content", StatusCode: http.StatusNoContent, }, nil) + initialiseUser() + user.Labels = map[string]string{"test": userName} + initialiseManager("test", userName) }) It("raises an event to indicate a successful deletion", func() { diff --git a/controllers/vhost_controller_test.go b/controllers/vhost_controller_test.go index 14478af0..7171cd6a 100644 --- a/controllers/vhost_controller_test.go +++ b/controllers/vhost_controller_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" + "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/controllers" "io" + "k8s.io/apimachinery/pkg/labels" "net/http" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -36,14 +39,28 @@ var _ = Describe("vhost-controller", func() { vhostLimits *topology.VhostLimits ) - BeforeEach(func() { + initialiseManager := func(keyValPair ...string) { + var sel labels.Selector + if len(keyValPair) == 2 { + var err error + sel, err = labels.Parse(fmt.Sprintf("%s == %s", keyValPair[0], keyValPair[1])) + Expect(err).NotTo(HaveOccurred()) + } + var err error vhostMgr, err = ctrl.NewManager(testEnv.Config, ctrl.Options{ Metrics: server.Options{ BindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite }, Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{vhostNamespace: {}}, + DefaultNamespaces: map[string]cache.Config{vhostNamespace: { + LabelSelector: sel, + }}, + ByObject: map[runtimeClient.Object]cache.ByObject{ + &v1beta1.RabbitmqCluster{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Secret{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + &corev1.Service{}: {Namespaces: map[string]cache.Config{cache.AllNamespaces: {}}}, + }, }, Logger: GinkgoLogr, Controller: config.Controller{ @@ -68,21 +85,9 @@ var _ = Describe("vhost-controller", func() { RabbitmqClientFactory: fakeRabbitMQClientFactory, ReconcileFunc: &controllers.VhostReconciler{Client: vhostMgr.GetClient()}, }).SetupWithManager(vhostMgr)).To(Succeed()) - }) - - AfterEach(func() { - managerCancel() - // Sad workaround to avoid controllers racing for the reconciliation of other's - // test cases. Without this wait, the last run test consistently fails because - // the previous cancelled manager is just in time to reconcile the Queue of the - // new/last test, and use the wrong/unexpected arguments in the queue declare call - // - // Eventual consistency is nice when you have good means of awaiting. That's not the - // case with testenv and kubernetes controllers. - <-time.After(time.Second) - }) + } - JustBeforeEach(func() { + initialiseVhost := func() { vhost = topology.Vhost{ ObjectMeta: metav1.ObjectMeta{ Name: vhostName, @@ -96,13 +101,21 @@ var _ = Describe("vhost-controller", func() { VhostLimits: vhostLimits, }, } + } + + AfterEach(func() { + managerCancel() + // Sad workaround to avoid controllers racing for the reconciliation of other's + // test cases. Without this wait, the last run test consistently fails because + // the previous cancelled manager is just in time to reconcile the Queue of the + // new/last test, and use the wrong/unexpected arguments in the queue declare call + // + // Eventual consistency is nice when you have good means of awaiting. That's not the + // case with testenv and kubernetes controllers. + <-time.After(time.Second) }) Context("creation", func() { - AfterEach(func() { - Expect(k8sClient.Delete(ctx, &vhost)).To(Succeed()) - }) - When("the RabbitMQ Client returns a HTTP error response", func() { BeforeEach(func() { vhostName = "test-http-error" @@ -110,6 +123,9 @@ var _ = Describe("vhost-controller", func() { Status: "418 I'm a teapot", StatusCode: 418, }, errors.New("a failure")) + initialiseVhost() + vhost.Labels = map[string]string{"test": "creation-http-error"} + initialiseManager("test", "creation-http-error") }) It("sets the status condition", func() { @@ -138,6 +154,9 @@ var _ = Describe("vhost-controller", func() { BeforeEach(func() { vhostName = "test-go-error" fakeRabbitMQClient.PutVhostReturns(nil, errors.New("a go failure")) + initialiseVhost() + vhost.Labels = map[string]string{"test": "creation-go-error"} + initialiseManager("test", "creation-go-error") }) It("sets the status condition to indicate a failure to reconcile", func() { @@ -165,6 +184,11 @@ var _ = Describe("vhost-controller", func() { Context("vhost limits", func() { var connections, queues int32 + AfterEach(func() { + // Must reset the vhost limits to avoid test pollution + vhostLimits = nil + }) + When("vhost limits are provided", func() { BeforeEach(func() { connections = 708 @@ -187,6 +211,9 @@ var _ = Describe("vhost-controller", func() { Message: "Object Not Found", Reason: "Not Found", }) + initialiseVhost() + vhost.Labels = map[string]string{"test": "vhost-with-limits"} + initialiseManager("test", "vhost-with-limits") }) It("puts the vhost limits", func() { @@ -224,6 +251,9 @@ var _ = Describe("vhost-controller", func() { Message: "Object Not Found", Reason: "Not Found", }) + initialiseVhost() + vhost.Labels = map[string]string{"test": "vhost-without-limits"} + initialiseManager("test", "vhost-without-limits") }) It("does not set vhost limits", func() { @@ -260,6 +290,9 @@ var _ = Describe("vhost-controller", func() { Status: "204 No Content", StatusCode: http.StatusNoContent, }, nil) + initialiseVhost() + vhost.Labels = map[string]string{"test": "vhost-updated-limits"} + initialiseManager("test", "vhost-updated-limits") }) It("updates the provided limits and removes unspecified limits", func() { @@ -299,7 +332,7 @@ var _ = Describe("vhost-controller", func() { }) Context("deletion", func() { - JustBeforeEach(func() { + createVhost := func() { fakeRabbitMQClient.PutVhostReturns(&http.Response{ Status: "201 Created", StatusCode: http.StatusCreated, @@ -321,11 +354,17 @@ var _ = Describe("vhost-controller", func() { "Reason": Equal("SuccessfulCreateOrUpdate"), "Status": Equal(corev1.ConditionTrue), }))) - }) + } When("the RabbitMQ Client returns a HTTP error response", func() { BeforeEach(func() { vhostName = "delete-vhost-http-error" + initialiseVhost() + vhost.Labels = map[string]string{"test": vhostName} + initialiseManager("test", vhostName) + + createVhost() + fakeRabbitMQClient.DeleteVhostReturns(&http.Response{ Status: "502 Bad Gateway", StatusCode: http.StatusBadGateway, @@ -349,6 +388,12 @@ var _ = Describe("vhost-controller", func() { When("the RabbitMQ Client returns a Go error response", func() { BeforeEach(func() { vhostName = "delete-go-error" + initialiseVhost() + vhost.Labels = map[string]string{"test": vhostName} + initialiseManager("test", vhostName) + + createVhost() + fakeRabbitMQClient.DeleteVhostReturns(nil, errors.New("some error")) }) @@ -369,26 +414,34 @@ var _ = Describe("vhost-controller", func() { When("the Vhost has DeletionPolicy set to retain", func() { BeforeEach(func() { vhostName = "vhost-with-retain-policy" - vhost.Spec.DeletionPolicy = "retain" fakeRabbitMQClient.DeleteVhostReturns(&http.Response{ Status: "200 OK", StatusCode: http.StatusOK, }, nil) + fakeRabbitMQClient.PutVhostReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil) + initialiseVhost() + vhost.Labels = map[string]string{"test": "vhost-with-retain-policy"} + initialiseManager("test", "vhost-with-retain-policy") }) It("deletes the k8s resource but preserves the vhost in RabbitMQ server", func() { + vhost.Spec.DeletionPolicy = "retain" Expect(k8sClient.Create(ctx, &vhost)).To(Succeed()) - Expect(k8sClient.Delete(ctx, &vhost)).To(Succeed()) + Eventually(fakeRabbitMQClient.PutVhostCallCount). + WithPolling(time.Second). + Within(time.Second*3). + Should(BeNumerically(">=", 1), "Expected to call RMQ API to create vhost") + Expect(k8sClient.Delete(ctx, &vhost)).To(Succeed()) Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: vhost.Name, Namespace: vhost.Namespace}, &vhost) return apierrors.IsNotFound(err) }). Within(statusEventsUpdateTimeout). WithPolling(time.Second). - Should(BeTrue()) + Should(BeTrue(), "vhost should not be found") - Expect(fakeRabbitMQClient.DeleteVhostCallCount()).To(Equal(0)) + Expect(fakeRabbitMQClient.DeleteVhostCallCount()).To(Equal(0), "Expected vhost to be deleted and no calls to RMQ API") }) }) })