fix: record the failed message in the status condition if the restore… (

#7362) record the failed message in the status condition
apecloud · May 17, 2024 · 5b1a49b · 5b1a49b
1 parent c5b629c
commit 5b1a49b
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 17 deletions.
diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go
@@ -206,17 +206,24 @@ func (r *RestoreReconciler) newAction(reqCtx intctrlutil.RequestCtx, restore *dp
 	repoName, err := CheckBackupRepoForRestore(reqCtx, r.Client, restore)
 	switch {
 	case intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal):
+		dprestore.SetRestoreCheckBackupRepoCondition(restore, dprestore.ReasonCheckBackupRepoFailed, err.Error())
 		restore.Status.Phase = dpv1alpha1.RestorePhaseFailed
 		restore.Status.CompletionTimestamp = &metav1.Time{Time: time.Now()}
 		r.Recorder.Event(restore, corev1.EventTypeWarning, dprestore.ReasonRestoreFailed, err.Error())
 	case intctrlutil.IsTargetError(err, dperrors.ErrorTypeWaitForBackupRepoPreparation):
+		dprestore.SetRestoreCheckBackupRepoCondition(restore, dprestore.ReasonWaitForBackupRepo, err.Error())
 		waitBackupRepo = true
 		restore.Labels[dataProtectionBackupRepoKey] = repoName
 		restore.Labels[dataProtectionWaitRepoPreparationKey] = trueVal
 	case err != nil:
-		return RecorderEventAndRequeue(reqCtx, r.Recorder, restore, err)
+		dprestore.SetRestoreCheckBackupRepoCondition(restore, ReasonUnknownError, err.Error())
+		sendWarningEventForError(r.Recorder, restore, err)
+		if patchErr := r.Client.Status().Patch(reqCtx.Ctx, restore, patch); patchErr != nil {
+			return intctrlutil.CheckedRequeueWithError(patchErr, reqCtx.Log, "")
+		}
+		return intctrlutil.RequeueWithError(err, reqCtx.Log, "")
 	default:
-		// no error, fallthrough
+		dprestore.SetRestoreCheckBackupRepoCondition(restore, dprestore.ReasonCheckBackupRepoSuccessfully, "")
 	}
 	if !reflect.DeepEqual(restore.ObjectMeta, oldRestore.ObjectMeta) {
 		if err := r.Client.Patch(reqCtx.Ctx, restore, patch); err != nil {
@@ -238,7 +245,7 @@ func (r *RestoreReconciler) newAction(reqCtx intctrlutil.RequestCtx, restore *dp
 		restore.Status.Phase = dpv1alpha1.RestorePhaseAsDataSource
 	} else {
 		// check if restore CR is legal
-		err := dprestore.ValidateAndInitRestoreMGR(reqCtx, r.Client, dprestore.NewRestoreManager(restore, r.Recorder, r.Scheme))
+		err := r.validateAndBuildMGR(reqCtx, dprestore.NewRestoreManager(restore, r.Recorder, r.Scheme))
 		switch {
 		case intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal):
 			restore.Status.Phase = dpv1alpha1.RestorePhaseFailed

diff --git a/controllers/dataprotection/restore_controller_test.go b/controllers/dataprotection/restore_controller_test.go
@@ -29,6 +29,7 @@ import (
 
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -245,6 +246,32 @@ var _ = Describe("Restore Controller test", func() {
 					}, nil)
 			})
 
+			It("test restore is failed when check failed in new action", func() {
+				By("expect for restore is Failed")
+				restore := initResourcesAndWaitRestore(true, false, true, dpv1alpha1.RestorePhaseFailed,
+					func(f *testdp.MockRestoreFactory) {
+						f.Get().Spec.Backup.Name = "wrongBackup"
+					}, nil)
+				By("check status.conditions")
+				Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) {
+					val := meta.IsStatusConditionFalse(r.Status.Conditions, dprestore.ConditionTypeRestoreCheckBackupRepo)
+					g.Expect(val).Should(BeTrue())
+				})).Should(Succeed())
+			})
+
+			It("test restore is failed when validate failed in new action", func() {
+				By("expect for restore is Failed")
+				restore := initResourcesAndWaitRestore(false, false, true, dpv1alpha1.RestorePhaseFailed, func(f *testdp.MockRestoreFactory) {
+					f.SetVolumeClaimsTemplate(testdp.MysqlTemplateName, testdp.DataVolumeName,
+						testdp.DataVolumeMountPath, "", int32(3), int32(0), nil)
+				}, nil)
+				By("check status.conditions")
+				Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) {
+					val := meta.IsStatusConditionFalse(r.Status.Conditions, dprestore.ConditionTypeRestoreValidationPassed)
+					g.Expect(val).Should(BeTrue())
+				})).Should(Succeed())
+			})
+
 			It("test restore is Failed when restore job is not Failed", func() {
 				By("expect for restore is Failed ")
 				restore := initResourcesAndWaitRestore(true, false, true, dpv1alpha1.RestorePhaseRunning,

diff --git a/controllers/dataprotection/utils.go b/controllers/dataprotection/utils.go
@@ -229,14 +229,13 @@ func getClusterLabelKeys() []string {
 	return []string{constant.AppInstanceLabelKey, constant.KBAppComponentLabelKey, constant.KBAppShardingNameLabelKey}
 }
 
-// sendWarningEventForError sends warning event for backup controller error
+// sendWarningEventForError sends warning event for controller error
 func sendWarningEventForError(recorder record.EventRecorder, obj client.Object, err error) {
 	controllerErr := intctrlutil.UnwrapControllerError(err)
 	if controllerErr != nil {
 		recorder.Eventf(obj, corev1.EventTypeWarning, string(controllerErr.Type), err.Error())
 	} else {
-		recorder.Eventf(obj, corev1.EventTypeWarning, "FailedCreatedBackup",
-			"Creating backup failed, error: %s", err.Error())
+		recorder.Eventf(obj, corev1.EventTypeWarning, "ReconcileFailed", "Reconciling failed, error: %s", err.Error())
 	}
 }
 

diff --git a/pkg/dataprotection/restore/types.go b/pkg/dataprotection/restore/types.go
@@ -28,18 +28,21 @@ const (
 	ConditionTypeRestorePreparedData     = "PrepareData"
 	ConditionTypeReadinessProbe          = "ReadinessProbe"
 	ConditionTypeRestorePostReady        = "PostReady"
-
+	ConditionTypeRestoreCheckBackupRepo  = "CheckBackupRepo"
 	// condition reasons
-	ReasonRestoreStarting      = "RestoreStarting"
-	ReasonRestoreCompleted     = "RestoreCompleted"
-	ReasonRestoreFailed        = "RestoreFailed"
-	ReasonValidateFailed       = "ValidateFailed"
-	ReasonValidateSuccessfully = "ValidateSuccessfully"
-	ReasonProcessing           = "Processing"
-	ReasonFailed               = "Failed"
-	ReasonSucceed              = "Succeed"
-	reasonCreateRestoreJob     = "CreateRestoreJob"
-	reasonCreateRestorePVC     = "CreateRestorePVC"
+	ReasonRestoreStarting             = "RestoreStarting"
+	ReasonRestoreCompleted            = "RestoreCompleted"
+	ReasonRestoreFailed               = "RestoreFailed"
+	ReasonCheckBackupRepoFailed       = "CheckBackupRepoFailed"
+	ReasonWaitForBackupRepo           = "WaitForBackupRepo"
+	ReasonCheckBackupRepoSuccessfully = "CheckBackupRepoSuccessfully"
+	ReasonValidateFailed              = "ValidateFailed"
+	ReasonValidateSuccessfully        = "ValidateSuccessfully"
+	ReasonProcessing                  = "Processing"
+	ReasonFailed                      = "Failed"
+	ReasonSucceed                     = "Succeed"
+	reasonCreateRestoreJob            = "CreateRestoreJob"
+	reasonCreateRestorePVC            = "CreateRestorePVC"
 )
 
 // labels key

diff --git a/pkg/dataprotection/restore/utils.go b/pkg/dataprotection/restore/utils.go
@@ -50,6 +50,13 @@ func SetRestoreCondition(restore *dpv1alpha1.Restore, status metav1.ConditionSta
 	}
 	meta.SetStatusCondition(&restore.Status.Conditions, condition)
 }
+func SetRestoreCheckBackupRepoCondition(restore *dpv1alpha1.Restore, reason, message string) {
+	status := metav1.ConditionFalse
+	if reason == ReasonCheckBackupRepoSuccessfully {
+		status = metav1.ConditionTrue
+	}
+	SetRestoreCondition(restore, status, ConditionTypeRestoreCheckBackupRepo, reason, message)
+}
 
 // SetRestoreValidationCondition sets restore condition which type is ConditionTypeRestoreValidationPassed.
 func SetRestoreValidationCondition(restore *dpv1alpha1.Restore, reason, message string) {