Skip to content

Commit 3e85bbc

Browse files
authored
fix(dataprotection): preserve OneToOne source Pod identity during scale-out restore (#10826)
1 parent 49f14a1 commit 3e85bbc

11 files changed

Lines changed: 444 additions & 48 deletions

File tree

pkg/controller/instancetemplate/ordinal.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,23 @@ func getOrdinal(podName string) (int32, error) {
6666
return int32(ordinal), nil
6767
}
6868

69+
// GetTemplateNameAndOrdinal parses the instance template name and ordinal from
70+
// a Pod name that belongs to the specified workload.
71+
func GetTemplateNameAndOrdinal(workloadName, podName string) (string, int32, error) {
72+
parentName, ordinal := parseParentNameAndOrdinal(podName)
73+
if ordinal < 0 {
74+
return "", 0, fmt.Errorf("failed to obtain pod ordinal from %q", podName)
75+
}
76+
if parentName == workloadName {
77+
return "", int32(ordinal), nil
78+
}
79+
prefix := workloadName + "-"
80+
if !strings.HasPrefix(parentName, prefix) {
81+
return "", 0, fmt.Errorf("pod %q does not belong to workload %q", podName, workloadName)
82+
}
83+
return strings.TrimPrefix(parentName, prefix), int32(ordinal), nil
84+
}
85+
6986
// parseParentNameAndOrdinal parses parent (instance template) Name and ordinal from the give instance name.
7087
// -1 will be returned if no numeric suffix contained.
7188
func parseParentNameAndOrdinal(s string) (string, int) {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
Copyright (C) 2022-2026 ApeCloud Co., Ltd
3+
4+
This file is part of KubeBlocks project
5+
6+
This program is free software: you can redistribute it and/or modify
7+
it under the terms of the GNU Affero General Public License as published by
8+
the Free Software Foundation, either version 3 of the License, or
9+
(at your option) any later version.
10+
11+
This program is distributed in the hope that it will be useful,
12+
but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
GNU Affero General Public License for more details.
15+
16+
You should have received a copy of the GNU Affero General Public License
17+
along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*/
19+
20+
package instancetemplate
21+
22+
import "testing"
23+
24+
func TestGetTemplateNameAndOrdinal(t *testing.T) {
25+
testCases := []struct {
26+
name string
27+
workloadName string
28+
podName string
29+
templateName string
30+
ordinal int32
31+
wantErr bool
32+
}{
33+
{name: "default template", workloadName: "cluster-comp", podName: "cluster-comp-3", ordinal: 3},
34+
{name: "template with dashes", workloadName: "cluster-comp", podName: "cluster-comp-b-a-3", templateName: "b-a", ordinal: 3},
35+
{name: "workload suffix is not a template", workloadName: "cluster-comp-a", podName: "cluster-comp-a-3", ordinal: 3},
36+
{name: "different workload", workloadName: "cluster-comp", podName: "other-comp-3", wantErr: true},
37+
{name: "missing ordinal", workloadName: "cluster-comp", podName: "cluster-comp-template-", wantErr: true},
38+
{name: "invalid ordinal", workloadName: "cluster-comp", podName: "cluster-comp-template-x", wantErr: true},
39+
}
40+
for _, testCase := range testCases {
41+
t.Run(testCase.name, func(t *testing.T) {
42+
templateName, ordinal, err := GetTemplateNameAndOrdinal(testCase.workloadName, testCase.podName)
43+
if testCase.wantErr {
44+
if err == nil {
45+
t.Fatal("expected an error")
46+
}
47+
return
48+
}
49+
if err != nil {
50+
t.Fatalf("GetTemplateNameAndOrdinal() error = %v", err)
51+
}
52+
if templateName != testCase.templateName || ordinal != testCase.ordinal {
53+
t.Fatalf("GetTemplateNameAndOrdinal() = %q, %d; want %q, %d", templateName, ordinal, testCase.templateName, testCase.ordinal)
54+
}
55+
})
56+
}
57+
}

pkg/controller/plan/restore.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ type RestoreManager struct {
6060
replicas int32
6161
restoreLabels map[string]string
6262
RestoreNamePrefix string
63-
SourceTargetName string
63+
// SourceTargetName overrides the source target for prepareData Restores.
64+
// It is primarily used by scale-out restoration.
65+
SourceTargetName string
6466
}
6567

6668
func NewRestoreManager(ctx context.Context,
@@ -129,16 +131,29 @@ func (r *RestoreManager) DoPrepareData(comp *component.SynthesizedComponent,
129131
// is nothing to prepareData-restore for this component. Callers must handle
130132
// the nil Restore instead of using it.
131133
func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, template *appsv1.InstanceTemplate) (*dpv1alpha1.Restore, error) {
132-
templateName := ""
133134
startingIndex := r.startingIndex
134135
if template != nil {
135-
templateName = template.Name
136136
if len(template.Ordinals.Ranges) > 0 {
137137
// todo: currently restore api does not support multiple ranges, if implement in current way it
138138
// need to use multiple restore objects
139139
startingIndex = template.Ordinals.Ranges[0].Start
140140
}
141141
}
142+
return r.buildPrepareDataRestore(comp, backupObj, template, startingIndex)
143+
}
144+
145+
// BuildPrepareDataRestoreForPod builds the Restore for one known Pod during
146+
// scale-out. The manager's startingIndex is the Pod's actual ordinal and must
147+
// not be replaced by the start of its instance template's ordinal range.
148+
func (r *RestoreManager) BuildPrepareDataRestoreForPod(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, template *appsv1.InstanceTemplate) (*dpv1alpha1.Restore, error) {
149+
return r.buildPrepareDataRestore(comp, backupObj, template, r.startingIndex)
150+
}
151+
152+
func (r *RestoreManager) buildPrepareDataRestore(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, template *appsv1.InstanceTemplate, startingIndex int32) (*dpv1alpha1.Restore, error) {
153+
templateName := ""
154+
if template != nil {
155+
templateName = template.Name
156+
}
142157
backupMethod := backupObj.Status.BackupMethod
143158
if backupMethod == nil {
144159
return nil, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRestoreFailed, `status.backupMethod of backup "%s" can not be empty`, backupObj.Name)

pkg/dataprotection/restore/manager.go

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,14 +385,10 @@ func (r *RestoreManager) RestorePVCFromSnapshot(reqCtx intctrlutil.RequestCtx, c
385385
if prepareDataConfig == nil {
386386
return nil
387387
}
388-
createPVCWithSnapshot := func(claim dpv1alpha1.RestoreVolumeClaim) error {
388+
createPVCWithSnapshot := func(claim dpv1alpha1.RestoreVolumeClaim, sourceTargetPodName string) error {
389389
if claim.VolumeSource == "" {
390390
return intctrlutil.NewFatalError(fmt.Sprintf(`claim "%s"" volumeSource can not be empty if the backup uses volume snapshot`, claim.Name))
391391
}
392-
sourceTargetPodName, err := GetSourcePodNameFromTarget(target, prepareDataConfig.RequiredPolicyForAllPodSelection, 0)
393-
if err != nil {
394-
return err
395-
}
396392
var volumeSnapshotName string
397393
if target.PodSelector.Strategy == dpv1alpha1.PodSelectionStrategyAny || sourceTargetPodName != "" {
398394
snapshotGroup := GetVolumeSnapshotsBySourcePod(backupSet.Backup, target, sourceTargetPodName)
@@ -413,7 +409,11 @@ func (r *RestoreManager) RestorePVCFromSnapshot(reqCtx intctrlutil.RequestCtx, c
413409
return r.createPVCIfNotExist(reqCtx, cli, claim.ObjectMeta, claim.VolumeClaimSpec)
414410
}
415411
for i := range prepareDataConfig.RestoreVolumeClaims {
416-
if err := createPVCWithSnapshot(prepareDataConfig.RestoreVolumeClaims[i]); err != nil {
412+
sourceTargetPodName, err := GetSourcePodNameFromTarget(target, prepareDataConfig.RequiredPolicyForAllPodSelection, 0)
413+
if err != nil {
414+
return err
415+
}
416+
if err := createPVCWithSnapshot(prepareDataConfig.RestoreVolumeClaims[i], sourceTargetPodName); err != nil {
417417
return err
418418
}
419419
}
@@ -422,13 +422,31 @@ func (r *RestoreManager) RestorePVCFromSnapshot(reqCtx intctrlutil.RequestCtx, c
422422
restoreJobReplicas := GetRestoreActionsCountForPrepareData(prepareDataConfig)
423423
for i := 0; i < restoreJobReplicas; i++ {
424424
// create pvc from claims template, build volumes and volumeMounts
425-
for _, claim := range prepareDataConfig.RestoreVolumeClaimsTemplate.Templates {
425+
for _, c := range prepareDataConfig.RestoreVolumeClaimsTemplate.Templates {
426+
// Deep-copy metadata maps so each replica gets its own pod identity.
427+
claim := *c.DeepCopy()
426428
index := i + int(claimTemplate.StartingIndex)
427429
claim.Name = fmt.Sprintf("%s-%d", claim.Name, index)
428430
// HACK: add InstanceSet related labels to the PVC,
429431
// so that it can be managed by InstanceSet
430432
addItsManagingLabels(&claim, index)
431-
if err := createPVCWithSnapshot(claim); err != nil {
433+
var sourceTargetPodName string
434+
var err error
435+
if targetPodName := claim.Labels[constant.KBAppPodNameLabelKey]; targetPodName != "" {
436+
sourceTargetPodName, err = GetSourcePodNameForTargetPod(target,
437+
prepareDataConfig.RequiredPolicyForAllPodSelection,
438+
targetPodName,
439+
claim.Labels[constant.KBAppInstanceTemplateLabelKey])
440+
} else {
441+
// Preserve positional selection for generic claims-template Restores
442+
// that do not carry a KubeBlocks target Pod identity.
443+
sourceTargetPodName, err = GetSourcePodNameFromTarget(target,
444+
prepareDataConfig.RequiredPolicyForAllPodSelection, i)
445+
}
446+
if err != nil {
447+
return err
448+
}
449+
if err := createPVCWithSnapshot(claim, sourceTargetPodName); err != nil {
432450
return err
433451
}
434452
}
@@ -541,7 +559,17 @@ func (r *RestoreManager) BuildPrepareDataJobs(reqCtx intctrlutil.RequestCtx, cli
541559
jobBuilder.addToSpecificVolumesAndMounts(volume, volumeMount)
542560
}
543561
}
544-
sourceTargetPodName, err := GetSourcePodNameFromTarget(target, prepareDataConfig.RequiredPolicyForAllPodSelection, i)
562+
var sourceTargetPodName string
563+
var err error
564+
targetPodName := jobBuilder.labels[constant.KBAppPodNameLabelKey]
565+
if claimsTemplate == nil || targetPodName == "" {
566+
sourceTargetPodName, err = GetSourcePodNameFromTarget(target, prepareDataConfig.RequiredPolicyForAllPodSelection, i)
567+
} else {
568+
sourceTargetPodName, err = GetSourcePodNameForTargetPod(target,
569+
prepareDataConfig.RequiredPolicyForAllPodSelection,
570+
targetPodName,
571+
jobBuilder.labels[constant.KBAppInstanceTemplateLabelKey])
572+
}
545573
if err != nil {
546574
return nil, err
547575
}

pkg/dataprotection/restore/manager_test.go

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -257,29 +257,72 @@ var _ = Describe("RestoreManager Test", func() {
257257

258258
It("test with RestorePVCFromSnapshot function", func() {
259259
reqCtx := getReqCtx()
260-
startingIndex := 0
260+
startingIndex := 3
261+
templateName := "az-a"
262+
cmpName := "mysql"
261263
useVolumeSnapshot := true
262264
restoreMGR, backupSet := initResources(reqCtx, startingIndex, useVolumeSnapshot, func(f *testdp.MockRestoreFactory) {
263265
f.SetVolumeClaimsTemplate(testdp.MysqlTemplateName, testdp.DataVolumeName,
264-
testdp.DataVolumeMountPath, "", int32(replicas), int32(startingIndex), nil)
266+
testdp.DataVolumeMountPath, "", int32(replicas), int32(startingIndex), map[string]string{
267+
constant.AppInstanceLabelKey: instanceName,
268+
constant.KBAppComponentLabelKey: cmpName,
269+
constant.KBAppInstanceTemplateLabelKey: templateName,
270+
}).
271+
SetPrepareDataRequiredPolicy(dpv1alpha1.OneToOneRestorePolicy, "")
265272
})
273+
backupSet.Backup.Status.Target.PodSelector.Strategy = dpv1alpha1.PodSelectionStrategyAll
274+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.AppInstanceLabelKey] = "source"
275+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.KBAppComponentLabelKey] = cmpName
276+
backupSet.Backup.Status.Target.SelectedTargetPods = []string{"source-mysql-az-a-4", "source-mysql-az-a-3"}
277+
backupSet.Backup.Status.Actions = []dpv1alpha1.ActionStatus{
278+
{
279+
TargetPodName: "source-mysql-az-a-4",
280+
VolumeSnapshots: []dpv1alpha1.VolumeSnapshotStatus{{
281+
Name: "snapshot-4",
282+
VolumeName: testdp.DataVolumeName,
283+
}},
284+
},
285+
{
286+
TargetPodName: "source-mysql-az-a-3",
287+
VolumeSnapshots: []dpv1alpha1.VolumeSnapshotStatus{{
288+
Name: "snapshot-3",
289+
VolumeName: testdp.DataVolumeName,
290+
}},
291+
},
292+
}
266293

267294
By("test RestorePVCFromSnapshot function")
268295
target := utils.GetBackupStatusTarget(backupSet.Backup, restoreMGR.Restore.Spec.Backup.SourceTargetName)
269296
Expect(restoreMGR.RestorePVCFromSnapshot(reqCtx, k8sClient, *backupSet, target)).Should(Succeed())
270297

271-
checkPVC(startingIndex, useVolumeSnapshot, "restore")
298+
checkPVC(startingIndex, useVolumeSnapshot, constant.AppName)
299+
for i := 0; i < replicas; i++ {
300+
pvc := &corev1.PersistentVolumeClaim{}
301+
Expect(k8sClient.Get(ctx, client.ObjectKey{
302+
Namespace: testCtx.DefaultNamespace,
303+
Name: fmt.Sprintf("%s-%d", testdp.MysqlTemplateName, startingIndex+i),
304+
}, pvc)).Should(Succeed())
305+
Expect(pvc.Spec.DataSource).ShouldNot(BeNil())
306+
Expect(pvc.Spec.DataSource.Name).Should(Equal(fmt.Sprintf("snapshot-%d", startingIndex+i)))
307+
}
272308
})
273309

274310
It("test with BuildPrepareDataJobs function and Parallel volumeRestorePolicy", func() {
275311
reqCtx := getReqCtx()
276-
startingIndex := 1
312+
startingIndex := 3
313+
cmpName := "mysql"
277314
restoreMGR, backupSet := initResources(reqCtx, startingIndex, false, func(f *testdp.MockRestoreFactory) {
278315
f.SetVolumeClaimsTemplate(testdp.MysqlTemplateName, testdp.DataVolumeName,
279316
testdp.DataVolumeMountPath, "", int32(replicas), int32(startingIndex), map[string]string{
280-
constant.AppInstanceLabelKey: instanceName,
281-
})
317+
constant.AppInstanceLabelKey: instanceName,
318+
constant.KBAppComponentLabelKey: cmpName,
319+
}).SetPrepareDataRequiredPolicy(dpv1alpha1.OneToOneRestorePolicy, "")
282320
})
321+
backupSet.Backup.Status.Path = "/repo/test/backup"
322+
backupSet.Backup.Status.Target.PodSelector.Strategy = dpv1alpha1.PodSelectionStrategyAll
323+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.AppInstanceLabelKey] = "source"
324+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.KBAppComponentLabelKey] = cmpName
325+
backupSet.Backup.Status.Target.SelectedTargetPods = []string{"source-mysql-4", "source-mysql-3"}
283326

284327
By(fmt.Sprintf("test BuildPrepareDataJobs function, expect for %d jobs", replicas))
285328
actionSetName := "preparedata-0"
@@ -291,8 +334,12 @@ var _ = Describe("RestoreManager Test", func() {
291334
Expect(len(jobs)).Should(Equal(replicas))
292335
// image should be expanded by env
293336
Expect(jobs[0].Spec.Template.Spec.Containers[0].Image).Should(ContainSubstring(testdp.ImageTag))
337+
for i := 0; i < replicas; i++ {
338+
env := utils.CovertEnvToMap(jobs[i].Spec.Template.Spec.Containers[0].Env)
339+
Expect(env[dptypes.DPTargetRelativePath]).Should(Equal(fmt.Sprintf("source-mysql-%d", startingIndex+i)))
340+
}
294341

295-
checkPVC(startingIndex, false, "restore")
342+
checkPVC(startingIndex, false, constant.AppName)
296343
})
297344

298345
It("test with BuildPrepareDataJobs function with InstanceTemplates claims", func() {
@@ -306,8 +353,18 @@ var _ = Describe("RestoreManager Test", func() {
306353
constant.AppInstanceLabelKey: instanceName,
307354
constant.KBAppComponentLabelKey: cmpName,
308355
constant.KBAppInstanceTemplateLabelKey: templateName,
309-
})
356+
}).SetPrepareDataRequiredPolicy(dpv1alpha1.OneToOneRestorePolicy, "")
310357
})
358+
backupSet.Backup.Status.Path = "/repo/test/backup"
359+
backupSet.Backup.Status.Target.PodSelector.Strategy = dpv1alpha1.PodSelectionStrategyAll
360+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.AppInstanceLabelKey] = "source"
361+
backupSet.Backup.Status.Target.PodSelector.MatchLabels[constant.KBAppComponentLabelKey] = cmpName
362+
backupSet.Backup.Status.Target.SelectedTargetPods = []string{
363+
"source-mysql-other-301",
364+
"source-mysql-abc-301",
365+
"source-mysql-other-300",
366+
"source-mysql-abc-300",
367+
}
311368
By(fmt.Sprintf("test BuildPrepareDataJobs function, expect job label pod name contains template '%s' and ordinal correct", templateName))
312369
actionSetName := "preparedata-0"
313370
target := utils.GetBackupStatusTarget(backupSet.Backup, restoreMGR.Restore.Spec.Backup.SourceTargetName)
@@ -317,6 +374,8 @@ var _ = Describe("RestoreManager Test", func() {
317374
// job label contains pod name and ordinal match
318375
for i := 0; i < replicas; i++ {
319376
Expect(jobs[i].Spec.Template.Labels[constant.KBAppPodNameLabelKey]).Should(Equal(fmt.Sprintf("%s-%s-%s-%d", instanceName, cmpName, templateName, startingIndex+i)))
377+
env := utils.CovertEnvToMap(jobs[i].Spec.Template.Spec.Containers[0].Env)
378+
Expect(env[dptypes.DPTargetRelativePath]).Should(Equal(fmt.Sprintf("source-mysql-%s-%d", templateName, startingIndex+i)))
320379
}
321380

322381
checkPVC(startingIndex, false, constant.AppName)

0 commit comments

Comments
 (0)