[DNM] refactor backup CR related logic

pkg/apis/pingcap/v1alpha1/backup.go

@@ -118,23 +118,18 @@ func IsBackupScheduled(backup *Backup) bool {
 	return condition != nil && condition.Status == corev1.ConditionTrue
 }
 
-// IsBackupClean returns true if a Backup has been successfully cleaned up
-func IsBackupClean(backup *Backup) bool {
+// IsBackupCleanedUp returns true if a Backup has been successfully cleaned up
+func IsBackupCleanedUp(backup *Backup) bool {
 	_, condition := GetBackupCondition(&backup.Status, BackupClean)
 	return condition != nil && condition.Status == corev1.ConditionTrue
 }
 
-// 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
-	}
+// CleanPolicyIsNotRetain returns true if backup's clean policy is not CleanPolicyTypeRetain
+func CleanPolicyIsNotRetain(backup *Backup) bool {
+	return backup.Spec.CleanPolicy != CleanPolicyTypeRetain
 }
 
-// NeedNotClean returns true if a Backup need not to be cleaned up according to cleanPolicy
-func NeedNotClean(backup *Backup) bool {
+// DontCleanSuccessBackup returns true if a Backup need not to be cleaned up according to cleanPolicy
+func DontCleanSuccessBackup(backup *Backup) bool {
 	return backup.Spec.CleanPolicy == CleanPolicyTypeOnFailure && !IsBackupFailed(backup)
 }

pkg/backup/backup/backup_cleaner.go

@@ -49,7 +49,9 @@ func NewBackupCleaner(deps *controller.Dependencies, statusUpdater controller.Ba
 }
 
 func (bc *backupCleaner) Clean(backup *v1alpha1.Backup) error {
-	if backup.DeletionTimestamp == nil || !v1alpha1.IsCleanCandidate(backup) || v1alpha1.NeedNotClean(backup) {
+	if backup.DeletionTimestamp == nil || // Backup CR is created or updated, not deleted
+		!v1alpha1.CleanPolicyIsNotRetain(backup) || // cleanPolicy is CleanPolicyTypeRetain
+		v1alpha1.DontCleanSuccessBackup(backup) { // cleanPolicy is CleanPolicyTypeOnFailure but backup succeeded
 		// The backup object has not been deleted or we need to retain backup data，do nothing
 		return nil
 	}

pkg/controller/backup/backup_control.go

@@ -99,16 +99,21 @@ func (c *defaultBackupControl) removeProtectionFinalizer(backup *v1alpha1.Backup
 }
 
 func needToAddFinalizer(backup *v1alpha1.Backup) bool {
-	return backup.DeletionTimestamp == nil && v1alpha1.IsCleanCandidate(backup) && !slice.ContainsString(backup.Finalizers, label.BackupProtectionFinalizer, nil)
+	return backup.DeletionTimestamp == nil && // not onDelete
+		!isProtectedByFinalizer(backup) &&
+		v1alpha1.CleanPolicyIsNotRetain(backup)
+
 }
 
 func needToRemoveFinalizer(backup *v1alpha1.Backup) bool {
-	return v1alpha1.IsCleanCandidate(backup) && isDeletionCandidate(backup) &&
-		(v1alpha1.IsBackupClean(backup) || v1alpha1.NeedNotClean(backup))
+	return backup.DeletionTimestamp != nil && // onDelete
+		isProtectedByFinalizer(backup) &&
+		v1alpha1.CleanPolicyIsNotRetain(backup) &&
+		(v1alpha1.IsBackupCleanedUp(backup) || v1alpha1.DontCleanSuccessBackup(backup))
 }
 
-func isDeletionCandidate(backup *v1alpha1.Backup) bool {
-	return backup.DeletionTimestamp != nil && slice.ContainsString(backup.Finalizers, label.BackupProtectionFinalizer, nil)
+func isProtectedByFinalizer(backup *v1alpha1.Backup) bool {
+	return slice.ContainsString(backup.Finalizers, label.BackupProtectionFinalizer, nil)
 }
 
 var _ ControlInterface = &defaultBackupControl{}