diff --git a/.drone.yml b/.drone.yml index 9923b893b..ce117145d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -153,7 +153,7 @@ pipeline: # start-kubernetes: - image: quay.io/presslabs/kluster-toolbox + image: quay.io/presslabs/bfc group: publish secrets: - GOOGLE_CREDENTIALS @@ -199,7 +199,7 @@ pipeline: event: push stop-kubernetes: - image: quay.io/presslabs/kluster-toolbox + image: quay.io/presslabs/bfc secrets: - GOOGLE_CREDENTIALS environment: diff --git a/config/crds/mysql_v1alpha1_mysqlbackup.yaml b/config/crds/mysql_v1alpha1_mysqlbackup.yaml index 60258f516..66d3b5fbb 100644 --- a/config/crds/mysql_v1alpha1_mysqlbackup.yaml +++ b/config/crds/mysql_v1alpha1_mysqlbackup.yaml @@ -45,9 +45,9 @@ spec: clusterName: description: ClustterName represents the cluster for which to take backup type: string - deletePolicy: - description: DeletePolicy the deletion policy that specify how to treat - the data from remote storage. By default it's used softDelete. + remoteDeletePolicy: + description: RemoteDeletePolicy the deletion policy that specify how + to treat the data from remote storage. By default it's used softDelete. type: string required: - clusterName diff --git a/examples/example-backup.yaml b/examples/example-backup.yaml index ab0680321..8af9f0b2d 100644 --- a/examples/example-backup.yaml +++ b/examples/example-backup.yaml @@ -15,3 +15,6 @@ spec: ## specify a secret where to find credentials to access the ## bucket # backupSecretName: backup-secret + + ## specify the remote deletion policy. It can be on of ["retain", "delete"] + # remoteDeletePolicy: retain diff --git a/examples/example-cluster-secret.yaml b/examples/example-cluster-secret.yaml index dda1a9266..f8f943ab3 100644 --- a/examples/example-cluster-secret.yaml +++ b/examples/example-cluster-secret.yaml @@ -6,3 +6,7 @@ type: Opaque data: # root password is required to be specified ROOT_PASSWORD: bXlwYXNz + ## application credentials that will be created at cluster bootstrap + # DATABASE: + # USER: + # PASSWORD: diff --git a/pkg/apis/mysql/v1alpha1/mysqlbackup_defaults.go b/pkg/apis/mysql/v1alpha1/mysqlbackup_defaults.go index 8b51ef8c4..e9b28e123 100644 --- a/pkg/apis/mysql/v1alpha1/mysqlbackup_defaults.go +++ b/pkg/apis/mysql/v1alpha1/mysqlbackup_defaults.go @@ -19,7 +19,7 @@ package v1alpha1 // SetDefaults_MysqlBackup sets the defaults for a mysqlbackup object // nolint: golint func SetDefaults_MysqlBackup(b *MysqlBackup) { - if len(b.Spec.DeletePolicy) == 0 { - b.Spec.DeletePolicy = SoftDelete + if len(b.Spec.RemoteDeletePolicy) == 0 { + b.Spec.RemoteDeletePolicy = Retain } } diff --git a/pkg/apis/mysql/v1alpha1/mysqlbackup_types.go b/pkg/apis/mysql/v1alpha1/mysqlbackup_types.go index 7aa164355..ab9b1b885 100644 --- a/pkg/apis/mysql/v1alpha1/mysqlbackup_types.go +++ b/pkg/apis/mysql/v1alpha1/mysqlbackup_types.go @@ -46,10 +46,10 @@ type MysqlBackupSpec struct { // +optional BackupSecretName string `json:"backupSecretName,omitempty"` - // DeletePolicy the deletion policy that specify how to treat the data from remote storage. By + // RemoteDeletePolicy the deletion policy that specify how to treat the data from remote storage. By // default it's used softDelete. // +optional - DeletePolicy DeletePolicy `json:"deletePolicy,omitempty"` + RemoteDeletePolicy DeletePolicy `json:"remoteDeletePolicy,omitempty"` } // BackupCondition defines condition struct for backup resource @@ -81,11 +81,12 @@ const ( type DeletePolicy string const ( - // HardDelete when used it will delete the backup from remote storage then will remove the - // MysqlBackup resource from Kubernetes. - HardDelete DeletePolicy = "hardDelete" - // SoftDelete when used it will delete only the MysqlBackup resource from Kuberentes. - SoftDelete DeletePolicy = "softDelete" + // Delete when used it will try to delete the backup from remote storage then will remove the + // MysqlBackup resource from Kubernetes. The remote deletion is not guaranteed that will succeed. + Delete DeletePolicy = "delete" + // Retain when used it will delete only the MysqlBackup resource from Kuberentes and will keep the backup + // on remote storage. + Retain DeletePolicy = "retain" ) // MysqlBackupStatus defines the observed state of MysqlBackup diff --git a/pkg/controller/mysqlbackup/internal/syncer/deletionJob.go b/pkg/controller/mysqlbackup/internal/syncer/deletionjob.go similarity index 66% rename from pkg/controller/mysqlbackup/internal/syncer/deletionJob.go rename to pkg/controller/mysqlbackup/internal/syncer/deletionjob.go index 0de78bda6..00a003232 100644 --- a/pkg/controller/mysqlbackup/internal/syncer/deletionJob.go +++ b/pkg/controller/mysqlbackup/internal/syncer/deletionjob.go @@ -17,34 +17,46 @@ limitations under the License. package syncer import ( + "fmt" + "strings" + "github.com/imdario/mergo" "github.com/presslabs/controller-util/mergo/transformers" "github.com/presslabs/controller-util/syncer" - "github.com/presslabs/mysql-operator/pkg/internal/mysqlbackup" batch "k8s.io/api/batch/v1" core "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1" + "github.com/presslabs/mysql-operator/pkg/internal/mysqlbackup" + "github.com/presslabs/mysql-operator/pkg/internal/mysqlcluster" "github.com/presslabs/mysql-operator/pkg/options" ) const ( // RemoteStorageFinalizer is the finalizer name used when hardDelete policy is used - RemoteStorageFinalizer = "backups.mysql.presslabs.org/remote-storage" + RemoteStorageFinalizer = "backups.mysql.presslabs.org/remote-storage-cleanup" + + // RemoteDeletionFailedEvent is the event that is set on the cluster when the cleanup job fails + RemoteDeletionFailedEvent = "RemoteDeletionFailed" ) type deletionJobSyncer struct { - backup *mysqlbackup.MysqlBackup - opt *options.Options + backup *mysqlbackup.MysqlBackup + cluster *mysqlcluster.MysqlCluster + opt *options.Options + schema *runtime.Scheme + recorder record.EventRecorder } -// NewRemoteJobSyncer returns a job syncer for hard deletion job. The job which removes the backup +// NewDeleteJobSyncer returns a job syncer for hard deletion job. The job which removes the backup // from remote storage. -func NewRemoteJobSyncer(c client.Client, s *runtime.Scheme, - backup *mysqlbackup.MysqlBackup, opt *options.Options) syncer.Interface { +func NewDeleteJobSyncer(c client.Client, s *runtime.Scheme, backup *mysqlbackup.MysqlBackup, + cluster *mysqlcluster.MysqlCluster, opt *options.Options, r record.EventRecorder) syncer.Interface { job := &batch.Job{ ObjectMeta: metav1.ObjectMeta{ @@ -54,17 +66,21 @@ func NewRemoteJobSyncer(c client.Client, s *runtime.Scheme, } jobSyncer := deletionJobSyncer{ - backup: backup, - opt: opt, + cluster: cluster, + backup: backup, + opt: opt, + schema: s, + recorder: r, } - return syncer.NewObjectSyncer("Backup", backup.Unwrap(), job, c, s, jobSyncer.SyncFn) + return syncer.NewObjectSyncer("BackupCleaner", nil, job, c, s, jobSyncer.SyncFn) } +// nolint: gocyclo func (s *deletionJobSyncer) SyncFn(in runtime.Object) error { out := in.(*batch.Job) - if s.backup.Spec.DeletePolicy == api.SoftDelete { + if s.backup.Spec.RemoteDeletePolicy == api.Retain { // do nothing return syncer.ErrIgnore } @@ -83,6 +99,10 @@ func (s *deletionJobSyncer) SyncFn(in runtime.Object) error { return syncer.ErrIgnore } + if len(s.backup.Spec.BackupURL) == 0 { + return fmt.Errorf("empty .spec.backupURL") + } + // check if the job is created and if not create it if out.ObjectMeta.CreationTimestamp.IsZero() { out.Labels = map[string]string{ @@ -95,19 +115,32 @@ func (s *deletionJobSyncer) SyncFn(in runtime.Object) error { if err != nil { return err } + + // explicit set owner reference on job because the owner has set deletionTimestamp, at this point, and + // the syncer will not set it + err = controllerutil.SetControllerReference(s.backup.Unwrap(), out, s.schema) + if err != nil { + return err + } } completed, failed := getJobStatus(out) - if completed && !failed { + if completed { removeFinalizer(s.backup.Unwrap(), RemoteStorageFinalizer) } + // announce the cluster if deletion from remote storage failed + if failed { + s.recordWEventOnCluster(RemoteDeletionFailedEvent, "job failed") + } + return nil } func (s *deletionJobSyncer) ensurePodSpec() core.PodSpec { return core.PodSpec{ - Containers: s.ensureContainers(), + RestartPolicy: core.RestartPolicyNever, + Containers: s.ensureContainers(), ImagePullSecrets: []core.LocalObjectReference{ {Name: s.opt.ImagePullSecretName}, }, @@ -122,12 +155,29 @@ func (s *deletionJobSyncer) ensureContainers() []core.Container { ImagePullPolicy: s.opt.ImagePullPolicy, Args: []string{ "rclone", "--config=/etc/rclone.conf", "delete", - s.backup.Status.BackupURI, + bucketForRclone(s.backup.Spec.BackupURL), + }, + EnvFrom: []core.EnvFromSource{ + { + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{ + Name: s.backup.Spec.BackupSecretName, + }, + }, + }, }, }, } } +func (s *deletionJobSyncer) recordWEventOnCluster(reason, msg string) { + s.recorder.Eventf(s.cluster, "Warning", reason, msg) +} + +func bucketForRclone(name string) string { + return strings.Replace(name, "://", ":", 1) +} + func getJobStatus(job *batch.Job) (bool, bool) { completed := false if completCond := jobCondition(batch.JobComplete, job); completCond != nil { diff --git a/pkg/controller/mysqlbackup/internal/syncer/deletionjob_test.go b/pkg/controller/mysqlbackup/internal/syncer/deletionjob_test.go index 0c38eaee0..c8a9918f5 100644 --- a/pkg/controller/mysqlbackup/internal/syncer/deletionjob_test.go +++ b/pkg/controller/mysqlbackup/internal/syncer/deletionjob_test.go @@ -27,6 +27,8 @@ import ( batch "k8s.io/api/batch/v1" core "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1" "github.com/presslabs/mysql-operator/pkg/internal/mysqlbackup" @@ -36,14 +38,16 @@ import ( var _ = Describe("MysqlBackup remove job syncer", func() { var ( - cluster *mysqlcluster.MysqlCluster - backup *mysqlbackup.MysqlBackup - syncer *deletionJobSyncer + cluster *mysqlcluster.MysqlCluster + backup *mysqlbackup.MysqlBackup + syncer *deletionJobSyncer + recorder *record.FakeRecorder ) BeforeEach(func() { clusterName := fmt.Sprintf("cluster-%d", rand.Int31()) name := fmt.Sprintf("backup-%d", rand.Int31()) + recorder = record.NewFakeRecorder(100) ns := "default" two := int32(2) @@ -64,18 +68,21 @@ var _ = Describe("MysqlBackup remove job syncer", func() { }) syncer = &deletionJobSyncer{ - backup: backup, - opt: options.GetOptions(), + cluster: cluster, + backup: backup, + opt: options.GetOptions(), + recorder: recorder, + schema: scheme.Scheme, } }) It("should skip job creation when no needed", func() { delJob := &batch.Job{} - backup.Spec.DeletePolicy = api.SoftDelete + backup.Spec.RemoteDeletePolicy = api.Retain // skip job creation because backup is set to soft delete Expect(syncer.SyncFn(delJob)).To(Equal(syncerpkg.ErrIgnore)) - backup.Spec.DeletePolicy = api.HardDelete + backup.Spec.RemoteDeletePolicy = api.Delete // skip job creation because backup is not deleted Expect(syncer.SyncFn(delJob)).To(Equal(syncerpkg.ErrIgnore)) Expect(backup.Finalizers).To(ContainElement(RemoteStorageFinalizer)) @@ -88,9 +95,9 @@ var _ = Describe("MysqlBackup remove job syncer", func() { Expect(backup.Finalizers).ToNot(ContainElement(RemoteStorageFinalizer)) }) - It("should create the job", func() { + It("should create the job and update backup finalizer", func() { delJob := &batch.Job{} - backup.Spec.DeletePolicy = api.HardDelete + backup.Spec.RemoteDeletePolicy = api.Delete deletionTime := metav1.NewTime(time.Now()) backup.DeletionTimestamp = &deletionTime Expect(syncer.SyncFn(delJob)).To(Succeed()) @@ -110,6 +117,16 @@ var _ = Describe("MysqlBackup remove job syncer", func() { }, } Expect(syncer.SyncFn(delJob)).To(Succeed()) + Expect(backup.Finalizers).ToNot(ContainElement(RemoteStorageFinalizer)) + Expect(recorder.Events).To(Receive(ContainSubstring(RemoteDeletionFailedEvent))) + + delJob.Status.Conditions = []batch.JobCondition{ + batch.JobCondition{ + Type: batch.JobComplete, + Status: core.ConditionFalse, + }, + } + Expect(syncer.SyncFn(delJob)).To(Succeed()) Expect(backup.Finalizers).To(ContainElement(RemoteStorageFinalizer)) delJob.Status.Conditions = []batch.JobCondition{ diff --git a/pkg/controller/mysqlbackup/internal/syncer/job.go b/pkg/controller/mysqlbackup/internal/syncer/job.go index 1a3e76081..0fd72f9c4 100644 --- a/pkg/controller/mysqlbackup/internal/syncer/job.go +++ b/pkg/controller/mysqlbackup/internal/syncer/job.go @@ -63,6 +63,12 @@ func NewJobSyncer(c client.Client, s *runtime.Scheme, backup *mysqlbackup.MysqlB func (s *jobSyncer) SyncFn(in runtime.Object) error { out := in.(*batch.Job) + if s.backup.Status.Completed { + log.V(1).Info("backup already completed", "name", s.backup.Name) + // skip doing anything + return syncer.ErrIgnore + } + if len(s.backup.GetBackupURL(s.cluster)) == 0 { log.Info("can't get bucketURI", "cluster", s.cluster, "backup", s.backup) return fmt.Errorf("can't get bucketURI") @@ -82,14 +88,6 @@ func (s *jobSyncer) SyncFn(in runtime.Object) error { return nil } -func (s *jobSyncer) getBackupSecretName() string { - if len(s.backup.Spec.BackupSecretName) > 0 { - return s.backup.Spec.BackupSecretName - } - - return s.cluster.Spec.BackupSecretName -} - // getBackupCandidate returns the hostname of the first not-lagged and // replicating slave node, else returns the master node. func (s *jobSyncer) getBackupCandidate() string { @@ -163,12 +161,12 @@ func (s *jobSyncer) ensurePodSpec(in core.PodSpec) core.PodSpec { }, } - if len(s.getBackupSecretName()) != 0 { + if len(s.backup.Spec.BackupSecretName) != 0 { in.Containers[0].EnvFrom = []core.EnvFromSource{ core.EnvFromSource{ SecretRef: &core.SecretEnvSource{ LocalObjectReference: core.LocalObjectReference{ - Name: s.getBackupSecretName(), + Name: s.backup.Spec.BackupSecretName, }, }, }, diff --git a/pkg/controller/mysqlbackup/internal/syncer/syncer_suite_test.go b/pkg/controller/mysqlbackup/internal/syncer/syncer_suite_test.go index 9f65c8c82..02db767aa 100644 --- a/pkg/controller/mysqlbackup/internal/syncer/syncer_suite_test.go +++ b/pkg/controller/mysqlbackup/internal/syncer/syncer_suite_test.go @@ -18,13 +18,46 @@ limitations under the License. package syncer import ( + "path/filepath" "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + api "github.com/presslabs/mysql-operator/pkg/apis/mysql/v1alpha1" ) +var t *envtest.Environment +var cfg *rest.Config +var c client.Client + func TestSyncers(t *testing.T) { RegisterFailHandler(Fail) RunSpecsWithDefaultAndCustomReporters(t, "Backup syncers suit", []Reporter{}) } + +var _ = BeforeSuite(func() { + var err error + + t = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "..", "..", "config", "crds")}, + } + + err = api.SchemeBuilder.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + cfg, err = t.Start() + Expect(err).NotTo(HaveOccurred()) + + c, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) +}) + +var _ = AfterSuite(func() { + t.Stop() +}) diff --git a/pkg/controller/mysqlbackup/mysqlbackup_controller.go b/pkg/controller/mysqlbackup/mysqlbackup_controller.go index a639f3971..d8a67f869 100644 --- a/pkg/controller/mysqlbackup/mysqlbackup_controller.go +++ b/pkg/controller/mysqlbackup/mysqlbackup_controller.go @@ -139,19 +139,6 @@ func (r *ReconcileMysqlBackup) Reconcile(request reconcile.Request) (reconcile.R return reconcile.Result{}, fmt.Errorf("cluster name is not specified") } - deletionJobSyncer := backupSyncer.NewRemoteJobSyncer(r.Client, r.scheme, backup, r.opt) - err = syncer.Sync(context.TODO(), deletionJobSyncer, r.recorder) - if err != nil { - return reconcile.Result{}, err - } - - // if the backup is completed then skip the reconciliation - if backup.Status.Completed { - // silence skip it - log.V(1).Info("backup already completed", "name", backup.Name) - return reconcile.Result{}, nil - } - // get related cluster var cluster *mysqlcluster.MysqlCluster if cluster, err = r.getRelatedCluster(backup); err != nil { @@ -161,18 +148,17 @@ func (r *ReconcileMysqlBackup) Reconcile(request reconcile.Request) (reconcile.R // set defaults for the backup base on the related cluster backup.SetDefaults(cluster) - // create the backup job syncer and run it - jobSyncer := backupSyncer.NewJobSyncer(r.Client, r.scheme, backup, cluster, r.opt) - err = syncer.Sync(context.TODO(), jobSyncer, r.recorder) - if err != nil { + syncers := []syncer.Interface{ + backupSyncer.NewDeleteJobSyncer(r.Client, r.scheme, backup, cluster, r.opt, r.recorder), + backupSyncer.NewJobSyncer(r.Client, r.scheme, backup, cluster, r.opt), + } + + if err = r.sync(context.TODO(), syncers); err != nil { return reconcile.Result{}, err } - // update spec if modified - if !reflect.DeepEqual(savedBackup, backup.Unwrap()) { - if err = r.Update(context.TODO(), backup.Unwrap()); err != nil { - return reconcile.Result{}, err - } + if err = r.updateBackup(savedBackup, backup); err != nil { + return reconcile.Result{}, err } return reconcile.Result{}, nil @@ -187,3 +173,21 @@ func (r *ReconcileMysqlBackup) getRelatedCluster(backup *mysqlbackup.MysqlBackup return cluster, nil } + +func (r *ReconcileMysqlBackup) updateBackup(savedBackup *mysqlv1alpha1.MysqlBackup, backup *mysqlbackup.MysqlBackup) error { + if !reflect.DeepEqual(savedBackup, backup.Unwrap()) { + if err := r.Update(context.TODO(), backup.Unwrap()); err != nil { + return err + } + } + return nil +} + +func (r *ReconcileMysqlBackup) sync(ctx context.Context, syncers []syncer.Interface) error { + for _, s := range syncers { + if err := syncer.Sync(ctx, s, r.recorder); err != nil { + return err + } + } + return nil +} diff --git a/pkg/controller/mysqlbackup/mysqlbackup_controller_test.go b/pkg/controller/mysqlbackup/mysqlbackup_controller_test.go index 89ef2a841..c68b16670 100644 --- a/pkg/controller/mysqlbackup/mysqlbackup_controller_test.go +++ b/pkg/controller/mysqlbackup/mysqlbackup_controller_test.go @@ -225,6 +225,8 @@ var _ = Describe("MysqlBackup controller", func() { Expect(c.Create(context.TODO(), backup.Unwrap())).To(Succeed()) Eventually(requests, timeout).Should(Receive(Equal(expectedRequest))) + // update backup defaults from cluster + Eventually(requests, timeout).Should(Receive(Equal(expectedRequest))) }) AfterEach(func() { diff --git a/pkg/internal/mysqlbackup/mysqlbackup.go b/pkg/internal/mysqlbackup/mysqlbackup.go index 72c2bfa86..c50d9a33e 100644 --- a/pkg/internal/mysqlbackup/mysqlbackup.go +++ b/pkg/internal/mysqlbackup/mysqlbackup.go @@ -52,11 +52,6 @@ func (b *MysqlBackup) Unwrap() *api.MysqlBackup { return b.MysqlBackup } -// GetNameForJob returns the name of the job -func (b *MysqlBackup) GetNameForJob() string { - return fmt.Sprintf("%s-bjob", b.Name) -} - // GetBackupURL returns a backup URL func (b *MysqlBackup) GetBackupURL(cluster *mysqlcluster.MysqlCluster) string { if strings.HasSuffix(b.Spec.BackupURL, BackupSuffix) { @@ -87,7 +82,12 @@ func (b *MysqlBackup) composeBackupURL(base string) string { return base + fileName } +// GetNameForJob returns the name of the job +func (b *MysqlBackup) GetNameForJob() string { + return fmt.Sprintf("%s-backup", b.Name) +} + //GetNameForDeletionJob returns the name for the hard deletion job. func (b *MysqlBackup) GetNameForDeletionJob() string { - return fmt.Sprintf("%s-djob", b.Name) + return fmt.Sprintf("%s-backup-cleanup", b.Name) }