diff --git a/apis/dataprotection/v1alpha1/actionset_types.go b/apis/dataprotection/v1alpha1/actionset_types.go index 8dd1dc9455d..47af6e63903 100644 --- a/apis/dataprotection/v1alpha1/actionset_types.go +++ b/apis/dataprotection/v1alpha1/actionset_types.go @@ -174,7 +174,8 @@ type JobActionSpec struct { // runOnTargetPodNode specifies whether to run the job workload on the // target pod node. If backup container should mount the target pod's - // volume, this field should be set to true. + // volumes, this field should be set to true. otherwise the target pod's + // volumes will be ignored. // +optional // +kubebuilder:default=false RunOnTargetPodNode *bool `json:"runOnTargetPodNode,omitempty"` diff --git a/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml b/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml index 85c9df43f48..7b18d68b4f2 100644 --- a/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml +++ b/config/crd/bases/dataprotection.kubeblocks.io_actionsets.yaml @@ -76,8 +76,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup container - should mount the target pod's volume, this field should - be set to true. + should mount the target pod's volumes, this field should + be set to true. otherwise the target pod's volumes will + be ignored. type: boolean syncProgress: description: syncProgress specifies whether to sync the backup @@ -163,8 +164,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -236,8 +238,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -485,8 +488,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -518,8 +522,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup container - should mount the target pod's volume, this field should - be set to true. + should mount the target pod's volumes, this field should + be set to true. otherwise the target pod's volumes will + be ignored. type: boolean required: - command diff --git a/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml b/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml index 85c9df43f48..7b18d68b4f2 100644 --- a/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml +++ b/deploy/helm/crds/dataprotection.kubeblocks.io_actionsets.yaml @@ -76,8 +76,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup container - should mount the target pod's volume, this field should - be set to true. + should mount the target pod's volumes, this field should + be set to true. otherwise the target pod's volumes will + be ignored. type: boolean syncProgress: description: syncProgress specifies whether to sync the backup @@ -163,8 +164,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -236,8 +238,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -485,8 +488,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup - container should mount the target pod's volume, this - field should be set to true. + container should mount the target pod's volumes, this + field should be set to true. otherwise the target + pod's volumes will be ignored. type: boolean required: - command @@ -518,8 +522,9 @@ spec: default: false description: runOnTargetPodNode specifies whether to run the job workload on the target pod node. If backup container - should mount the target pod's volume, this field should - be set to true. + should mount the target pod's volumes, this field should + be set to true. otherwise the target pod's volumes will + be ignored. type: boolean required: - command diff --git a/deploy/mongodb/templates/backuppolicytemplate.yaml b/deploy/mongodb/templates/backuppolicytemplate.yaml index 1c93927fdca..c15827de782 100644 --- a/deploy/mongodb/templates/backuppolicytemplate.yaml +++ b/deploy/mongodb/templates/backuppolicytemplate.yaml @@ -11,7 +11,7 @@ spec: - componentDefRef: mongodb retentionPeriod: 7d target: - role: follower + role: secondary backupMethods: - name: datafile snapshotVolumes: false diff --git a/pkg/dataprotection/backup/request.go b/pkg/dataprotection/backup/request.go index 45294958b47..1c36da8f41b 100644 --- a/pkg/dataprotection/backup/request.go +++ b/pkg/dataprotection/backup/request.go @@ -302,27 +302,33 @@ func (r *Request) buildJobActionPodSpec(name string, job *dpv1alpha1.JobActionSp } buildVolumes := func() []corev1.Volume { - return append( - []corev1.Volume{ - { - Name: syncProgressSharedVolumeName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, + volumes := []corev1.Volume{ + { + Name: syncProgressSharedVolumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, - getVolumesByVolumeInfo(targetPod, r.BackupMethod.TargetVolumes)...) + } + // only mount the volumes when the backup pod is running on the target pod node. + if boolptr.IsSetToTrue(job.RunOnTargetPodNode) { + volumes = append(volumes, getVolumesByVolumeInfo(targetPod, r.BackupMethod.TargetVolumes)...) + } + return volumes } buildVolumeMounts := func() []corev1.VolumeMount { - return append( - []corev1.VolumeMount{ - { - Name: syncProgressSharedVolumeName, - MountPath: syncProgressSharedMountPath, - }, + volumesMount := []corev1.VolumeMount{ + { + Name: syncProgressSharedVolumeName, + MountPath: syncProgressSharedMountPath, }, - getVolumeMountsByVolumeInfo(targetPod, r.BackupMethod.TargetVolumes)...) + } + // only mount the volumes when the backup pod is running on the target pod node. + if boolptr.IsSetToTrue(job.RunOnTargetPodNode) { + volumesMount = append(volumesMount, getVolumeMountsByVolumeInfo(targetPod, r.BackupMethod.TargetVolumes)...) + } + return volumesMount } runAsUser := int64(0) diff --git a/pkg/dataprotection/restore/manager.go b/pkg/dataprotection/restore/manager.go index aca7fa43dfb..70a995202ad 100644 --- a/pkg/dataprotection/restore/manager.go +++ b/pkg/dataprotection/restore/manager.go @@ -407,16 +407,17 @@ func (r *RestoreManager) BuildPostReadyActionJobs(reqCtx intctrlutil.RequestCtx, return nil, err } targetPod := targetPodList[0] - for _, volumeMount := range jobAction.Target.VolumeMounts { - for _, volume := range targetPod.Spec.Volumes { - if volume.Name != volumeMount.Name { - continue - } - jobBuilder.addToSpecificVolumesAndMounts(&volume, &volumeMount) - } - } if boolptr.IsSetToTrue(actionSpec.Job.RunOnTargetPodNode) { jobBuilder.setNodeNameToNodeSelector(targetPod.Spec.NodeName) + // mount the targe pod's volumes when RunOnTargetPodNode is true + for _, volumeMount := range jobAction.Target.VolumeMounts { + for _, volume := range targetPod.Spec.Volumes { + if volume.Name != volumeMount.Name { + continue + } + jobBuilder.addToSpecificVolumesAndMounts(&volume, &volumeMount) + } + } } job := jobBuilder.setImage(actionSpec.Job.Image). setJobName(buildJobName(0)). diff --git a/pkg/dataprotection/restore/manager_test.go b/pkg/dataprotection/restore/manager_test.go index 96fcdac4ac2..8a2b790e181 100644 --- a/pkg/dataprotection/restore/manager_test.go +++ b/pkg/dataprotection/restore/manager_test.go @@ -163,6 +163,17 @@ var _ = Describe("Backup Deleter Test", func() { } } + checkVolumes := func(job *batchv1.Job, volumeName string, exist bool) { + var volumeExist bool + for _, v := range job.Spec.Template.Spec.Volumes { + if v.Name == volumeName { + volumeExist = true + break + } + } + Expect(volumeExist).Should(Equal(exist)) + } + It("test with RestorePVCFromSnapshot function", func() { reqCtx := getReqCtx() startingIndex := 0 @@ -269,7 +280,7 @@ var _ = Describe("Backup Deleter Test", func() { Expect(job).ShouldNot(BeNil()) }) - It("test with BuildPostReadyActionJobs function", func() { + testPostReady := func(existVolume bool) { reqCtx := getReqCtx() matchLabels := map[string]string{ constant.AppInstanceLabelKey: testdp.ClusterName, @@ -294,8 +305,21 @@ var _ = Describe("Backup Deleter Test", func() { Expect(err).ShouldNot(HaveOccurred()) // count of job should equal to 1 Expect(len(jobs)).Should(Equal(1)) + + checkVolumes(jobs[0], testdp.DataVolumeName, existVolume) + } + + It("test with BuildPostReadyActionJobs function and run target pod node", func() { + testPostReady(true) }) + It("test with BuildPostReadyActionJobs function and no need to run target pod node", func() { + Expect(testapps.ChangeObj(&testCtx, actionSet, func(set *dpv1alpha1.ActionSet) { + runTargetPodNode := false + actionSet.Spec.Restore.PostReady[1].Job.RunOnTargetPodNode = &runTargetPodNode + })).Should(Succeed()) + testPostReady(false) + }) }) }) diff --git a/pkg/testutil/dataprotection/restore_factory.go b/pkg/testutil/dataprotection/restore_factory.go index efe6f891e16..578e45ef3c2 100644 --- a/pkg/testutil/dataprotection/restore_factory.go +++ b/pkg/testutil/dataprotection/restore_factory.go @@ -162,6 +162,12 @@ func (f *MockRestoreFactory) SetJobActionConfig(matchLabels map[string]string) * PodSelector: metav1.LabelSelector{ MatchLabels: matchLabels, }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: DataVolumeName, + MountPath: DataVolumeMountPath, + }, + }, }, } return f diff --git a/test/testdata/backup/actionset.yaml b/test/testdata/backup/actionset.yaml index 446b9d2e258..1c9dcaddc80 100644 --- a/test/testdata/backup/actionset.yaml +++ b/test/testdata/backup/actionset.yaml @@ -45,6 +45,7 @@ spec: container: postgres - job: image: infracreate-registry.cn-zhangjiakou.cr.aliyuncs.com/apecloud/percona-xtrabackup:$(IMAGE_TAG) + runOnTargetPodNode: true command: - sh - -c