Skip to content

Commit

Permalink
Update controller-runtime to 0.16
Browse files Browse the repository at this point in the history
This version of controller runtime has breaking changes. Refactored the
Topology Controller tests because they were flaky. This group of tests
was defining their own queue reconciler in the same manager that has all
the controllers (included one for Queue types). This caused a "race"
between all setup reconcilers, and the test passed when the controller
registered in the specific test case was the first to reconcile, which
seemed to happen often enough to hide the flake.

After the refactor, each test case defines its own manager, which
watches a dedicated namespace for the Topology Controller tests, and
cancels/stops the manager at the end of each test case. This ensures
that only one controller is reconciling the Queue object. The
controllers defined in the BeforeSuite now watch only the default
namespace. All our tests were using the default namespace anyway, so
there are no failures as part of this subtle change.

Signed-off-by: Aitor Perez Cedres <[email protected]>
  • Loading branch information
Zerpet committed Dec 1, 2023
1 parent dc798d7 commit 03f4458
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 238 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ $(KUBEBUILDER_ASSETS):
### Targets

.PHONY: unit-tests
unit-tests: install-tools $(KUBEBUILDER_ASSETS) generate fmt vet vuln manifests ## Run unit tests
unit-tests: install-tools $(KUBEBUILDER_ASSETS) generate fmt vet manifests ## Run unit tests
ginkgo -r --randomize-all api/ internal/ rabbitmqclient/

.PHONY: integration-tests
integration-tests: install-tools $(KUBEBUILDER_ASSETS) generate fmt vet vuln manifests ## Run integration tests
integration-tests: install-tools $(KUBEBUILDER_ASSETS) generate fmt vet manifests ## Run integration tests
ginkgo -r --randomize-all controllers/

just-integration-tests: $(KUBEBUILDER_ASSETS) vet
Expand Down
1 change: 0 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion controllers/binding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

type BindingReconciler struct{}

func (r *BindingReconciler) DeclareFunc(ctx context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
func (r *BindingReconciler) DeclareFunc(_ context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
binding := obj.(*topology.Binding)
info, err := internal.GenerateBindingInfo(binding)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/exchange_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

type ExchangeReconciler struct{}

func (r *ExchangeReconciler) DeclareFunc(ctx context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
func (r *ExchangeReconciler) DeclareFunc(_ context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
exchange := obj.(*topology.Exchange)
settings, err := internal.GenerateExchangeSettings(exchange)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/policy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
type PolicyReconciler struct{}

// DeclareFunc creates or updates a given policy using rabbithole client.PutPolicy
func (r *PolicyReconciler) DeclareFunc(ctx context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
func (r *PolicyReconciler) DeclareFunc(_ context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
policy := obj.(*topology.Policy)
generatePolicy, err := internal.GeneratePolicy(policy)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/queue_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

type QueueReconciler struct{}

func (r *QueueReconciler) DeclareFunc(ctx context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
func (r *QueueReconciler) DeclareFunc(_ context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
queue := obj.(*topology.Queue)
queueSettings, err := internal.GenerateQueueSettings(queue)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/schemareplication_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (r *SchemaReplicationReconciler) DeclareFunc(ctx context.Context, client ra
return validateResponse(client.PutGlobalParameter(SchemaReplicationParameterName, endpoints))
}

func (r *SchemaReplicationReconciler) DeleteFunc(ctx context.Context, client rabbitmqclient.Client, obj topology.TopologyResource) error {
func (r *SchemaReplicationReconciler) DeleteFunc(ctx context.Context, client rabbitmqclient.Client, _ topology.TopologyResource) error {
logger := ctrl.LoggerFrom(ctx)
err := validateResponseForDeletion(client.DeleteGlobalParameter(SchemaReplicationParameterName))
if errors.Is(err, NotFound) {
Expand Down
164 changes: 74 additions & 90 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ package controllers_test
import (
"context"
"crypto/x509"
"fmt"
"go/build"
"path/filepath"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"testing"
"time"

Expand All @@ -31,7 +34,7 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -87,8 +90,9 @@ var _ = BeforeSuite(func() {
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{
filepath.Join("..", "config", "crd", "bases"),
filepath.Join(build.Default.GOPATH, "pkg", "mod", "github.com", "rabbitmq", "cluster-operator@v1.14.0", "config", "crd", "bases"),
filepath.Join(build.Default.GOPATH, "pkg", "mod", "github.com", "rabbitmq", "cluster-operator", "[email protected].0", "config", "crd", "bases"),
},
ErrorIfCRDPathMissing: true,
}

cfg, err := testEnv.Start()
Expand All @@ -104,8 +108,13 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())

mgr, err = ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme.Scheme,
MetricsBindAddress: "0", // To avoid MacOS firewall pop-up every time you run this suite
Scheme: scheme.Scheme,
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{"default": {}},
},
})
Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -228,34 +237,6 @@ var _ = BeforeSuite(func() {
komega.SetClient(client)
komega.SetContext(ctx)

rmqCreds := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "example-rabbit-user-credentials",
Namespace: "default",
},
}
Expect(client.Create(ctx, &rmqCreds)).To(Succeed())

rmqSrv := corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "example-rabbit",
Namespace: "default",
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: 15672,
Name: "management",
},
{
Port: 15671,
Name: "management-tls",
},
},
},
}
Expect(client.Create(ctx, &rmqSrv)).To(Succeed())

rmq := rabbitmqv1beta1.RabbitmqCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "example-rabbit",
Expand All @@ -270,49 +251,7 @@ var _ = BeforeSuite(func() {
},
},
}
Expect(client.Create(ctx, &rmq)).To(Succeed())

rmq.Status = rabbitmqv1beta1.RabbitmqClusterStatus{
Binding: &corev1.LocalObjectReference{
Name: "example-rabbit-user-credentials",
},
DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{
ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{
Name: "example-rabbit",
Namespace: "default",
},
},
}
rmq.Status.SetConditions([]runtime.Object{})
Expect(client.Status().Update(ctx, &rmq)).To(Succeed())

rmqCreds = corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-all-rabbit-user-credentials",
Namespace: "default",
},
}
Expect(client.Create(ctx, &rmqCreds)).To(Succeed())

rmqSrv = corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-all-rabbit",
Namespace: "default",
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: 15672,
Name: "management",
},
{
Port: 15671,
Name: "management-tls",
},
},
},
}
Expect(client.Create(ctx, &rmqSrv)).To(Succeed())
Expect(createRabbitmqClusterResources(client, &rmq)).To(Succeed())

rmq = rabbitmqv1beta1.RabbitmqCluster{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -323,21 +262,7 @@ var _ = BeforeSuite(func() {
},
},
}
Expect(client.Create(ctx, &rmq)).To(Succeed())

rmq.Status = rabbitmqv1beta1.RabbitmqClusterStatus{
Binding: &corev1.LocalObjectReference{
Name: "allow-all-rabbit-user-credentials",
},
DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{
ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{
Name: "allow-all-rabbit",
Namespace: "default",
},
},
}
rmq.Status.SetConditions([]runtime.Object{})
Expect(client.Status().Update(ctx, &rmq)).To(Succeed())
Expect(createRabbitmqClusterResources(client, &rmq)).To(Succeed())

endpointsSecretBody := map[string][]byte{
"username": []byte("a-random-user"),
Expand Down Expand Up @@ -420,3 +345,62 @@ func FakeRabbitMQClientFactoryArgsForCall(i int) (map[string]string, bool, *x509
argsForCall := fakeRabbitMQClientFactoryArgsForCall[i]
return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3
}

func createRabbitmqClusterResources(client runtimeClient.Client, rabbitmqObj *rabbitmqv1beta1.RabbitmqCluster) error {
rmqCreds := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-user-credentials", rabbitmqObj.Name),
Namespace: rabbitmqObj.Namespace,
},
}
err := client.Create(ctx, &rmqCreds)
if err != nil {
return err
}

rmqSrv := corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: rabbitmqObj.Name,
Namespace: rabbitmqObj.Namespace,
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: 15672,
Name: "management",
},
{
Port: 15671,
Name: "management-tls",
},
},
},
}
err = client.Create(ctx, &rmqSrv)
if err != nil {
return err
}

err = client.Create(ctx, rabbitmqObj)
if err != nil {
return err
}

rabbitmqObj.Status = rabbitmqv1beta1.RabbitmqClusterStatus{
Binding: &corev1.LocalObjectReference{
Name: fmt.Sprintf("%s-user-credentials", rabbitmqObj.Name),
},
DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{
ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{
Name: rabbitmqObj.Name,
Namespace: rabbitmqObj.Namespace,
},
},
}
rabbitmqObj.Status.SetConditions([]runtime.Object{})
err = client.Status().Update(ctx, rabbitmqObj)
if err != nil {
return err
}
return nil
}
2 changes: 1 addition & 1 deletion controllers/super_stream_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"strconv"

"github.com/go-logr/logr"
rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1"
topologyv1alpha1 "github.com/rabbitmq/messaging-topology-operator/api/v1alpha1"
topology "github.com/rabbitmq/messaging-topology-operator/api/v1beta1"
"github.com/rabbitmq/messaging-topology-operator/internal/managedresource"
Expand Down
Loading

0 comments on commit 03f4458

Please sign in to comment.