From 4796de4056af845cece31890e0d6f20a67cbb322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ege=20G=C3=BCne=C5=9F?= Date: Mon, 5 May 2025 12:53:48 +0300 Subject: [PATCH 1/3] K8SPSMDB-1339: Validate PiTR target before starting restore --- ...rcona.com_perconaservermongodbbackups.yaml | 3 ++ ...cona.com_perconaservermongodbrestores.yaml | 3 ++ deploy/bundle.yaml | 6 ++++ deploy/crd.yaml | 6 ++++ deploy/cw-bundle.yaml | 6 ++++ e2e-tests/version-service/conf/crd.yaml | 6 ++++ .../v1/perconaservermongodbbackup_types.go | 34 ++++++++++--------- pkg/apis/psmdb/v1/zz_generated.deepcopy.go | 28 ++++++++------- .../perconaservermongodbbackup/backup.go | 3 ++ .../perconaservermongodbrestore_controller.go | 4 +-- .../perconaservermongodbrestore/validate.go | 23 +++++++++++++ 11 files changed, 92 insertions(+), 30 deletions(-) diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml index c3c4314e5a..c7158677dc 100644 --- a/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml +++ b/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml @@ -107,6 +107,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml index 6261d0a41b..595a7c17b1 100644 --- a/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml +++ b/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml @@ -75,6 +75,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/deploy/bundle.yaml b/deploy/bundle.yaml index 9d863e068a..fc7db0b9a7 100644 --- a/deploy/bundle.yaml +++ b/deploy/bundle.yaml @@ -106,6 +106,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string @@ -260,6 +263,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/deploy/crd.yaml b/deploy/crd.yaml index ed0880ca25..e5375989db 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -106,6 +106,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string @@ -260,6 +263,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/deploy/cw-bundle.yaml b/deploy/cw-bundle.yaml index 010ee9f9f9..9749612585 100644 --- a/deploy/cw-bundle.yaml +++ b/deploy/cw-bundle.yaml @@ -106,6 +106,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string @@ -260,6 +263,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/e2e-tests/version-service/conf/crd.yaml b/e2e-tests/version-service/conf/crd.yaml index ed0880ca25..e5375989db 100644 --- a/e2e-tests/version-service/conf/crd.yaml +++ b/e2e-tests/version-service/conf/crd.yaml @@ -106,6 +106,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string @@ -260,6 +263,9 @@ spec: lastTransition: format: date-time type: string + lastWriteAt: + format: date-time + type: string latestRestorableTime: format: date-time type: string diff --git a/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go b/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go index 1b1c61eeec..cd937e85a4 100644 --- a/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go +++ b/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go @@ -35,24 +35,26 @@ const ( // PerconaServerMongoDBBackupStatus defines the observed state of PerconaServerMongoDBBackup type PerconaServerMongoDBBackupStatus struct { - Type defs.BackupType `json:"type,omitempty"` - State BackupState `json:"state,omitempty"` - StartAt *metav1.Time `json:"start,omitempty"` - CompletedAt *metav1.Time `json:"completed,omitempty"` - LastTransition *metav1.Time `json:"lastTransition,omitempty"` - Destination string `json:"destination,omitempty"` - StorageName string `json:"storageName,omitempty"` - S3 *BackupStorageS3Spec `json:"s3,omitempty"` - Azure *BackupStorageAzureSpec `json:"azure,omitempty"` - Filesystem *BackupStorageFilesystemSpec `json:"filesystem,omitempty"` - ReplsetNames []string `json:"replsetNames,omitempty"` - PBMname string `json:"pbmName,omitempty"` + Type defs.BackupType `json:"type,omitempty"` + State BackupState `json:"state,omitempty"` + Destination string `json:"destination,omitempty"` + StorageName string `json:"storageName,omitempty"` + S3 *BackupStorageS3Spec `json:"s3,omitempty"` + Azure *BackupStorageAzureSpec `json:"azure,omitempty"` + Filesystem *BackupStorageFilesystemSpec `json:"filesystem,omitempty"` + ReplsetNames []string `json:"replsetNames,omitempty"` + PBMname string `json:"pbmName,omitempty"` // Deprecated: Use PBMPods instead - PBMPod string `json:"pbmPod,omitempty"` - PBMPods map[string]string `json:"pbmPods,omitempty"` - Error string `json:"error,omitempty"` - LatestRestorableTime *metav1.Time `json:"latestRestorableTime,omitempty"` + PBMPod string `json:"pbmPod,omitempty"` + PBMPods map[string]string `json:"pbmPods,omitempty"` + Error string `json:"error,omitempty"` + + StartAt *metav1.Time `json:"start,omitempty"` + CompletedAt *metav1.Time `json:"completed,omitempty"` + LastWriteAt *metav1.Time `json:"lastWriteAt,omitempty"` + LastTransition *metav1.Time `json:"lastTransition,omitempty"` + LatestRestorableTime *metav1.Time `json:"latestRestorableTime,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/psmdb/v1/zz_generated.deepcopy.go b/pkg/apis/psmdb/v1/zz_generated.deepcopy.go index 89525f7472..95130cceb9 100644 --- a/pkg/apis/psmdb/v1/zz_generated.deepcopy.go +++ b/pkg/apis/psmdb/v1/zz_generated.deepcopy.go @@ -1079,18 +1079,6 @@ func (in *PerconaServerMongoDBBackupSpec) DeepCopy() *PerconaServerMongoDBBackup // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PerconaServerMongoDBBackupStatus) DeepCopyInto(out *PerconaServerMongoDBBackupStatus) { *out = *in - if in.StartAt != nil { - in, out := &in.StartAt, &out.StartAt - *out = (*in).DeepCopy() - } - if in.CompletedAt != nil { - in, out := &in.CompletedAt, &out.CompletedAt - *out = (*in).DeepCopy() - } - if in.LastTransition != nil { - in, out := &in.LastTransition, &out.LastTransition - *out = (*in).DeepCopy() - } if in.S3 != nil { in, out := &in.S3, &out.S3 *out = new(BackupStorageS3Spec) @@ -1118,6 +1106,22 @@ func (in *PerconaServerMongoDBBackupStatus) DeepCopyInto(out *PerconaServerMongo (*out)[key] = val } } + if in.StartAt != nil { + in, out := &in.StartAt, &out.StartAt + *out = (*in).DeepCopy() + } + if in.CompletedAt != nil { + in, out := &in.CompletedAt, &out.CompletedAt + *out = (*in).DeepCopy() + } + if in.LastWriteAt != nil { + in, out := &in.LastWriteAt, &out.LastWriteAt + *out = (*in).DeepCopy() + } + if in.LastTransition != nil { + in, out := &in.LastTransition, &out.LastTransition + *out = (*in).DeepCopy() + } if in.LatestRestorableTime != nil { in, out := &in.LatestRestorableTime, &out.LatestRestorableTime *out = (*in).DeepCopy() diff --git a/pkg/controller/perconaservermongodbbackup/backup.go b/pkg/controller/perconaservermongodbbackup/backup.go index 298fc0c525..41de9573b4 100644 --- a/pkg/controller/perconaservermongodbbackup/backup.go +++ b/pkg/controller/perconaservermongodbbackup/backup.go @@ -187,6 +187,9 @@ func (b *Backup) Status(ctx context.Context, cr *api.PerconaServerMongoDBBackup) status.CompletedAt = &metav1.Time{ Time: time.Unix(meta.LastTransitionTS, 0), } + status.LastWriteAt = &metav1.Time{ + Time: time.Unix(int64(meta.LastWriteTS.T), 0), + } case defs.StatusStarting: passed := time.Now().UTC().Sub(time.Unix(meta.StartTS, 0)) if passed >= pbmStartingDeadline { diff --git a/pkg/controller/perconaservermongodbrestore/perconaservermongodbrestore_controller.go b/pkg/controller/perconaservermongodbrestore/perconaservermongodbrestore_controller.go index 7ee08855cc..6a5b8079a6 100644 --- a/pkg/controller/perconaservermongodbrestore/perconaservermongodbrestore_controller.go +++ b/pkg/controller/perconaservermongodbrestore/perconaservermongodbrestore_controller.go @@ -188,7 +188,7 @@ func (r *ReconcilePerconaServerMongoDBRestore) Reconcile(ctx context.Context, re } if cr.Status.State == psmdbv1.RestoreStateNew { - err := r.validate(ctx, cr, cluster) + err = r.validate(ctx, cr, cluster) if err != nil { if errors.Is(err, errWaitingPBM) { err = nil @@ -209,7 +209,7 @@ func (r *ReconcilePerconaServerMongoDBRestore) Reconcile(ctx context.Context, re return rr, nil } - return rr, errors.Wrap(err, "failed to validate restore") + return reconcile.Result{}, errors.Wrap(err, "failed to validate restore") } if cluster.Spec.Sharding.Enabled { diff --git a/pkg/controller/perconaservermongodbrestore/validate.go b/pkg/controller/perconaservermongodbrestore/validate.go index 5516195d68..17727beb20 100644 --- a/pkg/controller/perconaservermongodbrestore/validate.go +++ b/pkg/controller/perconaservermongodbrestore/validate.go @@ -49,5 +49,28 @@ func (r *ReconcilePerconaServerMongoDBRestore) validate(ctx context.Context, cr return errors.Wrap(err, "failed to validate backup") } + if pitr := cr.Spec.PITR; pitr != nil { + switch { + case pitr.Type == psmdbv1.PITRestoreTypeDate && pitr.Date != nil: + if bcp.Status.LastWriteAt != nil { + if pitr.Date.Time.Equal(bcp.Status.LastWriteAt) { + return errors.New("backup's last write is equal to target time") + } + if pitr.Date.Time.Before(bcp.Status.LastWriteAt) { + return errors.New("backup's last write is later than target time") + } + } + ts := pitr.Date.Unix() + if _, err := pbmc.GetPITRChunkContains(ctx, ts); err != nil { + return err + } + case pitr.Type == psmdbv1.PITRestoreTypeLatest: + _, err := pbmc.GetLatestTimelinePITR(ctx) + if err != nil { + return err + } + } + } + return nil } From 2134ff10e348fc8a28934d5460e7eb91b0139c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ege=20G=C3=BCne=C5=9F?= Date: Mon, 5 May 2025 20:06:47 +0300 Subject: [PATCH 2/3] add unit tests --- .../perconaservermongodbrestore/validate.go | 1 - .../validate_test.go | 265 ++++++++++++++++++ 2 files changed, 265 insertions(+), 1 deletion(-) diff --git a/pkg/controller/perconaservermongodbrestore/validate.go b/pkg/controller/perconaservermongodbrestore/validate.go index 17727beb20..cfb4e065e4 100644 --- a/pkg/controller/perconaservermongodbrestore/validate.go +++ b/pkg/controller/perconaservermongodbrestore/validate.go @@ -6,7 +6,6 @@ import ( "github.com/pkg/errors" "github.com/percona/percona-backup-mongodb/pbm/defs" - psmdbv1 "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/backup" ) diff --git a/pkg/controller/perconaservermongodbrestore/validate_test.go b/pkg/controller/perconaservermongodbrestore/validate_test.go index 68905e1a7a..7b9f351862 100644 --- a/pkg/controller/perconaservermongodbrestore/validate_test.go +++ b/pkg/controller/perconaservermongodbrestore/validate_test.go @@ -3,13 +3,16 @@ package perconaservermongodbrestore import ( "context" "testing" + "time" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/percona/percona-backup-mongodb/pbm/defs" psmdbv1 "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" fakeBackup "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/backup/fake" ) @@ -152,6 +155,268 @@ func TestValidate(t *testing.T) { } } +func TestValidatePiTR(t *testing.T) { + ctx := context.Background() + + ns := "validate-pitr" + clusterName := ns + "-cr" + restoreName := ns + "-restore" + backupName := ns + "-backup" + secretName := ns + "-secret" + secret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: ns, + }, + Data: map[string][]byte{}, + } + + storageName := ns + "-stg" + + cluster := readDefaultCluster(t, clusterName, ns) + cluster.Spec.Backup.Storages = map[string]psmdbv1.BackupStorageSpec{ + storageName: { + Type: psmdbv1.BackupStorageS3, + S3: psmdbv1.BackupStorageS3Spec{ + Bucket: "some-bucket", + Prefix: "some-prefix", + Region: "some-region", + EndpointURL: "some-endpoint", + CredentialsSecret: secretName, + }, + }, + } + + tests := []struct { + name string + backup *psmdbv1.PerconaServerMongoDBBackup + restore *psmdbv1.PerconaServerMongoDBRestore + expectedErr string + }{ + { + "logical: pitr target time is equal to backup's last write", + &psmdbv1.PerconaServerMongoDBBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: backupName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBBackupSpec{ + Type: defs.LogicalBackup, + ClusterName: clusterName, + StorageName: storageName, + }, + Status: psmdbv1.PerconaServerMongoDBBackupStatus{ + State: psmdbv1.BackupStateReady, + Type: defs.LogicalBackup, + LastWriteAt: &metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + &psmdbv1.PerconaServerMongoDBRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: restoreName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBRestoreSpec{ + BackupName: backupName, + ClusterName: clusterName, + PITR: &psmdbv1.PITRestoreSpec{ + Type: psmdbv1.PITRestoreTypeDate, + Date: &psmdbv1.PITRestoreDate{ + Time: metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + }, + }, + "backup's last write is equal to target time", + }, + { + "physical: pitr target time is equal to backup's last write", + &psmdbv1.PerconaServerMongoDBBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: backupName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBBackupSpec{ + Type: defs.LogicalBackup, + ClusterName: clusterName, + StorageName: storageName, + }, + Status: psmdbv1.PerconaServerMongoDBBackupStatus{ + State: psmdbv1.BackupStateReady, + Type: defs.PhysicalBackup, + LastWriteAt: &metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + &psmdbv1.PerconaServerMongoDBRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: restoreName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBRestoreSpec{ + BackupName: backupName, + ClusterName: clusterName, + PITR: &psmdbv1.PITRestoreSpec{ + Type: psmdbv1.PITRestoreTypeDate, + Date: &psmdbv1.PITRestoreDate{ + Time: metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + }, + }, + "backup's last write is equal to target time", + }, + { + "logical: pitr target time is later than backup's last write", + &psmdbv1.PerconaServerMongoDBBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: backupName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBBackupSpec{ + Type: defs.LogicalBackup, + ClusterName: clusterName, + StorageName: storageName, + }, + Status: psmdbv1.PerconaServerMongoDBBackupStatus{ + State: psmdbv1.BackupStateReady, + Type: defs.LogicalBackup, + LastWriteAt: &metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:58Z"), + }, + }, + }, + &psmdbv1.PerconaServerMongoDBRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: restoreName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBRestoreSpec{ + BackupName: backupName, + ClusterName: clusterName, + PITR: &psmdbv1.PITRestoreSpec{ + Type: psmdbv1.PITRestoreTypeDate, + Date: &psmdbv1.PITRestoreDate{ + Time: metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + }, + }, + "backup's last write is later than target time", + }, + { + "physical: pitr target time is later than backup's last write", + &psmdbv1.PerconaServerMongoDBBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: backupName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBBackupSpec{ + Type: defs.LogicalBackup, + ClusterName: clusterName, + StorageName: storageName, + }, + Status: psmdbv1.PerconaServerMongoDBBackupStatus{ + State: psmdbv1.BackupStateReady, + Type: defs.PhysicalBackup, + LastWriteAt: &metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:58Z"), + }, + }, + }, + &psmdbv1.PerconaServerMongoDBRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: restoreName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBRestoreSpec{ + BackupName: backupName, + ClusterName: clusterName, + PITR: &psmdbv1.PITRestoreSpec{ + Type: psmdbv1.PITRestoreTypeDate, + Date: &psmdbv1.PITRestoreDate{ + Time: metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + }, + }, + "backup's last write is later than target time", + }, + { + "logical: pitr target time is valid", + &psmdbv1.PerconaServerMongoDBBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: backupName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBBackupSpec{ + Type: defs.LogicalBackup, + ClusterName: clusterName, + StorageName: storageName, + }, + Status: psmdbv1.PerconaServerMongoDBBackupStatus{ + State: psmdbv1.BackupStateReady, + Type: defs.PhysicalBackup, + LastWriteAt: &metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-04T21:00:57Z"), + }, + }, + }, + &psmdbv1.PerconaServerMongoDBRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: restoreName, + Namespace: ns, + }, + Spec: psmdbv1.PerconaServerMongoDBRestoreSpec{ + BackupName: backupName, + ClusterName: clusterName, + PITR: &psmdbv1.PITRestoreSpec{ + Type: psmdbv1.PITRestoreTypeDate, + Date: &psmdbv1.PITRestoreDate{ + Time: metav1.Time{ + Time: mustParseTime(time.RFC3339, "2010-02-05T12:10:58Z"), + }, + }, + }, + }, + }, + "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runtimeObjs := []client.Object{&secret, tt.restore, cluster, tt.backup} + r := fakeReconciler(runtimeObjs...) + err := r.validate(ctx, tt.restore, cluster) + if len(tt.expectedErr) > 0 { + assert.EqualError(t, err, tt.expectedErr) + } else { + assert.NoError(t, err) + } + }) + } +} + +func mustParseTime(layout string, value string) time.Time { + time, err := time.Parse(layout, value) + if err != nil { + panic(err) + } + return time +} + func fakeReconciler(objs ...client.Object) *ReconcilePerconaServerMongoDBRestore { s := scheme.Scheme From 779781535ecde5dc7e052e62dbf54e44ad5029d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ege=20G=C3=BCne=C5=9F?= Date: Mon, 5 May 2025 20:11:03 +0300 Subject: [PATCH 3/3] fix golangci-lint --- pkg/controller/perconaservermongodbrestore/validate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/perconaservermongodbrestore/validate.go b/pkg/controller/perconaservermongodbrestore/validate.go index cfb4e065e4..9134beae5c 100644 --- a/pkg/controller/perconaservermongodbrestore/validate.go +++ b/pkg/controller/perconaservermongodbrestore/validate.go @@ -52,10 +52,10 @@ func (r *ReconcilePerconaServerMongoDBRestore) validate(ctx context.Context, cr switch { case pitr.Type == psmdbv1.PITRestoreTypeDate && pitr.Date != nil: if bcp.Status.LastWriteAt != nil { - if pitr.Date.Time.Equal(bcp.Status.LastWriteAt) { + if pitr.Date.Equal(bcp.Status.LastWriteAt) { return errors.New("backup's last write is equal to target time") } - if pitr.Date.Time.Before(bcp.Status.LastWriteAt) { + if pitr.Date.Before(bcp.Status.LastWriteAt) { return errors.New("backup's last write is later than target time") } }