Skip to content

Commit 0084e4a

Browse files
shubham-pampattiwarshubham-pampattiwar
committed
Extend PVCAction itemblock plugin to support grouping PVCs under VolumeGroupSnapshot label
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com> Add changelog file Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com> Update VGS label key and address PR feedback Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com> update log level to debug for edge cases Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
1 parent d2c6b6b commit 0084e4a

5 files changed

Lines changed: 172 additions & 4 deletions

File tree

‎changelogs/unreleased/8944-shubham-pampattiwar‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Extend PVCAction itemblock plugin to support grouping PVCs under VGS label key

‎pkg/cmd/server/config/config.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const (
3737
defaultBackupTTL = 30 * 24 * time.Hour
3838

3939
// defaultVGSLabelKey is the default label key used to group PVCs under a VolumeGroupSnapshot
40-
defaultVGSLabelKey = "velero.io/volume-group-snapshot"
40+
defaultVGSLabelKey = "velero.io/volume-group"
4141

4242
defaultCSISnapshotTimeout = 10 * time.Minute
4343
defaultItemOperationTimeout = 4 * time.Hour
@@ -248,7 +248,7 @@ func (c *Config) BindFlags(flags *pflag.FlagSet) {
248248
flags.StringVar(&c.ProfilerAddress, "profiler-address", c.ProfilerAddress, "The address to expose the pprof profiler.")
249249
flags.DurationVar(&c.ResourceTerminatingTimeout, "terminating-resource-timeout", c.ResourceTerminatingTimeout, "How long to wait on persistent volumes and namespaces to terminate during a restore before timing out.")
250250
flags.DurationVar(&c.DefaultBackupTTL, "default-backup-ttl", c.DefaultBackupTTL, "How long to wait by default before backups can be garbage collected.")
251-
flags.StringVar(&c.DefaultVGSLabelKey, "volume-group-snapshot-label-key", c.DefaultVGSLabelKey, "Label key for grouping PVCs into VolumeGroupSnapshot. Default value is 'velero.io/volume-group-snapshot'")
251+
flags.StringVar(&c.DefaultVGSLabelKey, "volume-group-snapshot-label-key", c.DefaultVGSLabelKey, "Label key for grouping PVCs into VolumeGroupSnapshot. Default value is 'velero.io/volume-group'")
252252
flags.DurationVar(&c.RepoMaintenanceFrequency, "default-repo-maintain-frequency", c.RepoMaintenanceFrequency, "How often 'maintain' is run for backup repositories by default.")
253253
flags.DurationVar(&c.GarbageCollectionFrequency, "garbage-collection-frequency", c.GarbageCollectionFrequency, "How often garbage collection is run for expired backups.")
254254
flags.DurationVar(&c.ItemOperationSyncFrequency, "item-operation-sync-frequency", c.ItemOperationSyncFrequency, "How often to check status on backup/restore operations after backup/restore processing. Default is 10 seconds")

‎pkg/controller/backup_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ func TestPrepareBackupRequest_SetsVGSLabelKey(t *testing.T) {
484484
require.NoError(t, err)
485485
now = now.Local()
486486

487-
defaultVGSLabelKey := "velero.io/volume-group-snapshot"
487+
defaultVGSLabelKey := "velero.io/volume-group"
488488

489489
tests := []struct {
490490
name string

‎pkg/itemblock/actions/pvc_action.go‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,67 @@ func (a *PVCAction) GetRelatedItems(item runtime.Unstructured, backup *v1.Backup
105105
}
106106
}
107107

108+
// Gather groupedPVCs based on VGS label provided in the backup
109+
groupedPVCs, err := a.getGroupedPVCs(context.Background(), pvc, backup)
110+
if err != nil {
111+
return nil, err
112+
}
113+
114+
// Add the groupedPVCs to relatedItems so that they processed in a single item block
115+
relatedItems = append(relatedItems, groupedPVCs...)
116+
108117
return relatedItems, nil
109118
}
110119

111120
func (a *PVCAction) Name() string {
112-
return "PodItemBlockAction"
121+
return "PVCItemBlockAction"
122+
}
123+
124+
// getGroupedPVCs returns other PVCs in the same group based on the VGS label key in the Backup spec.
125+
func (a *PVCAction) getGroupedPVCs(ctx context.Context, pvc *corev1api.PersistentVolumeClaim, backup *v1.Backup) ([]velero.ResourceIdentifier, error) {
126+
var related []velero.ResourceIdentifier
127+
128+
vgsLabelKey := backup.Spec.VolumeGroupSnapshotLabelKey
129+
if vgsLabelKey == "" {
130+
a.log.Debug("No VolumeGroupSnapshotLabelKey provided in backup spec; skipping PVC grouping")
131+
return nil, nil
132+
}
133+
134+
groupID, ok := pvc.Labels[vgsLabelKey]
135+
if !ok || groupID == "" {
136+
// PVC does not belong to any VGS group or groupID has empty value
137+
a.log.Debug("PVC does not belong to any PVC group or group label value is empty; skipping PVC grouping")
138+
return nil, nil
139+
}
140+
141+
pvcList := new(corev1api.PersistentVolumeClaimList)
142+
if err := a.crClient.List(
143+
ctx,
144+
pvcList,
145+
crclient.InNamespace(pvc.Namespace),
146+
crclient.MatchingLabels{vgsLabelKey: groupID},
147+
); err != nil {
148+
return nil, errors.Wrapf(err, "failed to list PVCs for VGS grouping with label %s=%s in namespace %s", vgsLabelKey, groupID, pvc.Namespace)
149+
}
150+
151+
if len(pvcList.Items) <= 1 {
152+
// Only the current PVC exists in this group
153+
return nil, nil
154+
}
155+
156+
for _, groupPVC := range pvcList.Items {
157+
if groupPVC.Name == pvc.Name {
158+
continue
159+
}
160+
161+
a.log.Infof("Adding grouped PVC %s (group %s) to relatedItems for PVC %s", groupPVC.Name, groupID, pvc.Name)
162+
163+
related = append(related, velero.ResourceIdentifier{
164+
GroupResource: kuberesource.PersistentVolumeClaims,
165+
Namespace: groupPVC.Namespace,
166+
Name: groupPVC.Name,
167+
})
168+
}
169+
170+
return related, nil
113171
}

‎pkg/itemblock/actions/pvc_action_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,22 @@ func TestBackupPVAction(t *testing.T) {
124124
{GroupResource: kuberesource.Pods, Namespace: "velero", Name: "testPod2"},
125125
},
126126
},
127+
{
128+
name: "Test with PVC grouping via VGS label",
129+
pvc: builder.ForPersistentVolumeClaim("velero", "testPVC-1").ObjectMeta(builder.WithLabels("velero.io/group", "db")).VolumeName("testPV-1").Phase(corev1api.ClaimBound).Result(),
130+
pods: []*corev1api.Pod{
131+
builder.ForPod("velero", "testPod-1").
132+
Volumes(builder.ForVolume("testPV-1").PersistentVolumeClaimSource("testPVC-1").Result()).
133+
NodeName("node").
134+
Phase(corev1api.PodRunning).Result(),
135+
},
136+
expectedErr: nil,
137+
expectedRelated: []velero.ResourceIdentifier{
138+
{GroupResource: kuberesource.PersistentVolumes, Name: "testPV-1"},
139+
{GroupResource: kuberesource.Pods, Namespace: "velero", Name: "testPod-1"},
140+
{GroupResource: kuberesource.PersistentVolumeClaims, Namespace: "velero", Name: "groupedPVC"},
141+
},
142+
},
127143
}
128144

129145
backup := &v1.Backup{}
@@ -152,6 +168,12 @@ func TestBackupPVAction(t *testing.T) {
152168
require.NoError(t, crClient.Create(context.Background(), pod))
153169
}
154170

171+
if tc.name == "Test with PVC grouping via VGS label" {
172+
groupedPVC := builder.ForPersistentVolumeClaim("velero", "groupedPVC").ObjectMeta(builder.WithLabels("velero.io/group", "db")).VolumeName("groupedPV").Phase(corev1api.ClaimBound).Result()
173+
require.NoError(t, crClient.Create(context.Background(), groupedPVC))
174+
backup.Spec.VolumeGroupSnapshotLabelKey = "velero.io/group"
175+
}
176+
155177
pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.pvc)
156178
require.NoError(t, err)
157179

@@ -165,3 +187,90 @@ func TestBackupPVAction(t *testing.T) {
165187
})
166188
}
167189
}
190+
191+
// Test_getGroupedPVCs verifies the PVC grouping logic for VolumeGroupSnapshots.
192+
// This ensures only same-namespace PVCs with the same label key and value are included.
193+
func Test_getGroupedPVCs(t *testing.T) {
194+
tests := []struct {
195+
name string
196+
labelKey string
197+
groupValue string
198+
existingPVCs []*corev1api.PersistentVolumeClaim
199+
targetPVC *corev1api.PersistentVolumeClaim
200+
expectedRelated []velero.ResourceIdentifier
201+
expectError bool
202+
}{
203+
{
204+
name: "No label key in spec",
205+
labelKey: "",
206+
targetPVC: builder.ForPersistentVolumeClaim("ns", "pvc-1").Result(),
207+
expectError: false,
208+
},
209+
{
210+
name: "No group value",
211+
labelKey: "velero.io/group",
212+
groupValue: "",
213+
targetPVC: builder.ForPersistentVolumeClaim("ns", "pvc-1").Result(),
214+
expectError: false,
215+
},
216+
{
217+
name: "Target PVC does not have the label",
218+
labelKey: "velero.io/group",
219+
targetPVC: builder.ForPersistentVolumeClaim("ns", "pvc-1").Result(),
220+
expectError: false,
221+
},
222+
{
223+
name: "Target PVC has label, but no group matches",
224+
labelKey: "velero.io/group",
225+
groupValue: "group-1",
226+
targetPVC: builder.ForPersistentVolumeClaim("ns", "pvc-1").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
227+
existingPVCs: []*corev1api.PersistentVolumeClaim{
228+
builder.ForPersistentVolumeClaim("ns", "pvc-1").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
229+
},
230+
expectError: false,
231+
expectedRelated: nil,
232+
},
233+
{
234+
name: "Multiple PVCs in the same group",
235+
labelKey: "velero.io/group",
236+
groupValue: "group-1",
237+
targetPVC: builder.ForPersistentVolumeClaim("ns", "pvc-1").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
238+
existingPVCs: []*corev1api.PersistentVolumeClaim{
239+
builder.ForPersistentVolumeClaim("ns", "pvc-1").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
240+
builder.ForPersistentVolumeClaim("ns", "pvc-2").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
241+
builder.ForPersistentVolumeClaim("ns", "pvc-3").ObjectMeta(builder.WithLabels("velero.io/group", "group-1")).Result(),
242+
},
243+
expectError: false,
244+
expectedRelated: []velero.ResourceIdentifier{
245+
{GroupResource: kuberesource.PersistentVolumeClaims, Namespace: "ns", Name: "pvc-2"},
246+
{GroupResource: kuberesource.PersistentVolumeClaims, Namespace: "ns", Name: "pvc-3"},
247+
},
248+
},
249+
}
250+
251+
for _, tc := range tests {
252+
t.Run(tc.name, func(t *testing.T) {
253+
crClient := velerotest.NewFakeControllerRuntimeClient(t)
254+
for _, pvc := range tc.existingPVCs {
255+
require.NoError(t, crClient.Create(context.Background(), pvc))
256+
}
257+
258+
logger := logrus.New()
259+
a := &PVCAction{
260+
log: logger,
261+
crClient: crClient,
262+
}
263+
264+
backup := builder.ForBackup("ns", "bkp").VolumeGroupSnapshotLabelKey(tc.labelKey).Result()
265+
266+
related, err := a.getGroupedPVCs(context.Background(), tc.targetPVC, backup)
267+
if tc.expectError {
268+
require.Error(t, err)
269+
} else {
270+
require.NoError(t, err)
271+
}
272+
273+
assert.ElementsMatch(t, tc.expectedRelated, related)
274+
})
275+
}
276+
}

0 commit comments

Comments
 (0)