K8SPSMDB-1319 Don't run scheduled backup if DB is unhealthy and startingDeadlineSeconds added. (#1940)

nmarukovich · Natalia Marukovich · hors · web-flow · commit 3d88eb85ddbb · 2025-06-04T11:59:24.000+03:00
* K8SPSMDB-1319

* fix test

* fix test

* fix imports

* fix

* fix images

* fix images

* fix

* fix PR comments

* fix PR comments

* fix labels after 1304 merge

* fix labels

* fix

* fix PR comments

* fix PR comments

---------

Co-authored-by: Natalia Marukovich &lt;nataliamarukovich@Natalias-MacBook-Pro.local&gt;
Co-authored-by: Viacheslav Sarzhan &lt;slava.sarzhan@percona.com&gt;
diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml
@@ -67,6 +67,9 @@ spec:
                 type: integer
               compressionType:
                 type: string
+              startingDeadlineSeconds:
+                format: int64
+                type: integer
               storageName:
                 type: string
               type:
diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbs.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbs.yaml
@@ -298,6 +298,9 @@ spec:
                     type: string
                   serviceAccountName:
                     type: string
+                  startingDeadlineSeconds:
+                    format: int64
+                    type: integer
                   storages:
                     additionalProperties:
                       properties:
diff --git a/deploy/backup/backup.yaml b/deploy/backup/backup.yaml
@@ -10,3 +10,4 @@ spec:
 #  type: physical
 #  compressionType: gzip
 #  compressionLevel: 6
+#  startingDeadlineSeconds: 300
diff --git a/deploy/bundle.yaml b/deploy/bundle.yaml
@@ -71,6 +71,9 @@ spec:
                 type: integer
               compressionType:
                 type: string
+              startingDeadlineSeconds:
+                format: int64
+                type: integer
               storageName:
                 type: string
               type:
@@ -1023,6 +1026,9 @@ spec:
                     type: string
                   serviceAccountName:
                     type: string
+                  startingDeadlineSeconds:
+                    format: int64
+                    type: integer
                   storages:
                     additionalProperties:
                       properties:
diff --git a/deploy/cr.yaml b/deploy/cr.yaml
@@ -684,6 +684,7 @@ spec:
   backup:
     enabled: true
     image: perconalab/percona-server-mongodb-operator:main-backup
+#    startingDeadlineSeconds: 300
 #    annotations:
 #      iam.amazonaws.com/role: role-arn
 #    resources:
diff --git a/deploy/crd.yaml b/deploy/crd.yaml
@@ -71,6 +71,9 @@ spec:
                 type: integer
               compressionType:
                 type: string
+              startingDeadlineSeconds:
+                format: int64
+                type: integer
               storageName:
                 type: string
               type:
@@ -1023,6 +1026,9 @@ spec:
                     type: string
                   serviceAccountName:
                     type: string
+                  startingDeadlineSeconds:
+                    format: int64
+                    type: integer
                   storages:
                     additionalProperties:
                       properties:
diff --git a/deploy/cw-bundle.yaml b/deploy/cw-bundle.yaml
@@ -71,6 +71,9 @@ spec:
                 type: integer
               compressionType:
                 type: string
+              startingDeadlineSeconds:
+                format: int64
+                type: integer
               storageName:
                 type: string
               type:
@@ -1023,6 +1026,9 @@ spec:
                     type: string
                   serviceAccountName:
                     type: string
+                  startingDeadlineSeconds:
+                    format: int64
+                    type: integer
                   storages:
                     additionalProperties:
                       properties:
diff --git a/e2e-tests/version-service/conf/crd.yaml b/e2e-tests/version-service/conf/crd.yaml
@@ -71,6 +71,9 @@ spec:
                 type: integer
               compressionType:
                 type: string
+              startingDeadlineSeconds:
+                format: int64
+                type: integer
               storageName:
                 type: string
               type:
@@ -1023,6 +1026,9 @@ spec:
                     type: string
                   serviceAccountName:
                     type: string
+                  startingDeadlineSeconds:
+                    format: int64
+                    type: integer
                   storages:
                     additionalProperties:
                       properties:
diff --git a/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go b/pkg/apis/psmdb/v1/perconaservermongodbbackup_types.go
@@ -17,7 +17,8 @@ type PerconaServerMongoDBBackupSpec struct {
 	CompressionLevel *int                     `json:"compressionLevel,omitempty"`
 
 	// +kubebuilder:validation:Enum={logical,physical,incremental,incremental-base}
-	Type defs.BackupType `json:"type,omitempty"`
+	Type                    defs.BackupType `json:"type,omitempty"`
+	StartingDeadlineSeconds *int64          `json:"startingDeadlineSeconds,omitempty"`
 }
 
 type BackupState string
@@ -26,7 +27,6 @@ const (
 	BackupStateNew       BackupState = ""
 	BackupStateWaiting   BackupState = "waiting"
 	BackupStateRequested BackupState = "requested"
-	BackupStateRejected  BackupState = "rejected"
 	BackupStateRunning   BackupState = "running"
 	BackupStateError     BackupState = "error"
 	BackupStateReady     BackupState = "ready"
diff --git a/pkg/apis/psmdb/v1/psmdb_types.go b/pkg/apis/psmdb/v1/psmdb_types.go
@@ -1106,6 +1106,7 @@ type BackupSpec struct {
 	PITR                     PITRSpec                     `json:"pitr,omitempty"`
 	Configuration            BackupConfig                 `json:"configuration,omitempty"`
 	VolumeMounts             []corev1.VolumeMount         `json:"volumeMounts,omitempty"`
+	StartingDeadlineSeconds  *int64                       `json:"startingDeadlineSeconds,omitempty"`
 }
 
 func (b BackupSpec) IsPITREnabled() bool {
diff --git a/pkg/apis/psmdb/v1/zz_generated.deepcopy.go b/pkg/apis/psmdb/v1/zz_generated.deepcopy.go
diff --git a/pkg/controller/perconaservermongodbbackup/backup.go b/pkg/controller/perconaservermongodbbackup/backup.go
@@ -25,7 +25,7 @@ import (
 const (
 	// pbmStartingDeadline is timeout after which continuous starting state is considered as error
 	pbmStartingDeadline       = time.Duration(120) * time.Second
-	pbmStartingDeadlineErrMsg = "starting deadline exceeded"
+	pbmStartingDeadlineErrMsg = "backup did not progress from 'starting' state within the allowed timeout"
 )
 
 var defaultBackoff = wait.Backoff{
diff --git a/pkg/controller/perconaservermongodbbackup/deadline.go b/pkg/controller/perconaservermongodbbackup/deadline.go
@@ -0,0 +1,41 @@
+package perconaservermongodbbackup
+
+import (
+	"context"
+	"time"
+
+	"github.com/pkg/errors"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+
+	psmdbv1 "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
+)
+
+func checkStartingDeadline(ctx context.Context, cluster *psmdbv1.PerconaServerMongoDB, cr *psmdbv1.PerconaServerMongoDBBackup) error {
+	log := logf.FromContext(ctx)
+
+	if cr.Status.State != psmdbv1.BackupStateNew {
+		return nil
+	}
+
+	var deadlineSeconds *int64
+	if cr.Spec.StartingDeadlineSeconds != nil {
+		deadlineSeconds = cr.Spec.StartingDeadlineSeconds
+	} else if cluster.Spec.Backup.StartingDeadlineSeconds != nil {
+		deadlineSeconds = cluster.Spec.Backup.StartingDeadlineSeconds
+	}
+
+	if deadlineSeconds == nil {
+		return nil
+	}
+
+	since := time.Since(cr.CreationTimestamp.Time).Seconds()
+	if since < float64(*deadlineSeconds) {
+		return nil
+	}
+
+	log.Info("Backup didn't start in startingDeadlineSeconds, failing the backup",
+		"startingDeadlineSeconds", *deadlineSeconds,
+		"passedSeconds", since)
+
+	return errors.New("starting deadline seconds exceeded")
+}
diff --git a/pkg/controller/perconaservermongodbbackup/deadline_test.go b/pkg/controller/perconaservermongodbbackup/deadline_test.go
@@ -0,0 +1,121 @@
+package perconaservermongodbbackup
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	_ "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+	"sigs.k8s.io/yaml"
+
+	psmdbv1 "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
+)
+
+var _ = Describe("Starting deadline", func() {
+	It("should be optional", func() {
+		cluster, err := readDefaultCR("cluster1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		bcp, err := readDefaultBackup("backup1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		cluster.Spec.Backup.StartingDeadlineSeconds = nil
+
+		bcp.Spec.StartingDeadlineSeconds = nil
+		bcp.Status.State = psmdbv1.BackupStateNew
+
+		err = checkStartingDeadline(context.Background(), cluster, bcp)
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("should use universal value if defined", func() {
+		cluster, err := readDefaultCR("cluster1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		bcp, err := readDefaultBackup("backup1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		cluster.Spec.Backup.StartingDeadlineSeconds = ptr.To(int64(60))
+
+		bcp.Status.State = psmdbv1.BackupStateNew
+		bcp.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute))
+
+		err = checkStartingDeadline(context.Background(), cluster, bcp)
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("should use particular value if defined", func() {
+		cluster, err := readDefaultCR("cluster1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		bcp, err := readDefaultBackup("backup1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		cluster.Spec.Backup.StartingDeadlineSeconds = ptr.To(int64(600))
+
+		bcp.Status.State = psmdbv1.BackupStateNew
+		bcp.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute))
+		bcp.Spec.StartingDeadlineSeconds = ptr.To(int64(60))
+
+		err = checkStartingDeadline(context.Background(), cluster, bcp)
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("should not return an error", func() {
+		cluster, err := readDefaultCR("cluster1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		bcp, err := readDefaultBackup("backup1", "test")
+		Expect(err).ToNot(HaveOccurred())
+
+		cluster.Spec.Backup.StartingDeadlineSeconds = ptr.To(int64(600))
+
+		bcp.Status.State = psmdbv1.BackupStateNew
+		bcp.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute))
+		bcp.Spec.StartingDeadlineSeconds = ptr.To(int64(300))
+
+		err = checkStartingDeadline(context.Background(), cluster, bcp)
+		Expect(err).ToNot(HaveOccurred())
+	})
+})
+
+func readDefaultCR(name, namespace string) (*psmdbv1.PerconaServerMongoDB, error) {
+	data, err := os.ReadFile(filepath.Join("..", "..", "..", "deploy", "cr.yaml"))
+	if err != nil {
+		return nil, err
+	}
+
+	cr := &psmdbv1.PerconaServerMongoDB{}
+
+	if err := yaml.Unmarshal(data, cr); err != nil {
+		return nil, err
+	}
+
+	cr.Name = name
+	cr.Namespace = namespace
+	cr.Spec.InitImage = "perconalab/percona-server-mongodb-operator:main"
+	return cr, nil
+}
+
+func readDefaultBackup(name, namespace string) (*psmdbv1.PerconaServerMongoDBBackup, error) {
+	data, err := os.ReadFile(filepath.Join("..", "..", "..", "deploy", "backup", "backup.yaml"))
+	if err != nil {
+		return nil, err
+	}
+
+	cr := &psmdbv1.PerconaServerMongoDBBackup{}
+
+	if err := yaml.Unmarshal(data, cr); err != nil {
+		return cr, err
+	}
+
+	cr.Name = name
+	cr.Namespace = namespace
+
+	return cr, nil
+}
diff --git a/pkg/controller/perconaservermongodbbackup/perconaservermongodbbackup_controller.go b/pkg/controller/perconaservermongodbbackup/perconaservermongodbbackup_controller.go
@@ -178,6 +178,10 @@ func (r *ReconcilePerconaServerMongoDBBackup) Reconcile(ctx context.Context, req
 		cluster = nil
 	}
 
+	if err = checkStartingDeadline(ctx, cluster, cr); err != nil {
+		return reconcile.Result{}, err
+	}
+
 	if cluster != nil {
 		var svr *version.ServerVersion
 		svr, err = version.Server(r.clientcmd)
@@ -187,7 +191,7 @@ func (r *ReconcilePerconaServerMongoDBBackup) Reconcile(ctx context.Context, req
 
 		err = cluster.CheckNSetDefaults(ctx, svr.Platform)
 		if err != nil {
-			return rr, errors.Wrapf(err, "set defaults for %s/%s", cluster.Namespace, cluster.Name)
+			return reconcile.Result{}, errors.Wrapf(err, "invalid cr oprions used for %s/%s", cluster.Namespace, cluster.Name)
 		}
 	}
 
@@ -236,7 +240,8 @@ func (r *ReconcilePerconaServerMongoDBBackup) reconcile(
 	}
 
 	if err := cluster.CanBackup(ctx); err != nil {
-		return status, errors.Wrap(err, "failed to run backup")
+		log.Error(err, "Cluster is not ready for backup")
+		return status, nil
 	}
 
 	cjobs, err := backup.HasActiveJobs(ctx, r.newPBMFunc, r.client, cluster, backup.NewBackupJob(cr.Name), backup.NotPITRLock)

Original file line number	Diff line number	Diff line change
`@@ -1106,6 +1106,7 @@ type BackupSpec struct {`
`1106`	`1106`	PITR PITRSpec `json:"pitr,omitempty"`
`1107`	`1107`	Configuration BackupConfig `json:"configuration,omitempty"`
`1108`	`1108`	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
	`1109`	+ StartingDeadlineSeconds *int64 `json:"startingDeadlineSeconds,omitempty"`
`1109`	`1110`	`}`
`1110`	`1111`
`1111`	`1112`	`func (b BackupSpec) IsPITREnabled() bool {`