Skip to content

Commit 571aea1

Browse files
Joeavaikathclaude
Joeavaikath
and
claude
committed
Apply ResourceModifier rules during restore finalization for dynamic PVs
Fixes velero-io#9358. For FSB/CSI restores, PVs are dynamically provisioned after the initial restore phase. The restore finalizer patches these PVs with labels from the backup, but ResourceModifier rules were never applied during this phase — leaving users unable to modify PV labels for cross-cloud migrations or zone mismatch scenarios. Load ResourceModifier rules from the restore spec's ConfigMap reference in the finalizer controller and apply them to PVs after setting backup labels but before patching. On failure, log a warning and fall through to the original behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
1 parent 4c19509 commit 571aea1

2 files changed

Lines changed: 155 additions & 20 deletions

File tree

‎pkg/controller/restore_finalizer_controller.go‎

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controller
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223
"sync"
2324
"time"
2425

@@ -30,13 +31,16 @@ import (
3031
storagev1api "k8s.io/api/storage/v1"
3132
apierrors "k8s.io/apimachinery/pkg/api/errors"
3233
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
34+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
35+
"k8s.io/apimachinery/pkg/runtime"
3336
"k8s.io/apimachinery/pkg/runtime/schema"
3437
"k8s.io/apimachinery/pkg/util/wait"
3538
"k8s.io/utils/clock"
3639
ctrl "sigs.k8s.io/controller-runtime"
3740
"sigs.k8s.io/controller-runtime/pkg/client"
3841

3942
"github.com/vmware-tanzu/velero/internal/hook"
43+
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
4044
"github.com/vmware-tanzu/velero/internal/volume"
4145
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
4246
"github.com/vmware-tanzu/velero/pkg/constant"
@@ -167,14 +171,32 @@ func (r *restoreFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Req
167171
return ctrl.Result{}, errors.Wrap(err, "error getting itemOperationList")
168172
}
169173

174+
var resModifiers *resourcemodifiers.ResourceModifiers
175+
if restore.Spec.ResourceModifier != nil && strings.EqualFold(restore.Spec.ResourceModifier.Kind, resourcemodifiers.ConfigmapRefType) {
176+
cm := &corev1api.ConfigMap{}
177+
if err := r.crClient.Get(ctx, client.ObjectKey{Namespace: restore.Namespace, Name: restore.Spec.ResourceModifier.Name}, cm); err != nil {
178+
log.WithError(err).Warnf("failed to get resource modifier configmap %s/%s, proceeding without resource modifiers", restore.Namespace, restore.Spec.ResourceModifier.Name)
179+
} else {
180+
resModifiers, err = resourcemodifiers.GetResourceModifiersFromConfig(cm)
181+
if err != nil {
182+
log.WithError(err).Warn("failed to parse resource modifier rules, proceeding without resource modifiers")
183+
resModifiers = nil
184+
} else if err = resModifiers.Validate(); err != nil {
185+
log.WithError(err).Warn("resource modifier validation failed, proceeding without resource modifiers")
186+
resModifiers = nil
187+
}
188+
}
189+
}
190+
170191
finalizerCtx := &finalizerContext{
171-
logger: log,
172-
restore: restore,
173-
crClient: r.crClient,
174-
volumeInfo: volumeInfo,
175-
restoredPVCList: restoredPVCList,
176-
multiHookTracker: r.multiHookTracker,
177-
resourceTimeout: r.resourceTimeout,
192+
logger: log,
193+
restore: restore,
194+
crClient: r.crClient,
195+
volumeInfo: volumeInfo,
196+
restoredPVCList: restoredPVCList,
197+
multiHookTracker: r.multiHookTracker,
198+
resourceTimeout: r.resourceTimeout,
199+
resourceModifiers: resModifiers,
178200
restoreItemOperationList: restoreItemOperationList{
179201
items: restoreItemOperations,
180202
},
@@ -292,6 +314,7 @@ type finalizerContext struct {
292314
restoreItemOperationList restoreItemOperationList
293315
multiHookTracker *hook.MultiHookTracker
294316
resourceTimeout time.Duration
317+
resourceModifiers *resourcemodifiers.ResourceModifiers
295318
}
296319

297320
func (ctx *finalizerContext) execute() (results.Result, results.Result) {
@@ -423,6 +446,13 @@ func (ctx *finalizerContext) patchDynamicPVWithVolumeInfo() (errs results.Result
423446
updatedPV := pv.DeepCopy()
424447
updatedPV.Labels = volInfo.PVInfo.Labels
425448
updatedPV.Spec.PersistentVolumeReclaimPolicy = corev1api.PersistentVolumeReclaimPolicy(volInfo.PVInfo.ReclaimPolicy)
449+
450+
if ctx.resourceModifiers != nil {
451+
if err := ctx.applyResourceModifiersToPV(updatedPV, log); err != nil {
452+
log.WithError(err).Warn("failed to apply resource modifier rules to PV, patching without modifications")
453+
}
454+
}
455+
426456
if err := kubeutil.PatchResource(pv, updatedPV, ctx.crClient); err != nil {
427457
return false, err
428458
}
@@ -565,6 +595,25 @@ func needPatch(newPV *corev1api.PersistentVolume, pvInfo *volume.PVInfo) bool {
565595
return false
566596
}
567597

598+
func (ctx *finalizerContext) applyResourceModifiersToPV(pv *corev1api.PersistentVolume, log logrus.FieldLogger) error {
599+
pvMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pv)
600+
if err != nil {
601+
return errors.Wrap(err, "failed to convert PV to unstructured")
602+
}
603+
obj := &unstructured.Unstructured{Object: pvMap}
604+
obj.SetAPIVersion("v1")
605+
obj.SetKind("PersistentVolume")
606+
607+
if errList := ctx.resourceModifiers.ApplyResourceModifierRules(obj, "persistentvolumes", ctx.crClient.Scheme(), log); len(errList) > 0 {
608+
return fmt.Errorf("resource modifier rule errors: %v", errList)
609+
}
610+
611+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, pv); err != nil {
612+
return errors.Wrap(err, "failed to convert unstructured back to PV")
613+
}
614+
return nil
615+
}
616+
568617
// WaitRestoreExecHook waits for restore exec hooks to finish then update the hook execution results
569618
func (ctx *finalizerContext) WaitRestoreExecHook() (errs results.Result) {
570619
log := ctx.logger.WithField("restore", ctx.restore.Name)

‎pkg/controller/restore_finalizer_controller_test.go‎

Lines changed: 99 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
crclient "sigs.k8s.io/controller-runtime/pkg/client"
3838

3939
"github.com/vmware-tanzu/velero/internal/hook"
40+
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
4041
"github.com/vmware-tanzu/velero/internal/volume"
4142
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
4243
"github.com/vmware-tanzu/velero/pkg/builder"
@@ -221,14 +222,15 @@ func TestUpdateResult(t *testing.T) {
221222

222223
func TestPatchDynamicPVWithVolumeInfo(t *testing.T) {
223224
tests := []struct {
224-
name string
225-
volumeInfo []*volume.BackupVolumeInfo
226-
restoredPVCNames map[string]struct{}
227-
restore *velerov1api.Restore
228-
restoredPVC []*corev1api.PersistentVolumeClaim
229-
restoredPV []*corev1api.PersistentVolume
230-
expectedPatch map[string]volume.PVInfo
231-
expectedErrNum int
225+
name string
226+
volumeInfo []*volume.BackupVolumeInfo
227+
restoredPVCNames map[string]struct{}
228+
restore *velerov1api.Restore
229+
restoredPVC []*corev1api.PersistentVolumeClaim
230+
restoredPV []*corev1api.PersistentVolume
231+
resourceModifiers *resourcemodifiers.ResourceModifiers
232+
expectedPatch map[string]volume.PVInfo
233+
expectedErrNum int
232234
}{
233235
{
234236
name: "no applicable volumeInfo",
@@ -429,6 +431,89 @@ func TestPatchDynamicPVWithVolumeInfo(t *testing.T) {
429431
},
430432
expectedErrNum: 1,
431433
},
434+
{
435+
name: "resource modifier removes a label",
436+
volumeInfo: []*volume.BackupVolumeInfo{{
437+
BackupMethod: "PodVolumeBackup",
438+
PVCName: "pvc1",
439+
PVName: "pv1",
440+
PVCNamespace: "ns1",
441+
PVInfo: &volume.PVInfo{
442+
ReclaimPolicy: string(corev1api.PersistentVolumeReclaimDelete),
443+
Labels: map[string]string{"label1": "label1-val", "zone": "us-east-1a"},
444+
},
445+
}},
446+
restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore").Result(),
447+
restoredPVCNames: map[string]struct{}{"ns1/pvc1": {}},
448+
restoredPV: []*corev1api.PersistentVolume{
449+
builder.ForPersistentVolume("new-pv1").ClaimRef("ns1", "pvc1").Phase(corev1api.VolumeBound).ReclaimPolicy(corev1api.PersistentVolumeReclaimRetain).Result()},
450+
restoredPVC: []*corev1api.PersistentVolumeClaim{
451+
builder.ForPersistentVolumeClaim("ns1", "pvc1").VolumeName("new-pv1").Phase(corev1api.ClaimBound).Result(),
452+
},
453+
resourceModifiers: &resourcemodifiers.ResourceModifiers{
454+
Version: "v1",
455+
ResourceModifierRules: []resourcemodifiers.ResourceModifierRule{
456+
{
457+
Conditions: resourcemodifiers.Conditions{
458+
GroupResource: "persistentvolumes",
459+
},
460+
Patches: []resourcemodifiers.JSONPatch{
461+
{
462+
Operation: "remove",
463+
Path: "/metadata/labels/zone",
464+
},
465+
},
466+
},
467+
},
468+
},
469+
expectedPatch: map[string]volume.PVInfo{"new-pv1": {
470+
ReclaimPolicy: string(corev1api.PersistentVolumeReclaimDelete),
471+
Labels: map[string]string{"label1": "label1-val"},
472+
}},
473+
expectedErrNum: 0,
474+
},
475+
{
476+
name: "resource modifier replaces a label value",
477+
volumeInfo: []*volume.BackupVolumeInfo{{
478+
BackupMethod: "CSISnapshot",
479+
PVCName: "pvc1",
480+
PVName: "pv1",
481+
PVCNamespace: "ns1",
482+
PVInfo: &volume.PVInfo{
483+
ReclaimPolicy: string(corev1api.PersistentVolumeReclaimDelete),
484+
Labels: map[string]string{"zone": "us-east-1a"},
485+
},
486+
}},
487+
restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore").Result(),
488+
restoredPVCNames: map[string]struct{}{"ns1/pvc1": {}},
489+
restoredPV: []*corev1api.PersistentVolume{
490+
builder.ForPersistentVolume("new-pv1").ClaimRef("ns1", "pvc1").Phase(corev1api.VolumeBound).ReclaimPolicy(corev1api.PersistentVolumeReclaimRetain).Result()},
491+
restoredPVC: []*corev1api.PersistentVolumeClaim{
492+
builder.ForPersistentVolumeClaim("ns1", "pvc1").VolumeName("new-pv1").Phase(corev1api.ClaimBound).Result(),
493+
},
494+
resourceModifiers: &resourcemodifiers.ResourceModifiers{
495+
Version: "v1",
496+
ResourceModifierRules: []resourcemodifiers.ResourceModifierRule{
497+
{
498+
Conditions: resourcemodifiers.Conditions{
499+
GroupResource: "persistentvolumes",
500+
},
501+
Patches: []resourcemodifiers.JSONPatch{
502+
{
503+
Operation: "replace",
504+
Path: "/metadata/labels/zone",
505+
Value: "eastus-1",
506+
},
507+
},
508+
},
509+
},
510+
},
511+
expectedPatch: map[string]volume.PVInfo{"new-pv1": {
512+
ReclaimPolicy: string(corev1api.PersistentVolumeReclaimDelete),
513+
Labels: map[string]string{"zone": "eastus-1"},
514+
}},
515+
expectedErrNum: 0,
516+
},
432517
}
433518

434519
for _, tc := range tests {
@@ -437,11 +522,12 @@ func TestPatchDynamicPVWithVolumeInfo(t *testing.T) {
437522
logger = velerotest.NewLogger()
438523
)
439524
ctx := &finalizerContext{
440-
logger: logger,
441-
crClient: fakeClient,
442-
restore: tc.restore,
443-
restoredPVCList: tc.restoredPVCNames,
444-
volumeInfo: tc.volumeInfo,
525+
logger: logger,
526+
crClient: fakeClient,
527+
restore: tc.restore,
528+
restoredPVCList: tc.restoredPVCNames,
529+
volumeInfo: tc.volumeInfo,
530+
resourceModifiers: tc.resourceModifiers,
445531
}
446532

447533
for _, pv := range tc.restoredPV {

0 commit comments

Comments
 (0)