Fix recurrent backups

pkg/controller/internal/testutil/backup.go

@@ -0,0 +1,64 @@
+/*
+Copyright 2018 Pressinfra SRL
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// nolint: golint, errcheck
+package testutil
+
+import (
+	"context"
+
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
+	gomegatypes "github.com/onsi/gomega/types"
+
+	core "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1"
+)
+
+// ListAllBackupsFn returns a helper function that can be used with gomega
+// Eventually and Consistently
+func ListAllBackupsFn(c client.Client, options *client.ListOptions) func() []api.MysqlBackup {
+	return func() []api.MysqlBackup {
+		backups := &api.MysqlBackupList{}
+		c.List(context.TODO(), options, backups)
+		return backups.Items
+	}
+}
+
+// BackupHaveCondition is a helper func that returns a matcher to check for an
+// existing condition in condition list list
+func BackupHaveCondition(condType api.BackupConditionType, status core.ConditionStatus) gomegatypes.GomegaMatcher {
+	return PointTo(MatchFields(IgnoreExtras, Fields{
+		"Status": MatchFields(IgnoreExtras, Fields{
+			"Conditions": ContainElement(MatchFields(IgnoreExtras, Fields{
+				"Type":   Equal(condType),
+				"Status": Equal(status),
+			})),
+		}),
+	}))
+}
+
+// BackupForCluster is gomega matcher that matches a backup which is for given
+// cluster
+func BackupForCluster(cluster *api.MysqlCluster) gomegatypes.GomegaMatcher {
+	return MatchFields(IgnoreExtras, Fields{
+		"Spec": MatchFields(IgnoreExtras, Fields{
+			"ClusterName": Equal(cluster.Name),
+		}),
+	})
+}

pkg/controller/internal/testutil/testutil.go

@@ -21,20 +21,15 @@ import (
 	"time"
 
 	g "github.com/onsi/gomega"
-	gs "github.com/onsi/gomega/gstruct"
-	gomegatypes "github.com/onsi/gomega/types"
 
 	// loggging
 	"github.com/go-logr/logr"
 	"github.com/go-logr/zapr"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 
-	core "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-
-	api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1"
 )
 
 const (
@@ -53,19 +48,6 @@ func DrainChan(requests <-chan reconcile.Request) {
 	}
 }
 
-// BackupHaveCondition is a helper func that returns a matcher to check for an
-// existing condition in condition list list
-func BackupHaveCondition(condType api.BackupConditionType, status core.ConditionStatus) gomegatypes.GomegaMatcher {
-	return gs.PointTo(gs.MatchFields(gs.IgnoreExtras, gs.Fields{
-		"Status": gs.MatchFields(gs.IgnoreExtras, gs.Fields{
-			"Conditions": g.ContainElement(gs.MatchFields(gs.IgnoreExtras, gs.Fields{
-				"Type":   g.Equal(condType),
-				"Status": g.Equal(status),
-			})),
-		}),
-	}))
-}
-
 // NewTestLogger returns a logger good for tests
 func NewTestLogger(w io.Writer, options ...zap.Option) logr.Logger {
 	encoderCfg := zapcore.EncoderConfig{

pkg/controller/mysqlbackupcron/job_backup.go

@@ -77,7 +77,8 @@ func (j job) Run() {
 			var err error
 			cluster := &api.MysqlBackup{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: backupName,
+					Name:      backupName,
+					Namespace: j.Namespace,
 					Labels: map[string]string{
 						"recurrent": "true",
 					},
@@ -89,14 +90,16 @@ func (j job) Run() {
 			if err = j.c.Create(context.TODO(), cluster); err == nil {
 				break
 			}
-			log.V(1).Info("failed to create backup", "backup", backupName, "error", err)
 
 			if tries > 5 {
 				log.Error(err, "fail to create backup, max tries exeded",
 					"cluster", j.Name, "retries", tries, "backup", backupName)
 				return false
 			}
 
+			log.Info("failed to create backup, retring", "backup", backupName,
+				"error", err, "tries", tries)
+
 			time.Sleep(5 * time.Second)
 			tries++
 		}

pkg/controller/mysqlbackupcron/mysqlbackupcron_controller_suite_test.go

@@ -28,8 +28,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
 	"github.com/presslabs/mysql-operator/pkg/apis"
+	"github.com/presslabs/mysql-operator/pkg/controller/internal/testutil"
 )
 
 var cfg *rest.Config
@@ -43,6 +45,8 @@ func TestMysqlBackupController(t *testing.T) {
 var _ = BeforeSuite(func() {
 	var err error
 
+	logf.SetLogger(testutil.NewTestLogger(GinkgoWriter))
+
 	t = &envtest.Environment{
 		CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crds")},
 	}

pkg/controller/mysqlbackupcron/mysqlbackupcron_controller_test.go

@@ -30,12 +30,14 @@ import (
 	cronpkg "github.com/wgliang/cron"
 	"golang.org/x/net/context"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"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"
 )
 
 const timeout = time.Second * 2
@@ -56,8 +58,13 @@ var _ = Describe("MysqlBackupCron controller", func() {
 		var recFn reconcile.Reconciler
 		cron = cronpkg.New()
 
+		// start the cron here instead of using manager to start it because of a
+		// DATA RACE happens when in Start() and Entris() methods.
+		// Expect(mgr.Add(sscron)).To(Succeed())
+		cron.Start()
+
 		mgr, err := manager.New(cfg, manager.Options{})
-		Expect(err).NotTo(HaveOccurred())
+		Expect(err).To(Succeed())
 		c = mgr.GetClient()
 
 		recFn, requests = SetupTestReconcile(newReconciler(mgr, cron))
@@ -95,7 +102,7 @@ var _ = Describe("MysqlBackupCron controller", func() {
 				Replicas:   &two,
 				SecretName: "a-secret",
 
-				BackupSchedule:   "* * * * *",
+				BackupSchedule:   "0 0 0 * *",
 				BackupSecretName: "a-backup-secret",
 				BackupURL:        "gs://bucket/",
 			},
@@ -177,6 +184,40 @@ var _ = Describe("MysqlBackupCron controller", func() {
 				}),
 			}))))
 		})
+
+		When("backup is executed once per second", func() {
+			var (
+				timeout = 5 * time.Second
+			)
+
+			BeforeEach(func() {
+				// update cluster scheduler to run every second
+				cluster.Spec.BackupSchedule = "* * * * * *"
+				Expect(c.Update(context.TODO(), cluster)).To(Succeed())
+			})
+
+			AfterEach(func() {
+				// delete all created backups
+				lo := &client.ListOptions{}
+				for _, b := range testutil.ListAllBackupsFn(c, lo)() {
+					c.Delete(context.TODO(), &b)
+				}
+			})
+
+			It("should create the mysqlbackup", func() {
+				lo := &client.ListOptions{
+					LabelSelector: labels.SelectorFromSet(labels.Set{
+						"recurrent": "true",
+					}),
+					Namespace: cluster.Namespace,
+				}
+				Eventually(testutil.ListAllBackupsFn(c, lo), timeout).Should(
+					ContainElement(testutil.BackupForCluster(cluster)))
+
+				// it should have only a backup created
+				Consistently(testutil.ListAllBackupsFn(c, lo), "2s").Should(HaveLen(1))
+			})
+		})
 	})
 })
 

pkg/controller/orchestrator/orchestrator_controller_test.go

@@ -137,14 +137,6 @@ var _ = Describe("Orchestrator controller", func() {
 			By("wait for a first reconcile event")
 			// this is a sincronization event
 			Eventually(requests, 4*time.Second).Should(Receive(Equal(expectedRequest)))
-
-			// expect to not receive any event when a cluster is created, but
-			// just after reconcile time passed then receive a reconcile event
-			Consistently(requests, noReconcileTime).ShouldNot(Receive(Equal(expectedRequest)))
-
-			By("waiting a reconcile event")
-			Eventually(requests, reconcileTimeout).Should(Receive(Equal(expectedRequest)))
-
 		})
 
 		AfterEach(func() {
@@ -160,7 +152,11 @@ var _ = Describe("Orchestrator controller", func() {
 		})
 
 		It("should trigger reconciliation after noReconcileTime", func() {
+			// expect to not receive any event when a cluster is created, but
+			// just after reconcile time passed then receive a reconcile event
 			Consistently(requests, noReconcileTime).ShouldNot(Receive(Equal(expectedRequest)))
+
+			// wait for the second request
 			Eventually(requests, reconcileTimeout).Should(Receive(Equal(expectedRequest)))
 		})