mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-07-29 19:42:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

remove VolumeSnapshotContents from resourceMustHave list

Browse Source 
Stop force-including VolumeSnapshotContents via resourceMustHave on
every restore; CSI VolumeSnapshot/PVC RestoreItemActions now set
`restore.velero.io/must-include-additional-items` so bound snapshot
dependencies are restored only when their parent is restored.

Fixes: #9957

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
This commit is contained in:
Adam Zhang
2026-07-28 14:04:15 +08:00
parent c95597720a
commit ef100da89b
9 changed files with 155 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10087-adam-jian-zhang
+1
View File
@@ -0,0 +1 @@
Stop force-including VolumeSnapshotContents via resourceMustHave on every restore; CSI VolumeSnapshot/PVC RestoreItemActions now set restore.velero.io/must-include-additional-items so bound snapshot dependencies are restored only when their parent is restored (fixes #9957)
pkg/restore/actions/csi/pvc_action.go
+9
View File
@@ -175,6 +175,15 @@ func (p *pvcRestoreItemAction) Execute(
Name: vsName,
Namespace: pvc.Namespace,
})
// Force-restore the VolumeSnapshot even when restore resource filters
// would otherwise exclude it (mirrors backup-side must-include).
annotations := pvc.GetAnnotations()
if annotations == nil {
annotations = map[string]string{}
}
annotations[velerov1api.MustIncludeAdditionalItemRestoreAnnotation] = "true"
pvc.SetAnnotations(annotations)
}
}
pkg/restore/actions/csi/pvc_action_test.go
+20 -7
View File
@@ -402,15 +402,22 @@ func TestExecute(t *testing.T) {
vs: builder.ForVolumeSnapshot("velero", vsName).ObjectMeta(
builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi"),
).Result(),
expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName")).Result(),
expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(
velerov1api.VolumeSnapshotLabel, "vsName",
velerov1api.MustIncludeAdditionalItemRestoreAnnotation, "true",
)).Result(),
},
{
name: "Restore from VolumeSnapshot without volume-snapshot-name annotation",
backup: builder.ForBackup("velero", "testBackup").Result(),
restore: builder.ForRestore("velero", "testRestore").Backup("testBackup").Result(),
pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName", AnnSelectedNode, "node1")).Result(),
vs: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi")).Result(),
expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName", AnnSelectedNode, "node1")).Result(),
name: "Restore from VolumeSnapshot without volume-snapshot-name annotation",
backup: builder.ForBackup("velero", "testBackup").Result(),
restore: builder.ForRestore("velero", "testRestore").Backup("testBackup").Result(),
pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName", AnnSelectedNode, "node1")).Result(),
vs: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi")).Result(),
expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(
velerov1api.VolumeSnapshotLabel, "vsName",
AnnSelectedNode, "node1",
velerov1api.MustIncludeAdditionalItemRestoreAnnotation, "true",
)).Result(),
},
{
name: "DataUploadResult cannot be found",
@@ -508,6 +515,12 @@ func TestExecute(t *testing.T) {
err := runtime.DefaultUnstructuredConverter.FromUnstructured(output.UpdatedItem.UnstructuredContent(), pvc)
require.NoError(t, err)
require.Equal(t, tc.expectedPVC.GetObjectMeta(), pvc.GetObjectMeta())
if tc.name == "Restore from VolumeSnapshot" {
require.Equal(t, "true", pvc.GetAnnotations()[velerov1api.MustIncludeAdditionalItemRestoreAnnotation])
require.Len(t, output.AdditionalItems, 1)
require.Equal(t, "volumesnapshots.snapshot.storage.k8s.io", output.AdditionalItems[0].GroupResource.String())
require.Equal(t, "vsName", output.AdditionalItems[0].Name)
}
if pvc.Spec.Selector != nil && pvc.Spec.Selector.MatchLabels != nil {
// This is used for long name and namespace case.
if len(tc.pvc.Namespace+"."+tc.pvc.Name) >= validation.DNS1035LabelMaxLength {
pkg/restore/actions/csi/volumesnapshot_action.go
+15 -6
View File
@@ -282,12 +282,6 @@ func (p *volumeSnapshotRestoreItemAction) Execute(
vs.Namespace, vs.Name)
}
vsMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&vs)
if err != nil {
p.log.Errorf("Fail to convert VS %s to unstructured", vs.Namespace+"/"+vs.Name)
return nil, errors.WithStack(err)
}
if vsFromBackup.Status == nil ||
vsFromBackup.Status.BoundVolumeSnapshotContentName == nil {
p.log.Errorf("VS %s doesn't have bound VSC", vsFromBackup.Name)
@@ -299,6 +293,21 @@ func (p *volumeSnapshotRestoreItemAction) Execute(
Name: *vsFromBackup.Status.BoundVolumeSnapshotContentName,
}
// Force-restore the bound VSC even when restore resource filters would
// otherwise exclude it (mirrors backup-side must-include for CSI deps).
annotations := vs.GetAnnotations()
if annotations == nil {
annotations = map[string]string{}
}
annotations[velerov1api.MustIncludeAdditionalItemRestoreAnnotation] = "true"
vs.SetAnnotations(annotations)
vsMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&vs)
if err != nil {
p.log.Errorf("Fail to convert VS %s to unstructured", vs.Namespace+"/"+vs.Name)
return nil, errors.WithStack(err)
}
p.log.Infof(`Returning from VolumeSnapshotRestoreItemAction with
VolumeSnapshotContent in additionalItems`)
pkg/restore/actions/csi/volumesnapshot_action_test.go
+4
View File
@@ -184,6 +184,10 @@ func TestVSExecute(t *testing.T) {
require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(
result.UpdatedItem.UnstructuredContent(), &vs))
require.Equal(t, test.expectedVS.Spec, vs.Spec)
require.Equal(t, "true", vs.GetAnnotations()[velerov1api.MustIncludeAdditionalItemRestoreAnnotation])
require.Len(t, result.AdditionalItems, 1)
require.Equal(t, "volumesnapshotcontents.snapshot.storage.k8s.io", result.AdditionalItems[0].GroupResource.String())
require.Equal(t, "vscName", result.AdditionalItems[0].Name)
}
})
}
pkg/restore/restore.go
-1
View File
@@ -87,7 +87,6 @@ const ObjectStatusRestoreAnnotationKey = "velero.io/restore-status"
var resourceMustHave = []string{
"datauploads.velero.io",
"volumesnapshotcontents.snapshot.storage.k8s.io",
}
type VolumeSnapshotterGetter interface {
pkg/restore/restore_test.go
+69
View File
@@ -754,6 +754,29 @@ func TestRestoreResourceFiltering(t *testing.T) {
apiResources: []*test.APIResource{test.ServiceAccounts()},
want: map[*test.APIResource][]string{test.ServiceAccounts(): {"ns-1/sa-1"}},
},
{
// Regression for #9957: VSC must not be force-included via resourceMustHave
// when the restore only selects unrelated resource types.
name: "volumesnapshotcontents are not force-included for selective resource restores",
restore: defaultRestore().IncludedResources("storageclasses").IncludeClusterResources(true).Result(),
backup: defaultBackup().Result(),
tarball: test.NewTarWriter(t).
AddItems("storageclasses.storage.k8s.io",
builder.ForStorageClass("sc-1").Result(),
).
AddItems("volumesnapshotcontents.snapshot.storage.k8s.io",
builder.ForVolumeSnapshotContent("vsc-1").Result(),
).
Done(),
apiResources: []*test.APIResource{
test.StorageClasses(),
test.VolumeSnapshotContents(),
},
want: map[*test.APIResource][]string{
test.StorageClasses(): {"/sc-1"},
test.VolumeSnapshotContents(): nil,
},
},
}
for _, tc := range tests {
@@ -2592,6 +2615,52 @@ func TestRestoreMustIncludeAdditionalItems(t *testing.T) {
test.PVCs(): nil,
})
})
t.Run("VS must-include restores excluded VolumeSnapshotContent additional item", func(t *testing.T) {
h := newHarness(t)
h.AddItems(t, test.VolumeSnapshots())
h.AddItems(t, test.VolumeSnapshotContents())
data := &Request{
Log: h.log,
Restore: defaultRestore().IncludedResources("volumesnapshots.snapshot.storage.k8s.io").IncludeClusterResources(true).Result(),
Backup: defaultBackup().Result(),
BackupReader: test.NewTarWriter(t).
AddItems("volumesnapshots.snapshot.storage.k8s.io", builder.ForVolumeSnapshot("ns-1", "vs-1").Result()).
AddItems("volumesnapshotcontents.snapshot.storage.k8s.io", builder.ForVolumeSnapshotContent("vsc-1").Result()).
Done(),
}
warnings, errs := h.restorer.Restore(
data,
[]riav2.RestoreItemAction{
&pluggableAction{
selector: velero.ResourceSelector{IncludedResources: []string{"volumesnapshots.snapshot.storage.k8s.io"}},
executeFunc: func(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) {
item := input.Item.(*unstructured.Unstructured)
annotations := item.GetAnnotations()
if annotations == nil {
annotations = map[string]string{}
}
annotations[velerov1api.MustIncludeAdditionalItemRestoreAnnotation] = "true"
item.SetAnnotations(annotations)
return &velero.RestoreItemActionExecuteOutput{
UpdatedItem: item,
AdditionalItems: []velero.ResourceIdentifier{
{GroupResource: kuberesource.VolumeSnapshotContents, Name: "vsc-1"},
},
}, nil
},
},
},
nil,
)
assertEmptyResults(t, warnings, errs)
assertAPIContents(t, h, map[*test.APIResource][]string{
test.VolumeSnapshots(): {"ns-1/vs-1"},
test.VolumeSnapshotContents(): {"/vsc-1"},
})
})
}
// TestShouldRestore runs the ShouldRestore function for various permutations of
pkg/test/api_server.go
+3
View File
@@ -58,6 +58,9 @@ func NewAPIServer(t *testing.T) *APIServer {
{Group: "velero.io", Version: "v2alpha1", Resource: "datauploads"}: "DataUploadsList",
{Group: "mygroup.io", Version: "v1", Resource: "mycustomkinds"}: "MyCustomKindList",
{Group: "mygroup.io", Version: "v1", Resource: "myclustercustomkinds"}: "MyClusterCustomKindList",
{Group: "storage.k8s.io", Version: "v1", Resource: "storageclasses"}: "StorageClassList",
{Group: "snapshot.storage.k8s.io", Version: "v1", Resource: "volumesnapshots"}: "VolumeSnapshotList",
{Group: "snapshot.storage.k8s.io", Version: "v1", Resource: "volumesnapshotcontents"}: "VolumeSnapshotContentList",
})
discoveryClient = &DiscoveryClient{FakeDiscovery: kubeClient.Discovery().(*discoveryfake.FakeDiscovery)}
)
pkg/test/resources.go
+34
View File
@@ -220,3 +220,37 @@ func DataUploads(items ...metav1.Object) *APIResource {
Items: items,
}
}
func StorageClasses(items ...metav1.Object) *APIResource {
return &APIResource{
Group: "storage.k8s.io",
Version: "v1",
Name: "storageclasses",
ShortName: "sc",
Kind: "StorageClass",
Namespaced: false,
Items: items,
}
}
func VolumeSnapshotContents(items ...metav1.Object) *APIResource {
return &APIResource{
Group: "snapshot.storage.k8s.io",
Version: "v1",
Name: "volumesnapshotcontents",
Kind: "VolumeSnapshotContent",
Namespaced: false,
Items: items,
}
}
func VolumeSnapshots(items ...metav1.Object) *APIResource {
return &APIResource{
Group: "snapshot.storage.k8s.io",
Version: "v1",
Name: "volumesnapshots",
Kind: "VolumeSnapshot",
Namespaced: true,
Items: items,
}
}