Skip to content

Commit

Permalink
Refactor integration tests: controller isolation
Browse files Browse the repository at this point in the history 
Controllers now are scoped to a dedicated namespace. Controllers now are
started and stopped for each test case. This provides isolation from
other test suites, ensuring there's no environment pollution.

We also bumped Kubernetes to 1.26, because this version had some changes
regarding Pod Admission Controllers, and it will be the minimum
supported Kubernetes version in the next minor release.

Signed-off-by: Aitor Perez Cedres <acedres@vmware.com>
  • Loading branch information
@Zerpet
Zerpet committed Dec 11, 2023
1 parent 9430e0d commit e2d091f
Show file tree
Hide file tree
Showing 18 changed files with 1,471 additions and 747 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ install-tools:
@$(get_mod_code_generator)
go install golang.org/x/vuln/cmd/govulncheck@latest

ENVTEST_K8S_VERSION = 1.22.1
ENVTEST_K8S_VERSION = 1.26.1
ARCHITECTURE = amd64
LOCAL_TESTBIN = $(CURDIR)/testbin

Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/user_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ var _ = Describe("user spec", func() {
username = "invalid-user"
})
It("fails to create the user", func() {
Expect(k8sClient.Create(ctx, &user)).To(MatchError(`User.rabbitmq.com "invalid-user" is invalid: spec.tags: Unsupported value: "invalid": supported values: "management", "policymaker", "monitoring", "administrator"`))
Expect(k8sClient.Create(ctx, &user)).To(MatchError(`User.rabbitmq.com "invalid-user" is invalid: spec.tags[1]: Unsupported value: "invalid": supported values: "management", "policymaker", "monitoring", "administrator"`))
})
})
})
Expand Down
85 changes: 70 additions & 15 deletions controllers/binding_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@ package controllers_test

import (
"bytes"
"context"
"errors"
"io/ioutil"
"github.com/rabbitmq/messaging-topology-operator/controllers"
"io"
"net/http"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
runtimeClient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"time"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -18,14 +24,63 @@ import (
)

var _ = Describe("bindingController", func() {
var binding topology.Binding
var bindingName string
var (
binding topology.Binding
bindingName string
bindingMgr ctrl.Manager
managerCtx context.Context
managerCancel context.CancelFunc
k8sClient runtimeClient.Client
)

BeforeEach(func() {
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: {}},
},
Logger: GinkgoLogr,
})
Expect(err).ToNot(HaveOccurred())

managerCtx, managerCancel = context.WithCancel(context.Background())
go func(ctx context.Context) {
defer GinkgoRecover()
Expect(bindingMgr.Start(ctx)).To(Succeed())
}(managerCtx)

k8sClient = bindingMgr.GetClient()

Expect((&controllers.TopologyReconciler{
Client: bindingMgr.GetClient(),
Type: &topology.Binding{},
Scheme: bindingMgr.GetScheme(),
Recorder: fakeRecorder,
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() {
binding = topology.Binding{
ObjectMeta: metav1.ObjectMeta{
Name: bindingName,
Namespace: "default",
Namespace: bindingNamespace,
},
Spec: topology.BindingSpec{
RabbitmqClusterReference: topology.RabbitmqClusterReference{
Expand All @@ -46,9 +101,9 @@ var _ = Describe("bindingController", func() {
})

It("sets the status condition to indicate a failure to reconcile", func() {
Expect(client.Create(ctx, &binding)).To(Succeed())
Expect(k8sClient.Create(ctx, &binding)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace},
&binding,
Expand All @@ -71,9 +126,9 @@ var _ = Describe("bindingController", func() {
})

It("sets the status condition to indicate a failure to reconcile", func() {
Expect(client.Create(ctx, &binding)).To(Succeed())
Expect(k8sClient.Create(ctx, &binding)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace},
&binding,
Expand All @@ -96,9 +151,9 @@ var _ = Describe("bindingController", func() {
Status: "201 Created",
StatusCode: http.StatusCreated,
}, nil)
Expect(client.Create(ctx, &binding)).To(Succeed())
Expect(k8sClient.Create(ctx, &binding)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace},
&binding,
Expand All @@ -118,14 +173,14 @@ var _ = Describe("bindingController", func() {
fakeRabbitMQClient.DeleteBindingReturns(&http.Response{
Status: "502 Bad Gateway",
StatusCode: http.StatusBadGateway,
Body: ioutil.NopCloser(bytes.NewBufferString("Hello World")),
Body: io.NopCloser(bytes.NewBufferString("Hello World")),
}, nil)
})

It("raises an event to indicate a failure to delete", func() {
Expect(client.Delete(ctx, &binding)).To(Succeed())
Expect(k8sClient.Delete(ctx, &binding)).To(Succeed())
Consistently(func() bool {
err := client.Get(ctx, types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace}, &topology.Binding{})
err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace}, &topology.Binding{})
return apierrors.IsNotFound(err)
}, statusEventsUpdateTimeout).Should(BeFalse())
Expect(observedEvents()).To(ContainElement("Warning FailedDelete failed to delete binding"))
Expand All @@ -139,9 +194,9 @@ var _ = Describe("bindingController", func() {
})

It("raises an event to indicate a failure to delete", func() {
Expect(client.Delete(ctx, &binding)).To(Succeed())
Expect(k8sClient.Delete(ctx, &binding)).To(Succeed())
Consistently(func() bool {
err := client.Get(ctx, types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace}, &topology.Binding{})
err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace}, &topology.Binding{})
return apierrors.IsNotFound(err)
}, 5).Should(BeFalse())
Expect(observedEvents()).To(ContainElement("Warning FailedDelete failed to delete binding"))
Expand Down
94 changes: 75 additions & 19 deletions controllers/exchange_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@ package controllers_test

import (
"bytes"
"context"
"errors"
"github.com/rabbitmq/messaging-topology-operator/controllers"
"io/ioutil"
"net/http"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
runtimeClient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"time"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -18,14 +24,64 @@ import (
)

var _ = Describe("exchange-controller", func() {
var exchange topology.Exchange
var exchangeName string
var (
exchange topology.Exchange
exchangeName string
exchangeMgr ctrl.Manager
managerCtx context.Context
managerCancel context.CancelFunc
k8sClient runtimeClient.Client
)

BeforeEach(func() {
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: {}},
},
Logger: GinkgoLogr,
})
Expect(err).ToNot(HaveOccurred())

managerCtx, managerCancel = context.WithCancel(context.Background())
go func(ctx context.Context) {
defer GinkgoRecover()
Expect(exchangeMgr.Start(ctx)).To(Succeed())
}(managerCtx)

k8sClient = exchangeMgr.GetClient()

Expect((&controllers.TopologyReconciler{
Client: exchangeMgr.GetClient(),
Type: &topology.Exchange{},
Scheme: exchangeMgr.GetScheme(),
Recorder: fakeRecorder,
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
exchange = topology.Exchange{
ObjectMeta: metav1.ObjectMeta{
Name: exchangeName,
Namespace: "default",
Namespace: exchangeNamespace,
},
Spec: topology.ExchangeSpec{
RabbitmqClusterReference: topology.RabbitmqClusterReference{
Expand All @@ -46,9 +102,9 @@ var _ = Describe("exchange-controller", func() {
})

It("sets the status condition", func() {
Expect(client.Create(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Create(ctx, &exchange)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace},
&exchange,
Expand All @@ -71,9 +127,9 @@ var _ = Describe("exchange-controller", func() {
})

It("sets the status condition to indicate a failure to reconcile", func() {
Expect(client.Create(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Create(ctx, &exchange)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace},
&exchange,
Expand All @@ -99,9 +155,9 @@ var _ = Describe("exchange-controller", func() {
})
It("changes only if status changes", func() {
By("setting LastTransitionTime when transitioning to status Ready=true")
Expect(client.Create(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Create(ctx, &exchange)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Namespace: exchange.Namespace, Name: exchange.Name},
&exchange,
Expand All @@ -120,9 +176,9 @@ var _ = Describe("exchange-controller", func() {
StatusCode: http.StatusNoContent,
}, nil)
exchange.Labels = map[string]string{"k1": "v1"}
Expect(client.Update(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Update(ctx, &exchange)).To(Succeed())
ConsistentlyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Namespace: exchange.Namespace, Name: exchange.Name},
&exchange,
Expand All @@ -140,9 +196,9 @@ var _ = Describe("exchange-controller", func() {
StatusCode: http.StatusInternalServerError,
}, errors.New("something went wrong"))
exchange.Labels = map[string]string{"k1": "v2"}
Expect(client.Update(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Update(ctx, &exchange)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Namespace: exchange.Namespace, Name: exchange.Name},
&exchange,
Expand All @@ -164,9 +220,9 @@ var _ = Describe("exchange-controller", func() {
Status: "201 Created",
StatusCode: http.StatusCreated,
}, nil)
Expect(client.Create(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Create(ctx, &exchange)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
_ = k8sClient.Get(
ctx,
types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace},
&exchange,
Expand All @@ -191,9 +247,9 @@ var _ = Describe("exchange-controller", func() {
})

It("publishes a 'warning' event", func() {
Expect(client.Delete(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Delete(ctx, &exchange)).To(Succeed())
Consistently(func() bool {
err := client.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &topology.Exchange{})
err := k8sClient.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &topology.Exchange{})
return apierrors.IsNotFound(err)
}, statusEventsUpdateTimeout).Should(BeFalse())
Expect(observedEvents()).To(ContainElement("Warning FailedDelete failed to delete exchange"))
Expand All @@ -207,9 +263,9 @@ var _ = Describe("exchange-controller", func() {
})

It("publishes a 'warning' event", func() {
Expect(client.Delete(ctx, &exchange)).To(Succeed())
Expect(k8sClient.Delete(ctx, &exchange)).To(Succeed())
Consistently(func() bool {
err := client.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &topology.Exchange{})
err := k8sClient.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &topology.Exchange{})
return apierrors.IsNotFound(err)
}, statusEventsUpdateTimeout).Should(BeFalse())
Expect(observedEvents()).To(ContainElement("Warning FailedDelete failed to delete exchange"))
Expand Down
Loading

0 comments on commit e2d091f

Please sign in to comment.