fix tests

Kuadrant · Jul 19, 2021 · 4868802 · 4868802
1 parent a2b76bd
commit 4868802
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 58 deletions.
diff --git a/Makefile b/Makefile
@@ -33,7 +33,7 @@ ENVTEST_ASSETS_DIR = $(shell pwd)/testbin
 test: generate fmt vet manifests
 	mkdir -p $(ENVTEST_ASSETS_DIR)
 	test -f $(ENVTEST_ASSETS_DIR)/setup-envtest.sh || curl -sSLo $(ENVTEST_ASSETS_DIR)/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.6.3/hack/setup-envtest.sh
-	source $(ENVTEST_ASSETS_DIR)/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out
+	source $(ENVTEST_ASSETS_DIR)/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out -v
 
 # Build manager binary
 manager: generate fmt vet

diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go
@@ -45,8 +45,8 @@ func (r *LimitadorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	reqLogger := r.Log.WithValues("limitador", req.NamespacedName)
 
 	// Delete Limitador deployment and service if needed
-	limitadorObj := limitadorv1alpha1.Limitador{}
-	if err := r.Get(context.TODO(), req.NamespacedName, &limitadorObj); err != nil {
+	limitadorObj := &limitadorv1alpha1.Limitador{}
+	if err := r.Get(context.TODO(), req.NamespacedName, limitadorObj); err != nil {
 		if errors.IsNotFound(err) {
 			// The deployment and the service should be deleted automatically
 			// because they have an owner ref to Limitador
@@ -58,17 +58,19 @@ func (r *LimitadorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	if limitadorObj.GetDeletionTimestamp() != nil { // Marked to be deleted
+		reqLogger.V(1).Info("marked to be deleted")
 		return ctrl.Result{}, nil
 	}
 
-	if err := r.ensureLimitadorServiceExists(&limitadorObj); err != nil {
+	err := r.ensureLimitadorServiceExists(limitadorObj)
+	reqLogger.V(1).Info("reconcile service", "error", err)
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	desiredDeployment := limitador.LimitadorDeployment(&limitadorObj)
-
-	if err := r.reconcileDeployment(desiredDeployment); err != nil {
-		reqLogger.Error(err, "Failed to update Limitador deployment.")
+	err = r.reconcileDeployment(limitador.LimitadorDeployment(limitadorObj))
+	reqLogger.V(1).Info("reconcile deployment", "error", err)
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 

diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go
@@ -2,7 +2,6 @@ package controllers
 
 import (
 	"context"
-	limitadorv1alpha1 "github.com/3scale/limitador-operator/api/v1alpha1"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
@@ -13,6 +12,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"time"
+
+	limitadorv1alpha1 "github.com/3scale/limitador-operator/api/v1alpha1"
+	"github.com/3scale/limitador-operator/pkg/limitador"
 )
 
 var _ = Describe("Limitador controller", func() {
@@ -28,11 +30,11 @@ var _ = Describe("Limitador controller", func() {
 
 	replicas := LimitadorReplicas
 	version := LimitadorVersion
-	newLimitador := func() limitadorv1alpha1.Limitador {
+	newLimitador := func() *limitadorv1alpha1.Limitador {
 		// The name can't start with a number.
 		name := "a" + string(uuid.NewUUID())
 
-		return limitadorv1alpha1.Limitador{
+		return &limitadorv1alpha1.Limitador{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "Limitador",
 				APIVersion: "limitador.3scale.net/v1alpha1",
@@ -51,14 +53,14 @@ var _ = Describe("Limitador controller", func() {
 	deletePropagationPolicy := client.PropagationPolicy(metav1.DeletePropagationForeground)
 
 	Context("Creating a new Limitador object", func() {
-		var limitadorObj limitadorv1alpha1.Limitador
+		var limitadorObj *limitadorv1alpha1.Limitador
 
 		BeforeEach(func() {
 			limitadorObj = newLimitador()
-			err := k8sClient.Delete(context.TODO(), limitadorObj.DeepCopy(), deletePropagationPolicy)
+			err := k8sClient.Delete(context.TODO(), limitadorObj, deletePropagationPolicy)
 			Expect(err == nil || errors.IsNotFound(err))
 
-			Expect(k8sClient.Create(context.TODO(), limitadorObj.DeepCopy())).Should(Succeed())
+			Expect(k8sClient.Create(context.TODO(), limitadorObj)).Should(Succeed())
 		})
 
 		It("Should create a new deployment with the right number of replicas and version", func() {
@@ -90,7 +92,7 @@ var _ = Describe("Limitador controller", func() {
 					context.TODO(),
 					types.NamespacedName{
 						Namespace: LimitadorNamespace,
-						Name:      "limitador", // Hardcoded for now
+						Name:      limitador.ServiceName, // Hardcoded for now
 					},
 					&createdLimitadorService)
 
@@ -99,55 +101,15 @@ var _ = Describe("Limitador controller", func() {
 		})
 	})
 
-	Context("Deleting a Limitador object", func() {
-		var limitadorObj limitadorv1alpha1.Limitador
-
-		BeforeEach(func() {
-			limitadorObj = newLimitador()
-			Expect(k8sClient.Create(context.TODO(), limitadorObj.DeepCopy())).Should(Succeed())
-			Expect(k8sClient.Delete(context.TODO(), limitadorObj.DeepCopy(), deletePropagationPolicy)).Should(Succeed())
-		})
-
-		It("Should delete the limitador deployment", func() {
-			createdLimitadorDeployment := appsv1.Deployment{}
-			Eventually(func() bool {
-				err := k8sClient.Get(
-					context.TODO(),
-					types.NamespacedName{
-						Namespace: LimitadorNamespace,
-						Name:      limitadorObj.Name,
-					},
-					&createdLimitadorDeployment)
-
-				return errors.IsNotFound(err)
-			}, timeout, interval).Should(BeTrue())
-		})
-
-		It("Should delete the limitador service", func() {
-			createdLimitadorService := v1.Service{}
-			Eventually(func() bool {
-				err := k8sClient.Get(
-					context.TODO(),
-					types.NamespacedName{
-						Namespace: LimitadorNamespace,
-						Name:      "limitador", // Hardcoded for now
-					},
-					&createdLimitadorService)
-
-				return errors.IsNotFound(err)
-			}, timeout, interval).Should(BeTrue())
-		})
-	})
-
 	Context("Updating a limitador object", func() {
-		var limitadorObj limitadorv1alpha1.Limitador
+		var limitadorObj *limitadorv1alpha1.Limitador
 
 		BeforeEach(func() {
 			limitadorObj = newLimitador()
-			err := k8sClient.Delete(context.TODO(), limitadorObj.DeepCopy(), deletePropagationPolicy)
+			err := k8sClient.Delete(context.TODO(), limitadorObj, deletePropagationPolicy)
 			Expect(err == nil || errors.IsNotFound(err))
 
-			Expect(k8sClient.Create(context.TODO(), limitadorObj.DeepCopy())).Should(Succeed())
+			Expect(k8sClient.Create(context.TODO(), limitadorObj)).Should(Succeed())
 		})
 
 		It("Should modify the limitador deployment", func() {