From 47f235188c261bdaf37f6d5a2308e4c78f4ae862 Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Tue, 6 Jul 2021 17:08:23 +0900 Subject: [PATCH 1/4] Fix delete BackupPolicy error Signed-off-by: Yusuke Suzuki --- api/v1beta1/backuppolicy_webhook.go | 5 +++- api/v1beta1/backuppolicy_webhook_test.go | 35 ++++++++++++++++++++++-- api/v1beta1/mysqlcluster_webhook_test.go | 34 ++++++++++++++++------- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/api/v1beta1/backuppolicy_webhook.go b/api/v1beta1/backuppolicy_webhook.go index 54307b5fb..76cce1c41 100644 --- a/api/v1beta1/backuppolicy_webhook.go +++ b/api/v1beta1/backuppolicy_webhook.go @@ -53,7 +53,10 @@ func (r *BackupPolicy) ValidateDelete() error { if cluster.Spec.BackupPolicyName == nil { continue } - return fmt.Errorf("MySQLCluster %s/%s has a reference to this policy", cluster.Namespace, cluster.Name) + + if *cluster.Spec.BackupPolicyName == r.Name { + return fmt.Errorf("MySQLCluster %s/%s has a reference to this policy", cluster.Namespace, cluster.Name) + } } return nil } diff --git a/api/v1beta1/backuppolicy_webhook_test.go b/api/v1beta1/backuppolicy_webhook_test.go index 4171541c4..046af77cc 100644 --- a/api/v1beta1/backuppolicy_webhook_test.go +++ b/api/v1beta1/backuppolicy_webhook_test.go @@ -2,7 +2,6 @@ package v1beta1 import ( "context" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" batchv1beta1 "k8s.io/api/batch/v1beta1" @@ -25,8 +24,11 @@ var _ = Describe("BackupPolicy Webhook", func() { ctx := context.TODO() BeforeEach(func() { + err := deleteMySQLCluster() + Expect(err).NotTo(HaveOccurred()) + r := &BackupPolicy{} - err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "test"}, r) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "test"}, r) if apierrors.IsNotFound(err) { return } @@ -74,4 +76,33 @@ var _ = Describe("BackupPolicy Webhook", func() { err := k8sClient.Create(ctx, r) Expect(err).To(HaveOccurred()) }) + + It("should delete BackupPolicy", func() { + cluster := makeMySQLCluster() + cluster.Spec.BackupPolicyName = pointer.String("no-test") + err := k8sClient.Create(ctx, cluster) + Expect(err).NotTo(HaveOccurred()) + + backup := makeBackupPolicy() + err = k8sClient.Create(ctx, backup) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Delete(ctx, backup) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should NOT delete BackupPolicy which is referenced by MySQLCluster", func() { + cluster := makeMySQLCluster() + cluster.Spec.BackupPolicyName = pointer.String("test") + err := k8sClient.Create(ctx, cluster) + Expect(err).NotTo(HaveOccurred()) + + backup := makeBackupPolicy() + err = k8sClient.Create(ctx, backup) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Delete(ctx, backup) + Expect(err).To(HaveOccurred()) + }) + }) diff --git a/api/v1beta1/mysqlcluster_webhook_test.go b/api/v1beta1/mysqlcluster_webhook_test.go index d431dc161..9c5299db6 100644 --- a/api/v1beta1/mysqlcluster_webhook_test.go +++ b/api/v1beta1/mysqlcluster_webhook_test.go @@ -41,20 +41,34 @@ func makeMySQLCluster() *MySQLCluster { } } +func deleteMySQLCluster() error { + r := &MySQLCluster{} + err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "test"}, r) + if apierrors.IsNotFound(err) { + return nil + } + + if err != nil { + return err + } + + r.Finalizers = nil + if err := k8sClient.Update(ctx, r); err != nil { + return err + } + + if err := k8sClient.Delete(ctx, r); err != nil { + return err + } + + return nil +} + var _ = Describe("MySQLCluster Webhook", func() { ctx := context.TODO() BeforeEach(func() { - r := &MySQLCluster{} - err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "test"}, r) - if apierrors.IsNotFound(err) { - return - } - Expect(err).NotTo(HaveOccurred()) - r.Finalizers = nil - err = k8sClient.Update(ctx, r) - Expect(err).NotTo(HaveOccurred()) - err = k8sClient.Delete(ctx, r) + err := deleteMySQLCluster() Expect(err).NotTo(HaveOccurred()) }) From 11f924c82940dd8629c209daddff62d6c9bcda6c Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Tue, 6 Jul 2021 21:08:50 +0900 Subject: [PATCH 2/4] Fixed not to be affected by the system time zone Signed-off-by: Yusuke Suzuki --- backup/backup.go | 2 +- backup/integration_test.go | 2 +- controllers/mysqlcluster_controller_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backup/backup.go b/backup/backup.go index b45e06ffc..8dddaa528 100644 --- a/backup/backup.go +++ b/backup/backup.go @@ -418,7 +418,7 @@ func (bm *BackupManager) backupBinlog(ctx context.Context, op bkop.Operator) err pw2 = nil bw := &ByteCountWriter{} - key := calcKey(bm.cluster.Namespace, bm.cluster.Name, constants.BinlogFilename, lastBackup.Time.Time) + key := calcKey(bm.cluster.Namespace, bm.cluster.Name, constants.BinlogFilename, lastBackup.Time.Time.UTC()) if err := bm.bucket.Put(ctx, key, io.TeeReader(pr2, bw)); err != nil { return fmt.Errorf("failed to put binlog.tar.zst: %w", err) } diff --git a/backup/integration_test.go b/backup/integration_test.go index ea3751ed6..c5361ed24 100644 --- a/backup/integration_test.go +++ b/backup/integration_test.go @@ -194,7 +194,7 @@ var _ = Describe("Backup/Restore", func() { Expect(bc.contents).To(HaveLen(1)) time.Sleep(1100 * time.Millisecond) - restorePoint := time.Now() + restorePoint := time.Now().UTC() time.Sleep(1100 * time.Millisecond) newOperator = func(host string, port int, user, password string, threads int) (bkop.Operator, error) { diff --git a/controllers/mysqlcluster_controller_test.go b/controllers/mysqlcluster_controller_test.go index 20b32051b..23a9d3dad 100644 --- a/controllers/mysqlcluster_controller_test.go +++ b/controllers/mysqlcluster_controller_test.go @@ -1150,7 +1150,7 @@ var _ = Describe("MySQLCluster reconciler", func() { "single", "test", "test", - now.Format(constants.BackupTimeFormat), + now.UTC().Format(constants.BackupTimeFormat), })) Expect(c.EnvFrom).To(HaveLen(1)) Expect(c.Env).To(HaveLen(2)) From 4b8905db7525ef093fd9032f2daf2d3facc6ec70 Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Tue, 6 Jul 2021 21:22:05 +0900 Subject: [PATCH 3/4] Fix wrong format Signed-off-by: Yusuke Suzuki --- api/v1beta1/backuppolicy_webhook_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/v1beta1/backuppolicy_webhook_test.go b/api/v1beta1/backuppolicy_webhook_test.go index 046af77cc..46b394b58 100644 --- a/api/v1beta1/backuppolicy_webhook_test.go +++ b/api/v1beta1/backuppolicy_webhook_test.go @@ -2,6 +2,7 @@ package v1beta1 import ( "context" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" batchv1beta1 "k8s.io/api/batch/v1beta1" From 2ca8860601ef067d1fdb23d91edb9534716906ba Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Wed, 7 Jul 2021 08:35:32 +0900 Subject: [PATCH 4/4] Reflect review comments Signed-off-by: Yusuke Suzuki --- backup/backup.go | 2 +- backup/integration_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backup/backup.go b/backup/backup.go index 8dddaa528..b45e06ffc 100644 --- a/backup/backup.go +++ b/backup/backup.go @@ -418,7 +418,7 @@ func (bm *BackupManager) backupBinlog(ctx context.Context, op bkop.Operator) err pw2 = nil bw := &ByteCountWriter{} - key := calcKey(bm.cluster.Namespace, bm.cluster.Name, constants.BinlogFilename, lastBackup.Time.Time.UTC()) + key := calcKey(bm.cluster.Namespace, bm.cluster.Name, constants.BinlogFilename, lastBackup.Time.Time) if err := bm.bucket.Put(ctx, key, io.TeeReader(pr2, bw)); err != nil { return fmt.Errorf("failed to put binlog.tar.zst: %w", err) } diff --git a/backup/integration_test.go b/backup/integration_test.go index c5361ed24..ea3751ed6 100644 --- a/backup/integration_test.go +++ b/backup/integration_test.go @@ -194,7 +194,7 @@ var _ = Describe("Backup/Restore", func() { Expect(bc.contents).To(HaveLen(1)) time.Sleep(1100 * time.Millisecond) - restorePoint := time.Now().UTC() + restorePoint := time.Now() time.Sleep(1100 * time.Millisecond) newOperator = func(host string, port int, user, password string, threads int) (bkop.Operator, error) {