Fix typos and make tests more stable

pkg/controller/mysqlbackup/mysqlbackup_controller_test.go

@@ -138,16 +138,19 @@ var _ = Describe("MysqlBackup controller", func() {
 
 			// some extra reconcile requests may appear
 			testutil.DrainChan(requests)
-			// We need to make sure that the controller does not create infinite
-			// loops
-			Consistently(requests, timeout).ShouldNot(Receive(Equal(expectedRequest)))
 		})
 
 		AfterEach(func() {
 			Expect(c.Delete(context.TODO(), cluster.Unwrap())).To(Succeed())
 			Expect(c.Delete(context.TODO(), backup.Unwrap())).To(Succeed())
 		})
 
+		It("should have only one reconcile request", func() {
+			// We need to make sure that the controller does not create infinite
+			// loops
+			Consistently(requests, 5*time.Second).ShouldNot(Receive(Equal(expectedRequest)))
+		})
+
 		It("should create the job", func() {
 			job := &batch.Job{}
 			Expect(c.Get(context.TODO(), jobKey, job)).To(Succeed())

pkg/controller/mysqlcluster/internal/cleaner/pvc.go

@@ -34,10 +34,10 @@ import (
 )
 
 const (
-	reasonPVCClinupSuccess = "SucessfulPVCClinup"
-	reasonPVCClinupFail    = "FailedPVCClinup"
-	messagePVCDeleted      = "delete Claim %s in StatefulSet %s successful"
-	messagePVCNotDeleted   = "delete Claim %s in StatefulSet %s failed"
+	reasonPVCCleanupSuccessfull = "SucessfulPVCCleanup"
+	reasonPVCCleanupFailed      = "FailedPVCCleanup"
+	messageCleanupSuccessfull   = "delete Claim %s in StatefulSet %s successful"
+	messageCleanupFailed        = "delete Claim %s in StatefulSet %s failed"
 )
 
 var log = logf.Log.WithName("mysqlcluster.pvccleaner")
@@ -71,38 +71,40 @@ func (p *PVCCleaner) Run(ctx context.Context) error {
 	}
 
 	// Find any pvcs with higher ordinal than replicas and delete them
-	claims, err := p.getClaims(ctx)
+	pvcs, err := p.getPVCs(ctx)
 	if err != nil {
 		return err
 	}
 
-	for _, claim := range claims {
-		ord := getOrdinal(claim.Name)
-		if ord >= *cluster.Spec.Replicas && ord != 0 {
-			log.Info("cleanning up PVC", "pvc", claim)
-			if err := p.deleteClaim(ctx, &claim); err != nil {
+	for _, pvc := range pvcs {
+		ord, parseErr := getOrdinal(pvc.Name)
+		if parseErr == nil && ord >= *cluster.Spec.Replicas && ord != 0 {
+			log.Info("cleaning up PVC", "pvc", pvc)
+			if err := p.deletePVC(ctx, &pvc); err != nil {
 				return err
 			}
+		} else if parseErr != nil {
+			log.Error(parseErr, "pvc deletion error")
 		}
 	}
 
 	return nil
 }
 
-func (p *PVCCleaner) deleteClaim(ctx context.Context, pvc *core.PersistentVolumeClaim) error {
+func (p *PVCCleaner) deletePVC(ctx context.Context, pvc *core.PersistentVolumeClaim) error {
 	err := p.client.Delete(ctx, pvc)
 	if err != nil {
-		p.recorder.Event(p.cluster, core.EventTypeWarning, reasonPVCClinupFail,
-			fmt.Sprintf(messagePVCNotDeleted, pvc.Name, p.cluster.Name))
+		p.recorder.Event(p.cluster, core.EventTypeWarning, reasonPVCCleanupFailed,
+			fmt.Sprintf(messageCleanupFailed, pvc.Name, p.cluster.Name))
 		return err
 	}
 
-	p.recorder.Event(p.cluster, core.EventTypeNormal, reasonPVCClinupSuccess,
-		fmt.Sprintf(messagePVCDeleted, pvc.Name, p.cluster.Name))
+	p.recorder.Event(p.cluster, core.EventTypeNormal, reasonPVCCleanupSuccessfull,
+		fmt.Sprintf(messageCleanupSuccessfull, pvc.Name, p.cluster.Name))
 	return nil
 }
 
-func (p *PVCCleaner) getClaims(ctx context.Context) ([]core.PersistentVolumeClaim, error) {
+func (p *PVCCleaner) getPVCs(ctx context.Context) ([]core.PersistentVolumeClaim, error) {
 	pvcs := &core.PersistentVolumeClaimList{}
 	lo := &client.ListOptions{
 		Namespace:     p.cluster.Namespace,
@@ -131,6 +133,11 @@ func (p *PVCCleaner) getClaims(ctx context.Context) ([]core.PersistentVolumeClai
 }
 
 func isOwnedBy(pvc core.PersistentVolumeClaim, cluster *api.MysqlCluster) bool {
+	if pvc.Namespace != cluster.Namespace {
+		// check is that cluster is in the same namespace
+		return false
+	}
+
 	for _, ref := range pvc.ObjectMeta.GetOwnerReferences() {
 		if ref.Kind == "MysqlCluster" && ref.Name == cluster.Name {
 			return true
@@ -141,17 +148,16 @@ func isOwnedBy(pvc core.PersistentVolumeClaim, cluster *api.MysqlCluster) bool {
 	return false
 }
 
-func getOrdinal(name string) int32 {
+func getOrdinal(name string) (int32, error) {
 	idx := strings.LastIndexAny(name, "-")
 	if idx == -1 {
-		log.Error(nil, "failed to extract ordinal for pvc", "pvc", name)
-		return -1 // not a good name for PVC
+		return -1, fmt.Errorf("failed to extract ordinal from pvc name: %s", name)
 	}
 
 	ordinal, err := strconv.Atoi(name[idx+1:])
 	if err != nil {
 		log.Error(err, "failed to extract ordinal for pvc", "pvc", name)
-		return -1 // not a good name for PVC
+		return -1, fmt.Errorf("failed to extract ordinal from pvc name: %s", name)
 	}
-	return int32(ordinal)
+	return int32(ordinal), nil
 }

pkg/controller/mysqlcluster/internal/cleaner/pvc_test.go

@@ -36,7 +36,7 @@ import (
 	"github.com/presslabs/mysql-operator/pkg/options"
 )
 
-var _ = Describe("Pvc cleaner", func() {
+var _ = Describe("PVC cleaner", func() {
 	var (
 		cluster *mysqlcluster.MysqlCluster
 		rec     *record.FakeRecorder
@@ -131,18 +131,19 @@ var _ = Describe("Pvc cleaner", func() {
 		})
 
 		It("should remove extra PVCs when cluster is scaled down", func() {
+			// assert that here are multiple PVCs than needed
+			Expect(listClaimsForCluster(c, cluster).Items).To(HaveLen(5))
+
 			// run cleaner
 			pvcCleaner := NewPVCCleaner(cluster, options.GetOptions(), rec, c)
 			Expect(pvcCleaner.Run(context.TODO())).To(Succeed())
 
-			pvcs2 := listClaimsForCluster(c, cluster)
-			Expect(pvcs2.Items).To(HaveLen(3))
+			Expect(listClaimsForCluster(c, cluster).Items).To(HaveLen(3))
 
-			// run cleaner again, should nothing changes
+			// run cleaner again, should result in no changes
 			Expect(pvcCleaner.Run(context.TODO())).To(Succeed())
 
-			pvcs2 = listClaimsForCluster(c, cluster)
-			Expect(pvcs2.Items).To(HaveLen(3))
+			Expect(listClaimsForCluster(c, cluster).Items).To(HaveLen(3))
 		})
 
 		It("should not remove pvc with 0 index", func() {

pkg/controller/mysqlcluster/mysqlcluster_controller_test.go

@@ -39,6 +39,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1"
+	"github.com/presslabs/mysql-operator/pkg/controller/internal/testutil"
 	"github.com/presslabs/mysql-operator/pkg/internal/mysqlcluster"
 )
 
@@ -163,20 +164,8 @@ var _ = Describe("MysqlCluster controller", func() {
 			Eventually(requests, timeout).Should(Receive(Equal(expectedRequest)))
 
 			// some extra reconcile requests may appear
-		drain:
-			for {
-				select {
-				case <-requests:
-					continue
-				case <-time.After(200 * time.Millisecond):
-					break drain
-				}
-			}
+			testutil.DrainChan(requests)
 
-			// We need to make sure that the controller does not create infinite
-			// loops
-			By("wait for no more reconcile requests")
-			Consistently(requests).ShouldNot(Receive(Equal(expectedRequest)))
 		})
 
 		AfterEach(func() {
@@ -186,6 +175,12 @@ var _ = Describe("MysqlCluster controller", func() {
 			c.Delete(context.TODO(), cluster.Unwrap())
 		})
 
+		It("should have only one reconcile request", func() {
+			// We need to make sure that the controller does not create infinite
+			// loops
+			Consistently(requests, 5*time.Second).ShouldNot(Receive(Equal(expectedRequest)))
+		})
+
 		DescribeTable("the reconciler",
 			func(nameFmt string, obj runtime.Object) {
 				key := types.NamespacedName{