From 37c6256b702265238245ae6f4b09fe9a7949c9a7 Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 16:44:19 +0800 Subject: [PATCH 01/10] fix(dataprotection): serialize post-ready target jobs --- .../v1alpha1/actionset_types.go | 18 +++++ ...taprotection.kubeblocks.io_actionsets.yaml | 10 +++ .../dataprotection/restore_controller.go | 5 ++ ...taprotection.kubeblocks.io_actionsets.yaml | 10 +++ pkg/dataprotection/restore/manager.go | 79 ++++++++++++++++++- pkg/dataprotection/restore/manager_test.go | 49 ++++++++++++ 6 files changed, 169 insertions(+), 2 deletions(-) diff --git a/apis/dataprotection/v1alpha1/actionset_types.go b/apis/dataprotection/v1alpha1/actionset_types.go index 3df07149a4f..66f88a3faa7 100644 --- a/apis/dataprotection/v1alpha1/actionset_types.go +++ b/apis/dataprotection/v1alpha1/actionset_types.go @@ -168,6 +168,14 @@ type RestoreActionSpec struct { // +optional PostReady []ActionSpec `json:"postReady,omitempty"` + // Specifies how postReady actions run when they target multiple pods. + // Parallel preserves the existing behavior. Serial waits for each target + // action to complete before starting the next one. + // + // +optional + // +kubebuilder:default=Parallel + PostReadyExecutionPolicy PostReadyExecutionPolicy `json:"postReadyExecutionPolicy,omitempty"` + // Determines if a base backup is required during restoration. // // +optional @@ -180,6 +188,16 @@ type RestoreActionSpec struct { WithParameters []string `json:"withParameters,omitempty"` } +// PostReadyExecutionPolicy specifies how postReady actions execute across target pods. +// +enum +// +kubebuilder:validation:Enum={Parallel,Serial} +type PostReadyExecutionPolicy string + +const ( + PostReadyExecutionPolicyParallel PostReadyExecutionPolicy = "Parallel" + PostReadyExecutionPolicySerial PostReadyExecutionPolicy = "Serial" +) + // ActionSpec defines an action that should be executed. Only one of the fields may be set. type ActionSpec struct { // Specifies that the action should be executed using the pod's exec API within a container. diff --git a/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml b/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml index 15b2de1b464..4196b5ed700 100644 --- a/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml +++ b/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml @@ -553,6 +553,16 @@ spec: type: object type: object type: array + postReadyExecutionPolicy: + default: Parallel + description: |- + Specifies how postReady actions run when they target multiple pods. + Parallel preserves the existing behavior. Serial waits for each target + action to complete before starting the next one. + enum: + - Parallel + - Serial + type: string prepareData: description: Specifies the action required to prepare data for restoration. diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go index 4c94a3cd6e1..c51b784bb46 100644 --- a/controllers/dataprotection/restore_controller.go +++ b/controllers/dataprotection/restore_controller.go @@ -512,6 +512,11 @@ func (r *RestoreReconciler) handleBackupActionSet(reqCtx intctrlutil.RequestCtx, if err != nil { return false, err } + if stage == dpv1alpha1.PostReady { + if err = restoreMgr.ResumeNextSerialPostReadyJob(reqCtx, r.Client, backupSet, jobs); err != nil { + return false, err + } + } // 4. check if jobs are finished. allActionsFinished, existFailedAction, err = restoreMgr.CheckJobsDone(stage, actionName, backupSet, jobs) diff --git a/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml b/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml index 15b2de1b464..4196b5ed700 100644 --- a/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml +++ b/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml @@ -553,6 +553,16 @@ spec: type: object type: object type: array + postReadyExecutionPolicy: + default: Parallel + description: |- + Specifies how postReady actions run when they target multiple pods. + Parallel preserves the existing behavior. Serial waits for each target + action to complete before starting the next one. + enum: + - Parallel + - Serial + type: string prepareData: description: Specifies the action required to prepare data for restoration. diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index c456154ac92..10d7a63eeba 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -23,6 +23,7 @@ import ( "context" "fmt" "sort" + "strconv" "strings" "time" @@ -800,10 +801,75 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, return restoreJobs, nil } + var jobs []*batchv1.Job if actionSpec.Job != nil { - return buildJobsForJobAction() + jobs, err = buildJobsForJobAction() + } else { + jobs, err = buildJobsForExecAction() + } + if err != nil { + return nil, err + } + if isSerialPostReady(backupSet) { + for i := range jobs { + suspend := i > 0 + jobs[i].Spec.Suspend = &suspend + } + } + return jobs, nil +} + +func isSerialPostReady(backupSet BackupActionSet) bool { + return backupSet.ActionSet != nil && + backupSet.ActionSet.Spec.Restore != nil && + backupSet.ActionSet.Spec.Restore.PostReadyExecutionPolicy == dpv1alpha1.PostReadyExecutionPolicySerial +} + +// ResumeNextSerialPostReadyJob starts at most one suspended postReady job after +// every preceding job has completed successfully. +func (r *RestoreManager) ResumeNextSerialPostReadyJob( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + backupSet BackupActionSet, + jobs []*batchv1.Job, +) error { + if !isSerialPostReady(backupSet) { + return nil + } + sort.Slice(jobs, func(i, j int) bool { + return postReadyJobIndex(jobs[i].Name) < postReadyJobIndex(jobs[j].Name) + }) + for i := range jobs { + done, _, errMsg := utils.IsJobFinished(jobs[i]) + if errMsg != "" { + return nil + } + if done { + continue + } + if jobs[i].Spec.Suspend != nil && *jobs[i].Spec.Suspend { + updated := jobs[i].DeepCopy() + updated.Spec.Suspend = boolptr.False() + if err := cli.Patch(reqCtx.Ctx, updated, client.MergeFrom(jobs[i])); err != nil { + return err + } + jobs[i] = updated + } + return nil } - return buildJobsForExecAction() + return nil +} + +func postReadyJobIndex(name string) int { + separator := strings.LastIndexByte(name, '-') + if separator < 0 { + return int(^uint(0) >> 1) + } + index, err := strconv.Atoi(name[separator+1:]) + if err != nil { + return int(^uint(0) >> 1) + } + return index } func (r *RestoreManager) createPVCIfNotExist( @@ -883,6 +949,7 @@ func (r *RestoreManager) CheckJobsDone( if stage == dpv1alpha1.PostReady { restoreActions = &r.Restore.Status.Actions.PostReady } + serialPostReady := stage == dpv1alpha1.PostReady && isSerialPostReady(backupSet) // count the number of jobs that are completed, failed, // or have the normally terminated `restore` container finishedCount := 0 @@ -914,9 +981,17 @@ func (r *RestoreManager) CheckJobsDone( } if normalTerminated { finishedCount++ + if serialPostReady { + if err := r.StopManagerContainerByJob(fetchedJobs[i]); err != nil { + return false, false, err + } + } } } } + if serialPostReady && existFailedJob { + return true, true, nil + } // wait until all `restore` containers are terminated normally or jobs are completed or failed if finishedCount == len(fetchedJobs) { for i := range fetchedJobs { diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 7efc5d18c3d..0e49dcfe7c2 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -499,6 +499,9 @@ var _ = Describe("RestoreManager Test", func() { Expect(err).ShouldNot(HaveOccurred()) // the count of exec jobs should equal to the pods count of cluster Expect(len(jobs)).Should(Equal(2)) + for i := range jobs { + Expect(jobs[i].Spec.Suspend).Should(BeNil()) + } Expect(jobs[0].Namespace).Should(Equal(kbNamespace)) Expect(jobs[0].Spec.Template.Spec.ServiceAccountName).Should(Equal(execWorkerServiceAccountName)) @@ -532,6 +535,52 @@ var _ = Describe("RestoreManager Test", func() { testPostReady(false) }) + It("serializes postReady jobs across selected pods", func() { + reqCtx := getReqCtx() + matchLabels := map[string]string{ + constant.AppInstanceLabelKey: testdp.ClusterName, + } + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PostReadyExecutionPolicy = dpv1alpha1.PostReadyExecutionPolicySerial + })).Should(Succeed()) + oldControllerNamespace := viper.GetString(constant.CfgKeyCtrlrMgrNS) + viper.Set(constant.CfgKeyCtrlrMgrNS, testCtx.DefaultNamespace) + DeferCleanup(func() { viper.Set(constant.CfgKeyCtrlrMgrNS, oldControllerNamespace) }) + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName).SetJobActionConfig(matchLabels).SetExecActionConfig(matchLabels) + }) + testdp.NewFakeCluster(&testCtx) + + target := utils.GetBackupStatusTarget(backupSet.Backup, restoreMGR.Restore.Spec.Backup.SourceTargetName) + jobs, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 0) + Expect(err).ShouldNot(HaveOccurred()) + Expect(jobs).Should(HaveLen(2)) + Expect(jobs[0].Spec.Suspend).ShouldNot(BeNil()) + Expect(*jobs[0].Spec.Suspend).Should(BeFalse()) + Expect(jobs[1].Spec.Suspend).ShouldNot(BeNil()) + Expect(*jobs[1].Spec.Suspend).Should(BeTrue()) + for i := range jobs { + for j := range jobs[i].Spec.Template.Spec.Containers { + jobs[i].Spec.Template.Spec.Containers[j].Image = "test-image" + } + } + + jobs, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, jobs) + Expect(err).ShouldNot(HaveOccurred()) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, *backupSet, jobs)).Should(Succeed()) + second := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), second)).Should(Succeed()) + Expect(*second.Spec.Suspend).Should(BeTrue()) + + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[0]), batchv1.JobComplete) + for i := range jobs { + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), jobs[i])).Should(Succeed()) + } + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, *backupSet, jobs)).Should(Succeed()) + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), second)).Should(Succeed()) + Expect(*second.Spec.Suspend).Should(BeFalse()) + }) + Context("BuildContinuousRestoreManager", func() { const ( continuousBackupStartTime = "2023-01-01T09:00:00Z" From 415f954fb1252c12102ace8e461c9c86ae34536b Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 16:48:20 +0800 Subject: [PATCH 02/10] docs: regenerate dataprotection API reference --- .../api-reference/dataprotection.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/developer_docs/api-reference/dataprotection.md b/docs/developer_docs/api-reference/dataprotection.md index 56e9d7c59a1..1ce3361cc12 100644 --- a/docs/developer_docs/api-reference/dataprotection.md +++ b/docs/developer_docs/api-reference/dataprotection.md @@ -4796,6 +4796,27 @@ And only takes effect when the ‘strategy’ is set to ‘Any&rsquo +

PostReadyExecutionPolicy +(string alias)

+

+(Appears on:RestoreActionSpec) +

+
+

PostReadyExecutionPolicy specifies how postReady actions execute across target pods.

+
+ + + + + + + + + + + + +
ValueDescription

"Parallel"

"Serial"

PrepareDataConfig

@@ -5180,6 +5201,22 @@ JobActionSpec +postReadyExecutionPolicy
+ + +PostReadyExecutionPolicy + + + + +(Optional) +

Specifies how postReady actions run when they target multiple pods. +Parallel preserves the existing behavior. Serial waits for each target +action to complete before starting the next one.

+ + + + baseBackupRequired
bool From 09b8892afc631928ee9ebe1540b3f5825e98bd97 Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 17:18:54 +0800 Subject: [PATCH 03/10] fix: make serial post-ready retries deterministic --- .../dataprotection/restore_controller.go | 7 +- pkg/dataprotection/restore/manager.go | 118 ++++++++++++++++-- pkg/dataprotection/restore/manager_test.go | 91 +++++++++++++- 3 files changed, 203 insertions(+), 13 deletions(-) diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go index c51b784bb46..51f686243f6 100644 --- a/controllers/dataprotection/restore_controller.go +++ b/controllers/dataprotection/restore_controller.go @@ -507,13 +507,18 @@ func (r *RestoreReconciler) handleBackupActionSet(reqCtx intctrlutil.RequestCtx, if len(jobs) == 0 { return true, nil } + if stage == dpv1alpha1.PostReady { + if err = restoreMgr.FreezePostReadyExecutionPolicy(reqCtx, r.Client, jobs); err != nil { + return false, err + } + } // 3. create jobs jobs, err = restoreMgr.CreateJobsIfNotExist(reqCtx, r.Client, restoreMgr.Restore, jobs) if err != nil { return false, err } if stage == dpv1alpha1.PostReady { - if err = restoreMgr.ResumeNextSerialPostReadyJob(reqCtx, r.Client, backupSet, jobs); err != nil { + if err = restoreMgr.ResumeNextSerialPostReadyJob(reqCtx, r.Client, jobs); err != nil { return false, err } } diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 10d7a63eeba..02bf56c5ff4 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -49,7 +49,8 @@ import ( ) const ( - restoreManagerContainerName = "restore-manager" + restoreManagerContainerName = "restore-manager" + postReadyExecutionPolicyAnnotationKey = "dataprotection.kubeblocks.io/post-ready-execution-policy" ) type BackupActionSet struct { @@ -777,6 +778,7 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, if err != nil { return nil, err } + sort.Sort(intctrlutil.ByPodName(targetPodList.Items)) var restoreJobs []*batchv1.Job for i := range targetPodList.Items { containerName := actionSpec.Exec.Container @@ -810,11 +812,12 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, if err != nil { return nil, err } + policy := dpv1alpha1.PostReadyExecutionPolicyParallel if isSerialPostReady(backupSet) { - for i := range jobs { - suspend := i > 0 - jobs[i].Spec.Suspend = &suspend - } + policy = dpv1alpha1.PostReadyExecutionPolicySerial + } + for i := range jobs { + setPostReadyExecutionPolicy(jobs[i], policy) } return jobs, nil } @@ -825,15 +828,107 @@ func isSerialPostReady(backupSet BackupActionSet) bool { backupSet.ActionSet.Spec.Restore.PostReadyExecutionPolicy == dpv1alpha1.PostReadyExecutionPolicySerial } +func setPostReadyExecutionPolicy(job *batchv1.Job, policy dpv1alpha1.PostReadyExecutionPolicy) { + if job.Annotations == nil { + job.Annotations = map[string]string{} + } + job.Annotations[postReadyExecutionPolicyAnnotationKey] = string(policy) + if policy == dpv1alpha1.PostReadyExecutionPolicySerial { + job.Spec.Suspend = boolptr.True() + } else { + job.Spec.Suspend = nil + } +} + +func postReadyExecutionPolicyForJob(job *batchv1.Job) (dpv1alpha1.PostReadyExecutionPolicy, error) { + if job.Annotations == nil || job.Annotations[postReadyExecutionPolicyAnnotationKey] == "" { + return dpv1alpha1.PostReadyExecutionPolicyParallel, nil + } + policy := dpv1alpha1.PostReadyExecutionPolicy(job.Annotations[postReadyExecutionPolicyAnnotationKey]) + if policy != dpv1alpha1.PostReadyExecutionPolicyParallel && policy != dpv1alpha1.PostReadyExecutionPolicySerial { + return "", intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has invalid frozen execution policy %q", + job.Namespace, job.Name, policy)) + } + return policy, nil +} + +func serialPostReadyJobs(jobs []*batchv1.Job) (bool, error) { + if len(jobs) == 0 { + return false, nil + } + policy, err := postReadyExecutionPolicyForJob(jobs[0]) + if err != nil { + return false, err + } + for i := 1; i < len(jobs); i++ { + jobPolicy, err := postReadyExecutionPolicyForJob(jobs[i]) + if err != nil { + return false, err + } + if jobPolicy != policy { + return false, intctrlutil.NewFatalError("postReady jobs have inconsistent frozen execution policies") + } + } + return policy == dpv1alpha1.PostReadyExecutionPolicySerial, nil +} + +// FreezePostReadyExecutionPolicy preserves the policy chosen by the first +// created job across partial creation retries and ActionSet updates. +func (r *RestoreManager) FreezePostReadyExecutionPolicy( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + jobs []*batchv1.Job, +) error { + if len(jobs) == 0 { + return nil + } + policy, err := postReadyExecutionPolicyForJob(jobs[0]) + if err != nil { + return err + } + foundExisting := false + for i := range jobs { + existing := &batchv1.Job{} + if err := cli.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), existing); err != nil { + if apierrors.IsNotFound(err) { + continue + } + return err + } + if !r.isJobForRestoreAction(existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "restore job name collision: existing job %s/%s does not belong to restore %s/%s", + existing.Namespace, existing.Name, r.Restore.Namespace, r.Restore.Name)) + } + existingPolicy, err := postReadyExecutionPolicyForJob(existing) + if err != nil { + return err + } + if foundExisting && existingPolicy != policy { + return intctrlutil.NewFatalError("postReady jobs have inconsistent frozen execution policies") + } + policy = existingPolicy + foundExisting = true + } + for i := range jobs { + setPostReadyExecutionPolicy(jobs[i], policy) + } + return nil +} + // ResumeNextSerialPostReadyJob starts at most one suspended postReady job after // every preceding job has completed successfully. func (r *RestoreManager) ResumeNextSerialPostReadyJob( reqCtx intctrlutil.RequestCtx, cli client.Client, - backupSet BackupActionSet, jobs []*batchv1.Job, ) error { - if !isSerialPostReady(backupSet) { + serial, err := serialPostReadyJobs(jobs) + if err != nil { + return err + } + if !serial { return nil } sort.Slice(jobs, func(i, j int) bool { @@ -949,7 +1044,14 @@ func (r *RestoreManager) CheckJobsDone( if stage == dpv1alpha1.PostReady { restoreActions = &r.Restore.Status.Actions.PostReady } - serialPostReady := stage == dpv1alpha1.PostReady && isSerialPostReady(backupSet) + serialPostReady := false + if stage == dpv1alpha1.PostReady { + var err error + serialPostReady, err = serialPostReadyJobs(fetchedJobs) + if err != nil { + return false, false, err + } + } // count the number of jobs that are completed, failed, // or have the normally terminated `restore` container finishedCount := 0 diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 0e49dcfe7c2..da2bfe9ac34 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -20,6 +20,7 @@ along with this program. If not, see . package restore import ( + "context" "fmt" "strconv" "strings" @@ -47,6 +48,24 @@ import ( viper "github.com/apecloud/kubeblocks/pkg/viperx" ) +type reversePodListClient struct { + client.Client +} + +func (c reversePodListClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + if err := c.Client.List(ctx, list, opts...); err != nil { + return err + } + pods, ok := list.(*corev1.PodList) + if !ok { + return nil + } + for i, j := 0, len(pods.Items)-1; i < j; i, j = i+1, j-1 { + pods.Items[i], pods.Items[j] = pods.Items[j], pods.Items[i] + } + return nil +} + var _ = Describe("RestoreManager Test", func() { cleanEnv := func() { @@ -495,13 +514,15 @@ var _ = Describe("RestoreManager Test", func() { By("test with execAction and expect for creating 2 exec job") target := utils.GetBackupStatusTarget(backupSet.Backup, restoreMGR.Restore.Spec.Backup.SourceTargetName) // step 0 is the execAction in actionSet - jobs, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 0) + jobs, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, reversePodListClient{Client: k8sClient}, *backupSet, target, 0) Expect(err).ShouldNot(HaveOccurred()) // the count of exec jobs should equal to the pods count of cluster Expect(len(jobs)).Should(Equal(2)) for i := range jobs { Expect(jobs[i].Spec.Suspend).Should(BeNil()) + Expect(jobs[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicyParallel))) } + Expect(jobs[0].Spec.Template.Spec.Containers[0].Args[3] < jobs[1].Spec.Template.Spec.Containers[0].Args[3]).Should(BeTrue()) Expect(jobs[0].Namespace).Should(Equal(kbNamespace)) Expect(jobs[0].Spec.Template.Spec.ServiceAccountName).Should(Equal(execWorkerServiceAccountName)) @@ -556,31 +577,93 @@ var _ = Describe("RestoreManager Test", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(jobs).Should(HaveLen(2)) Expect(jobs[0].Spec.Suspend).ShouldNot(BeNil()) - Expect(*jobs[0].Spec.Suspend).Should(BeFalse()) + Expect(*jobs[0].Spec.Suspend).Should(BeTrue()) Expect(jobs[1].Spec.Suspend).ShouldNot(BeNil()) Expect(*jobs[1].Spec.Suspend).Should(BeTrue()) + for i := range jobs { + Expect(jobs[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicySerial))) + } for i := range jobs { for j := range jobs[i].Spec.Template.Spec.Containers { jobs[i].Spec.Template.Spec.Containers[j].Image = "test-image" } } + Expect(restoreMGR.FreezePostReadyExecutionPolicy(reqCtx, k8sClient, jobs)).Should(Succeed()) jobs, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, jobs) Expect(err).ShouldNot(HaveOccurred()) - Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, *backupSet, jobs)).Should(Succeed()) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, jobs)).Should(Succeed()) + first := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[0]), first)).Should(Succeed()) + Expect(*first.Spec.Suspend).Should(BeFalse()) second := &batchv1.Job{} Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), second)).Should(Succeed()) Expect(*second.Spec.Suspend).Should(BeTrue()) + By("keep the frozen serial policy after the ActionSet changes") + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PostReadyExecutionPolicy = dpv1alpha1.PostReadyExecutionPolicyParallel + })).Should(Succeed()) + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[0]), batchv1.JobComplete) for i := range jobs { Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), jobs[i])).Should(Succeed()) } - Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, *backupSet, jobs)).Should(Succeed()) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, jobs)).Should(Succeed()) Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), second)).Should(Succeed()) Expect(*second.Spec.Suspend).Should(BeFalse()) }) + It("freezes serial policy across a partial creation retry", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + first := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-partial-0", labels) + setPostReadyExecutionPolicy(first, dpv1alpha1.PostReadyExecutionPolicySerial) + Expect(k8sClient.Create(reqCtx.Ctx, first)).Should(Succeed()) + + desiredFirst := newRestoreJob(testCtx.DefaultNamespace, first.Name, labels) + desiredSecond := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-partial-1", labels) + setPostReadyExecutionPolicy(desiredFirst, dpv1alpha1.PostReadyExecutionPolicyParallel) + setPostReadyExecutionPolicy(desiredSecond, dpv1alpha1.PostReadyExecutionPolicyParallel) + desired := []*batchv1.Job{desiredFirst, desiredSecond} + + Expect(restoreMGR.FreezePostReadyExecutionPolicy(reqCtx, k8sClient, desired)).Should(Succeed()) + for i := range desired { + Expect(desired[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicySerial))) + Expect(*desired[i].Spec.Suspend).Should(BeTrue()) + } + }) + + It("does not resume a serial job after a failed predecessor", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + jobs := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-failed-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-failed-1", labels), + } + for i := range jobs { + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + Expect(k8sClient.Create(reqCtx.Ctx, jobs[i])).Should(Succeed()) + } + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[0]), batchv1.JobFailed) + for i := range jobs { + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), jobs[i])).Should(Succeed()) + } + + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, jobs)).Should(Succeed()) + second := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), second)).Should(Succeed()) + Expect(*second.Spec.Suspend).Should(BeTrue()) + }) + Context("BuildContinuousRestoreManager", func() { const ( continuousBackupStartTime = "2023-01-01T09:00:00Z" From c1829e7c6c158957ebc9deefd94f5340b385f1b7 Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 17:43:45 +0800 Subject: [PATCH 04/10] fix: freeze post-ready target plan --- .../dataprotection/restore_controller.go | 2 +- pkg/dataprotection/restore/manager.go | 238 ++++++++++++++++-- pkg/dataprotection/restore/manager_test.go | 148 +++++++++-- pkg/dataprotection/restore/utils_test.go | 97 +++++++ 4 files changed, 450 insertions(+), 35 deletions(-) diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go index 51f686243f6..ff5556ddc3c 100644 --- a/controllers/dataprotection/restore_controller.go +++ b/controllers/dataprotection/restore_controller.go @@ -508,7 +508,7 @@ func (r *RestoreReconciler) handleBackupActionSet(reqCtx intctrlutil.RequestCtx, return true, nil } if stage == dpv1alpha1.PostReady { - if err = restoreMgr.FreezePostReadyExecutionPolicy(reqCtx, r.Client, jobs); err != nil { + if jobs, err = restoreMgr.FreezePostReadyExecutionPlan(reqCtx, r.Client, jobs); err != nil { return false, err } } diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 02bf56c5ff4..81750e7c860 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -21,6 +21,7 @@ package restore import ( "context" + "encoding/json" "fmt" "sort" "strconv" @@ -51,6 +52,8 @@ import ( const ( restoreManagerContainerName = "restore-manager" postReadyExecutionPolicyAnnotationKey = "dataprotection.kubeblocks.io/post-ready-execution-policy" + postReadyTargetIdentityAnnotationKey = "dataprotection.kubeblocks.io/post-ready-target-identity" + postReadyTargetPlanAnnotationKey = "dataprotection.kubeblocks.io/post-ready-target-plan" ) type BackupActionSet struct { @@ -764,7 +767,9 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, // no need to recover the volume when the pod selection policy is 'All' and sourceTargetPodName is not found. continue } - jobs = append(jobs, buildJob(&targetPodList.Items[i], sourceTargetPodName, i)) + job := buildJob(&targetPodList.Items[i], sourceTargetPodName, i) + setPostReadyTargetIdentity(job, &targetPodList.Items[i], sourceTargetPodName) + jobs = append(jobs, job) } return jobs, nil } @@ -798,6 +803,7 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, job.Spec.Template.Spec.ServiceAccountName = viper.GetString(dptypes.CfgKeyExecWorkerServiceAccountName) } job.Labels[DataProtectionRestoreNamespaceLabelKey] = r.Restore.Namespace + setPostReadyTargetIdentity(job, &targetPodList.Items[i], "") restoreJobs = append(restoreJobs, job) } return restoreJobs, nil @@ -819,6 +825,9 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, for i := range jobs { setPostReadyExecutionPolicy(jobs[i], policy) } + if err := setPostReadyTargetPlan(jobs); err != nil { + return nil, err + } return jobs, nil } @@ -840,6 +849,69 @@ func setPostReadyExecutionPolicy(job *batchv1.Job, policy dpv1alpha1.PostReadyEx } } +func setPostReadyTargetIdentity(job *batchv1.Job, pod *corev1.Pod, sourceTargetPodName string) { + if job.Annotations == nil { + job.Annotations = map[string]string{} + } + target := types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }.String() + job.Annotations[postReadyTargetIdentityAnnotationKey] = fmt.Sprintf("%s|source=%s", target, sourceTargetPodName) +} + +func postReadyTargetIdentity(job *batchv1.Job) string { + if job.Annotations == nil { + return "" + } + return job.Annotations[postReadyTargetIdentityAnnotationKey] +} + +func hasPostReadyFrozenContract(job *batchv1.Job) bool { + if job.Annotations == nil { + return false + } + return job.Annotations[postReadyExecutionPolicyAnnotationKey] != "" || + job.Annotations[postReadyTargetIdentityAnnotationKey] != "" || + job.Annotations[postReadyTargetPlanAnnotationKey] != "" +} + +func setPostReadyTargetPlan(jobs []*batchv1.Job) error { + plan := make([]string, 0, len(jobs)) + for i := range jobs { + identity := postReadyTargetIdentity(jobs[i]) + if identity == "" { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has empty target identity", jobs[i].Namespace, jobs[i].Name)) + } + plan = append(plan, identity) + } + serialized, err := json.Marshal(plan) + if err != nil { + return err + } + for i := range jobs { + jobs[i].Annotations[postReadyTargetPlanAnnotationKey] = string(serialized) + } + return nil +} + +func postReadyTargetPlan(job *batchv1.Job) ([]string, error) { + if job.Annotations == nil || job.Annotations[postReadyTargetPlanAnnotationKey] == "" { + return nil, nil + } + var plan []string + if err := json.Unmarshal([]byte(job.Annotations[postReadyTargetPlanAnnotationKey]), &plan); err != nil { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has invalid frozen target plan: %v", job.Namespace, job.Name, err)) + } + if len(plan) == 0 { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has empty frozen target plan", job.Namespace, job.Name)) + } + return plan, nil +} + func postReadyExecutionPolicyForJob(job *batchv1.Job) (dpv1alpha1.PostReadyExecutionPolicy, error) { if job.Annotations == nil || job.Annotations[postReadyExecutionPolicyAnnotationKey] == "" { return dpv1alpha1.PostReadyExecutionPolicyParallel, nil @@ -873,50 +945,171 @@ func serialPostReadyJobs(jobs []*batchv1.Job) (bool, error) { return policy == dpv1alpha1.PostReadyExecutionPolicySerial, nil } -// FreezePostReadyExecutionPolicy preserves the policy chosen by the first -// created job across partial creation retries and ActionSet updates. -func (r *RestoreManager) FreezePostReadyExecutionPolicy( +// FreezePostReadyExecutionPlan preserves the policy and ordered target set +// chosen by the first created job across partial retries and ActionSet updates. +func (r *RestoreManager) FreezePostReadyExecutionPlan( reqCtx intctrlutil.RequestCtx, cli client.Client, jobs []*batchv1.Job, -) error { +) ([]*batchv1.Job, error) { if len(jobs) == 0 { - return nil + return jobs, nil } policy, err := postReadyExecutionPolicyForJob(jobs[0]) if err != nil { - return err + return nil, err } foundExisting := false + var frozenPlan []string + allInputJobsPersisted := true + for i := range jobs { + if jobs[i].ResourceVersion == "" { + allInputJobsPersisted = false + break + } + } for i := range jobs { existing := &batchv1.Job{} if err := cli.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), existing); err != nil { if apierrors.IsNotFound(err) { continue } - return err + return nil, err } if !r.isJobForRestoreAction(existing) { - return intctrlutil.NewFatalError(fmt.Sprintf( + return nil, intctrlutil.NewFatalError(fmt.Sprintf( "restore job name collision: existing job %s/%s does not belong to restore %s/%s", existing.Namespace, existing.Name, r.Restore.Namespace, r.Restore.Name)) } + if !hasPostReadyFrozenContract(existing) { + if allInputJobsPersisted { + for j := range jobs { + if hasPostReadyFrozenContract(jobs[j]) { + return nil, intctrlutil.NewFatalError("legacy and frozen postReady jobs cannot be mixed") + } + } + return jobs, nil + } + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s cannot be combined with a new frozen target plan", + existing.Namespace, existing.Name)) + } existingPolicy, err := postReadyExecutionPolicyForJob(existing) if err != nil { - return err + return nil, err } if foundExisting && existingPolicy != policy { - return intctrlutil.NewFatalError("postReady jobs have inconsistent frozen execution policies") + return nil, intctrlutil.NewFatalError("postReady jobs have inconsistent frozen execution policies") } policy = existingPolicy + plan, err := postReadyTargetPlan(existing) + if err != nil { + return nil, err + } + if len(plan) == 0 { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has no frozen target plan", existing.Namespace, existing.Name)) + } + if len(frozenPlan) > 0 && strings.Join(plan, "\x00") != strings.Join(frozenPlan, "\x00") { + return nil, intctrlutil.NewFatalError("postReady jobs have inconsistent frozen target plans") + } + index := postReadyJobIndex(existing.Name) + if index < 0 || index >= len(plan) || postReadyTargetIdentity(existing) != plan[index] { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s target identity does not match its frozen plan", + existing.Namespace, existing.Name)) + } + frozenPlan = plan foundExisting = true } + if !foundExisting { + return jobs, nil + } + + jobsByTarget := make(map[string]*batchv1.Job, len(jobs)) for i := range jobs { - setPostReadyExecutionPolicy(jobs[i], policy) + identity := postReadyTargetIdentity(jobs[i]) + if identity == "" { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has empty target identity", jobs[i].Namespace, jobs[i].Name)) + } + if _, ok := jobsByTarget[identity]; ok { + return nil, intctrlutil.NewFatalError(fmt.Sprintf("duplicate postReady target identity %s", identity)) + } + jobsByTarget[identity] = jobs[i] + } + frozenJobs := make([]*batchv1.Job, 0, len(frozenPlan)) + for i, identity := range frozenPlan { + job, ok := jobsByTarget[identity] + if !ok { + return nil, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRequeue, + "postReady frozen target %s is not currently available", identity) + } + job = job.DeepCopy() + job.Name = postReadyJobNameForIndex(jobs[0].Name, i) + setPostReadyExecutionPolicy(job, policy) + serializedPlan, err := json.Marshal(frozenPlan) + if err != nil { + return nil, err + } + job.Annotations[postReadyTargetPlanAnnotationKey] = string(serializedPlan) + frozenJobs = append(frozenJobs, job) + } + return frozenJobs, nil +} + +func (r *RestoreManager) validateExistingRestoreActionJob(desired, existing *batchv1.Job) error { + if !r.isJobForRestoreAction(existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "restore job name collision: existing job %s/%s does not belong to restore %s/%s", + existing.Namespace, existing.Name, r.Restore.Namespace, r.Restore.Name)) + } + desiredIdentity := postReadyTargetIdentity(desired) + if desiredIdentity == "" { + return nil + } + if postReadyTargetIdentity(existing) != desiredIdentity { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s target identity does not match desired target", + existing.Namespace, existing.Name)) + } + desiredPolicy, err := postReadyExecutionPolicyForJob(desired) + if err != nil { + return err + } + existingPolicy, err := postReadyExecutionPolicyForJob(existing) + if err != nil { + return err + } + if existingPolicy != desiredPolicy { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s execution policy does not match desired policy", + existing.Namespace, existing.Name)) + } + desiredPlan, err := postReadyTargetPlan(desired) + if err != nil { + return err + } + existingPlan, err := postReadyTargetPlan(existing) + if err != nil { + return err + } + if len(desiredPlan) == 0 || strings.Join(existingPlan, "\x00") != strings.Join(desiredPlan, "\x00") { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s frozen target plan does not match desired plan", + existing.Namespace, existing.Name)) } return nil } +func postReadyJobNameForIndex(name string, index int) string { + separator := strings.LastIndexByte(name, '-') + if separator < 0 { + return name + } + return fmt.Sprintf("%s-%d", name[:separator], index) +} + // ResumeNextSerialPostReadyJob starts at most one suspended postReady job after // every preceding job has completed successfully. func (r *RestoreManager) ResumeNextSerialPostReadyJob( @@ -1012,17 +1205,26 @@ func (r *RestoreManager) CreateJobsIfNotExist(reqCtx intctrlutil.RequestCtx, return nil, err } } - if err = cli.Create(reqCtx.Ctx, objs[i]); err != nil && !apierrors.IsAlreadyExists(err) { - return nil, err + if err = cli.Create(reqCtx.Ctx, objs[i]); err != nil { + if !apierrors.IsAlreadyExists(err) { + return nil, err + } + fetchedJob = &batchv1.Job{} + if err = cli.Get(reqCtx.Ctx, client.ObjectKeyFromObject(objs[i]), fetchedJob); err != nil { + return nil, err + } + if err = r.validateExistingRestoreActionJob(objs[i], fetchedJob); err != nil { + return nil, err + } + fetchedJobs = append(fetchedJobs, fetchedJob) + continue } msg := fmt.Sprintf("created job %s/%s", objs[i].Namespace, objs[i].Name) r.Recorder.Event(r.Restore, corev1.EventTypeNormal, reasonCreateRestoreJob, msg) fetchedJobs = append(fetchedJobs, objs[i]) } else { - if !r.isJobForRestoreAction(fetchedJob) { - err := fmt.Sprintf("restore job name collision: existing job %s/%s does not belong to restore %s/%s", - fetchedJob.Namespace, fetchedJob.Name, r.Restore.Namespace, r.Restore.Name) - return nil, intctrlutil.NewFatalError(err) + if err = r.validateExistingRestoreActionJob(objs[i], fetchedJob); err != nil { + return nil, err } fetchedJobs = append(fetchedJobs, fetchedJob) } diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index da2bfe9ac34..1f72d6fc637 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -32,6 +32,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -52,6 +53,32 @@ type reversePodListClient struct { client.Client } +type createAlreadyExistsClient struct { + client.Client + foreign *batchv1.Job +} + +type failNthCreateClient struct { + client.Client + n int + count int +} + +func (c *failNthCreateClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + c.count++ + if c.count == c.n { + return fmt.Errorf("injected create failure at %d", c.n) + } + return c.Client.Create(ctx, obj, opts...) +} + +func (c createAlreadyExistsClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if err := c.Client.Create(ctx, c.foreign.DeepCopy(), opts...); err != nil { + return err + } + return apierrors.NewAlreadyExists(schema.GroupResource{Group: "batch", Resource: "jobs"}, obj.GetName()) +} + func (c reversePodListClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { if err := c.Client.List(ctx, list, opts...); err != nil { return err @@ -589,7 +616,8 @@ var _ = Describe("RestoreManager Test", func() { } } - Expect(restoreMGR.FreezePostReadyExecutionPolicy(reqCtx, k8sClient, jobs)).Should(Succeed()) + jobs, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) jobs, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, jobs) Expect(err).ShouldNot(HaveOccurred()) Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, jobs)).Should(Succeed()) @@ -614,28 +642,68 @@ var _ = Describe("RestoreManager Test", func() { Expect(*second.Spec.Suspend).Should(BeFalse()) }) - It("freezes serial policy across a partial creation retry", func() { + It("freezes target membership and serial policy across a partial creation retry", func() { reqCtx := getReqCtx() restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) labels := map[string]string{ DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, } - first := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-partial-0", labels) - setPostReadyExecutionPolicy(first, dpv1alpha1.PostReadyExecutionPolicySerial) - Expect(k8sClient.Create(reqCtx.Ctx, first)).Should(Succeed()) - - desiredFirst := newRestoreJob(testCtx.DefaultNamespace, first.Name, labels) - desiredSecond := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-partial-1", labels) - setPostReadyExecutionPolicy(desiredFirst, dpv1alpha1.PostReadyExecutionPolicyParallel) - setPostReadyExecutionPolicy(desiredSecond, dpv1alpha1.PostReadyExecutionPolicyParallel) - desired := []*batchv1.Job{desiredFirst, desiredSecond} - - Expect(restoreMGR.FreezePostReadyExecutionPolicy(reqCtx, k8sClient, desired)).Should(Succeed()) - for i := range desired { - Expect(desired[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicySerial))) - Expect(*desired[i].Spec.Suspend).Should(BeTrue()) + newTargetJob := func(index int, target string, policy dpv1alpha1.PostReadyExecutionPolicy) *batchv1.Job { + job := newRestoreJob(testCtx.DefaultNamespace, fmt.Sprintf("restore-post-ready-partial-%d", index), labels) + job.Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: target} + setPostReadyExecutionPolicy(job, policy) + return job + } + + oldPlan := []*batchv1.Job{ + newTargetJob(0, "default/pod-b", dpv1alpha1.PostReadyExecutionPolicySerial), + newTargetJob(1, "default/pod-c", dpv1alpha1.PostReadyExecutionPolicySerial), + } + Expect(setPostReadyTargetPlan(oldPlan)).Should(Succeed()) + failingClient := &failNthCreateClient{Client: k8sClient, n: 2} + created, err := restoreMGR.CreateJobsIfNotExist(reqCtx, failingClient, restoreMGR.Restore, oldPlan) + Expect(err).Should(HaveOccurred()) + Expect(created).Should(BeNil()) + firstAfterFailure := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(oldPlan[0]), firstAfterFailure)).Should(Succeed()) + Expect(*firstAfterFailure.Spec.Suspend).Should(BeTrue()) + Expect(apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(oldPlan[1]), &batchv1.Job{}))).Should(BeTrue()) + + missingTarget := []*batchv1.Job{ + newTargetJob(0, "default/pod-a", dpv1alpha1.PostReadyExecutionPolicyParallel), + newTargetJob(1, "default/pod-b", dpv1alpha1.PostReadyExecutionPolicyParallel), + } + Expect(setPostReadyTargetPlan(missingTarget)).Should(Succeed()) + _, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, missingTarget) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("frozen target default/pod-c is not currently available")) + + current := []*batchv1.Job{ + newTargetJob(0, "default/pod-a", dpv1alpha1.PostReadyExecutionPolicyParallel), + newTargetJob(1, "default/pod-b", dpv1alpha1.PostReadyExecutionPolicyParallel), + newTargetJob(2, "default/pod-c", dpv1alpha1.PostReadyExecutionPolicyParallel), } + Expect(setPostReadyTargetPlan(current)).Should(Succeed()) + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, current) + Expect(err).ShouldNot(HaveOccurred()) + Expect(frozen).Should(HaveLen(2)) + for i, target := range []string{"default/pod-b", "default/pod-c"} { + Expect(frozen[i].Name).Should(Equal(fmt.Sprintf("restore-post-ready-partial-%d", i))) + Expect(postReadyTargetIdentity(frozen[i])).Should(Equal(target)) + Expect(frozen[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicySerial))) + Expect(*frozen[i].Spec.Suspend).Should(BeTrue()) + } + + frozen, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, frozen) + Expect(err).ShouldNot(HaveOccurred()) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, frozen)).Should(Succeed()) + first := &batchv1.Job{} + second := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(frozen[0]), first)).Should(Succeed()) + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(frozen[1]), second)).Should(Succeed()) + Expect(*first.Spec.Suspend).Should(BeFalse()) + Expect(*second.Spec.Suspend).Should(BeTrue()) }) It("does not resume a serial job after a failed predecessor", func() { @@ -651,6 +719,10 @@ var _ = Describe("RestoreManager Test", func() { } for i := range jobs { setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + jobs[i].Annotations[postReadyTargetIdentityAnnotationKey] = fmt.Sprintf("default/pod-%d", i) + } + Expect(setPostReadyTargetPlan(jobs)).Should(Succeed()) + for i := range jobs { Expect(k8sClient.Create(reqCtx.Ctx, jobs[i])).Should(Succeed()) } testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[0]), batchv1.JobFailed) @@ -664,6 +736,50 @@ var _ = Describe("RestoreManager Test", func() { Expect(*second.Spec.Suspend).Should(BeTrue()) }) + It("refetches and rejects a foreign Job after an AlreadyExists create race", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + desired := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-race-0", labels) + desired.Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: "default/pod-0"} + setPostReadyExecutionPolicy(desired, dpv1alpha1.PostReadyExecutionPolicySerial) + Expect(setPostReadyTargetPlan([]*batchv1.Job{desired})).Should(Succeed()) + + foreign := desired.DeepCopy() + foreign.Labels[DataProtectionRestoreLabelKey] = "another-restore" + jobs, err := restoreMGR.CreateJobsIfNotExist(reqCtx, + createAlreadyExistsClient{Client: k8sClient, foreign: foreign}, restoreMGR.Restore, []*batchv1.Job{desired}) + Expect(err).Should(HaveOccurred()) + Expect(jobs).Should(BeNil()) + Expect(err.Error()).Should(ContainSubstring("does not belong to restore")) + }) + + It("keeps a fully persisted legacy postReady action on the Parallel path", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + legacy := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-legacy-0", map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + }) + Expect(k8sClient.Create(reqCtx.Ctx, legacy)).Should(Succeed()) + persisted := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(legacy), persisted)).Should(Succeed()) + Expect(persisted.ResourceVersion).ShouldNot(BeEmpty()) + + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, []*batchv1.Job{persisted}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(frozen).Should(HaveLen(1)) + Expect(frozen[0].Annotations).Should(BeEmpty()) + serial, err := serialPostReadyJobs(frozen) + Expect(err).ShouldNot(HaveOccurred()) + Expect(serial).Should(BeFalse()) + Expect(frozen[0].Spec.Suspend).ShouldNot(BeNil()) + Expect(*frozen[0].Spec.Suspend).Should(BeFalse()) + }) + Context("BuildContinuousRestoreManager", func() { const ( continuousBackupStartTime = "2023-01-01T09:00:00Z" diff --git a/pkg/dataprotection/restore/utils_test.go b/pkg/dataprotection/restore/utils_test.go index 6ce44c95b7d..ee42cd18d9d 100644 --- a/pkg/dataprotection/restore/utils_test.go +++ b/pkg/dataprotection/restore/utils_test.go @@ -21,6 +21,7 @@ package restore import ( "context" + "strconv" "testing" "time" @@ -345,3 +346,99 @@ func TestRestoreManagerStopsManagerContainer(t *testing.T) { assert.NoError(t, mgr.StopManagerContainer(got)) assert.NoError(t, mgr.StopManagerContainerByJob(job)) } + +func TestSerialPostReadyNormalExitHandsOffToNextJob(t *testing.T) { + scheme := runtime.NewScheme() + assert.NoError(t, corev1.AddToScheme(scheme)) + assert.NoError(t, batchv1.AddToScheme(scheme)) + + jobs := []*batchv1.Job{ + {ObjectMeta: metav1.ObjectMeta{Name: "restore-post-ready-0", Namespace: "ns"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "restore-post-ready-1", Namespace: "ns"}}, + } + for i := range jobs { + jobs[i].Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: "ns/pod-" + strconv.Itoa(i)} + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + } + assert.NoError(t, setPostReadyTargetPlan(jobs)) + jobs[0].Spec.Suspend = func() *bool { v := false; return &v }() + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restore-post-ready-0-pod", + Namespace: "ns", + Labels: map[string]string{"job-name": jobs[0].Name}, + }, + Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{ + Name: Restore, + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}}, + }}}, + } + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(jobs[0], jobs[1], pod).Build() + mgr := &RestoreManager{ + Restore: &dpv1alpha1.Restore{}, + Client: cli, + } + backupSet := BackupActionSet{Backup: &dpv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "backup"}}} + + allDone, failed, err := mgr.CheckJobsDone(dpv1alpha1.PostReady, "postready-0", backupSet, jobs) + assert.NoError(t, err) + assert.False(t, allDone) + assert.False(t, failed) + gotPod := &corev1.Pod{} + assert.NoError(t, cli.Get(context.Background(), client.ObjectKeyFromObject(pod), gotPod)) + assert.Equal(t, "true", gotPod.Annotations[DataProtectionStopRestoreManagerAnnotationKey]) + + jobs[0].Status.Conditions = []batchv1.JobCondition{{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}} + reqCtx := intctrlutil.RequestCtx{Ctx: context.Background()} + assert.NoError(t, mgr.ResumeNextSerialPostReadyJob(reqCtx, cli, jobs)) + second := &batchv1.Job{} + assert.NoError(t, cli.Get(context.Background(), client.ObjectKeyFromObject(jobs[1]), second)) + assert.NotNil(t, second.Spec.Suspend) + assert.False(t, *second.Spec.Suspend) +} + +func TestSerialPostReadyUsesNumericOrderForElevenJobs(t *testing.T) { + scheme := runtime.NewScheme() + assert.NoError(t, batchv1.AddToScheme(scheme)) + jobs := make([]*batchv1.Job, 11) + for i := range jobs { + jobs[i] = &batchv1.Job{ObjectMeta: metav1.ObjectMeta{ + Name: "restore-post-ready-" + strconv.Itoa(i), + Namespace: "ns", + Annotations: map[string]string{postReadyTargetIdentityAnnotationKey: "ns/pod-" + strconv.Itoa(i)}, + }} + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + if i < 3 { + jobs[i].Status.Conditions = []batchv1.JobCondition{{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}} + } + } + assert.NoError(t, setPostReadyTargetPlan(jobs)) + objects := make([]client.Object, 0, len(jobs)) + for i := range jobs { + objects = append(objects, jobs[i]) + } + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + shuffled := []*batchv1.Job{jobs[10], jobs[4], jobs[2], jobs[8], jobs[0], jobs[6], jobs[1], jobs[9], jobs[5], jobs[3], jobs[7]} + assert.NoError(t, (&RestoreManager{}).ResumeNextSerialPostReadyJob(intctrlutil.RequestCtx{Ctx: context.Background()}, cli, shuffled)) + for i := 3; i < len(jobs); i++ { + got := &batchv1.Job{} + assert.NoError(t, cli.Get(context.Background(), client.ObjectKeyFromObject(jobs[i]), got)) + assert.NotNil(t, got.Spec.Suspend) + assert.Equal(t, i != 3, *got.Spec.Suspend) + } +} + +func TestSerialPostReadyRejectsInvalidAndMixedFrozenPolicies(t *testing.T) { + invalid := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "invalid", Annotations: map[string]string{ + postReadyExecutionPolicyAnnotationKey: "Unknown", + }}} + _, err := serialPostReadyJobs([]*batchv1.Job{invalid}) + assert.Error(t, err) + + parallel := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "parallel"}} + serial := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "serial"}} + setPostReadyExecutionPolicy(parallel, dpv1alpha1.PostReadyExecutionPolicyParallel) + setPostReadyExecutionPolicy(serial, dpv1alpha1.PostReadyExecutionPolicySerial) + _, err = serialPostReadyJobs([]*batchv1.Job{parallel, serial}) + assert.Error(t, err) +} From 938f8239ded53759cb32228e6401ece8ed366196 Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 17:52:18 +0800 Subject: [PATCH 05/10] fix: reject incomplete post-ready contracts --- pkg/dataprotection/restore/manager.go | 5 +++++ pkg/dataprotection/restore/manager_test.go | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 81750e7c860..f012d21ea36 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -914,6 +914,11 @@ func postReadyTargetPlan(job *batchv1.Job) ([]string, error) { func postReadyExecutionPolicyForJob(job *batchv1.Job) (dpv1alpha1.PostReadyExecutionPolicy, error) { if job.Annotations == nil || job.Annotations[postReadyExecutionPolicyAnnotationKey] == "" { + if hasPostReadyFrozenContract(job) { + return "", intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a frozen target contract without an execution policy", + job.Namespace, job.Name)) + } return dpv1alpha1.PostReadyExecutionPolicyParallel, nil } policy := dpv1alpha1.PostReadyExecutionPolicy(job.Annotations[postReadyExecutionPolicyAnnotationKey]) diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 1f72d6fc637..63f36f1e64b 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -772,7 +772,9 @@ var _ = Describe("RestoreManager Test", func() { frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, []*batchv1.Job{persisted}) Expect(err).ShouldNot(HaveOccurred()) Expect(frozen).Should(HaveLen(1)) - Expect(frozen[0].Annotations).Should(BeEmpty()) + Expect(frozen[0].Annotations).ShouldNot(HaveKey(postReadyExecutionPolicyAnnotationKey)) + Expect(frozen[0].Annotations).ShouldNot(HaveKey(postReadyTargetIdentityAnnotationKey)) + Expect(frozen[0].Annotations).ShouldNot(HaveKey(postReadyTargetPlanAnnotationKey)) serial, err := serialPostReadyJobs(frozen) Expect(err).ShouldNot(HaveOccurred()) Expect(serial).Should(BeFalse()) @@ -780,6 +782,19 @@ var _ = Describe("RestoreManager Test", func() { Expect(*frozen[0].Spec.Suspend).Should(BeFalse()) }) + It("rejects a frozen target contract without an execution policy", func() { + job := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-missing-policy-0", nil) + job.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: "default/pod-0", + postReadyTargetPlanAnnotationKey: `["default/pod-0"]`, + } + + serial, err := serialPostReadyJobs([]*batchv1.Job{job}) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("frozen target contract without an execution policy")) + Expect(serial).Should(BeFalse()) + }) + Context("BuildContinuousRestoreManager", func() { const ( continuousBackupStartTime = "2023-01-01T09:00:00Z" From e772fb0d4f785281e80b33eb4d8a75274058426a Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 18:08:42 +0800 Subject: [PATCH 06/10] fix: preserve post-ready plan across reconciles --- pkg/dataprotection/restore/manager.go | 95 ++++++++++++++-- pkg/dataprotection/restore/manager_test.go | 121 +++++++++++++++++++++ 2 files changed, 209 insertions(+), 7 deletions(-) diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index f012d21ea36..868a25c85fe 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -614,8 +614,10 @@ func (r *RestoreManager) BuildVolumePopulateJob( } // GetExistingActionJobs returns jobs already recorded in Restore status for an in-flight action. -// If any recorded Processing Job is missing, it returns an empty list so callers can follow -// the original build/create path. +// PostReady needs completed predecessors as well as processing jobs because the persisted set +// carries the frozen serial plan across reconciles. PrepareData retains its processing-only path. +// If any required recorded Job is missing, it returns an empty list so callers can follow the +// original build/create path. func (r *RestoreManager) GetExistingActionJobs( reqCtx intctrlutil.RequestCtx, cli client.Client, @@ -638,10 +640,12 @@ func (r *RestoreManager) GetExistingActionJobs( action := restoreActions[i] if action.BackupName != backupName || action.Name != actionName || - action.Status != dpv1alpha1.RestoreActionProcessing || !strings.HasPrefix(action.ObjectKey, jobKeyPrefix) { continue } + if stage != dpv1alpha1.PostReady && action.Status != dpv1alpha1.RestoreActionProcessing { + continue + } jobName := strings.TrimPrefix(action.ObjectKey, jobKeyPrefix) found := false for _, namespace := range namespaces { @@ -725,6 +729,11 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, return nil, err } sort.Sort(intctrlutil.ByPodName(targetPodList.Items)) + frozenSourceByTarget, hasFrozenPlan, err := r.getFrozenPostReadySourceTargets( + reqCtx, cli, types.NamespacedName{Namespace: r.Restore.Namespace, Name: buildJobName(0)}) + if err != nil { + return nil, err + } buildJob := func(targetPod *corev1.Pod, sourceTargetPodName string, index int) *batchv1.Job { if boolptr.IsSetToTrue(actionSpec.Job.RunOnTargetPodNode) { jobBuilder.resetSpecificVolumesAndMounts() @@ -750,7 +759,7 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, build() } - if podSelector.Strategy == dpv1alpha1.PodSelectionStrategyAny { + if podSelector.Strategy == dpv1alpha1.PodSelectionStrategyAny && !hasFrozenPlan { targetPod := utils.GetFirstIndexRunningPod(targetPodList) if targetPod == nil { return nil, fmt.Errorf("can not found any running pod by spec.readyConfig.jobAction.target.podSelector") @@ -759,9 +768,19 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, } var jobs []*batchv1.Job for i := range targetPodList.Items { - sourceTargetPodName, err := GetSourcePodNameFromTarget(target, jobAction.RequiredPolicyForAllPodSelection, i) - if err != nil { - return nil, err + targetName := types.NamespacedName{ + Namespace: targetPodList.Items[i].Namespace, + Name: targetPodList.Items[i].Name, + }.String() + sourceTargetPodName, selectedByFrozenPlan := frozenSourceByTarget[targetName] + if hasFrozenPlan && !selectedByFrozenPlan { + continue + } + if !hasFrozenPlan { + sourceTargetPodName, err = GetSourcePodNameFromTarget(target, jobAction.RequiredPolicyForAllPodSelection, i) + if err != nil { + return nil, err + } } if target.PodSelector.Strategy == dpv1alpha1.PodSelectionStrategyAll && sourceTargetPodName == "" { // no need to recover the volume when the pod selection policy is 'All' and sourceTargetPodName is not found. @@ -771,6 +790,10 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, setPostReadyTargetIdentity(job, &targetPodList.Items[i], sourceTargetPodName) jobs = append(jobs, job) } + if hasFrozenPlan && len(jobs) != len(frozenSourceByTarget) { + return nil, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRequeue, + "not all frozen postReady target pods are currently available") + } return jobs, nil } @@ -867,6 +890,64 @@ func postReadyTargetIdentity(job *batchv1.Job) string { return job.Annotations[postReadyTargetIdentityAnnotationKey] } +func splitPostReadyTargetIdentity(identity string) (target, source string) { + const sourceMarker = "|source=" + parts := strings.SplitN(identity, sourceMarker, 2) + if len(parts) == 1 { + return identity, "" + } + return parts[0], parts[1] +} + +// getFrozenPostReadySourceTargets returns the original target-to-source mapping +// after a partial create. JobAction must use it while rebuilding specs so the +// backup path does not drift when the selected target Pod set changes. +func (r *RestoreManager) getFrozenPostReadySourceTargets( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + firstJobKey types.NamespacedName, +) (map[string]string, bool, error) { + existing := &batchv1.Job{} + if err := cli.Get(reqCtx.Ctx, firstJobKey, existing); err != nil { + if apierrors.IsNotFound(err) { + return nil, false, nil + } + return nil, false, err + } + if !r.isJobForRestoreAction(existing) { + return nil, false, intctrlutil.NewFatalError(fmt.Sprintf( + "restore job name collision: existing job %s/%s does not belong to restore %s/%s", + existing.Namespace, existing.Name, r.Restore.Namespace, r.Restore.Name)) + } + if !hasPostReadyFrozenContract(existing) { + return nil, false, nil + } + if _, err := postReadyExecutionPolicyForJob(existing); err != nil { + return nil, false, err + } + plan, err := postReadyTargetPlan(existing) + if err != nil { + return nil, false, err + } + if len(plan) == 0 { + return nil, false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has no frozen target plan", existing.Namespace, existing.Name)) + } + sourceByTarget := make(map[string]string, len(plan)) + for _, identity := range plan { + target, source := splitPostReadyTargetIdentity(identity) + if target == "" { + return nil, false, intctrlutil.NewFatalError("postReady frozen target plan has an empty target") + } + if _, ok := sourceByTarget[target]; ok { + return nil, false, intctrlutil.NewFatalError(fmt.Sprintf( + "duplicate postReady frozen target %s", target)) + } + sourceByTarget[target] = source + } + return sourceByTarget, true, nil +} + func hasPostReadyFrozenContract(job *batchv1.Job) bool { if job.Annotations == nil { return false diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 63f36f1e64b..aed789aeb92 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -396,6 +396,56 @@ var _ = Describe("RestoreManager Test", func() { Expect(jobs[0].Name).Should(Equal(jobName)) }) + It("loads completed postReady predecessors with the processing successor", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := "postready-0" + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + jobs := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-postready-existing-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-postready-existing-1", labels), + } + for i := range jobs { + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + jobs[i].Annotations[postReadyTargetIdentityAnnotationKey] = fmt.Sprintf("default/pod-%d", i) + } + Expect(setPostReadyTargetPlan(jobs)).Should(Succeed()) + for i := range jobs { + Expect(k8sClient.Create(reqCtx.Ctx, jobs[i])).Should(Succeed()) + } + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[0]), batchv1.JobComplete) + restoreMGR.Restore.Status.Actions.PostReady = []dpv1alpha1.RestoreStatusAction{ + { + Name: actionName, + ObjectKey: BuildJobKeyForActionStatus(jobs[0].Name), + BackupName: backupSet.Backup.Name, + Status: dpv1alpha1.RestoreActionCompleted, + }, + { + Name: actionName, + ObjectKey: BuildJobKeyForActionStatus(jobs[1].Name), + BackupName: backupSet.Backup.Name, + Status: dpv1alpha1.RestoreActionProcessing, + }, + } + + persisted, err := restoreMGR.GetExistingActionJobs( + reqCtx, k8sClient, dpv1alpha1.PostReady, backupSet.Backup.Name, actionName) + Expect(err).ShouldNot(HaveOccurred()) + Expect(persisted).Should(HaveLen(2)) + persisted, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, persisted) + Expect(err).ShouldNot(HaveOccurred()) + Expect(persisted).Should(HaveLen(2)) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, persisted)).Should(Succeed()) + + next := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[1]), next)).Should(Succeed()) + Expect(*next.Spec.Suspend).Should(BeFalse()) + }) + It("should not use a controller-namespace action job without the Restore namespace label", func() { controllerNamespace := "kb-system-wrong-restore-ns" viper.Set(constant.CfgKeyCtrlrMgrNS, controllerNamespace) @@ -706,6 +756,77 @@ var _ = Describe("RestoreManager Test", func() { Expect(*second.Spec.Suspend).Should(BeTrue()) }) + It("preserves the OneToOne source mapping through the real Job builder after target insertion", func() { + reqCtx := getReqCtx() + oldToolsImage := viper.GetString(constant.KBToolsImage) + viper.Set(constant.KBToolsImage, "kubeblocks-tools") + DeferCleanup(func() { viper.Set(constant.KBToolsImage, oldToolsImage) }) + matchLabels := map[string]string{constant.AppInstanceLabelKey: testdp.ClusterName} + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PostReadyExecutionPolicy = dpv1alpha1.PostReadyExecutionPolicySerial + })).Should(Succeed()) + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName).SetJobActionConfig(matchLabels) + }) + restoreMGR.Restore.Spec.ReadyConfig.JobAction.RequiredPolicyForAllPodSelection = + &dpv1alpha1.RequiredPolicyForAllPodSelection{DataRestorePolicy: dpv1alpha1.OneToOneRestorePolicy} + restoreMGR.Restore.Spec.ReadyConfig.JobAction.Target.PodSelector.Strategy = + dpv1alpha1.PodSelectionStrategyAll + testdp.NewFakeCluster(&testCtx) + target := utils.GetBackupStatusTarget(backupSet.Backup, restoreMGR.Restore.Spec.Backup.SourceTargetName) + target.PodSelector.Strategy = dpv1alpha1.PodSelectionStrategyAll + target.SelectedTargetPods = []string{"source-b", "source-c"} + backupSet.Backup.Status.Path = "/repo/default/test-backup" + + initial, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 1) + Expect(err).ShouldNot(HaveOccurred()) + Expect(initial).Should(HaveLen(2)) + initialIdentities := []string{postReadyTargetIdentity(initial[0]), postReadyTargetIdentity(initial[1])} + Expect(initialIdentities[0]).Should(ContainSubstring("|source=source-b")) + Expect(initialIdentities[1]).Should(ContainSubstring("|source=source-c")) + created, err := restoreMGR.CreateJobsIfNotExist(reqCtx, + &failNthCreateClient{Client: k8sClient, n: 2}, restoreMGR.Restore, initial) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("injected create failure at 2")) + Expect(created).Should(BeNil()) + persistedFirst := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(initial[0]), persistedFirst)).Should(Succeed()) + Expect(persistedFirst.Annotations).Should(HaveKey(postReadyTargetPlanAnnotationKey)) + + inserted := testapps.NewPodFactory(testCtx.DefaultNamespace, "aaa-inserted-target"). + AddAppInstanceLabel(testdp.ClusterName). + AddAppComponentLabel(testdp.ComponentName). + AddContainer(corev1.Container{Name: testdp.ContainerName, Image: testapps.ApeCloudMySQLImage}). + Create(&testCtx).GetObject() + Expect(testapps.ChangeObjStatus(&testCtx, inserted, func() { + inserted.Status.Phase = corev1.PodRunning + })).Should(Succeed()) + frozenSources, hasFrozenPlan, err := restoreMGR.getFrozenPostReadySourceTargets( + reqCtx, k8sClient, client.ObjectKeyFromObject(initial[0])) + Expect(err).ShouldNot(HaveOccurred()) + Expect(hasFrozenPlan).Should(BeTrue()) + Expect(frozenSources).Should(HaveLen(2)) + + rebuilt, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 1) + Expect(err).ShouldNot(HaveOccurred()) + Expect(rebuilt).Should(HaveLen(2)) + rebuilt, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, rebuilt) + Expect(err).ShouldNot(HaveOccurred()) + Expect(rebuilt).Should(HaveLen(2)) + Expect(postReadyTargetIdentity(rebuilt[0])).Should(Equal(initialIdentities[0])) + Expect(postReadyTargetIdentity(rebuilt[1])).Should(Equal(initialIdentities[1])) + + targetRelativePaths := make([]string, 0, len(rebuilt)) + for i := range rebuilt { + for _, env := range rebuilt[i].Spec.Template.Spec.Containers[0].Env { + if env.Name == dptypes.DPTargetRelativePath { + targetRelativePaths = append(targetRelativePaths, env.Value) + } + } + } + Expect(targetRelativePaths).Should(Equal([]string{"source-b", "source-c"})) + }) + It("does not resume a serial job after a failed predecessor", func() { reqCtx := getReqCtx() restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) From 521c7a4c5b42fb4d57c07378da903dbe54b22784 Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 18:18:10 +0800 Subject: [PATCH 07/10] fix: preserve post-ready job ordinals --- pkg/dataprotection/restore/manager.go | 37 ++++++++++++++-------- pkg/dataprotection/restore/manager_test.go | 20 ++++++++++++ 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 868a25c85fe..39dcfa1c379 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -729,7 +729,7 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, return nil, err } sort.Sort(intctrlutil.ByPodName(targetPodList.Items)) - frozenSourceByTarget, hasFrozenPlan, err := r.getFrozenPostReadySourceTargets( + frozenTargetByName, hasFrozenPlan, err := r.getFrozenPostReadySourceTargets( reqCtx, cli, types.NamespacedName{Namespace: r.Restore.Namespace, Name: buildJobName(0)}) if err != nil { return nil, err @@ -772,10 +772,15 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, Namespace: targetPodList.Items[i].Namespace, Name: targetPodList.Items[i].Name, }.String() - sourceTargetPodName, selectedByFrozenPlan := frozenSourceByTarget[targetName] + frozenTarget, selectedByFrozenPlan := frozenTargetByName[targetName] if hasFrozenPlan && !selectedByFrozenPlan { continue } + sourceTargetPodName := frozenTarget.source + jobIndex := i + if hasFrozenPlan { + jobIndex = frozenTarget.ordinal + } if !hasFrozenPlan { sourceTargetPodName, err = GetSourcePodNameFromTarget(target, jobAction.RequiredPolicyForAllPodSelection, i) if err != nil { @@ -786,11 +791,11 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, // no need to recover the volume when the pod selection policy is 'All' and sourceTargetPodName is not found. continue } - job := buildJob(&targetPodList.Items[i], sourceTargetPodName, i) + job := buildJob(&targetPodList.Items[i], sourceTargetPodName, jobIndex) setPostReadyTargetIdentity(job, &targetPodList.Items[i], sourceTargetPodName) jobs = append(jobs, job) } - if hasFrozenPlan && len(jobs) != len(frozenSourceByTarget) { + if hasFrozenPlan && len(jobs) != len(frozenTargetByName) { return nil, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRequeue, "not all frozen postReady target pods are currently available") } @@ -899,14 +904,20 @@ func splitPostReadyTargetIdentity(identity string) (target, source string) { return parts[0], parts[1] } -// getFrozenPostReadySourceTargets returns the original target-to-source mapping -// after a partial create. JobAction must use it while rebuilding specs so the -// backup path does not drift when the selected target Pod set changes. +type frozenPostReadyTarget struct { + source string + ordinal int +} + +// getFrozenPostReadySourceTargets returns the original target-to-source and +// target-to-ordinal mappings after a partial create. JobAction must use both +// while rebuilding specs so neither the backup path nor the Job name drifts +// when the selected target Pod set changes. func (r *RestoreManager) getFrozenPostReadySourceTargets( reqCtx intctrlutil.RequestCtx, cli client.Client, firstJobKey types.NamespacedName, -) (map[string]string, bool, error) { +) (map[string]frozenPostReadyTarget, bool, error) { existing := &batchv1.Job{} if err := cli.Get(reqCtx.Ctx, firstJobKey, existing); err != nil { if apierrors.IsNotFound(err) { @@ -933,19 +944,19 @@ func (r *RestoreManager) getFrozenPostReadySourceTargets( return nil, false, intctrlutil.NewFatalError(fmt.Sprintf( "postReady job %s/%s has no frozen target plan", existing.Namespace, existing.Name)) } - sourceByTarget := make(map[string]string, len(plan)) - for _, identity := range plan { + targets := make(map[string]frozenPostReadyTarget, len(plan)) + for ordinal, identity := range plan { target, source := splitPostReadyTargetIdentity(identity) if target == "" { return nil, false, intctrlutil.NewFatalError("postReady frozen target plan has an empty target") } - if _, ok := sourceByTarget[target]; ok { + if _, ok := targets[target]; ok { return nil, false, intctrlutil.NewFatalError(fmt.Sprintf( "duplicate postReady frozen target %s", target)) } - sourceByTarget[target] = source + targets[target] = frozenPostReadyTarget{source: source, ordinal: ordinal} } - return sourceByTarget, true, nil + return targets, true, nil } func hasPostReadyFrozenContract(job *batchv1.Job) bool { diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index aed789aeb92..c8fe750a3ab 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -810,6 +810,8 @@ var _ = Describe("RestoreManager Test", func() { rebuilt, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 1) Expect(err).ShouldNot(HaveOccurred()) Expect(rebuilt).Should(HaveLen(2)) + Expect(rebuilt[0].Name).Should(Equal(initial[0].Name)) + Expect(rebuilt[1].Name).Should(Equal(initial[1].Name)) rebuilt, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, rebuilt) Expect(err).ShouldNot(HaveOccurred()) Expect(rebuilt).Should(HaveLen(2)) @@ -825,6 +827,24 @@ var _ = Describe("RestoreManager Test", func() { } } Expect(targetRelativePaths).Should(Equal([]string{"source-b", "source-c"})) + + rebuilt, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, rebuilt) + Expect(err).ShouldNot(HaveOccurred()) + Expect(rebuilt).Should(HaveLen(2)) + Expect(restoreMGR.ResumeNextSerialPostReadyJob(reqCtx, k8sClient, rebuilt)).Should(Succeed()) + persistedJobs := &batchv1.JobList{} + Expect(k8sClient.List(reqCtx.Ctx, persistedJobs, + client.InNamespace(initial[0].Namespace), + client.MatchingLabels{DataProtectionRestoreLabelKey: restoreMGR.Restore.Name})).Should(Succeed()) + Expect(persistedJobs.Items).Should(HaveLen(2)) + Expect([]string{persistedJobs.Items[0].Name, persistedJobs.Items[1].Name}).Should( + ConsistOf(initial[0].Name, initial[1].Name)) + for i := range initial { + persisted := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(initial[i]), persisted)).Should(Succeed()) + Expect(postReadyTargetIdentity(persisted)).Should(Equal(initialIdentities[i])) + Expect(*persisted.Spec.Suspend).Should(Equal(i != 0)) + } }) It("does not resume a serial job after a failed predecessor", func() { From e2a2ce09077467c1659e903ed9434a78e71526ad Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 13 Jul 2026 18:30:19 +0800 Subject: [PATCH 08/10] fix: reject empty post-ready contracts --- pkg/dataprotection/restore/manager.go | 34 +++++++++++++++------- pkg/dataprotection/restore/manager_test.go | 22 ++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 39dcfa1c379..692fc30fcd6 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -963,9 +963,10 @@ func hasPostReadyFrozenContract(job *batchv1.Job) bool { if job.Annotations == nil { return false } - return job.Annotations[postReadyExecutionPolicyAnnotationKey] != "" || - job.Annotations[postReadyTargetIdentityAnnotationKey] != "" || - job.Annotations[postReadyTargetPlanAnnotationKey] != "" + _, hasPolicy := job.Annotations[postReadyExecutionPolicyAnnotationKey] + _, hasIdentity := job.Annotations[postReadyTargetIdentityAnnotationKey] + _, hasPlan := job.Annotations[postReadyTargetPlanAnnotationKey] + return hasPolicy || hasIdentity || hasPlan } func setPostReadyTargetPlan(jobs []*batchv1.Job) error { @@ -1005,15 +1006,28 @@ func postReadyTargetPlan(job *batchv1.Job) ([]string, error) { } func postReadyExecutionPolicyForJob(job *batchv1.Job) (dpv1alpha1.PostReadyExecutionPolicy, error) { - if job.Annotations == nil || job.Annotations[postReadyExecutionPolicyAnnotationKey] == "" { - if hasPostReadyFrozenContract(job) { - return "", intctrlutil.NewFatalError(fmt.Sprintf( - "postReady job %s/%s has a frozen target contract without an execution policy", - job.Namespace, job.Name)) - } + if !hasPostReadyFrozenContract(job) { return dpv1alpha1.PostReadyExecutionPolicyParallel, nil } - policy := dpv1alpha1.PostReadyExecutionPolicy(job.Annotations[postReadyExecutionPolicyAnnotationKey]) + policyValue, hasPolicy := job.Annotations[postReadyExecutionPolicyAnnotationKey] + if !hasPolicy || policyValue == "" { + return "", intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a frozen target contract without an execution policy", + job.Namespace, job.Name)) + } + identity, hasIdentity := job.Annotations[postReadyTargetIdentityAnnotationKey] + if !hasIdentity || identity == "" { + return "", intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a frozen target contract without a target identity", + job.Namespace, job.Name)) + } + plan, hasPlan := job.Annotations[postReadyTargetPlanAnnotationKey] + if !hasPlan || plan == "" { + return "", intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a frozen target contract without a target plan", + job.Namespace, job.Name)) + } + policy := dpv1alpha1.PostReadyExecutionPolicy(policyValue) if policy != dpv1alpha1.PostReadyExecutionPolicyParallel && policy != dpv1alpha1.PostReadyExecutionPolicySerial { return "", intctrlutil.NewFatalError(fmt.Sprintf( "postReady job %s/%s has invalid frozen execution policy %q", diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index c8fe750a3ab..8c9b0cf93e3 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -936,6 +936,28 @@ var _ = Describe("RestoreManager Test", func() { Expect(serial).Should(BeFalse()) }) + It("rejects a frozen target contract whose three keys have empty values", func() { + job := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-empty-contract-0", nil) + job.Annotations = map[string]string{ + postReadyExecutionPolicyAnnotationKey: "", + postReadyTargetIdentityAnnotationKey: "", + postReadyTargetPlanAnnotationKey: "", + } + + serial, err := serialPostReadyJobs([]*batchv1.Job{job}) + Expect(err).Should(HaveOccurred()) + Expect(serial).Should(BeFalse()) + }) + + It("rejects a single present frozen target key with an empty value", func() { + job := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-empty-identity-0", nil) + job.Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: ""} + + serial, err := serialPostReadyJobs([]*batchv1.Job{job}) + Expect(err).Should(HaveOccurred()) + Expect(serial).Should(BeFalse()) + }) + Context("BuildContinuousRestoreManager", func() { const ( continuousBackupStartTime = "2023-01-01T09:00:00Z" From c3a488c3296a876feab4cffaca61c8434048f68c Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Tue, 14 Jul 2026 20:10:16 +0800 Subject: [PATCH 09/10] fix(dataprotection): freeze post-ready action contract --- .../dataprotection/restore_controller.go | 9 +- .../dataprotection/restore_controller_test.go | 34 +++ pkg/dataprotection/restore/manager.go | 287 +++++++++++++++++- pkg/dataprotection/restore/manager_test.go | 244 ++++++++++++++- 4 files changed, 563 insertions(+), 11 deletions(-) diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go index ff5556ddc3c..cf0abee63c6 100644 --- a/controllers/dataprotection/restore_controller.go +++ b/controllers/dataprotection/restore_controller.go @@ -407,6 +407,10 @@ func (r *RestoreReconciler) prepareData(reqCtx intctrlutil.RequestCtx, restoreMg } func (r *RestoreReconciler) postReady(reqCtx intctrlutil.RequestCtx, restoreMgr *dprestore.RestoreManager) (bool, error) { + orphanedActionsCompleted, err := restoreMgr.ReconcileOrphanedPostReadyActions(reqCtx, r.Client) + if err != nil || !orphanedActionsCompleted { + return false, err + } readyConfig := restoreMgr.Restore.Spec.ReadyConfig if len(restoreMgr.PostReadyBackupSets) == 0 || readyConfig == nil { return true, nil @@ -415,10 +419,7 @@ func (r *RestoreReconciler) postReady(reqCtx intctrlutil.RequestCtx, restoreMgr return true, nil } dprestore.SetRestoreStageCondition(restoreMgr.Restore, dpv1alpha1.PostReady, dprestore.ReasonProcessing, "processing postReady stage") - var ( - err error - isCompleted bool - ) + var isCompleted bool defer func() { r.handleRestoreStageError(restoreMgr.Restore, dpv1alpha1.PrepareData, err) }() diff --git a/controllers/dataprotection/restore_controller_test.go b/controllers/dataprotection/restore_controller_test.go index 2f774fb5953..3f13905fd88 100644 --- a/controllers/dataprotection/restore_controller_test.go +++ b/controllers/dataprotection/restore_controller_test.go @@ -526,6 +526,40 @@ var _ = Describe("Restore Controller test", func() { Eventually(testapps.CheckObjExists(&testCtx, client.ObjectKeyFromObject(restore), restore, false)).Should(Succeed()) }) + It("keeps running persisted postReady jobs after the ActionSet removes them", func() { + By("remove the prepareData stage for testing post ready actions") + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PrepareData = nil + })).Should(Succeed()) + + matchLabels := map[string]string{ + constant.AppInstanceLabelKey: testdp.ClusterName, + } + restore := initResourcesAndWaitRestore(true, false, false, "", dpv1alpha1.RestorePhaseRunning, + func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName).SetJobActionConfig(matchLabels).SetExecActionConfig(matchLabels) + }, nil) + + By("wait for the first postReady step to create its Jobs") + Eventually(testapps.List(&testCtx, generics.JobSignature, + client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, + client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(2)) + + By("remove postReady from the mutable ActionSet while those Jobs are running") + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PostReady = nil + })).Should(Succeed()) + Consistently(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) { + g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseRunning)) + }), 2*time.Second, 100*time.Millisecond).Should(Succeed()) + + By("complete the persisted Jobs and expect the Restore to complete") + mockRestoreJobsCompleted(restore) + Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) { + g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseCompleted)) + })).Should(Succeed()) + }) + It("should complete an existing postReady job when target pod is no longer ready", func() { By("remove the prepareData stage for testing post ready actions") Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index 692fc30fcd6..a741e9ce770 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -21,6 +21,7 @@ package restore import ( "context" + "crypto/sha256" "encoding/json" "fmt" "sort" @@ -31,6 +32,7 @@ import ( vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -54,6 +56,9 @@ const ( postReadyExecutionPolicyAnnotationKey = "dataprotection.kubeblocks.io/post-ready-execution-policy" postReadyTargetIdentityAnnotationKey = "dataprotection.kubeblocks.io/post-ready-target-identity" postReadyTargetPlanAnnotationKey = "dataprotection.kubeblocks.io/post-ready-target-plan" + postReadyActionContractAnnotationKey = "dataprotection.kubeblocks.io/post-ready-action-contract" + postReadyBackupNameAnnotationKey = "dataprotection.kubeblocks.io/post-ready-backup-name" + postReadyActionNameAnnotationKey = "dataprotection.kubeblocks.io/post-ready-action-name" ) type BackupActionSet struct { @@ -674,6 +679,168 @@ func (r *RestoreManager) GetExistingActionJobs( return jobs, nil } +// ReconcileOrphanedPostReadyActions keeps persisted in-flight Jobs authoritative +// when a newer ActionSet removes or shortens postReady. Without this guard, the +// latest ActionSet loop can skip a still-running Job and mark postReady complete. +func (r *RestoreManager) ReconcileOrphanedPostReadyActions( + reqCtx intctrlutil.RequestCtx, + cli client.Client, +) (bool, error) { + currentActions := map[string]struct{}{} + if r.Restore.Spec.ReadyConfig != nil { + for i := range r.PostReadyBackupSets { + backupSet := r.PostReadyBackupSets[i] + if backupSet.Backup == nil || backupSet.ActionSet == nil || backupSet.ActionSet.Spec.Restore == nil { + continue + } + for step := range backupSet.ActionSet.Spec.Restore.PostReady { + currentActions[postReadyActionKey(backupSet.Backup.Name, fmt.Sprintf("%s-%d", dpv1alpha1.PostReady, step))] = struct{}{} + } + } + } + + namespaces := []string{r.Restore.Namespace} + if controllerNamespace := viper.GetString(constant.CfgKeyCtrlrMgrNS); controllerNamespace != "" && controllerNamespace != r.Restore.Namespace { + namespaces = append(namespaces, controllerNamespace) + } + orphaned := map[string][]*batchv1.Job{} + for _, namespace := range namespaces { + jobList := &batchv1.JobList{} + if err := cli.List(reqCtx.Ctx, jobList, + client.InNamespace(namespace), + client.MatchingLabels{DataProtectionRestoreLabelKey: r.Restore.Name}); err != nil { + return false, err + } + for i := range jobList.Items { + job := &jobList.Items[i] + if !r.isJobForRestoreAction(job) || !hasPostReadyFrozenContract(job) { + continue + } + backupName, actionName := postReadyActionIdentity(job) + if backupName == "" || actionName == "" { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has no recoverable action identity", job.Namespace, job.Name)) + } + key := postReadyActionKey(backupName, actionName) + if _, ok := currentActions[key]; ok { + continue + } + orphaned[key] = append(orphaned[key], job.DeepCopy()) + } + } + + orphanedKeys := make([]string, 0, len(orphaned)) + for key := range orphaned { + orphanedKeys = append(orphanedKeys, key) + } + sort.Strings(orphanedKeys) + for _, key := range orphanedKeys { + jobs := orphaned[key] + backupName, actionName := splitPostReadyActionKey(key) + plan, err := postReadyTargetPlan(jobs[0]) + if err != nil { + return false, err + } + if len(plan) != len(jobs) { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "in-flight postReady action %s for backup %s is incomplete and no longer exists in the ActionSet", + actionName, backupName)) + } + policy, err := postReadyExecutionPolicyForJob(jobs[0]) + if err != nil { + return false, err + } + contract := postReadyActionContractForJob(jobs[0]) + identities := map[string]struct{}{} + for i := range jobs { + jobPlan, err := postReadyTargetPlan(jobs[i]) + if err != nil { + return false, err + } + jobPolicy, err := postReadyExecutionPolicyForJob(jobs[i]) + if err != nil { + return false, err + } + identity := postReadyTargetIdentity(jobs[i]) + index := postReadyJobIndex(jobs[i].Name) + if strings.Join(jobPlan, "\x00") != strings.Join(plan, "\x00") || + jobPolicy != policy || postReadyActionContractForJob(jobs[i]) != contract || + index < 0 || index >= len(plan) || identity != plan[index] { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "in-flight postReady action %s for backup %s has an inconsistent frozen contract", + actionName, backupName)) + } + if _, ok := identities[identity]; ok { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "in-flight postReady action %s for backup %s has a duplicate frozen target", + actionName, backupName)) + } + identities[identity] = struct{}{} + } + backupSet := BackupActionSet{Backup: &dpv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Name: backupName}}} + completed, failed, err := r.CheckJobsDone(dpv1alpha1.PostReady, actionName, backupSet, jobs) + if err != nil { + return false, err + } + if failed { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "in-flight postReady action %s for backup %s failed after it was removed from the ActionSet", + actionName, backupName)) + } + if !completed { + return false, nil + } + } + return true, nil +} + +func postReadyActionKey(backupName, actionName string) string { + return backupName + "\x00" + actionName +} + +func splitPostReadyActionKey(key string) (string, string) { + parts := strings.SplitN(key, "\x00", 2) + if len(parts) != 2 { + return "", "" + } + return parts[0], parts[1] +} + +func postReadyActionIdentity(job *batchv1.Job) (string, string) { + if job.Annotations != nil { + if backupName := job.Annotations[postReadyBackupNameAnnotationKey]; backupName != "" { + if actionName := job.Annotations[postReadyActionNameAnnotationKey]; actionName != "" { + return backupName, actionName + } + } + } + backupName := "" + for _, container := range job.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == dptypes.DPBackupName { + backupName = env.Value + break + } + } + if backupName != "" { + break + } + } + lastSeparator := strings.LastIndexByte(job.Name, '-') + if lastSeparator < 0 { + return backupName, "" + } + stepSeparator := strings.LastIndexByte(job.Name[:lastSeparator], '-') + if stepSeparator < 0 || stepSeparator+1 == lastSeparator { + return backupName, "" + } + step := job.Name[stepSeparator+1 : lastSeparator] + if _, err := strconv.Atoi(step); err != nil { + return backupName, "" + } + return backupName, fmt.Sprintf("%s-%s", dpv1alpha1.PostReady, step) +} + func (r *RestoreManager) isJobForRestoreAction(job *batchv1.Job) bool { if job.Labels[DataProtectionRestoreLabelKey] != r.Restore.Name { return false @@ -856,6 +1023,9 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, if err := setPostReadyTargetPlan(jobs); err != nil { return nil, err } + if err := r.setPostReadyActionContract(jobs, backupSet, step); err != nil { + return nil, err + } return jobs, nil } @@ -895,6 +1065,54 @@ func postReadyTargetIdentity(job *batchv1.Job) string { return job.Annotations[postReadyTargetIdentityAnnotationKey] } +type postReadyActionContract struct { + ReadyConfig *dpv1alpha1.ReadyConfig `json:"readyConfig,omitempty"` + PostReady []dpv1alpha1.ActionSpec `json:"postReady,omitempty"` + KBToolsImage string `json:"kbToolsImage,omitempty"` + WorkerServiceAccount string `json:"workerServiceAccount,omitempty"` + ControllerNamespace string `json:"controllerNamespace,omitempty"` + ExecWorkerServiceAccount string `json:"execWorkerServiceAccount,omitempty"` +} + +func (r *RestoreManager) setPostReadyActionContract(jobs []*batchv1.Job, backupSet BackupActionSet, step int) error { + if backupSet.ActionSet == nil || backupSet.ActionSet.Spec.Restore == nil { + return intctrlutil.NewFatalError("postReady action has no ActionSet restore contract") + } + if step < 0 || step >= len(backupSet.ActionSet.Spec.Restore.PostReady) { + return intctrlutil.NewFatalError(fmt.Sprintf("postReady action step %d is out of range", step)) + } + contract := postReadyActionContract{ + ReadyConfig: r.Restore.Spec.ReadyConfig, + PostReady: backupSet.ActionSet.Spec.Restore.PostReady, + KBToolsImage: viper.GetString(constant.KBToolsImage), + WorkerServiceAccount: r.WorkerServiceAccount, + ControllerNamespace: viper.GetString(constant.CfgKeyCtrlrMgrNS), + ExecWorkerServiceAccount: viper.GetString(dptypes.CfgKeyExecWorkerServiceAccountName), + } + serialized, err := json.Marshal(contract) + if err != nil { + return err + } + digest := fmt.Sprintf("sha256:%x", sha256.Sum256(serialized)) + actionName := fmt.Sprintf("%s-%d", dpv1alpha1.PostReady, step) + for i := range jobs { + if jobs[i].Annotations == nil { + jobs[i].Annotations = map[string]string{} + } + jobs[i].Annotations[postReadyActionContractAnnotationKey] = digest + jobs[i].Annotations[postReadyBackupNameAnnotationKey] = backupSet.Backup.Name + jobs[i].Annotations[postReadyActionNameAnnotationKey] = actionName + } + return nil +} + +func postReadyActionContractForJob(job *batchv1.Job) string { + if job.Annotations == nil { + return "" + } + return job.Annotations[postReadyActionContractAnnotationKey] +} + func splitPostReadyTargetIdentity(identity string) (target, source string) { const sourceMarker = "|source=" parts := strings.SplitN(identity, sourceMarker, 2) @@ -1070,8 +1288,6 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( if err != nil { return nil, err } - foundExisting := false - var frozenPlan []string allInputJobsPersisted := true for i := range jobs { if jobs[i].ResourceVersion == "" { @@ -1079,6 +1295,23 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( break } } + desiredContract := postReadyActionContractForJob(jobs[0]) + if desiredContract == "" { + if allInputJobsPersisted { + // All executable Job specs already exist, so they remain the action + // fact source for upgrades from the previous frozen-plan format. + return jobs, nil + } + return nil, intctrlutil.NewFatalError("postReady jobs have no frozen action contract") + } + for i := 1; i < len(jobs); i++ { + if postReadyActionContractForJob(jobs[i]) != desiredContract { + return nil, intctrlutil.NewFatalError("postReady jobs have inconsistent frozen action contracts") + } + } + foundExisting := false + foundLegacyPartial := false + var frozenPlan []string for i := range jobs { existing := &batchv1.Job{} if err := cli.Get(reqCtx.Ctx, client.ObjectKeyFromObject(jobs[i]), existing); err != nil { @@ -1101,9 +1334,24 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( } return jobs, nil } - return nil, intctrlutil.NewFatalError(fmt.Sprintf( - "legacy postReady job %s/%s cannot be combined with a new frozen target plan", - existing.Namespace, existing.Name)) + if foundExisting { + return nil, intctrlutil.NewFatalError("legacy and frozen postReady jobs cannot be mixed") + } + if policy != dpv1alpha1.PostReadyExecutionPolicyParallel { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s cannot be migrated to serial execution", + existing.Namespace, existing.Name)) + } + if !apiequality.Semantic.DeepDerivative(jobs[i].Spec, existing.Spec) { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s executable action does not match the current ActionSet", + existing.Namespace, existing.Name)) + } + foundLegacyPartial = true + continue + } + if foundLegacyPartial { + return nil, intctrlutil.NewFatalError("legacy and frozen postReady jobs cannot be mixed") } existingPolicy, err := postReadyExecutionPolicyForJob(existing) if err != nil { @@ -1124,6 +1372,18 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( if len(frozenPlan) > 0 && strings.Join(plan, "\x00") != strings.Join(frozenPlan, "\x00") { return nil, intctrlutil.NewFatalError("postReady jobs have inconsistent frozen target plans") } + existingContract := postReadyActionContractForJob(existing) + if existingContract == "" { + if !allInputJobsPersisted { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has no complete frozen action contract", + existing.Namespace, existing.Name)) + } + } else if existingContract != desiredContract { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s executable action does not match its frozen contract", + existing.Namespace, existing.Name)) + } index := postReadyJobIndex(existing.Name) if index < 0 || index >= len(plan) || postReadyTargetIdentity(existing) != plan[index] { return nil, intctrlutil.NewFatalError(fmt.Sprintf( @@ -1133,6 +1393,16 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( frozenPlan = plan foundExisting = true } + if foundLegacyPartial { + for i := range jobs { + delete(jobs[i].Annotations, postReadyExecutionPolicyAnnotationKey) + delete(jobs[i].Annotations, postReadyTargetIdentityAnnotationKey) + delete(jobs[i].Annotations, postReadyTargetPlanAnnotationKey) + delete(jobs[i].Annotations, postReadyActionContractAnnotationKey) + jobs[i].Spec.Suspend = nil + } + return jobs, nil + } if !foundExisting { return jobs, nil } @@ -1159,6 +1429,7 @@ func (r *RestoreManager) FreezePostReadyExecutionPlan( job = job.DeepCopy() job.Name = postReadyJobNameForIndex(jobs[0].Name, i) setPostReadyExecutionPolicy(job, policy) + job.Annotations[postReadyActionContractAnnotationKey] = desiredContract serializedPlan, err := json.Marshal(frozenPlan) if err != nil { return nil, err @@ -1210,6 +1481,12 @@ func (r *RestoreManager) validateExistingRestoreActionJob(desired, existing *bat "postReady job %s/%s frozen target plan does not match desired plan", existing.Namespace, existing.Name)) } + desiredContract := postReadyActionContractForJob(desired) + if desiredContract == "" || postReadyActionContractForJob(existing) != desiredContract { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s executable action does not match desired contract", + existing.Namespace, existing.Name)) + } return nil } diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 8c9b0cf93e3..e82539a706c 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -598,10 +598,21 @@ var _ = Describe("RestoreManager Test", func() { for i := range jobs { Expect(jobs[i].Spec.Suspend).Should(BeNil()) Expect(jobs[i].Annotations[postReadyExecutionPolicyAnnotationKey]).Should(Equal(string(dpv1alpha1.PostReadyExecutionPolicyParallel))) + Expect(jobs[i].Annotations[postReadyActionContractAnnotationKey]).Should(HavePrefix("sha256:")) + Expect(jobs[i].Annotations[postReadyBackupNameAnnotationKey]).Should(Equal(backupSet.Backup.Name)) + Expect(jobs[i].Annotations[postReadyActionNameAnnotationKey]).Should(Equal("postReady-0")) } + execActionContract := jobs[0].Annotations[postReadyActionContractAnnotationKey] Expect(jobs[0].Spec.Template.Spec.Containers[0].Args[3] < jobs[1].Spec.Template.Spec.Containers[0].Args[3]).Should(BeTrue()) Expect(jobs[0].Namespace).Should(Equal(kbNamespace)) Expect(jobs[0].Spec.Template.Spec.ServiceAccountName).Should(Equal(execWorkerServiceAccountName)) + oldToolsImage := viper.GetString(constant.KBToolsImage) + viper.Set(constant.KBToolsImage, oldToolsImage+"-changed") + mutatedExecJobs, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 0) + Expect(err).ShouldNot(HaveOccurred()) + Expect(mutatedExecJobs).Should(HaveLen(2)) + Expect(mutatedExecJobs[0].Annotations[postReadyActionContractAnnotationKey]).ShouldNot(Equal(execActionContract)) + viper.Set(constant.KBToolsImage, oldToolsImage) By("test with jobAction and expect for creating 1 job") // step 0 is the execAction in actionSet @@ -609,6 +620,9 @@ var _ = Describe("RestoreManager Test", func() { Expect(err).ShouldNot(HaveOccurred()) // count of job should equal to 1 Expect(len(jobs)).Should(Equal(1)) + jobActionContract := jobs[0].Annotations[postReadyActionContractAnnotationKey] + Expect(jobActionContract).Should(Equal(execActionContract)) + Expect(jobs[0].Annotations[postReadyActionNameAnnotationKey]).Should(Equal("postReady-1")) // test timeZone transform var backupStopTimeEnv string for _, v := range jobs[0].Spec.Template.Spec.Containers[0].Env { @@ -619,6 +633,17 @@ var _ = Describe("RestoreManager Test", func() { } Expect(backupStopTimeEnv).Should(Equal("2023-01-01 18:00:00")) checkVolumes(jobs[0], testdp.DataVolumeName, existVolume) + + By("change an executable action and expect the complete action contract to change") + backupSet.ActionSet.Spec.Restore.PostReady[1].Job.Image += "-changed" + mutatedJobs, err := restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 1) + Expect(err).ShouldNot(HaveOccurred()) + Expect(mutatedJobs).Should(HaveLen(1)) + Expect(mutatedJobs[0].Annotations[postReadyActionContractAnnotationKey]).ShouldNot(Equal(jobActionContract)) + mutatedExecJobs, err = restoreMGR.BuildPostReadyActionJobs(reqCtx, k8sClient, *backupSet, target, 0) + Expect(err).ShouldNot(HaveOccurred()) + Expect(mutatedExecJobs).Should(HaveLen(2)) + Expect(mutatedExecJobs[0].Annotations[postReadyActionContractAnnotationKey]).ShouldNot(Equal(execActionContract)) } It("test with BuildPostReadyActionJobs function and run target pod node", func() { @@ -701,7 +726,10 @@ var _ = Describe("RestoreManager Test", func() { } newTargetJob := func(index int, target string, policy dpv1alpha1.PostReadyExecutionPolicy) *batchv1.Job { job := newRestoreJob(testCtx.DefaultNamespace, fmt.Sprintf("restore-post-ready-partial-%d", index), labels) - job.Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: target} + job.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: target, + postReadyActionContractAnnotationKey: "sha256:stable-action-contract", + } setPostReadyExecutionPolicy(job, policy) return job } @@ -756,6 +784,215 @@ var _ = Describe("RestoreManager Test", func() { Expect(*second.Spec.Suspend).Should(BeTrue()) }) + It("rejects executable action drift across a frozen partial creation retry", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + newFrozenJob := func(index int, contract, image string) *batchv1.Job { + job := newRestoreJob(testCtx.DefaultNamespace, fmt.Sprintf("restore-post-ready-drift-%d", index), labels) + job.Spec.Template.Spec.Containers[0].Image = image + job.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: fmt.Sprintf("default/pod-%d", index), + postReadyActionContractAnnotationKey: contract, + } + setPostReadyExecutionPolicy(job, dpv1alpha1.PostReadyExecutionPolicyParallel) + return job + } + + initial := []*batchv1.Job{ + newFrozenJob(0, "sha256:initial", "image:v1"), + newFrozenJob(1, "sha256:initial", "image:v1"), + } + Expect(setPostReadyTargetPlan(initial)).Should(Succeed()) + _, err := restoreMGR.CreateJobsIfNotExist(reqCtx, + &failNthCreateClient{Client: k8sClient, n: 2}, restoreMGR.Restore, initial) + Expect(err).Should(HaveOccurred()) + + mutated := []*batchv1.Job{ + newFrozenJob(0, "sha256:mutated", "image:v2"), + newFrozenJob(1, "sha256:mutated", "image:v2"), + } + Expect(setPostReadyTargetPlan(mutated)).Should(Succeed()) + _, err = restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, mutated) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("executable action does not match its frozen contract")) + }) + + It("continues an unchanged legacy Parallel action after partial creation", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + desired := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-legacy-partial-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-legacy-partial-1", labels), + } + for i := range desired { + desired[i].Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: fmt.Sprintf("default/pod-%d", i), + postReadyActionContractAnnotationKey: "sha256:current-action", + } + setPostReadyExecutionPolicy(desired[i], dpv1alpha1.PostReadyExecutionPolicyParallel) + } + Expect(setPostReadyTargetPlan(desired)).Should(Succeed()) + + legacyFirst := desired[0].DeepCopy() + legacyFirst.Annotations = nil + legacyFirst.Spec.Suspend = nil + Expect(k8sClient.Create(reqCtx.Ctx, legacyFirst)).Should(Succeed()) + + continued, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, desired) + Expect(err).ShouldNot(HaveOccurred()) + Expect(continued).Should(HaveLen(2)) + for i := range continued { + Expect(continued[i].Annotations).ShouldNot(HaveKey(postReadyExecutionPolicyAnnotationKey)) + Expect(continued[i].Annotations).ShouldNot(HaveKey(postReadyTargetIdentityAnnotationKey)) + Expect(continued[i].Annotations).ShouldNot(HaveKey(postReadyTargetPlanAnnotationKey)) + Expect(continued[i].Annotations).ShouldNot(HaveKey(postReadyActionContractAnnotationKey)) + Expect(continued[i].Spec.Suspend).Should(BeNil()) + } + continued, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, continued) + Expect(err).ShouldNot(HaveOccurred()) + Expect(continued).Should(HaveLen(2)) + }) + + It("rejects executable drift while recovering a legacy Parallel partial creation", func() { + reqCtx := getReqCtx() + restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + desired := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-legacy-drift-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-legacy-drift-1", labels), + } + for i := range desired { + desired[i].Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: fmt.Sprintf("default/pod-%d", i), + postReadyActionContractAnnotationKey: "sha256:current-action", + } + setPostReadyExecutionPolicy(desired[i], dpv1alpha1.PostReadyExecutionPolicyParallel) + } + Expect(setPostReadyTargetPlan(desired)).Should(Succeed()) + + legacyFirst := desired[0].DeepCopy() + legacyFirst.Annotations = nil + legacyFirst.Spec.Suspend = nil + legacyFirst.Spec.Template.Spec.Containers[0].Image = "legacy:image" + Expect(k8sClient.Create(reqCtx.Ctx, legacyFirst)).Should(Succeed()) + + continued, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, desired) + Expect(err).Should(HaveOccurred()) + Expect(continued).Should(BeNil()) + Expect(err.Error()).Should(ContainSubstring("executable action does not match the current ActionSet")) + }) + + It("waits for persisted postReady jobs after their ActionSet step is removed", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + Expect(backupSet.ActionSet.Spec.Restore.PostReady).Should(HaveLen(2)) + backupSet.ActionSet.Spec.Restore.PostReady = backupSet.ActionSet.Spec.Restore.PostReady[:1] + restoreMGR.PostReadyBackupSets = []BackupActionSet{*backupSet} + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + actionName := fmt.Sprintf("%s-1", dpv1alpha1.PostReady) + jobs := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-removed-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-removed-1", labels), + } + for i := range jobs { + jobs[i].Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: fmt.Sprintf("default/pod-%d", i), + postReadyActionContractAnnotationKey: "sha256:removed-action", + postReadyBackupNameAnnotationKey: backupSet.Backup.Name, + postReadyActionNameAnnotationKey: actionName, + } + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicyParallel) + } + Expect(setPostReadyTargetPlan(jobs)).Should(Succeed()) + for i := range jobs { + Expect(k8sClient.Create(reqCtx.Ctx, jobs[i])).Should(Succeed()) + } + + completed, err := restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeFalse()) + Expect(restoreMGR.Restore.Status.Actions.PostReady).Should(HaveLen(2)) + + for i := range jobs { + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(jobs[i]), batchv1.JobComplete) + } + completed, err = restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeTrue()) + for i := range restoreMGR.Restore.Status.Actions.PostReady { + Expect(restoreMGR.Restore.Status.Actions.PostReady[i].Status).Should(Equal(dpv1alpha1.RestoreActionCompleted)) + } + }) + + It("recovers an orphaned frozen-plan Job written before action-contract annotations", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + Expect(backupSet.ActionSet.Spec.Restore.PostReady).Should(HaveLen(2)) + backupSet.ActionSet.Spec.Restore.PostReady = backupSet.ActionSet.Spec.Restore.PostReady[:1] + restoreMGR.PostReadyBackupSets = []BackupActionSet{*backupSet} + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + job := newRestoreJob(testCtx.DefaultNamespace, + fmt.Sprintf("restore-post-ready-%s-%s-1-0", restoreMGR.Restore.UID[:8], backupSet.Backup.Name), labels) + job.Spec.Template.Spec.Containers[0].Env = append(job.Spec.Template.Spec.Containers[0].Env, + corev1.EnvVar{Name: dptypes.DPBackupName, Value: backupSet.Backup.Name}) + job.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: "default/pod-0", + } + setPostReadyExecutionPolicy(job, dpv1alpha1.PostReadyExecutionPolicyParallel) + Expect(setPostReadyTargetPlan([]*batchv1.Job{job})).Should(Succeed()) + Expect(k8sClient.Create(reqCtx.Ctx, job)).Should(Succeed()) + + completed, err := restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeFalse()) + Expect(restoreMGR.Restore.Status.Actions.PostReady).Should(HaveLen(1)) + Expect(restoreMGR.Restore.Status.Actions.PostReady[0].Name).Should(Equal("postReady-1")) + }) + + It("waits for persisted postReady jobs after readyConfig is removed", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + job := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-ready-config-removed-0", labels) + job.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: "default/pod-0", + postReadyActionContractAnnotationKey: "sha256:removed-ready-config", + postReadyBackupNameAnnotationKey: backupSet.Backup.Name, + postReadyActionNameAnnotationKey: actionName, + } + setPostReadyExecutionPolicy(job, dpv1alpha1.PostReadyExecutionPolicyParallel) + Expect(setPostReadyTargetPlan([]*batchv1.Job{job})).Should(Succeed()) + Expect(k8sClient.Create(reqCtx.Ctx, job)).Should(Succeed()) + restoreMGR.Restore.Spec.ReadyConfig = nil + + completed, err := restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeFalse()) + Expect(restoreMGR.Restore.Status.Actions.PostReady).Should(HaveLen(1)) + Expect(restoreMGR.Restore.Status.Actions.PostReady[0].Status).Should(Equal(dpv1alpha1.RestoreActionProcessing)) + }) + It("preserves the OneToOne source mapping through the real Job builder after target insertion", func() { reqCtx := getReqCtx() oldToolsImage := viper.GetString(constant.KBToolsImage) @@ -885,7 +1122,10 @@ var _ = Describe("RestoreManager Test", func() { DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, } desired := newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-race-0", labels) - desired.Annotations = map[string]string{postReadyTargetIdentityAnnotationKey: "default/pod-0"} + desired.Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: "default/pod-0", + postReadyActionContractAnnotationKey: "sha256:stable-action-contract", + } setPostReadyExecutionPolicy(desired, dpv1alpha1.PostReadyExecutionPolicySerial) Expect(setPostReadyTargetPlan([]*batchv1.Job{desired})).Should(Succeed()) From 7d4301bffc63f374c6e5637d1a04bb1e815668a0 Mon Sep 17 00:00:00 2001 From: Wyatt Review Artifact Date: Wed, 15 Jul 2026 08:21:26 +0800 Subject: [PATCH 10/10] fix(dataprotection): commit exact post-ready stage plan --- .../dataprotection/restore_controller.go | 83 +- .../dataprotection/restore_controller_test.go | 77 +- controllers/dataprotection/utils.go | 2 +- pkg/dataprotection/restore/manager.go | 1017 ++++++++++++++++- pkg/dataprotection/restore/manager_test.go | 634 ++++++++++ pkg/dataprotection/restore/utils.go | 6 + pkg/dataprotection/restore/utils_test.go | 150 +++ 7 files changed, 1933 insertions(+), 36 deletions(-) diff --git a/controllers/dataprotection/restore_controller.go b/controllers/dataprotection/restore_controller.go index cf0abee63c6..efd773c90a6 100644 --- a/controllers/dataprotection/restore_controller.go +++ b/controllers/dataprotection/restore_controller.go @@ -62,6 +62,7 @@ type RestoreReconciler struct { // +kubebuilder:rbac:groups=dataprotection.kubeblocks.io,resources=restores/status,verbs=get;update;patch // +kubebuilder:rbac:groups=dataprotection.kubeblocks.io,resources=restores/finalizers,verbs=update // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete @@ -105,6 +106,7 @@ func (r *RestoreReconciler) SetupWithManager(mgr ctrl.Manager) error { return intctrlutil.NewControllerManagedBy(mgr). For(&dpv1alpha1.Restore{}). Owns(&batchv1.Job{}). + Owns(&corev1.Secret{}). Watches(&batchv1.Job{}, handler.EnqueueRequestsFromMapFunc(r.parseRestoreJob)). // to watch the `restore` container if it is terminated Watches(&corev1.Pod{}, handler.EnqueueRequestsFromMapFunc(r.parseRestorePod)). @@ -155,7 +157,11 @@ func (r *RestoreReconciler) deleteExternalResources(reqCtx intctrlutil.RequestCt viper.GetString(constant.CfgKeyCtrlrMgrNS): {}, } - return deleteRelatedObjectList(reqCtx, r.Client, &batchv1.JobList{}, namespaces, labels) + if err := deleteRelatedObjectList(reqCtx, r.Client, &batchv1.JobList{}, namespaces, labels); err != nil { + return err + } + return deleteRelatedObjectList(reqCtx, r.Client, &corev1.SecretList{}, + map[string]sets.Empty{restore.Namespace: {}}, labels) } func CheckBackupRepoForRestore(reqCtx intctrlutil.RequestCtx, cli client.Client, restore *dpv1alpha1.Restore) (string, error) { @@ -407,38 +413,31 @@ func (r *RestoreReconciler) prepareData(reqCtx intctrlutil.RequestCtx, restoreMg } func (r *RestoreReconciler) postReady(reqCtx intctrlutil.RequestCtx, restoreMgr *dprestore.RestoreManager) (bool, error) { - orphanedActionsCompleted, err := restoreMgr.ReconcileOrphanedPostReadyActions(reqCtx, r.Client) - if err != nil || !orphanedActionsCompleted { + foundStage, err := restoreMgr.EnsurePostReadyStagePlan(reqCtx, r.Client) + if err != nil { return false, err } - readyConfig := restoreMgr.Restore.Spec.ReadyConfig - if len(restoreMgr.PostReadyBackupSets) == 0 || readyConfig == nil { - return true, nil + if !foundStage { + if restoreMgr.Restore.Spec.ReadyConfig == nil || len(restoreMgr.PostReadyBackupSets) == 0 { + return true, nil + } + return false, intctrlutil.NewFatalError("postReady stage has actions but no committed execution plan") } if meta.IsStatusConditionTrue(restoreMgr.Restore.Status.Conditions, dprestore.ConditionTypeRestorePostReady) { return true, nil } dprestore.SetRestoreStageCondition(restoreMgr.Restore, dpv1alpha1.PostReady, dprestore.ReasonProcessing, "processing postReady stage") - var isCompleted bool defer func() { r.handleRestoreStageError(restoreMgr.Restore, dpv1alpha1.PrepareData, err) }() - if readyConfig.ReadinessProbe != nil && !meta.IsStatusConditionTrue(restoreMgr.Restore.Status.Conditions, dprestore.ConditionTypeReadinessProbe) { + readyConfig := restoreMgr.Restore.Spec.ReadyConfig + if readyConfig != nil && readyConfig.ReadinessProbe != nil && !meta.IsStatusConditionTrue(restoreMgr.Restore.Status.Conditions, dprestore.ConditionTypeReadinessProbe) { // TODO: check readiness probe, use a job and kubectl exec? _ = klog.TODO() } - for _, v := range restoreMgr.PostReadyBackupSets { - // handle postReady actions - for i := range v.ActionSet.Spec.Restore.PostReady { - isCompleted, err = r.handleBackupActionSet(reqCtx, restoreMgr, v, dpv1alpha1.PostReady, i) - if err != nil { - return false, err - } - // waiting for restore jobs finished. - if !isCompleted { - return false, nil - } - } + isCompleted, err := restoreMgr.ReconcileOrphanedPostReadyActions(reqCtx, r.Client) + if err != nil || !isCompleted { + return false, err } dprestore.SetRestoreStageCondition(restoreMgr.Restore, dpv1alpha1.PostReady, dprestore.ReasonSucceed, "processing postReady stage successfully") return true, nil @@ -470,21 +469,39 @@ func (r *RestoreReconciler) handleBackupActionSet(reqCtx intctrlutil.RequestCtx, } actionName := fmt.Sprintf("%s-%d", stage, step) + var jobs []*batchv1.Job + var err error + expectedActionCount := 0 + if stage == dpv1alpha1.PostReady { + // Load the immutable plan before interpreting mutable completion + // status. Otherwise one completed ordinal can make a partially + // created multi-Job action look complete. + jobs, err = restoreMgr.GetExistingActionJobs(reqCtx, r.Client, stage, backupSet.Backup.Name, actionName) + if err != nil { + return false, err + } + expectedActionCount, err = dprestore.PostReadyActionExpectedJobCount(jobs) + if err != nil { + return false, err + } + } // 1. check if the restore actions are completed from status.actions firstly. - allActionsFinished, existFailedAction := restoreMgr.AnalysisRestoreActionsWithBackup(stage, backupSet.Backup.Name, actionName) + allActionsFinished, existFailedAction := restoreMgr.AnalysisRestoreActionsWithBackupExpected( + stage, backupSet.Backup.Name, actionName, expectedActionCount) isCompleted, err := checkIsCompleted(allActionsFinished, existFailedAction) if isCompleted || err != nil { return isCompleted, err } - var jobs []*batchv1.Job // For in-flight actions, check the recorded Job before rebuilding the Job // spec from the current target pod selector. The target pod may become // unavailable after the Job is created, but the existing Job is still the // action fact source that should drive convergence. - jobs, err = restoreMgr.GetExistingActionJobs(reqCtx, r.Client, stage, backupSet.Backup.Name, actionName) - if err != nil { - return false, err + if stage != dpv1alpha1.PostReady { + jobs, err = restoreMgr.GetExistingActionJobs(reqCtx, r.Client, stage, backupSet.Backup.Name, actionName) + if err != nil { + return false, err + } } switch stage { case dpv1alpha1.PrepareData: @@ -512,6 +529,22 @@ func (r *RestoreReconciler) handleBackupActionSet(reqCtx intctrlutil.RequestCtx, if jobs, err = restoreMgr.FreezePostReadyExecutionPlan(reqCtx, r.Client, jobs); err != nil { return false, err } + expectedActionCount, err = dprestore.PostReadyActionExpectedJobCount(jobs) + if err != nil { + return false, err + } + allActionsFinished, existFailedAction = restoreMgr.AnalysisRestoreActionsWithBackupExpected( + stage, backupSet.Backup.Name, actionName, expectedActionCount) + isCompleted, err = checkIsCompleted(allActionsFinished, existFailedAction) + if isCompleted || err != nil { + return isCompleted, err + } + jobs = restoreMgr.PendingPostReadyJobs(backupSet.Backup.Name, actionName, jobs) + if len(jobs) == 0 { + return false, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRequeue, + "postReady action %s for backup %s has no pending Job but is not complete", + actionName, backupSet.Backup.Name) + } } // 3. create jobs jobs, err = restoreMgr.CreateJobsIfNotExist(reqCtx, r.Client, restoreMgr.Restore, jobs) diff --git a/controllers/dataprotection/restore_controller_test.go b/controllers/dataprotection/restore_controller_test.go index 3f13905fd88..bf8ab471523 100644 --- a/controllers/dataprotection/restore_controller_test.go +++ b/controllers/dataprotection/restore_controller_test.go @@ -20,6 +20,7 @@ along with this program. If not, see . package dataprotection import ( + "encoding/json" "fmt" "strconv" "strings" @@ -489,6 +490,37 @@ var _ = Describe("Restore Controller test", func() { _ = testdp.NewFakeCluster(&testCtx) }) + expectCommittedFullStage := func(restore *dpv1alpha1.Restore) { + Eventually(func(g Gomega) { + secrets := &corev1.SecretList{} + g.Expect(k8sClient.List(ctx, secrets, + client.InNamespace(restore.Namespace), + client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name})).Should(Succeed()) + var payload []byte + for i := range secrets.Items { + if secrets.Items[i].Type == corev1.SecretType("dataprotection.kubeblocks.io/post-ready-plan") { + payload = secrets.Items[i].Data["plan.json"] + break + } + } + g.Expect(payload).ShouldNot(BeEmpty()) + var stage struct { + Actions []struct { + Order int `json:"order"` + BackupName string `json:"backupName"` + ActionName string `json:"actionName"` + } `json:"actions"` + } + g.Expect(json.Unmarshal(payload, &stage)).Should(Succeed()) + g.Expect(stage.Actions).Should(HaveLen(2)) + g.Expect(stage.Actions[0].Order).Should(Equal(0)) + g.Expect(stage.Actions[0].ActionName).Should(Equal("postReady-0")) + g.Expect(stage.Actions[1].Order).Should(Equal(1)) + g.Expect(stage.Actions[1].ActionName).Should(Equal("postReady-1")) + g.Expect(stage.Actions[1].BackupName).Should(Equal(stage.Actions[0].BackupName)) + }).Should(Succeed()) + } + It("test post ready actions", func() { By("remove the prepareData stage for testing post ready actions") Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { @@ -507,6 +539,7 @@ var _ = Describe("Restore Controller test", func() { Eventually(testapps.List(&testCtx, generics.JobSignature, client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(2)) + expectCommittedFullStage(restore) checkJobSA(restore, viper.GetString(dptypes.CfgKeyExecWorkerServiceAccountName)) @@ -544,6 +577,7 @@ var _ = Describe("Restore Controller test", func() { Eventually(testapps.List(&testCtx, generics.JobSignature, client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(2)) + expectCommittedFullStage(restore) By("remove postReady from the mutable ActionSet while those Jobs are running") Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { @@ -553,7 +587,48 @@ var _ = Describe("Restore Controller test", func() { g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseRunning)) }), 2*time.Second, 100*time.Millisecond).Should(Succeed()) - By("complete the persisted Jobs and expect the Restore to complete") + By("complete action zero and continue from the frozen action one") + mockRestoreJobsCompleted(restore) + Eventually(testapps.List(&testCtx, generics.JobSignature, + client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, + client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(3)) + mockRestoreJobsCompleted(restore) + Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) { + g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseCompleted)) + })).Should(Succeed()) + }) + + It("finishes a committed postReady plan after the ActionSet is deleted", func() { + By("remove the prepareData stage for testing post ready actions") + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + set.Spec.Restore.PrepareData = nil + })).Should(Succeed()) + + matchLabels := map[string]string{ + constant.AppInstanceLabelKey: testdp.ClusterName, + } + restore := initResourcesAndWaitRestore(true, false, false, "", dpv1alpha1.RestorePhaseRunning, + func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName).SetJobActionConfig(matchLabels).SetExecActionConfig(matchLabels) + }, nil) + + By("wait for the immutable plan and first postReady Jobs") + Eventually(testapps.List(&testCtx, generics.JobSignature, + client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, + client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(2)) + expectCommittedFullStage(restore) + + By("delete the mutable ActionSet while the committed Jobs are running") + Expect(k8sClient.Delete(ctx, actionSet)).Should(Succeed()) + Consistently(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) { + g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseRunning)) + }), 2*time.Second, 100*time.Millisecond).Should(Succeed()) + + By("complete action zero and create frozen action one without the deleted ActionSet") + mockRestoreJobsCompleted(restore) + Eventually(testapps.List(&testCtx, generics.JobSignature, + client.MatchingLabels{dprestore.DataProtectionRestoreLabelKey: restore.Name}, + client.InNamespace(testCtx.DefaultNamespace))).Should(HaveLen(3)) mockRestoreJobsCompleted(restore) Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(restore), func(g Gomega, r *dpv1alpha1.Restore) { g.Expect(r.Status.Phase).Should(Equal(dpv1alpha1.RestorePhaseCompleted)) diff --git a/controllers/dataprotection/utils.go b/controllers/dataprotection/utils.go index f7b3eb2bbac..4cbea049555 100644 --- a/controllers/dataprotection/utils.go +++ b/controllers/dataprotection/utils.go @@ -302,7 +302,7 @@ func getDefaultBackupRepo(ctx context.Context, cli client.Client) (*dpv1alpha1.B } type objectList interface { - *appsv1.StatefulSetList | *batchv1.JobList + *appsv1.StatefulSetList | *batchv1.JobList | *corev1.SecretList client.ObjectList } diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index a741e9ce770..d4a1ae753b6 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -59,16 +59,43 @@ const ( postReadyActionContractAnnotationKey = "dataprotection.kubeblocks.io/post-ready-action-contract" postReadyBackupNameAnnotationKey = "dataprotection.kubeblocks.io/post-ready-backup-name" postReadyActionNameAnnotationKey = "dataprotection.kubeblocks.io/post-ready-action-name" + postReadyPlanNameAnnotationKey = "dataprotection.kubeblocks.io/post-ready-plan-name" + postReadyPlanDigestAnnotationKey = "dataprotection.kubeblocks.io/post-ready-plan-digest" + postReadyPlanRestoreUIDAnnotationKey = "dataprotection.kubeblocks.io/post-ready-restore-uid" + postReadyPlanDataKey = "plan.json" + postReadyPlanVersion = "v2" + postReadyPlanMaxPayloadBytes = 1 << 20 + postReadyPlanMarkerAnnotationKey = "dataprotection.kubeblocks.io/post-ready-stage-plan" ) +const postReadyPlanSecretType corev1.SecretType = "dataprotection.kubeblocks.io/post-ready-plan" + +type postReadyActionExecutionPlan struct { + Order int `json:"order"` + BackupName string `json:"backupName"` + ActionName string `json:"actionName"` + Jobs []batchv1.Job `json:"jobs"` +} + +type postReadyExecutionPlan struct { + Version string `json:"version"` + RestoreNamespace string `json:"restoreNamespace"` + RestoreName string `json:"restoreName"` + RestoreUID string `json:"restoreUID"` + SourceBackupNamespace string `json:"sourceBackupNamespace"` + SourceBackupName string `json:"sourceBackupName"` + Actions []postReadyActionExecutionPlan `json:"actions"` +} + type BackupActionSet struct { Backup *dpv1alpha1.Backup // set it when the backup relies on incremental backups, such as Incremental backup AncestorIncrementalBackups []*dpv1alpha1.Backup // set it when the backup relies on a base backup, such as Continuous backup - BaseBackup *dpv1alpha1.Backup - ActionSet *dpv1alpha1.ActionSet - UseVolumeSnapshot bool + BaseBackup *dpv1alpha1.Backup + ActionSet *dpv1alpha1.ActionSet + UseVolumeSnapshot bool + UseDurablePostReadyPlan bool } type RestoreManager struct { @@ -113,6 +140,17 @@ func (r *RestoreManager) GetBackupActionSetByNamespaced(reqCtx intctrlutil.Reque useVolumeSnapshot := backupMethod.SnapshotVolumes != nil && *backupMethod.SnapshotVolumes actionSet, err := utils.GetActionSetByName(reqCtx, cli, backup.Status.BackupMethod.ActionSetName) if err != nil { + if apierrors.IsNotFound(err) { + _, foundPlan, planErr := r.loadPostReadyExecutionPlanStage(reqCtx, cli) + if planErr != nil { + return nil, planErr + } + if foundPlan { + return &BackupActionSet{ + Backup: backup, UseVolumeSnapshot: useVolumeSnapshot, UseDurablePostReadyPlan: true, + }, nil + } + } return nil, err } return &BackupActionSet{Backup: backup, ActionSet: actionSet, UseVolumeSnapshot: useVolumeSnapshot}, nil @@ -333,6 +371,18 @@ func (r *RestoreManager) SetBackupSets(backupSets ...BackupActionSet) { // AnalysisRestoreActionsWithBackup analysis the restore actions progress group by backup. // check if the restore jobs are completed or failed or processing. func (r *RestoreManager) AnalysisRestoreActionsWithBackup(stage dpv1alpha1.RestoreStage, backupName string, actionName string) (bool, bool) { + return r.AnalysisRestoreActionsWithBackupExpected(stage, backupName, actionName, 0) +} + +// AnalysisRestoreActionsWithBackupExpected prevents a partially recorded +// postReady action from looking complete when its immutable plan contains more +// Jobs than Restore status has observed so far. +func (r *RestoreManager) AnalysisRestoreActionsWithBackupExpected( + stage dpv1alpha1.RestoreStage, + backupName string, + actionName string, + expectedActionCount int, +) (bool, bool) { var ( restoreActionCount int finishedActionCount int @@ -360,6 +410,9 @@ func (r *RestoreManager) AnalysisRestoreActionsWithBackup(stage dpv1alpha1.Resto finishedActionCount += 1 } } + if stage == dpv1alpha1.PostReady && expectedActionCount > 0 { + restoreActionCount = expectedActionCount + } allActionsFinished := restoreActionCount > 0 && finishedActionCount == restoreActionCount return allActionsFinished, existFailedAction @@ -619,16 +672,24 @@ func (r *RestoreManager) BuildVolumePopulateJob( } // GetExistingActionJobs returns jobs already recorded in Restore status for an in-flight action. -// PostReady needs completed predecessors as well as processing jobs because the persisted set -// carries the frozen serial plan across reconciles. PrepareData retains its processing-only path. -// If any required recorded Job is missing, it returns an empty list so callers can follow the -// original build/create path. +// PostReady first loads its immutable plan Secret, including completed +// predecessors, because the plan survives Job GC. Legacy restores retain the +// status-recorded Job fallback. PrepareData keeps its processing-only path. func (r *RestoreManager) GetExistingActionJobs( reqCtx intctrlutil.RequestCtx, cli client.Client, stage dpv1alpha1.RestoreStage, backupName string, actionName string) ([]*batchv1.Job, error) { + if stage == dpv1alpha1.PostReady { + jobs, found, err := r.loadPostReadyExecutionPlan(reqCtx, cli, backupName, actionName) + if err != nil { + return nil, err + } + if found { + return jobs, nil + } + } restoreActions := r.Restore.Status.Actions.PrepareData if stage == dpv1alpha1.PostReady { restoreActions = r.Restore.Status.Actions.PostReady @@ -686,6 +747,74 @@ func (r *RestoreManager) ReconcileOrphanedPostReadyActions( reqCtx intctrlutil.RequestCtx, cli client.Client, ) (bool, error) { + stage, foundStage, err := r.loadPostReadyExecutionPlanStage(reqCtx, cli) + if err != nil { + return false, err + } + if foundStage { + secret := &corev1.Secret{} + if err := cli.Get(reqCtx.Ctx, types.NamespacedName{ + Namespace: r.Restore.Namespace, + Name: r.postReadyPlanSecretName(), + }, secret); err != nil { + return false, err + } + digest := secret.Annotations[postReadyPlanDigestAnnotationKey] + for i := range stage.Actions { + action := &stage.Actions[i] + jobs := attachPostReadyPlanReference( + action.Jobs, secret.Name, digest, string(r.Restore.UID)) + expectedCount, err := PostReadyActionExpectedJobCount(jobs) + if err != nil { + return false, err + } + completed, failed := r.AnalysisRestoreActionsWithBackupExpected( + dpv1alpha1.PostReady, action.BackupName, action.ActionName, expectedCount) + if failed { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady action %s for backup %s has a terminal failed Job", + action.ActionName, action.BackupName)) + } + if completed { + continue + } + pending := r.PendingPostReadyJobs(action.BackupName, action.ActionName, jobs) + if len(pending) == 0 { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady action %s for backup %s has no pending Job but is not complete", + action.ActionName, action.BackupName)) + } + pending, err = r.CreateJobsIfNotExist(reqCtx, cli, r.Restore, pending) + if err != nil { + return false, err + } + if err := r.ResumeNextSerialPostReadyJob(reqCtx, cli, pending); err != nil { + return false, err + } + backupSet := BackupActionSet{Backup: &dpv1alpha1.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: action.BackupName}, + }} + _, failed, err = r.CheckJobsDone(dpv1alpha1.PostReady, action.ActionName, backupSet, pending) + if err != nil { + return false, err + } + if failed { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady action %s for backup %s has a terminal failed Job", + action.ActionName, action.BackupName)) + } + completed, _ = r.AnalysisRestoreActionsWithBackupExpected( + dpv1alpha1.PostReady, action.BackupName, action.ActionName, expectedCount) + if !completed { + return false, nil + } + } + return true, nil + } + + // Before the stage marker exists, retain the legacy recovery path for an + // upgrade with already-running frozen Jobs. New reconciles commit the full + // stage plan before creating any Job and therefore never enter this path. currentActions := map[string]struct{}{} if r.Restore.Spec.ReadyConfig != nil { for i := range r.PostReadyBackupSets { @@ -698,6 +827,7 @@ func (r *RestoreManager) ReconcileOrphanedPostReadyActions( } } } + durablePlanKeys := map[string]struct{}{} namespaces := []string{r.Restore.Namespace} if controllerNamespace := viper.GetString(constant.CfgKeyCtrlrMgrNS); controllerNamespace != "" && controllerNamespace != r.Restore.Namespace { @@ -725,6 +855,9 @@ func (r *RestoreManager) ReconcileOrphanedPostReadyActions( if _, ok := currentActions[key]; ok { continue } + if _, ok := durablePlanKeys[key]; ok { + continue + } orphaned[key] = append(orphaned[key], job.DeepCopy()) } } @@ -806,6 +939,662 @@ func splitPostReadyActionKey(key string) (string, string) { return parts[0], parts[1] } +func (r *RestoreManager) postReadyPlanSecretName() string { + identity := strings.Join([]string{ + r.Restore.Namespace, + r.Restore.Name, + string(r.Restore.UID), + string(dpv1alpha1.PostReady), + }, "\x00") + digest := sha256.Sum256([]byte(identity)) + return fmt.Sprintf("postready-stage-%x", digest[:16]) +} + +func postReadyPlanMarkerValue(planName, digest string) string { + return planName + "@" + digest +} + +func (r *RestoreManager) postReadyPlanMarker() (string, bool) { + if r.Restore.Annotations == nil { + return "", false + } + value, ok := r.Restore.Annotations[postReadyPlanMarkerAnnotationKey] + return value, ok +} + +func (r *RestoreManager) ensurePostReadyPlanMarker( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + planName, digest string, +) error { + expected := postReadyPlanMarkerValue(planName, digest) + if current, ok := r.postReadyPlanMarker(); ok { + if current != expected { + return intctrlutil.NewFatalError(fmt.Sprintf( + "restore %s/%s has a conflicting postReady execution plan marker", + r.Restore.Namespace, r.Restore.Name)) + } + return nil + } + original := r.Restore.DeepCopy() + if r.Restore.Annotations == nil { + r.Restore.Annotations = map[string]string{} + } + r.Restore.Annotations[postReadyPlanMarkerAnnotationKey] = expected + return cli.Patch(reqCtx.Ctx, r.Restore, client.MergeFrom(original)) +} + +func postReadyPlanIdentityForJobs(jobs []*batchv1.Job) (string, string, bool, error) { + if len(jobs) == 0 { + return "", "", false, nil + } + backupName, actionName := postReadyActionIdentity(jobs[0]) + if backupName == "" && actionName == "" { + return "", "", false, nil + } + if backupName == "" || actionName == "" { + return "", "", false, intctrlutil.NewFatalError("postReady jobs have an incomplete action identity") + } + for i := 1; i < len(jobs); i++ { + jobBackupName, jobActionName := postReadyActionIdentity(jobs[i]) + if jobBackupName != backupName || jobActionName != actionName { + return "", "", false, intctrlutil.NewFatalError("postReady jobs have inconsistent action identities") + } + } + return backupName, actionName, true, nil +} + +func canonicalPostReadyPlanJobs(jobs []*batchv1.Job) ([]batchv1.Job, error) { + canonical := make([]batchv1.Job, 0, len(jobs)) + for i := range jobs { + if jobs[i] == nil { + return nil, intctrlutil.NewFatalError("postReady execution plan contains a nil Job") + } + job := jobs[i].DeepCopy() + job.TypeMeta = metav1.TypeMeta{APIVersion: batchv1.SchemeGroupVersion.String(), Kind: "Job"} + job.ResourceVersion = "" + job.UID = "" + job.Generation = 0 + job.CreationTimestamp = metav1.Time{} + job.DeletionTimestamp = nil + job.DeletionGracePeriodSeconds = nil + job.ManagedFields = nil + job.OwnerReferences = nil + job.Finalizers = []string{dptypes.DataProtectionFinalizerName} + job.Status = batchv1.JobStatus{} + if job.Annotations != nil { + delete(job.Annotations, postReadyPlanNameAnnotationKey) + delete(job.Annotations, postReadyPlanDigestAnnotationKey) + delete(job.Annotations, postReadyPlanRestoreUIDAnnotationKey) + } + canonical = append(canonical, *job) + } + sort.Slice(canonical, func(i, j int) bool { + return postReadyJobIndex(canonical[i].Name) < postReadyJobIndex(canonical[j].Name) + }) + var expectedPolicy dpv1alpha1.PostReadyExecutionPolicy + var expectedContract string + var expectedTargetPlan string + for i := range canonical { + if postReadyJobIndex(canonical[i].Name) != i { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady execution plan has non-contiguous Job ordinal at %s/%s", + canonical[i].Namespace, canonical[i].Name)) + } + policy, err := postReadyExecutionPolicyForJob(&canonical[i]) + if err != nil { + return nil, err + } + plan, err := postReadyTargetPlan(&canonical[i]) + if err != nil { + return nil, err + } + if len(plan) != len(canonical) || plan[i] != postReadyTargetIdentity(&canonical[i]) { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s target identity does not match the complete execution plan", + canonical[i].Namespace, canonical[i].Name)) + } + contract := postReadyActionContractForJob(&canonical[i]) + if contract == "" { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has no frozen action contract", + canonical[i].Namespace, canonical[i].Name)) + } + serializedTargetPlan := canonical[i].Annotations[postReadyTargetPlanAnnotationKey] + if i == 0 { + expectedPolicy = policy + expectedContract = contract + expectedTargetPlan = serializedTargetPlan + } else if policy != expectedPolicy || contract != expectedContract || serializedTargetPlan != expectedTargetPlan { + return nil, intctrlutil.NewFatalError("postReady execution plan contains inconsistent Job contracts") + } + } + return canonical, nil +} + +func postReadyPlanDigest(payload []byte) string { + return fmt.Sprintf("sha256:%x", sha256.Sum256(payload)) +} + +func attachPostReadyPlanReference(jobs []batchv1.Job, planName, digest, restoreUID string) []*batchv1.Job { + result := make([]*batchv1.Job, 0, len(jobs)) + for i := range jobs { + job := jobs[i].DeepCopy() + if job.Annotations == nil { + job.Annotations = map[string]string{} + } + job.Annotations[postReadyPlanNameAnnotationKey] = planName + job.Annotations[postReadyPlanDigestAnnotationKey] = digest + job.Annotations[postReadyPlanRestoreUIDAnnotationKey] = restoreUID + result = append(result, job) + } + return result +} + +func canonicalPostReadyStagePlan(plan postReadyExecutionPlan) (postReadyExecutionPlan, error) { + if len(plan.Actions) == 0 { + return postReadyExecutionPlan{}, intctrlutil.NewFatalError("postReady stage execution plan has no actions") + } + seen := map[string]struct{}{} + canonicalActions := make([]postReadyActionExecutionPlan, 0, len(plan.Actions)) + for i := range plan.Actions { + action := plan.Actions[i] + if action.Order != i || action.BackupName == "" || action.ActionName == "" || len(action.Jobs) == 0 { + return postReadyExecutionPlan{}, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage action %d has an invalid ordered identity", i)) + } + key := postReadyActionKey(action.BackupName, action.ActionName) + if _, ok := seen[key]; ok { + return postReadyExecutionPlan{}, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan has duplicate action %s/%s", action.BackupName, action.ActionName)) + } + seen[key] = struct{}{} + jobPointers := make([]*batchv1.Job, 0, len(action.Jobs)) + for j := range action.Jobs { + jobPointers = append(jobPointers, action.Jobs[j].DeepCopy()) + } + canonicalJobs, err := canonicalPostReadyPlanJobs(jobPointers) + if err != nil { + return postReadyExecutionPlan{}, err + } + for j := range canonicalJobs { + backupName, actionName := postReadyActionIdentity(&canonicalJobs[j]) + if backupName != action.BackupName || actionName != action.ActionName { + return postReadyExecutionPlan{}, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage action %s/%s contains a Job for another action", + action.BackupName, action.ActionName)) + } + } + canonicalActions = append(canonicalActions, postReadyActionExecutionPlan{ + Order: action.Order, BackupName: action.BackupName, ActionName: action.ActionName, Jobs: canonicalJobs, + }) + } + plan.Actions = canonicalActions + return plan, nil +} + +func (r *RestoreManager) findPostReadyStageAction( + plan *postReadyExecutionPlan, + backupName, actionName string, +) (*postReadyActionExecutionPlan, bool) { + for i := range plan.Actions { + if plan.Actions[i].BackupName == backupName && plan.Actions[i].ActionName == actionName { + return &plan.Actions[i], true + } + } + return nil, false +} + +func (r *RestoreManager) loadPostReadyExecutionPlanStage( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + expected ...*postReadyExecutionPlan, +) (*postReadyExecutionPlan, bool, error) { + secretName := r.postReadyPlanSecretName() + marker, hasMarker := r.postReadyPlanMarker() + secret := &corev1.Secret{} + if err := cli.Get(reqCtx.Ctx, types.NamespacedName{Namespace: r.Restore.Namespace, Name: secretName}, secret); err != nil { + if apierrors.IsNotFound(err) { + if hasMarker { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s is missing", r.Restore.Namespace, secretName)) + } + return nil, false, nil + } + return nil, false, err + } + if secret.DeletionTimestamp != nil { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s is terminating", secret.Namespace, secret.Name)) + } + if secret.Type != postReadyPlanSecretType || secret.Immutable == nil || !*secret.Immutable { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s is not an immutable plan Secret", secret.Namespace, secret.Name)) + } + controller := metav1.GetControllerOf(secret) + if controller == nil || + controller.APIVersion != dpv1alpha1.SchemeGroupVersion.String() || + controller.Kind != dptypes.RestoreKind || + controller.Name != r.Restore.Name || + controller.UID != r.Restore.UID || + controller.Controller == nil || !*controller.Controller || + controller.BlockOwnerDeletion == nil || !*controller.BlockOwnerDeletion || + secret.Annotations[postReadyPlanRestoreUIDAnnotationKey] != string(r.Restore.UID) { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s is not owned by restore %s/%s", + secret.Namespace, secret.Name, r.Restore.Namespace, r.Restore.Name)) + } + payload := secret.Data[postReadyPlanDataKey] + if len(payload) == 0 { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s has no canonical payload", secret.Namespace, secret.Name)) + } + digest := postReadyPlanDigest(payload) + if secret.Annotations == nil || secret.Annotations[postReadyPlanDigestAnnotationKey] != digest { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s has an invalid payload digest", secret.Namespace, secret.Name)) + } + var plan postReadyExecutionPlan + if err := json.Unmarshal(payload, &plan); err != nil { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s has an invalid payload: %v", secret.Namespace, secret.Name, err)) + } + if plan.Version != postReadyPlanVersion || + plan.RestoreNamespace != r.Restore.Namespace || plan.RestoreName != r.Restore.Name || + plan.RestoreUID != string(r.Restore.UID) || + plan.SourceBackupNamespace != r.Restore.Spec.Backup.Namespace || + plan.SourceBackupName != r.Restore.Spec.Backup.Name { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s does not match restore identity", secret.Namespace, secret.Name)) + } + canonical, err := canonicalPostReadyStagePlan(plan) + if err != nil { + return nil, true, err + } + canonicalPayload, err := json.Marshal(canonical) + if err != nil { + return nil, true, err + } + if string(canonicalPayload) != string(payload) { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s payload is not canonical", secret.Namespace, secret.Name)) + } + expectedMarker := postReadyPlanMarkerValue(secret.Name, digest) + if hasMarker && marker != expectedMarker { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "restore %s/%s postReady stage execution plan marker does not match %s/%s", + r.Restore.Namespace, r.Restore.Name, secret.Namespace, secret.Name)) + } + if !hasMarker { + if len(expected) != 1 || expected[0] == nil { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s has no committed marker and no complete expected stage", + secret.Namespace, secret.Name)) + } + expectedCanonical, err := canonicalPostReadyStagePlan(*expected[0]) + if err != nil { + return nil, true, err + } + expectedPayload, err := json.Marshal(expectedCanonical) + if err != nil { + return nil, true, err + } + if string(expectedPayload) != string(payload) { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan %s/%s does not match the complete expected stage", + secret.Namespace, secret.Name)) + } + if err := r.migratePostReadyJobsBeforeStageCommit(reqCtx, cli, &canonical, secret.Name, digest); err != nil { + return nil, true, err + } + } + if err := r.ensurePostReadyPlanMarker(reqCtx, cli, secret.Name, digest); err != nil { + return nil, true, err + } + return &canonical, true, nil +} + +func (r *RestoreManager) persistPostReadyExecutionPlanStage( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + actions []postReadyActionExecutionPlan, +) (*postReadyExecutionPlan, error) { + canonical, err := canonicalPostReadyStagePlan(postReadyExecutionPlan{ + Version: postReadyPlanVersion, + RestoreNamespace: r.Restore.Namespace, + RestoreName: r.Restore.Name, + RestoreUID: string(r.Restore.UID), + SourceBackupNamespace: r.Restore.Spec.Backup.Namespace, + SourceBackupName: r.Restore.Spec.Backup.Name, + Actions: actions, + }) + if err != nil { + return nil, err + } + payload, err := json.Marshal(canonical) + if err != nil { + return nil, err + } + if len(payload) > postReadyPlanMaxPayloadBytes { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan payload is %d bytes and exceeds the Secret limit", len(payload))) + } + digest := postReadyPlanDigest(payload) + immutable := true + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.Restore.Namespace, + Name: r.postReadyPlanSecretName(), + Labels: map[string]string{ + DataProtectionRestoreLabelKey: r.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: r.Restore.Namespace, + }, + Annotations: map[string]string{ + postReadyPlanDigestAnnotationKey: digest, + postReadyPlanRestoreUIDAnnotationKey: string(r.Restore.UID), + }, + }, + Immutable: &immutable, + Type: postReadyPlanSecretType, + Data: map[string][]byte{postReadyPlanDataKey: payload}, + } + if err := controllerutil.SetControllerReference(r.Restore, secret, r.Schema); err != nil { + return nil, err + } + if err := cli.Create(reqCtx.Ctx, secret); err != nil { + if !apierrors.IsAlreadyExists(err) { + return nil, err + } + persisted, found, loadErr := r.loadPostReadyExecutionPlanStage(reqCtx, cli, &canonical) + if loadErr != nil { + return nil, loadErr + } + if !found { + return nil, intctrlutil.NewFatalError("postReady stage execution plan disappeared after AlreadyExists") + } + return persisted, nil + } + if err := r.migratePostReadyJobsBeforeStageCommit(reqCtx, cli, &canonical, secret.Name, digest); err != nil { + return nil, err + } + if err := r.ensurePostReadyPlanMarker(reqCtx, cli, secret.Name, digest); err != nil { + return nil, err + } + return &canonical, nil +} + +func (r *RestoreManager) loadPostReadyExecutionPlan( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + backupName, actionName string, +) ([]*batchv1.Job, bool, error) { + stage, found, err := r.loadPostReadyExecutionPlanStage(reqCtx, cli) + if err != nil || !found { + return nil, found, err + } + action, ok := r.findPostReadyStageAction(stage, backupName, actionName) + if !ok { + return nil, true, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan does not contain action %s/%s", backupName, actionName)) + } + secretName := r.postReadyPlanSecretName() + secret := &corev1.Secret{} + if err := cli.Get(reqCtx.Ctx, types.NamespacedName{Namespace: r.Restore.Namespace, Name: secretName}, secret); err != nil { + return nil, true, err + } + digest := secret.Annotations[postReadyPlanDigestAnnotationKey] + return attachPostReadyPlanReference(action.Jobs, secret.Name, digest, string(r.Restore.UID)), true, nil +} + +func (r *RestoreManager) persistPostReadyExecutionPlan( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + backupName, actionName string, + jobs []*batchv1.Job, +) ([]*batchv1.Job, error) { + canonical, err := canonicalPostReadyPlanJobs(jobs) + if err != nil { + return nil, err + } + stage, err := r.persistPostReadyExecutionPlanStage(reqCtx, cli, []postReadyActionExecutionPlan{{ + Order: 0, BackupName: backupName, ActionName: actionName, Jobs: canonical, + }}) + if err != nil { + return nil, err + } + action, ok := r.findPostReadyStageAction(stage, backupName, actionName) + if !ok { + return nil, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage execution plan does not contain action %s/%s", backupName, actionName)) + } + secretName := r.postReadyPlanSecretName() + secret := &corev1.Secret{} + if err := cli.Get(reqCtx.Ctx, types.NamespacedName{Namespace: r.Restore.Namespace, Name: secretName}, secret); err != nil { + return nil, err + } + return attachPostReadyPlanReference( + action.Jobs, secret.Name, secret.Annotations[postReadyPlanDigestAnnotationKey], string(r.Restore.UID)), nil +} + +func normalizePostReadyJobSpec(job *batchv1.Job, scheme *runtime.Scheme) batchv1.JobSpec { + copy := job.DeepCopy() + if scheme != nil { + scheme.Default(copy) + } + if copy.Spec.Parallelism != nil && *copy.Spec.Parallelism == 1 { + copy.Spec.Parallelism = nil + } + if copy.Spec.Completions != nil && *copy.Spec.Completions == 1 { + copy.Spec.Completions = nil + } + if copy.Spec.BackoffLimit != nil && *copy.Spec.BackoffLimit == 6 { + copy.Spec.BackoffLimit = nil + } + if copy.Spec.CompletionMode != nil && *copy.Spec.CompletionMode == batchv1.NonIndexedCompletion { + copy.Spec.CompletionMode = nil + } + if copy.Spec.Suspend != nil && !*copy.Spec.Suspend { + copy.Spec.Suspend = nil + } + if copy.Spec.ManualSelector != nil && !*copy.Spec.ManualSelector { + copy.Spec.ManualSelector = nil + } + if isGeneratedPostReadyJobSelector(copy.Spec.Selector, string(copy.UID)) { + copy.Spec.Selector = nil + } + generatedLabels := map[string]string{ + "batch.kubernetes.io/controller-uid": string(copy.UID), + "controller-uid": string(copy.UID), + "batch.kubernetes.io/job-name": copy.Name, + "job-name": copy.Name, + } + for key, expected := range generatedLabels { + if expected != "" && copy.Spec.Template.Labels[key] == expected { + delete(copy.Spec.Template.Labels, key) + } + } + if len(copy.Spec.Template.Labels) == 0 { + copy.Spec.Template.Labels = nil + } + podSpec := ©.Spec.Template.Spec + if podSpec.DeprecatedServiceAccount == podSpec.ServiceAccountName { + podSpec.DeprecatedServiceAccount = "" + } + for i := range podSpec.Volumes { + normalizePostReadyVolumeDefaults(&podSpec.Volumes[i]) + } + if podSpec.TerminationGracePeriodSeconds != nil && *podSpec.TerminationGracePeriodSeconds == 30 { + podSpec.TerminationGracePeriodSeconds = nil + } + if podSpec.DNSPolicy == corev1.DNSClusterFirst { + podSpec.DNSPolicy = "" + } + if podSpec.SchedulerName == corev1.DefaultSchedulerName { + podSpec.SchedulerName = "" + } + if podSpec.SecurityContext != nil && apiequality.Semantic.DeepEqual( + podSpec.SecurityContext, &corev1.PodSecurityContext{}) { + podSpec.SecurityContext = nil + } + for i := range podSpec.InitContainers { + normalizePostReadyContainerDefaults(&podSpec.InitContainers[i]) + } + for i := range podSpec.Containers { + normalizePostReadyContainerDefaults(&podSpec.Containers[i]) + } + return copy.Spec +} + +func isGeneratedPostReadyJobSelector(selector *metav1.LabelSelector, jobUID string) bool { + if selector == nil || jobUID == "" || len(selector.MatchExpressions) != 0 || len(selector.MatchLabels) != 1 { + return false + } + for key, value := range selector.MatchLabels { + return (key == "controller-uid" || key == "batch.kubernetes.io/controller-uid") && value == jobUID + } + return false +} + +func normalizePostReadyContainerDefaults(container *corev1.Container) { + if container.TerminationMessagePath == corev1.TerminationMessagePathDefault { + container.TerminationMessagePath = "" + } + if container.TerminationMessagePolicy == corev1.TerminationMessageReadFile { + container.TerminationMessagePolicy = "" + } + imageName := container.Image + if slash := strings.LastIndexByte(imageName, '/'); slash >= 0 { + imageName = imageName[slash+1:] + } + defaultPullPolicy := corev1.PullIfNotPresent + if !strings.Contains(imageName, "@") { + colon := strings.LastIndexByte(imageName, ':') + if colon < 0 || imageName[colon+1:] == "latest" { + defaultPullPolicy = corev1.PullAlways + } + } + if container.ImagePullPolicy == defaultPullPolicy { + container.ImagePullPolicy = "" + } + for i := range container.Env { + if container.Env[i].ValueFrom != nil && container.Env[i].ValueFrom.FieldRef != nil && + container.Env[i].ValueFrom.FieldRef.APIVersion == "v1" { + container.Env[i].ValueFrom.FieldRef.APIVersion = "" + } + } +} + +func normalizePostReadyVolumeDefaults(volume *corev1.Volume) { + const defaultMode = int32(0o644) + if volume.DownwardAPI != nil { + if volume.DownwardAPI.DefaultMode != nil && *volume.DownwardAPI.DefaultMode == defaultMode { + volume.DownwardAPI.DefaultMode = nil + } + for i := range volume.DownwardAPI.Items { + if fieldRef := volume.DownwardAPI.Items[i].FieldRef; fieldRef != nil && fieldRef.APIVersion == "v1" { + fieldRef.APIVersion = "" + } + } + } + if volume.Secret != nil && volume.Secret.DefaultMode != nil && *volume.Secret.DefaultMode == defaultMode { + volume.Secret.DefaultMode = nil + } + if volume.ConfigMap != nil && volume.ConfigMap.DefaultMode != nil && *volume.ConfigMap.DefaultMode == defaultMode { + volume.ConfigMap.DefaultMode = nil + } + if volume.Projected != nil { + if volume.Projected.DefaultMode != nil && *volume.Projected.DefaultMode == defaultMode { + volume.Projected.DefaultMode = nil + } + for i := range volume.Projected.Sources { + downwardAPI := volume.Projected.Sources[i].DownwardAPI + if downwardAPI == nil { + continue + } + for j := range downwardAPI.Items { + if fieldRef := downwardAPI.Items[j].FieldRef; fieldRef != nil && fieldRef.APIVersion == "v1" { + fieldRef.APIVersion = "" + } + } + } + } +} + +func (r *RestoreManager) postReadyJobSpecsEqual(desired, existing *batchv1.Job) bool { + desiredSpec := normalizePostReadyJobSpec(desired, r.Schema) + existingSpec := normalizePostReadyJobSpec(existing, r.Schema) + return apiequality.Semantic.DeepEqual(desiredSpec, existingSpec) +} + +func (r *RestoreManager) migratePostReadyJobsBeforeStageCommit( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + stage *postReadyExecutionPlan, + planName, digest string, +) error { + for i := range stage.Actions { + desiredJobs := attachPostReadyPlanReference( + stage.Actions[i].Jobs, planName, digest, string(r.Restore.UID)) + for j := range desiredJobs { + existing := &batchv1.Job{} + if err := cli.Get(reqCtx.Ctx, client.ObjectKeyFromObject(desiredJobs[j]), existing); err != nil { + if apierrors.IsNotFound(err) { + continue + } + return err + } + if existing.DeletionTimestamp != nil || existing.UID == "" || !r.isJobForRestoreAction(existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s has no stable restore ownership", existing.Namespace, existing.Name)) + } + desiredBackupName, desiredActionName := postReadyActionIdentity(desiredJobs[j]) + existingBackupName, existingActionName := postReadyActionIdentity(existing) + if existingBackupName != desiredBackupName || existingActionName != desiredActionName { + return intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s action identity does not match the stage plan", + existing.Namespace, existing.Name)) + } + controller := metav1.GetControllerOf(existing) + if existing.Namespace == r.Restore.Namespace && (controller == nil || controller.UID != r.Restore.UID) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s is not owned by restore %s/%s", + existing.Namespace, existing.Name, r.Restore.Namespace, r.Restore.Name)) + } + existingPlanName := existing.Annotations[postReadyPlanNameAnnotationKey] + existingDigest := existing.Annotations[postReadyPlanDigestAnnotationKey] + existingRestoreUID := existing.Annotations[postReadyPlanRestoreUIDAnnotationKey] + if existingPlanName != "" || existingDigest != "" || existingRestoreUID != "" { + if existingPlanName != planName || existingDigest != digest || existingRestoreUID != string(r.Restore.UID) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a conflicting committed stage reference", + existing.Namespace, existing.Name)) + } + if err := r.validateExistingRestoreActionJob(desiredJobs[j], existing); err != nil { + return err + } + continue + } + if !r.postReadyJobSpecsEqual(desiredJobs[j], existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "legacy postReady job %s/%s executable spec does not match the stage plan", + existing.Namespace, existing.Name)) + } + original := existing.DeepCopy() + if existing.Annotations == nil { + existing.Annotations = map[string]string{} + } + existing.Annotations[postReadyPlanNameAnnotationKey] = planName + existing.Annotations[postReadyPlanDigestAnnotationKey] = digest + existing.Annotations[postReadyPlanRestoreUIDAnnotationKey] = string(r.Restore.UID) + controllerutil.AddFinalizer(existing, dptypes.DataProtectionFinalizerName) + if err := cli.Patch(reqCtx.Ctx, existing, client.MergeFrom(original)); err != nil { + return err + } + } + } + return nil +} + func postReadyActionIdentity(job *batchv1.Job) (string, string) { if job.Annotations != nil { if backupName := job.Annotations[postReadyBackupNameAnnotationKey]; backupName != "" { @@ -1274,12 +2063,144 @@ func serialPostReadyJobs(jobs []*batchv1.Job) (bool, error) { return policy == dpv1alpha1.PostReadyExecutionPolicySerial, nil } -// FreezePostReadyExecutionPlan preserves the policy and ordered target set -// chosen by the first created job across partial retries and ActionSet updates. +// PostReadyActionExpectedJobCount returns the immutable plan cardinality when +// available. This count belongs to the plan domain and does not change as +// mutable completion status advances. +func PostReadyActionExpectedJobCount(jobs []*batchv1.Job) (int, error) { + if len(jobs) == 0 { + return 0, nil + } + plan, err := postReadyTargetPlan(jobs[0]) + if err != nil { + return 0, err + } + if len(plan) > 0 { + return len(plan), nil + } + return len(jobs), nil +} + +// EnsurePostReadyStagePlan commits the ordered executable plan for every +// postReady action before the first Job of the stage can be created. +func (r *RestoreManager) EnsurePostReadyStagePlan( + reqCtx intctrlutil.RequestCtx, + cli client.Client, +) (bool, error) { + if _, hasMarker := r.postReadyPlanMarker(); hasMarker { + if _, found, err := r.loadPostReadyExecutionPlanStage(reqCtx, cli); err != nil || found { + return found, err + } + } + if r.Restore.Spec.ReadyConfig == nil || len(r.PostReadyBackupSets) == 0 { + return false, nil + } + actions := make([]postReadyActionExecutionPlan, 0) + for i := range r.PostReadyBackupSets { + backupSet := r.PostReadyBackupSets[i] + if backupSet.Backup == nil || backupSet.ActionSet == nil || backupSet.ActionSet.Spec.Restore == nil { + return false, intctrlutil.NewFatalError("postReady stage has an incomplete Backup/ActionSet definition") + } + target := utils.GetBackupStatusTarget(backupSet.Backup, r.Restore.Spec.Backup.SourceTargetName) + if target == nil { + return false, intctrlutil.NewFatalError("can not found any source targe in backup " + backupSet.Backup.Name) + } + for step := range backupSet.ActionSet.Spec.Restore.PostReady { + actionName := fmt.Sprintf("%s-%d", dpv1alpha1.PostReady, step) + jobs, err := r.BuildPostReadyActionJobs(reqCtx, cli, backupSet, target, step) + if err != nil { + return false, err + } + jobs, err = r.freezeLegacyPostReadyExecutionPlan(reqCtx, cli, jobs) + if err != nil { + return false, err + } + canonical, err := canonicalPostReadyPlanJobs(jobs) + if err != nil { + return false, err + } + if len(canonical) == 0 { + return false, intctrlutil.NewFatalError(fmt.Sprintf( + "postReady stage action %s/%s has no executable Jobs", backupSet.Backup.Name, actionName)) + } + actions = append(actions, postReadyActionExecutionPlan{ + Order: len(actions), BackupName: backupSet.Backup.Name, ActionName: actionName, Jobs: canonical, + }) + } + } + if len(actions) == 0 { + return false, nil + } + _, err := r.persistPostReadyExecutionPlanStage(reqCtx, cli, actions) + return err == nil, err +} + +// PendingPostReadyJobs filters Jobs already recorded terminal in mutable +// Restore status. A garbage-collected Completed or Failed Job is therefore +// never recreated from the immutable plan. +func (r *RestoreManager) PendingPostReadyJobs(backupName, actionName string, jobs []*batchv1.Job) []*batchv1.Job { + terminal := map[string]struct{}{} + for i := range r.Restore.Status.Actions.PostReady { + action := r.Restore.Status.Actions.PostReady[i] + if action.BackupName == backupName && action.Name == actionName && + (action.Status == dpv1alpha1.RestoreActionCompleted || action.Status == dpv1alpha1.RestoreActionFailed) { + terminal[action.ObjectKey] = struct{}{} + } + } + pending := make([]*batchv1.Job, 0, len(jobs)) + for i := range jobs { + if _, ok := terminal[BuildJobKeyForActionStatus(jobs[i].Name)]; ok { + continue + } + pending = append(pending, jobs[i]) + } + return pending +} + +// FreezePostReadyExecutionPlan persists the complete immutable executable plan +// before any Job is created. Once present, that plan remains authoritative +// across Job GC and ActionSet or ReadyConfig drift. func (r *RestoreManager) FreezePostReadyExecutionPlan( reqCtx intctrlutil.RequestCtx, cli client.Client, jobs []*batchv1.Job, +) ([]*batchv1.Job, error) { + backupName, actionName, hasIdentity, err := postReadyPlanIdentityForJobs(jobs) + if err != nil { + return nil, err + } + if hasIdentity { + if _, hasMarker := r.postReadyPlanMarker(); hasMarker { + persisted, found, err := r.loadPostReadyExecutionPlan(reqCtx, cli, backupName, actionName) + if err != nil { + return nil, err + } + if found { + return persisted, nil + } + } + } + + frozen, err := r.freezeLegacyPostReadyExecutionPlan(reqCtx, cli, jobs) + if err != nil || !hasIdentity { + return frozen, err + } + frozenBackupName, frozenActionName, frozenHasIdentity, err := postReadyPlanIdentityForJobs(frozen) + if err != nil { + return nil, err + } + if !frozenHasIdentity || frozenBackupName != backupName || frozenActionName != actionName { + return frozen, nil + } + return r.persistPostReadyExecutionPlan(reqCtx, cli, backupName, actionName, frozen) +} + +// freezeLegacyPostReadyExecutionPlan preserves compatibility with Jobs created +// before durable plan Secrets existed. It may migrate only a complete or +// provably equivalent legacy plan; ambiguous legacy state still fails closed. +func (r *RestoreManager) freezeLegacyPostReadyExecutionPlan( + reqCtx intctrlutil.RequestCtx, + cli client.Client, + jobs []*batchv1.Job, ) ([]*batchv1.Job, error) { if len(jobs) == 0 { return jobs, nil @@ -1455,6 +2376,36 @@ func (r *RestoreManager) validateExistingRestoreActionJob(desired, existing *bat "postReady job %s/%s target identity does not match desired target", existing.Namespace, existing.Name)) } + desiredHasPlanReference := desired.Annotations[postReadyPlanNameAnnotationKey] != "" || + desired.Annotations[postReadyPlanDigestAnnotationKey] != "" || + desired.Annotations[postReadyPlanRestoreUIDAnnotationKey] != "" + if desiredHasPlanReference { + desiredBackupName, hasDesiredBackupName := desired.Annotations[postReadyBackupNameAnnotationKey] + desiredActionName, hasDesiredActionName := desired.Annotations[postReadyActionNameAnnotationKey] + if !hasDesiredBackupName || desiredBackupName == "" || !hasDesiredActionName || desiredActionName == "" { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s desired committed action identity is incomplete", + desired.Namespace, desired.Name)) + } + existingBackupName, hasExistingBackupName := existing.Annotations[postReadyBackupNameAnnotationKey] + existingActionName, hasExistingActionName := existing.Annotations[postReadyActionNameAnnotationKey] + if !hasExistingBackupName || existingBackupName == "" || + !hasExistingActionName || existingActionName == "" || + existingBackupName != desiredBackupName || existingActionName != desiredActionName { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s committed action identity does not match desired action", + existing.Namespace, existing.Name)) + } + } else { + desiredBackupName, desiredActionName := postReadyActionIdentity(desired) + existingBackupName, existingActionName := postReadyActionIdentity(existing) + if (desiredBackupName != "" || desiredActionName != "") && + (existingBackupName != desiredBackupName || existingActionName != desiredActionName) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s action identity does not match desired action", + existing.Namespace, existing.Name)) + } + } desiredPolicy, err := postReadyExecutionPolicyForJob(desired) if err != nil { return err @@ -1487,6 +2438,54 @@ func (r *RestoreManager) validateExistingRestoreActionJob(desired, existing *bat "postReady job %s/%s executable action does not match desired contract", existing.Namespace, existing.Name)) } + desiredPlanName := desired.Annotations[postReadyPlanNameAnnotationKey] + desiredPlanDigest := desired.Annotations[postReadyPlanDigestAnnotationKey] + desiredRestoreUID := desired.Annotations[postReadyPlanRestoreUIDAnnotationKey] + if desiredPlanName == "" && desiredPlanDigest == "" && desiredRestoreUID == "" { + if existing.Annotations[postReadyPlanNameAnnotationKey] != "" || + existing.Annotations[postReadyPlanDigestAnnotationKey] != "" || + existing.Annotations[postReadyPlanRestoreUIDAnnotationKey] != "" { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has an unexpected committed stage reference", + existing.Namespace, existing.Name)) + } + if !r.postReadyJobSpecsEqual(desired, existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s executable spec does not match its frozen legacy plan", + existing.Namespace, existing.Name)) + } + return nil + } + if desiredPlanName == "" || desiredPlanDigest == "" || desiredRestoreUID == "" { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s desired execution plan reference is incomplete", + desired.Namespace, desired.Name)) + } + if existingPlanName := existing.Annotations[postReadyPlanNameAnnotationKey]; existingPlanName != desiredPlanName { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a missing or different immutable execution plan reference", + existing.Namespace, existing.Name)) + } + if existingPlanDigest := existing.Annotations[postReadyPlanDigestAnnotationKey]; existingPlanDigest != desiredPlanDigest { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a missing or different execution plan digest", + existing.Namespace, existing.Name)) + } + if existingRestoreUID := existing.Annotations[postReadyPlanRestoreUIDAnnotationKey]; existingRestoreUID != desiredRestoreUID || existingRestoreUID != string(r.Restore.UID) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s has a missing or different restore UID reference", + existing.Namespace, existing.Name)) + } + if !controllerutil.ContainsFinalizer(existing, dptypes.DataProtectionFinalizerName) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s is missing terminal-status protection", + existing.Namespace, existing.Name)) + } + if !r.postReadyJobSpecsEqual(desired, existing) { + return intctrlutil.NewFatalError(fmt.Sprintf( + "postReady job %s/%s executable spec does not match its immutable execution plan", + existing.Namespace, existing.Name)) + } return nil } diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index e82539a706c..58409d2b5a6 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -21,6 +21,7 @@ package restore import ( "context" + "encoding/json" "fmt" "strconv" "strings" @@ -34,8 +35,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" @@ -64,6 +67,17 @@ type failNthCreateClient struct { count int } +type failNthJobCreateClient struct { + client.Client + n int + count int +} + +type persistSecretThenErrorClient struct { + client.Client + failed bool +} + func (c *failNthCreateClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { c.count++ if c.count == c.n { @@ -72,6 +86,27 @@ func (c *failNthCreateClient) Create(ctx context.Context, obj client.Object, opt return c.Client.Create(ctx, obj, opts...) } +func (c *failNthJobCreateClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if _, ok := obj.(*batchv1.Job); ok { + c.count++ + if c.count == c.n { + return fmt.Errorf("injected Job create failure at %d", c.n) + } + } + return c.Client.Create(ctx, obj, opts...) +} + +func (c *persistSecretThenErrorClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if _, ok := obj.(*corev1.Secret); ok && !c.failed { + c.failed = true + if err := c.Client.Create(ctx, obj, opts...); err != nil { + return err + } + return fmt.Errorf("injected lost Secret create response") + } + return c.Client.Create(ctx, obj, opts...) +} + func (c createAlreadyExistsClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { if err := c.Client.Create(ctx, c.foreign.DeepCopy(), opts...); err != nil { return err @@ -112,6 +147,7 @@ var _ = Describe("RestoreManager Test", func() { Eventually(testapps.List(&testCtx, generics.BackupSignature, inNS)).Should(HaveLen(0)) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.JobSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.SecretSignature, true, inNS) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.RestoreSignature, true, inNS) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.PersistentVolumeClaimSignature, true, inNS) @@ -281,6 +317,32 @@ var _ = Describe("RestoreManager Test", func() { } } + newDurablePostReadyJobs := func( + restoreMGR *RestoreManager, + backupName, actionName, image string, + ) []*batchv1.Job { + labels := map[string]string{ + DataProtectionRestoreLabelKey: restoreMGR.Restore.Name, + DataProtectionRestoreNamespaceLabelKey: restoreMGR.Restore.Namespace, + } + jobs := []*batchv1.Job{ + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-durable-0", labels), + newRestoreJob(testCtx.DefaultNamespace, "restore-post-ready-durable-1", labels), + } + for i := range jobs { + jobs[i].Spec.Template.Spec.Containers[0].Image = image + jobs[i].Annotations = map[string]string{ + postReadyTargetIdentityAnnotationKey: fmt.Sprintf("default/pod-%d", i), + postReadyActionContractAnnotationKey: "sha256:stable-action-contract", + postReadyBackupNameAnnotationKey: backupName, + postReadyActionNameAnnotationKey: actionName, + } + setPostReadyExecutionPolicy(jobs[i], dpv1alpha1.PostReadyExecutionPolicySerial) + } + Expect(setPostReadyTargetPlan(jobs)).Should(Succeed()) + return jobs + } + checkVolumes := func(job *batchv1.Job, volumeName string, exist bool) { var volumeExist bool for _, v := range job.Spec.Template.Spec.Volumes { @@ -717,6 +779,464 @@ var _ = Describe("RestoreManager Test", func() { Expect(*second.Spec.Suspend).Should(BeFalse()) }) + It("persists the complete plan before the first Job and reuses it after drift", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + Expect(frozen).Should(HaveLen(2)) + planName := frozen[0].Annotations[postReadyPlanNameAnnotationKey] + planDigest := frozen[0].Annotations[postReadyPlanDigestAnnotationKey] + Expect(planName).ShouldNot(BeEmpty()) + Expect(planDigest).Should(HavePrefix("sha256:")) + planSecret := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: planName, + }, planSecret)).Should(Succeed()) + Expect(planSecret.Immutable).ShouldNot(BeNil()) + Expect(*planSecret.Immutable).Should(BeTrue()) + + _, err = restoreMGR.CreateJobsIfNotExist(reqCtx, + &failNthJobCreateClient{Client: k8sClient, n: 1}, restoreMGR.Restore, frozen) + Expect(err).Should(MatchError(ContainSubstring("injected Job create failure at 1"))) + for i := range frozen { + Expect(apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, + client.ObjectKeyFromObject(frozen[i]), &batchv1.Job{}))).Should(BeTrue()) + } + + drifted := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v2") + resumed, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, drifted) + Expect(err).ShouldNot(HaveOccurred()) + Expect(resumed).Should(HaveLen(2)) + for i := range resumed { + Expect(resumed[i].Spec.Template.Spec.Containers[0].Image).Should(Equal("image:v1")) + Expect(resumed[i].Annotations[postReadyPlanNameAnnotationKey]).Should(Equal(planName)) + Expect(resumed[i].Annotations[postReadyPlanDigestAnnotationKey]).Should(Equal(planDigest)) + } + }) + + It("commits every BackupSet and action in order before creating the first Job", func() { + reqCtx := getReqCtx() + matchLabels := map[string]string{constant.AppInstanceLabelKey: testdp.ClusterName} + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName). + SetJobActionConfig(matchLabels). + SetExecActionConfig(matchLabels) + }) + testdp.NewFakeCluster(&testCtx) + + second := *backupSet + second.Backup = backupSet.Backup.DeepCopy() + second.Backup.Name += "-second" + second.Backup.Status.Path += "-second" + restoreMGR.PostReadyBackupSets = []BackupActionSet{*backupSet, second} + + found, err := restoreMGR.EnsurePostReadyStagePlan(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(found).Should(BeTrue()) + jobList := &batchv1.JobList{} + Expect(k8sClient.List(reqCtx.Ctx, jobList, + client.InNamespace(restoreMGR.Restore.Namespace), + client.MatchingLabels{DataProtectionRestoreLabelKey: restoreMGR.Restore.Name})).Should(Succeed()) + Expect(jobList.Items).Should(BeEmpty()) + + stage, found, err := restoreMGR.loadPostReadyExecutionPlanStage(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(found).Should(BeTrue()) + Expect(stage.Actions).Should(HaveLen(4)) + Expect(stage.Actions).Should(ConsistOf( + HaveField("Order", 0), HaveField("Order", 1), HaveField("Order", 2), HaveField("Order", 3))) + Expect([]string{ + postReadyActionKey(stage.Actions[0].BackupName, stage.Actions[0].ActionName), + postReadyActionKey(stage.Actions[1].BackupName, stage.Actions[1].ActionName), + postReadyActionKey(stage.Actions[2].BackupName, stage.Actions[2].ActionName), + postReadyActionKey(stage.Actions[3].BackupName, stage.Actions[3].ActionName), + }).Should(Equal([]string{ + postReadyActionKey(backupSet.Backup.Name, "postReady-0"), + postReadyActionKey(backupSet.Backup.Name, "postReady-1"), + postReadyActionKey(second.Backup.Name, "postReady-0"), + postReadyActionKey(second.Backup.Name, "postReady-1"), + })) + }) + + It("rejects a canonical subset Secret before the stage marker is committed", func() { + reqCtx := getReqCtx() + matchLabels := map[string]string{constant.AppInstanceLabelKey: testdp.ClusterName} + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) { + f.SetConnectCredential(testdp.ClusterName). + SetJobActionConfig(matchLabels). + SetExecActionConfig(matchLabels) + }) + testdp.NewFakeCluster(&testCtx) + + second := *backupSet + second.Backup = backupSet.Backup.DeepCopy() + second.Backup.Name += "-second" + second.Backup.Status.Path += "-second" + restoreMGR.PostReadyBackupSets = []BackupActionSet{*backupSet, second} + + found, err := restoreMGR.EnsurePostReadyStagePlan(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(found).Should(BeTrue()) + secretKey := types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: restoreMGR.postReadyPlanSecretName(), + } + secret := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, secret)).Should(Succeed()) + Expect(k8sClient.Delete(reqCtx.Ctx, secret)).Should(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, secretKey, &corev1.Secret{})) + }).Should(BeTrue()) + + originalRestore := restoreMGR.Restore.DeepCopy() + delete(restoreMGR.Restore.Annotations, postReadyPlanMarkerAnnotationKey) + Expect(k8sClient.Patch(reqCtx.Ctx, restoreMGR.Restore, + client.MergeFrom(originalRestore))).Should(Succeed()) + + var subset postReadyExecutionPlan + Expect(json.Unmarshal(secret.Data[postReadyPlanDataKey], &subset)).Should(Succeed()) + Expect(subset.Actions).Should(HaveLen(4)) + subset.Actions = subset.Actions[:2] + payload, err := json.Marshal(subset) + Expect(err).ShouldNot(HaveOccurred()) + forged := secret.DeepCopy() + forged.ResourceVersion = "" + forged.UID = "" + forged.CreationTimestamp = metav1.Time{} + forged.DeletionTimestamp = nil + forged.ManagedFields = nil + forged.Data[postReadyPlanDataKey] = payload + forged.Annotations[postReadyPlanDigestAnnotationKey] = postReadyPlanDigest(payload) + Expect(k8sClient.Create(reqCtx.Ctx, forged)).Should(Succeed()) + + found, err = restoreMGR.EnsurePostReadyStagePlan(reqCtx, k8sClient) + Expect(found).Should(BeFalse()) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("does not match the complete expected stage")) + _, hasMarker := restoreMGR.postReadyPlanMarker() + Expect(hasMarker).Should(BeFalse()) + }) + + It("creates no Job when the plan create response is lost", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + + _, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, + &persistSecretThenErrorClient{Client: k8sClient}, jobs) + Expect(err).Should(MatchError(ContainSubstring("injected lost Secret create response"))) + jobList := &batchv1.JobList{} + Expect(k8sClient.List(reqCtx.Ctx, jobList, client.InNamespace(restoreMGR.Restore.Namespace), + client.MatchingLabels{DataProtectionRestoreLabelKey: restoreMGR.Restore.Name})).Should(Succeed()) + Expect(jobList.Items).Should(BeEmpty()) + + resumed, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + Expect(resumed).Should(HaveLen(2)) + Expect(resumed[0].Annotations[postReadyPlanDigestAnnotationKey]).Should(HavePrefix("sha256:")) + }) + + It("keeps completed ordinals outside the immutable plan and does not recreate a GCed Job", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + planDigest := frozen[0].Annotations[postReadyPlanDigestAnnotationKey] + created, err := restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, frozen) + Expect(err).ShouldNot(HaveOccurred()) + Expect(created).Should(HaveLen(2)) + + SetRestoreStatusAction(&restoreMGR.Restore.Status.Actions.PostReady, dpv1alpha1.RestoreStatusAction{ + Name: actionName, + BackupName: backupSet.Backup.Name, + ObjectKey: BuildJobKeyForActionStatus(created[0].Name), + Status: dpv1alpha1.RestoreActionCompleted, + }) + background := metav1.DeletePropagationBackground + zero := int64(0) + Expect(k8sClient.Delete(reqCtx.Ctx, created[0], + &client.DeleteOptions{PropagationPolicy: &background, GracePeriodSeconds: &zero})).Should(Succeed()) + + loaded, err := restoreMGR.GetExistingActionJobs(reqCtx, k8sClient, dpv1alpha1.PostReady, + backupSet.Backup.Name, actionName) + Expect(err).ShouldNot(HaveOccurred()) + Expect(loaded).Should(HaveLen(2)) + Expect(loaded[0].Annotations[postReadyPlanDigestAnnotationKey]).Should(Equal(planDigest)) + pending := restoreMGR.PendingPostReadyJobs(backupSet.Backup.Name, actionName, loaded) + Expect(pending).Should(HaveLen(1)) + Expect(pending[0].Name).Should(Equal(created[1].Name)) + _, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, pending) + Expect(err).ShouldNot(HaveOccurred()) + protected := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(created[0]), protected)).Should(Succeed()) + Expect(protected.DeletionTimestamp).ShouldNot(BeNil()) + Expect(protected.Finalizers).Should(ContainElement(dptypes.DataProtectionFinalizerName)) + + By("release terminal-status protection only after the Restore fact is durable") + original := protected.DeepCopy() + controllerutil.RemoveFinalizer(protected, dptypes.DataProtectionFinalizerName) + Expect(k8sClient.Patch(reqCtx.Ctx, protected, client.MergeFrom(original))).Should(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, + client.ObjectKeyFromObject(created[0]), &batchv1.Job{})) + }).Should(BeTrue()) + + secret := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: loaded[0].Annotations[postReadyPlanNameAnnotationKey], + }, secret)).Should(Succeed()) + Expect(secret.Annotations[postReadyPlanDigestAnnotationKey]).Should(Equal(planDigest)) + }) + + It("does not replay a failed Job after garbage collection", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + _, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + SetRestoreStatusAction(&restoreMGR.Restore.Status.Actions.PostReady, dpv1alpha1.RestoreStatusAction{ + Name: actionName, + BackupName: backupSet.Backup.Name, + ObjectKey: BuildJobKeyForActionStatus(jobs[0].Name), + Status: dpv1alpha1.RestoreActionFailed, + }) + + completed, err := restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(completed).Should(BeFalse()) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("terminal failed Job")) + jobList := &batchv1.JobList{} + Expect(k8sClient.List(reqCtx.Ctx, jobList, + client.InNamespace(restoreMGR.Restore.Namespace), + client.MatchingLabels{DataProtectionRestoreLabelKey: restoreMGR.Restore.Name})).Should(Succeed()) + Expect(jobList.Items).Should(BeEmpty()) + }) + + It("re-observes a terminal Job when the Restore status patch response is lost", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1")[:1] + Expect(setPostReadyTargetPlan(jobs)).Should(Succeed()) + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + created, err := restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, frozen) + Expect(err).ShouldNot(HaveOccurred()) + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(created[0]), batchv1.JobComplete) + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(created[0]), created[0])).Should(Succeed()) + + completed, failed, err := restoreMGR.CheckJobsDone( + dpv1alpha1.PostReady, actionName, *backupSet, created) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeTrue()) + Expect(failed).Should(BeFalse()) + + originalRestore := restoreMGR.OriginalRestore.DeepCopy() + freshManager := NewRestoreManager(originalRestore, nil, restoreMGR.Schema, k8sClient) + freshJob := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(created[0]), freshJob)).Should(Succeed()) + Expect(freshJob.Finalizers).Should(ContainElement(dptypes.DataProtectionFinalizerName)) + completed, failed, err = freshManager.CheckJobsDone( + dpv1alpha1.PostReady, actionName, *backupSet, []*batchv1.Job{freshJob}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeTrue()) + Expect(failed).Should(BeFalse()) + Expect(freshManager.Restore.Status.Actions.PostReady).Should(ContainElement( + HaveField("Status", dpv1alpha1.RestoreActionCompleted))) + }) + + It("fails closed when the durable plan payload is corrupted", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + secret := &corev1.Secret{} + secretKey := types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: frozen[0].Annotations[postReadyPlanNameAnnotationKey], + } + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, secret)).Should(Succeed()) + Expect(k8sClient.Delete(reqCtx.Ctx, secret)).Should(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, secretKey, &corev1.Secret{})) + }).Should(BeTrue()) + corrupted := secret.DeepCopy() + corrupted.ResourceVersion = "" + corrupted.UID = "" + corrupted.CreationTimestamp = metav1.Time{} + corrupted.Data[postReadyPlanDataKey] = []byte(`{"version":"v1","jobs":[]}`) + Expect(k8sClient.Create(reqCtx.Ctx, corrupted)).Should(Succeed()) + + _, err = restoreMGR.GetExistingActionJobs(reqCtx, k8sClient, dpv1alpha1.PostReady, + backupSet.Backup.Name, actionName) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("invalid payload digest")) + }) + + It("rejects untrusted, foreign, empty, and partial stage plans", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + secretKey := types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: frozen[0].Annotations[postReadyPlanNameAnnotationKey], + } + base := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, base)).Should(Succeed()) + + mutatePayload := func(secret *corev1.Secret, mutate func(*postReadyExecutionPlan)) { + var plan postReadyExecutionPlan + Expect(json.Unmarshal(secret.Data[postReadyPlanDataKey], &plan)).Should(Succeed()) + mutate(&plan) + payload, err := json.Marshal(plan) + Expect(err).ShouldNot(HaveOccurred()) + secret.Data[postReadyPlanDataKey] = payload + secret.Annotations[postReadyPlanDigestAnnotationKey] = postReadyPlanDigest(payload) + } + cases := []struct { + name string + mutate func(*corev1.Secret) + }{ + {name: "wrong owner", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences = nil + }}, + {name: "same UID wrong owner name", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences[0].Name = "foreign-restore" + }}, + {name: "same UID wrong owner kind", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences[0].Kind = "Backup" + }}, + {name: "same UID wrong owner API", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences[0].APIVersion = "v1" + }}, + {name: "same UID non-controller owner", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences[0].Controller = pointer.Bool(false) + }}, + {name: "same UID non-blocking owner", mutate: func(secret *corev1.Secret) { + secret.OwnerReferences[0].BlockOwnerDeletion = pointer.Bool(false) + }}, + {name: "wrong restore UID", mutate: func(secret *corev1.Secret) { + secret.Annotations[postReadyPlanRestoreUIDAnnotationKey] = "foreign-restore" + }}, + {name: "wrong digest", mutate: func(secret *corev1.Secret) { + secret.Annotations[postReadyPlanDigestAnnotationKey] = "sha256:forged" + }}, + {name: "empty shell", mutate: func(secret *corev1.Secret) { + secret.Data[postReadyPlanDataKey] = nil + secret.Annotations[postReadyPlanDigestAnnotationKey] = postReadyPlanDigest(nil) + }}, + {name: "foreign source backup", mutate: func(secret *corev1.Secret) { + mutatePayload(secret, func(plan *postReadyExecutionPlan) { + plan.SourceBackupName = "foreign-backup" + }) + }}, + {name: "partial action zero", mutate: func(secret *corev1.Secret) { + mutatePayload(secret, func(plan *postReadyExecutionPlan) { + plan.Actions[0].Jobs = nil + }) + }}, + {name: "no actions", mutate: func(secret *corev1.Secret) { + mutatePayload(secret, func(plan *postReadyExecutionPlan) { + plan.Actions = nil + }) + }}, + } + for _, tc := range cases { + By(tc.name) + current := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, current)).Should(Succeed()) + Expect(k8sClient.Delete(reqCtx.Ctx, current)).Should(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, secretKey, &corev1.Secret{})) + }).Should(BeTrue()) + forged := base.DeepCopy() + forged.ResourceVersion = "" + forged.UID = "" + forged.CreationTimestamp = metav1.Time{} + forged.DeletionTimestamp = nil + forged.ManagedFields = nil + tc.mutate(forged) + Expect(k8sClient.Create(reqCtx.Ctx, forged)).Should(Succeed()) + _, found, err := restoreMGR.loadPostReadyExecutionPlanStage(reqCtx, k8sClient) + Expect(found).Should(BeTrue()) + Expect(err).Should(HaveOccurred()) + } + }) + + It("fails closed when a committed durable plan is missing", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + marker, ok := restoreMGR.postReadyPlanMarker() + Expect(ok).Should(BeTrue()) + Expect(marker).Should(ContainSubstring(frozen[0].Annotations[postReadyPlanNameAnnotationKey])) + secret := &corev1.Secret{} + secretKey := types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: frozen[0].Annotations[postReadyPlanNameAnnotationKey], + } + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, secret)).Should(Succeed()) + background := metav1.DeletePropagationBackground + zero := int64(0) + Expect(k8sClient.Delete(reqCtx.Ctx, secret, + &client.DeleteOptions{PropagationPolicy: &background, GracePeriodSeconds: &zero})).Should(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, secretKey, &corev1.Secret{})) + }).Should(BeTrue()) + + _, err = restoreMGR.GetExistingActionJobs(reqCtx, k8sClient, dpv1alpha1.PostReady, + backupSet.Backup.Name, actionName) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("is missing")) + }) + + It("fails closed while the committed stage plan Secret is terminating", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + secretKey := types.NamespacedName{ + Namespace: restoreMGR.Restore.Namespace, + Name: frozen[0].Annotations[postReadyPlanNameAnnotationKey], + } + secret := &corev1.Secret{} + Expect(k8sClient.Get(reqCtx.Ctx, secretKey, secret)).Should(Succeed()) + original := secret.DeepCopy() + controllerutil.AddFinalizer(secret, "dataprotection.kubeblocks.io/test-hold") + Expect(k8sClient.Patch(reqCtx.Ctx, secret, client.MergeFrom(original))).Should(Succeed()) + Expect(k8sClient.Delete(reqCtx.Ctx, secret)).Should(Succeed()) + Eventually(func(g Gomega) { + terminating := &corev1.Secret{} + g.Expect(k8sClient.Get(reqCtx.Ctx, secretKey, terminating)).Should(Succeed()) + g.Expect(terminating.DeletionTimestamp).ShouldNot(BeNil()) + }).Should(Succeed()) + + _, found, err := restoreMGR.loadPostReadyExecutionPlanStage(reqCtx, k8sClient) + Expect(found).Should(BeTrue()) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("is terminating")) + }) + It("freezes target membership and serial policy across a partial creation retry", func() { reqCtx := getReqCtx() restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) @@ -845,6 +1365,10 @@ var _ = Describe("RestoreManager Test", func() { legacyFirst.Annotations = nil legacyFirst.Spec.Suspend = nil Expect(k8sClient.Create(reqCtx.Ctx, legacyFirst)).Should(Succeed()) + testdp.PatchK8sJobStatus(&testCtx, client.ObjectKeyFromObject(legacyFirst), batchv1.JobComplete) + persistedLegacy := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(legacyFirst), persistedLegacy)).Should(Succeed()) + legacyUID := persistedLegacy.UID continued, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, desired) Expect(err).ShouldNot(HaveOccurred()) @@ -859,6 +1383,10 @@ var _ = Describe("RestoreManager Test", func() { continued, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, continued) Expect(err).ShouldNot(HaveOccurred()) Expect(continued).Should(HaveLen(2)) + Expect(continued[0].UID).Should(Equal(legacyUID)) + done, condition, _ := utils.IsJobFinished(continued[0]) + Expect(done).Should(BeTrue()) + Expect(condition).Should(Equal(batchv1.JobComplete)) }) It("rejects executable drift while recovering a legacy Parallel partial creation", func() { @@ -938,6 +1466,29 @@ var _ = Describe("RestoreManager Test", func() { } }) + It("recovers a durable orphaned plan when no Job was ever created", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-1", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + Expect(frozen).Should(HaveLen(2)) + + restoreMGR.PostReadyBackupSets = nil + restoreMGR.Restore.Spec.ReadyConfig = nil + completed, err := restoreMGR.ReconcileOrphanedPostReadyActions(reqCtx, k8sClient) + Expect(err).ShouldNot(HaveOccurred()) + Expect(completed).Should(BeFalse()) + for i := range frozen { + persisted := &batchv1.Job{} + Expect(k8sClient.Get(reqCtx.Ctx, client.ObjectKeyFromObject(frozen[i]), persisted)).Should(Succeed()) + Expect(persisted.Annotations[postReadyPlanDigestAnnotationKey]).Should( + Equal(frozen[i].Annotations[postReadyPlanDigestAnnotationKey])) + } + Expect(restoreMGR.Restore.Status.Actions.PostReady).Should(HaveLen(2)) + }) + It("recovers an orphaned frozen-plan Job written before action-contract annotations", func() { reqCtx := getReqCtx() restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) @@ -1138,6 +1689,89 @@ var _ = Describe("RestoreManager Test", func() { Expect(err.Error()).Should(ContainSubstring("does not belong to restore")) }) + It("rejects an existing Job whose executable spec differs from the durable plan", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + forged := frozen[0].DeepCopy() + forged.Spec.Template.Spec.Containers[0].Image = "image:forged" + Expect(k8sClient.Create(reqCtx.Ctx, forged)).Should(Succeed()) + + _, err = restoreMGR.CreateJobsIfNotExist(reqCtx, k8sClient, restoreMGR.Restore, frozen) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("executable spec does not match its immutable execution plan")) + Expect(apierrors.IsNotFound(k8sClient.Get(reqCtx.Ctx, + client.ObjectKeyFromObject(frozen[1]), &batchv1.Job{}))).Should(BeTrue()) + }) + + It("rejects missing committed references and executable superset drift", func() { + reqCtx := getReqCtx() + restoreMGR, backupSet := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) + actionName := fmt.Sprintf("%s-0", dpv1alpha1.PostReady) + jobs := newDurablePostReadyJobs(restoreMGR, backupSet.Backup.Name, actionName, "image:v1") + frozen, err := restoreMGR.FreezePostReadyExecutionPlan(reqCtx, k8sClient, jobs) + Expect(err).ShouldNot(HaveOccurred()) + + cases := []struct { + name string + mutate func(*batchv1.Job) + }{ + {name: "missing plan name", mutate: func(job *batchv1.Job) { + delete(job.Annotations, postReadyPlanNameAnnotationKey) + }}, + {name: "missing plan digest", mutate: func(job *batchv1.Job) { + delete(job.Annotations, postReadyPlanDigestAnnotationKey) + }}, + {name: "missing restore UID", mutate: func(job *batchv1.Job) { + delete(job.Annotations, postReadyPlanRestoreUIDAnnotationKey) + }}, + {name: "missing committed backup identity", mutate: func(job *batchv1.Job) { + delete(job.Annotations, postReadyBackupNameAnnotationKey) + }}, + {name: "missing committed action identity", mutate: func(job *batchv1.Job) { + delete(job.Annotations, postReadyActionNameAnnotationKey) + }}, + {name: "wrong backup and missing action identity", mutate: func(job *batchv1.Job) { + job.Annotations[postReadyBackupNameAnnotationKey] = "foreign-backup" + delete(job.Annotations, postReadyActionNameAnnotationKey) + }}, + {name: "missing terminal finalizer", mutate: func(job *batchv1.Job) { + controllerutil.RemoveFinalizer(job, dptypes.DataProtectionFinalizerName) + }}, + {name: "extra init container", mutate: func(job *batchv1.Job) { + job.Spec.Template.Spec.InitContainers = append(job.Spec.Template.Spec.InitContainers, + corev1.Container{Name: "forged-init", Image: "busybox:1.36"}) + }}, + {name: "extra volume", mutate: func(job *batchv1.Job) { + job.Spec.Template.Spec.Volumes = append(job.Spec.Template.Spec.Volumes, + corev1.Volume{Name: "forged-volume", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}) + }}, + {name: "extra environment", mutate: func(job *batchv1.Job) { + job.Spec.Template.Spec.Containers[0].Env = append(job.Spec.Template.Spec.Containers[0].Env, + corev1.EnvVar{Name: "FORGED", Value: "true"}) + }}, + {name: "manual selector", mutate: func(job *batchv1.Job) { + job.Spec.ManualSelector = pointer.Bool(true) + }}, + {name: "forged selector", mutate: func(job *batchv1.Job) { + job.Spec.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"forged": "true"}} + if job.Spec.Template.Labels == nil { + job.Spec.Template.Labels = map[string]string{} + } + job.Spec.Template.Labels["forged"] = "true" + }}, + } + for _, tc := range cases { + By(tc.name) + existing := frozen[0].DeepCopy() + tc.mutate(existing) + Expect(restoreMGR.validateExistingRestoreActionJob(frozen[0], existing)).Should(HaveOccurred()) + } + }) + It("keeps a fully persisted legacy postReady action on the Parallel path", func() { reqCtx := getReqCtx() restoreMGR, _ := initResources(reqCtx, 0, false, func(f *testdp.MockRestoreFactory) {}) diff --git a/pkg/dataprotection/restore/utils.go b/pkg/dataprotection/restore/utils.go index 13e503ed306..fdd4e814afc 100644 --- a/pkg/dataprotection/restore/utils.go +++ b/pkg/dataprotection/restore/utils.go @@ -240,6 +240,12 @@ func ValidateAndInitRestoreMGR(reqCtx intctrlutil.RequestCtx, if err != nil { return err } + if backupSet.UseDurablePostReadyPlan { + // Reaching a committed postReady plan proves that prepareData already + // converged. If the mutable ActionSet is later deleted, the immutable + // plan remains the sole executable source for the unfinished stage. + return nil + } // validate restore parameters if backupSet.ActionSet != nil { diff --git a/pkg/dataprotection/restore/utils_test.go b/pkg/dataprotection/restore/utils_test.go index ee42cd18d9d..f07ac6430b7 100644 --- a/pkg/dataprotection/restore/utils_test.go +++ b/pkg/dataprotection/restore/utils_test.go @@ -21,6 +21,7 @@ package restore import ( "context" + "encoding/json" "strconv" "testing" "time" @@ -30,6 +31,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -320,6 +322,154 @@ func TestValidateAndInitRestoreMGRFullBackup(t *testing.T) { assert.Error(t, ValidateAndInitRestoreMGR(reqCtx, cli, mgr)) } +func TestValidateAndInitRestoreMGRUsesPersistedPlanAfterActionSetDeletion(t *testing.T) { + scheme := runtime.NewScheme() + assert.NoError(t, dpv1alpha1.AddToScheme(scheme)) + assert.NoError(t, corev1.AddToScheme(scheme)) + assert.NoError(t, batchv1.AddToScheme(scheme)) + + backup := &dpv1alpha1.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: "backup", Namespace: "ns"}, + Status: dpv1alpha1.BackupStatus{ + Phase: dpv1alpha1.BackupPhaseCompleted, + BackupMethod: &dpv1alpha1.BackupMethod{ + Name: "full", + ActionSetName: "deleted-action-set", + }, + }, + } + restoreObj := &dpv1alpha1.Restore{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restore", + Namespace: "ns", + UID: types.UID("restore-uid"), + }, + Spec: dpv1alpha1.RestoreSpec{Backup: dpv1alpha1.BackupRef{Name: "backup", Namespace: "ns"}}, + } + mgr := NewRestoreManager(restoreObj, nil, scheme, nil) + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restore-post-ready-0", + Namespace: "ns", + Finalizers: []string{dptypes.DataProtectionFinalizerName}, + Labels: map[string]string{ + DataProtectionRestoreLabelKey: "restore", + DataProtectionRestoreNamespaceLabelKey: "ns", + }, + Annotations: map[string]string{ + postReadyTargetIdentityAnnotationKey: "ns/pod-0", + postReadyActionContractAnnotationKey: "sha256:contract", + postReadyBackupNameAnnotationKey: "backup", + postReadyActionNameAnnotationKey: "postReady-0", + }, + }, + Spec: batchv1.JobSpec{Template: corev1.PodTemplateSpec{Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{{Name: Restore, Image: "busybox:1.36"}}, + }}}, + } + setPostReadyExecutionPolicy(job, dpv1alpha1.PostReadyExecutionPolicySerial) + assert.NoError(t, setPostReadyTargetPlan([]*batchv1.Job{job})) + stage, err := canonicalPostReadyStagePlan(postReadyExecutionPlan{ + Version: postReadyPlanVersion, + RestoreNamespace: "ns", + RestoreName: "restore", + RestoreUID: "restore-uid", + SourceBackupNamespace: "ns", + SourceBackupName: "backup", + Actions: []postReadyActionExecutionPlan{{ + Order: 0, BackupName: "backup", ActionName: "postReady-0", Jobs: []batchv1.Job{*job}, + }}, + }) + assert.NoError(t, err) + payload, err := json.Marshal(stage) + assert.NoError(t, err) + digest := postReadyPlanDigest(payload) + planName := mgr.postReadyPlanSecretName() + restoreObj.Annotations = map[string]string{ + postReadyPlanMarkerAnnotationKey: postReadyPlanMarkerValue(planName, digest), + } + immutable := true + controller := true + planSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: planName, + Namespace: "ns", + Labels: map[string]string{ + DataProtectionRestoreLabelKey: "restore", + }, + Annotations: map[string]string{ + postReadyPlanRestoreUIDAnnotationKey: "restore-uid", + postReadyPlanDigestAnnotationKey: digest, + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: dpv1alpha1.SchemeGroupVersion.String(), + Kind: "Restore", + Name: "restore", + UID: types.UID("restore-uid"), + Controller: &controller, + BlockOwnerDeletion: &controller, + }}, + }, + Immutable: &immutable, + Type: postReadyPlanSecretType, + Data: map[string][]byte{postReadyPlanDataKey: payload}, + } + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(backup, planSecret).Build() + reqCtx := intctrlutil.RequestCtx{ + Ctx: context.Background(), + Req: ctrl.Request{NamespacedName: client.ObjectKey{Namespace: "ns", Name: "restore"}}, + } + mgr = NewRestoreManager(restoreObj, nil, scheme, cli) + + assert.NoError(t, ValidateAndInitRestoreMGR(reqCtx, cli, mgr)) + assert.Empty(t, mgr.PrepareDataBackupSets) + assert.Empty(t, mgr.PostReadyBackupSets) +} + +func TestValidateAndInitRestoreMGRRejectsUntrustedPlanShellAfterActionSetDeletion(t *testing.T) { + scheme := runtime.NewScheme() + assert.NoError(t, dpv1alpha1.AddToScheme(scheme)) + assert.NoError(t, corev1.AddToScheme(scheme)) + + backup := &dpv1alpha1.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: "backup", Namespace: "ns"}, + Status: dpv1alpha1.BackupStatus{ + Phase: dpv1alpha1.BackupPhaseCompleted, + BackupMethod: &dpv1alpha1.BackupMethod{ + Name: "full", ActionSetName: "deleted-action-set", + }, + }, + } + restoreObj := &dpv1alpha1.Restore{ + ObjectMeta: metav1.ObjectMeta{Name: "restore", Namespace: "ns", UID: types.UID("restore-uid")}, + Spec: dpv1alpha1.RestoreSpec{Backup: dpv1alpha1.BackupRef{Name: "backup", Namespace: "ns"}}, + } + immutable := true + foreign := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foreign-postready-plan", + Namespace: "ns", + Labels: map[string]string{DataProtectionRestoreLabelKey: "restore"}, + Annotations: map[string]string{ + postReadyPlanRestoreUIDAnnotationKey: "restore-uid", + }, + }, + Immutable: &immutable, + Type: postReadyPlanSecretType, + } + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(backup, foreign).Build() + reqCtx := intctrlutil.RequestCtx{ + Ctx: context.Background(), + Req: ctrl.Request{NamespacedName: client.ObjectKey{Namespace: "ns", Name: "restore"}}, + } + mgr := NewRestoreManager(restoreObj, nil, scheme, cli) + + err := ValidateAndInitRestoreMGR(reqCtx, cli, mgr) + assert.Error(t, err) + assert.Contains(t, err.Error(), "deleted-action-set") +} + func TestRestoreManagerStopsManagerContainer(t *testing.T) { scheme := runtime.NewScheme() assert.NoError(t, corev1.AddToScheme(scheme))