Stop the running log backup when deleting CR (#5754)

Co-authored-by: csuzhangxc <csuzhangxc@gmail.com>
pingcap · Oct 16, 2024 · e42cb3b · e42cb3b
1 parent 92d7b75
commit e42cb3b
Show file tree

Hide file tree

Showing 10 changed files with 532 additions and 47 deletions.
diff --git a/cmd/backup-manager/app/backup/manager.go b/cmd/backup-manager/app/backup/manager.go
@@ -531,7 +531,7 @@ func (bm *Manager) startLogBackup(ctx context.Context, backup *v1alpha1.Backup)
 
 	if backupErr != nil {
 		klog.Errorf("Start log backup of cluster %s failed, err: %s", bm, backupErr)
-		return nil, "StartLogBackuFailed", backupErr
+		return nil, "StartLogBackupFailed", backupErr
 	}
 	klog.Infof("Start log backup of cluster %s to %s success", bm, backupFullPath)
 

diff --git a/hack/e2e-patch-codecov.sh b/hack/e2e-patch-codecov.sh
@@ -146,8 +146,9 @@ echo "$PATCH_BR_JOB"`` >> $TMP_BACKUP_MANAGER
 tail -n +$line $BACKUP_MANAGER >> $TMP_BACKUP_MANAGER
 mv -f $TMP_BACKUP_MANAGER $BACKUP_MANAGER
 
-line=$(grep -n 'bc.deps.JobControl.CreateJob(backup, job)' $BACKUP_CLEANER | cut -d ":" -f 1)
-head -n $(($line-1)) $BACKUP_CLEANER > $TMP_BACKUP_CLEANER
+# have more than one match, we only to hack the last one now
+line=$(grep -n 'bc.deps.JobControl.CreateJob(backup, job)' $BACKUP_CLEANER | tail -1 | cut -d ":" -f 1)
+head -n $(($line - 1)) $BACKUP_CLEANER > $TMP_BACKUP_CLEANER
 echo "$PATCH_BR_JOB"`` >> $TMP_BACKUP_CLEANER
 tail -n +$line $BACKUP_CLEANER >> $TMP_BACKUP_CLEANER
 mv -f $TMP_BACKUP_CLEANER $BACKUP_CLEANER
diff --git a/manifests/crd.yaml b/manifests/crd.yaml
@@ -1248,6 +1248,11 @@ spec:
                         type: number
                     type: object
                   cleanPolicy:
+                    default: Retain
+                    enum:
+                    - Retain
+                    - OnFailure
+                    - Delete
                     type: string
                   commitTs:
                     type: string
@@ -3494,6 +3499,11 @@ spec:
                         type: number
                     type: object
                   cleanPolicy:
+                    default: Retain
+                    enum:
+                    - Retain
+                    - OnFailure
+                    - Delete
                     type: string
                   commitTs:
                     type: string
@@ -5850,6 +5860,11 @@ spec:
                     type: number
                 type: object
               cleanPolicy:
+                default: Retain
+                enum:
+                - Retain
+                - OnFailure
+                - Delete
                 type: string
               commitTs:
                 type: string

diff --git a/manifests/crd/v1/pingcap.com_backups.yaml b/manifests/crd/v1/pingcap.com_backups.yaml
@@ -1273,6 +1273,11 @@ spec:
                     type: number
                 type: object
               cleanPolicy:
+                default: Retain
+                enum:
+                - Retain
+                - OnFailure
+                - Delete
                 type: string
               commitTs:
                 type: string

diff --git a/manifests/crd/v1/pingcap.com_backupschedules.yaml b/manifests/crd/v1/pingcap.com_backupschedules.yaml
@@ -1248,6 +1248,11 @@ spec:
                         type: number
                     type: object
                   cleanPolicy:
+                    default: Retain
+                    enum:
+                    - Retain
+                    - OnFailure
+                    - Delete
                     type: string
                   commitTs:
                     type: string
@@ -3494,6 +3499,11 @@ spec:
                         type: number
                     type: object
                   cleanPolicy:
+                    default: Retain
+                    enum:
+                    - Retain
+                    - OnFailure
+                    - Delete
                     type: string
                   commitTs:
                     type: string

diff --git a/pkg/apis/pingcap/v1alpha1/backup.go b/pkg/apis/pingcap/v1alpha1/backup.go
@@ -42,6 +42,11 @@ func (bk *Backup) GetCleanJobName() string {
 	return fmt.Sprintf("clean-%s", bk.GetName())
 }
 
+// GetCleanJobName return the clean job name for log backup
+func (bk *Backup) GetStopLogBackupJobName() string {
+	return fmt.Sprintf("stop-%s", bk.GetName())
+}
+
 // GetBackupJobName return the backup job name
 func (bk *Backup) GetBackupJobName() string {
 	if command := ParseLogBackupSubcommand(bk); command != "" {
@@ -287,6 +292,13 @@ func IsVolumeBackupFailed(backup *Backup) bool {
 
 // IsBackupClean returns true if a Backup has been successfully cleaned up
 func IsBackupClean(backup *Backup) bool {
+	// TODO: now we don't handle fault state, maybe we should consider it in the future
+	if backup.Spec.Mode == BackupModeLog && IsLogBackupOnTrack(backup) {
+		return false
+	}
+	if NeedRetainData(backup) {
+		return true
+	}
 	_, condition := GetBackupCondition(&backup.Status, BackupClean)
 	return condition != nil && condition.Status == corev1.ConditionTrue
 }
@@ -299,17 +311,15 @@ func IsBackupCleanFailed(backup *Backup) bool {
 
 // IsCleanCandidate returns true if a Backup should be added to clean candidate according to cleanPolicy
 func IsCleanCandidate(backup *Backup) bool {
-	switch backup.Spec.CleanPolicy {
-	case CleanPolicyTypeDelete, CleanPolicyTypeOnFailure:
-		return true
-	default:
-		return false
-	}
+	return backup.Spec.Mode == BackupModeLog ||
+		backup.Spec.CleanPolicy == CleanPolicyTypeDelete ||
+		backup.Spec.CleanPolicy == CleanPolicyTypeOnFailure
 }
 
-// NeedNotClean returns true if a Backup need not to be cleaned up according to cleanPolicy
-func NeedNotClean(backup *Backup) bool {
-	return backup.Spec.CleanPolicy == CleanPolicyTypeOnFailure && !IsBackupFailed(backup)
+// NeedRetainData returns true if a Backup need not to be cleaned up according to cleanPolicy
+func NeedRetainData(backup *Backup) bool {
+	return backup.Spec.CleanPolicy == CleanPolicyTypeRetain ||
+		(backup.Spec.CleanPolicy == CleanPolicyTypeOnFailure && !IsBackupFailed(backup))
 }
 
 // ParseLogBackupSubcommand parse the log backup subcommand from cr.
@@ -430,6 +440,20 @@ func IsLogBackupAlreadyStop(backup *Backup) bool {
 	return backup.Spec.Mode == BackupModeLog && backup.Status.Phase == BackupStopped
 }
 
+// IsLogBackupOnTrack returns whether log backup is on track.
+func IsLogBackupOnTrack(backup *Backup) bool {
+	if backup.Spec.Mode != BackupModeLog {
+		return false
+	}
+
+	switch backup.Status.Phase {
+	case BackupScheduled, BackupPrepare, BackupRunning, BackupPaused:
+		return true
+	default:
+		return false
+	}
+}
+
 // IsLogBackupAlreadyPaused return whether log backup has already paused.
 func IsLogBackupAlreadyPaused(backup *Backup) bool {
 	return backup.Spec.Mode == BackupModeLog && backup.Status.Phase == BackupPaused

diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go
@@ -2223,6 +2223,8 @@ type BackupSpec struct {
 	// Specify service account of backup
 	ServiceAccount string `json:"serviceAccount,omitempty"`
 	// CleanPolicy denotes whether to clean backup data when the object is deleted from the cluster, if not set, the backup data will be retained
+	// +kubebuilder:validation:Enum:=Retain;OnFailure;Delete
+	// +kubebuilder:default=Retain
 	CleanPolicy CleanPolicyType `json:"cleanPolicy,omitempty"`
 	// CleanOption controls the behavior of clean.
 	CleanOption *CleanOption `json:"cleanOption,omitempty"`