mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-01-07 13:55:20 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Address review feedback: remove deprecated functions

Browse Source 
Remove deprecated functions that were marked for removal per review:
- Remove GetPodsUsingPVC (replaced by GetPodsUsingPVCWithCache)
- Remove IsPVCDefaultToFSBackup (replaced by IsPVCDefaultToFSBackupWithCache)
- Remove associated tests for deprecated functions
- Add deprecation marker to NewVolumeHelperImpl
- Add deprecation marker to ShouldPerformSnapshotWithBackup

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit is contained in:
Shubham Pampattiwar
2025-12-17 10:44:15 -08:00
parent 10ef43e147
commit f592a264a6
5 changed files with 50 additions and 566 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
pkg/util/podvolume/pod_volume.go
View File

@@ -253,19 +253,8 @@ func GetVolumesToExclude(obj metav1.Object) []string {
return strings.Split(annotations[velerov1api.VolumesToExcludeAnnotation], ",")
}
// IsPVCDefaultToFSBackup checks if a PVC should default to fs-backup based on pod annotations.
// Deprecated: Use IsPVCDefaultToFSBackupWithCache for better performance.
func IsPVCDefaultToFSBackup(pvcNamespace, pvcName string, crClient crclient.Client, defaultVolumesToFsBackup bool) (bool, error) {
pods, err := GetPodsUsingPVC(pvcNamespace, pvcName, crClient)
if err != nil {
return false, errors.WithStack(err)
}
return checkPodsForFSBackup(pods, pvcName, defaultVolumesToFsBackup)
}
// IsPVCDefaultToFSBackupWithCache is the cached version of IsPVCDefaultToFSBackup.
// If cache is nil or not built, it falls back to the non-cached version.
// IsPVCDefaultToFSBackupWithCache checks if a PVC should default to fs-backup based on pod annotations.
// If cache is nil or not built, it falls back to listing pods directly.
// Note: In the main backup path, the cache is always built (via NewVolumeHelperImplWithNamespaces),
// so the fallback is only used by plugins that don't need cache optimization.
func IsPVCDefaultToFSBackupWithCache(
@@ -281,7 +270,7 @@ func IsPVCDefaultToFSBackupWithCache(
if cache != nil && cache.IsBuilt() {
pods = cache.GetPodsUsingPVC(pvcNamespace, pvcName)
} else {
pods, err = GetPodsUsingPVC(pvcNamespace, pvcName, crClient)
pods, err = getPodsUsingPVCDirect(pvcNamespace, pvcName, crClient)
if err != nil {
return false, errors.WithStack(err)
}
@@ -318,10 +307,32 @@ func getPodVolumeNameForPVC(pod corev1api.Pod, pvcName string) (string, error) {
return "", errors.Errorf("Pod %s/%s does not use PVC %s/%s", pod.Namespace, pod.Name, pod.Namespace, pvcName)
}
// GetPodsUsingPVC returns all pods in the given namespace that use the specified PVC.
// This function lists all pods in the namespace and filters them, which can be slow
// when there are many pods. Consider using GetPodsUsingPVCWithCache for better performance.
func GetPodsUsingPVC(
// GetPodsUsingPVCWithCache returns all pods that use the specified PVC.
// If cache is available and built, it uses the cache for O(1) lookup.
// Otherwise, it falls back to listing pods directly.
// Note: In the main backup path, the cache is always built (via NewVolumeHelperImplWithNamespaces),
// so the fallback is only used by plugins that don't need cache optimization.
func GetPodsUsingPVCWithCache(
pvcNamespace, pvcName string,
crClient crclient.Client,
cache *PVCPodCache,
) ([]corev1api.Pod, error) {
// Use cache if available
if cache != nil && cache.IsBuilt() {
pods := cache.GetPodsUsingPVC(pvcNamespace, pvcName)
if pods == nil {
return []corev1api.Pod{}, nil
}
return pods, nil
}
// Fall back to direct lookup (for plugins without cache)
return getPodsUsingPVCDirect(pvcNamespace, pvcName, crClient)
}
// getPodsUsingPVCDirect returns all pods in the given namespace that use the specified PVC.
// This is an internal function that lists all pods in the namespace and filters them.
func getPodsUsingPVCDirect(
pvcNamespace, pvcName string,
crClient crclient.Client,
) ([]corev1api.Pod, error) {
@@ -346,29 +357,6 @@ func GetPodsUsingPVC(
return podsUsingPVC, nil
}
// GetPodsUsingPVCWithCache returns all pods that use the specified PVC.
// If cache is available and built, it uses the cache for O(1) lookup.
// Otherwise, it falls back to the original GetPodsUsingPVC function.
// Note: In the main backup path, the cache is always built (via NewVolumeHelperImplWithNamespaces),
// so the fallback is only used by plugins that don't need cache optimization.
func GetPodsUsingPVCWithCache(
pvcNamespace, pvcName string,
crClient crclient.Client,
cache *PVCPodCache,
) ([]corev1api.Pod, error) {
// Use cache if available
if cache != nil && cache.IsBuilt() {
pods := cache.GetPodsUsingPVC(pvcNamespace, pvcName)
if pods == nil {
return []corev1api.Pod{}, nil
}
return pods, nil
}
// Fall back to direct lookup
return GetPodsUsingPVC(pvcNamespace, pvcName, crClient)
}
func GetVolumesToProcess(volumes []corev1api.Volume, volsToProcessByLegacyApproach []string) []corev1api.Volume {
volsToProcess := make([]corev1api.Volume, 0)

306
pkg/util/podvolume/pod_volume_test.go
View File

@@ -382,196 +382,6 @@ func TestGetVolumesByPod(t *testing.T) {
}
}
func TestIsPVCDefaultToFSBackup(t *testing.T) {
objs := []runtime.Object{
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod1",
Namespace: "default",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod2",
Namespace: "default",
Annotations: map[string]string{
"backup.velero.io/backup-volumes": "csi-vol1",
},
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod3",
Namespace: "default",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
EmptyDir: &corev1api.EmptyDirVolumeSource{},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "awesome-pod-1",
Namespace: "awesome-ns",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "awesome-csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "awesome-pod-2",
Namespace: "awesome-ns",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "awesome-csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod1",
Namespace: "uploader-ns",
Annotations: map[string]string{
"backup.velero.io/backup-volumes": "csi-vol1",
},
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod2",
Namespace: "uploader-ns",
Annotations: map[string]string{
"backup.velero.io/backup-volumes": "csi-vol1",
},
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
}
fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objs...)
testCases := []struct {
name string
inPVCNamespace string
inPVCName string
expectedIsFSUploaderUsed bool
defaultVolumesToFSBackup bool
}{
{
name: "2 pods using PVC, 1 pod using uploader",
inPVCNamespace: "default",
inPVCName: "csi-pvc1",
expectedIsFSUploaderUsed: true,
defaultVolumesToFSBackup: false,
},
{
name: "2 pods using PVC, 2 pods using uploader",
inPVCNamespace: "uploader-ns",
inPVCName: "csi-pvc1",
expectedIsFSUploaderUsed: true,
defaultVolumesToFSBackup: false,
},
{
name: "2 pods using PVC, 0 pods using uploader",
inPVCNamespace: "awesome-ns",
inPVCName: "awesome-csi-pvc1",
expectedIsFSUploaderUsed: false,
defaultVolumesToFSBackup: false,
},
{
name: "0 pods using PVC",
inPVCNamespace: "default",
inPVCName: "does-not-exist",
expectedIsFSUploaderUsed: false,
defaultVolumesToFSBackup: false,
},
{
name: "2 pods using PVC, using uploader by default",
inPVCNamespace: "awesome-ns",
inPVCName: "awesome-csi-pvc1",
expectedIsFSUploaderUsed: true,
defaultVolumesToFSBackup: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualIsFSUploaderUsed, _ := IsPVCDefaultToFSBackup(tc.inPVCNamespace, tc.inPVCName, fakeClient, tc.defaultVolumesToFSBackup)
assert.Equal(t, tc.expectedIsFSUploaderUsed, actualIsFSUploaderUsed)
})
}
}
func TestGetPodVolumeNameForPVC(t *testing.T) {
testCases := []struct {
name string
@@ -677,122 +487,6 @@ func TestGetPodVolumeNameForPVC(t *testing.T) {
}
}
func TestGetPodsUsingPVC(t *testing.T) {
objs := []runtime.Object{
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod1",
Namespace: "default",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod2",
Namespace: "default",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod3",
Namespace: "default",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
EmptyDir: &corev1api.EmptyDirVolumeSource{},
},
},
},
},
},
&corev1api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod1",
Namespace: "awesome-ns",
},
Spec: corev1api.PodSpec{
Volumes: []corev1api.Volume{
{
Name: "csi-vol1",
VolumeSource: corev1api.VolumeSource{
PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
ClaimName: "csi-pvc1",
},
},
},
},
},
},
}
fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objs...)
testCases := []struct {
name string
pvcNamespace string
pvcName string
expectedPodCount int
}{
{
name: "should find exactly 2 pods using the PVC",
pvcNamespace: "default",
pvcName: "csi-pvc1",
expectedPodCount: 2,
},
{
name: "should find exactly 1 pod using the PVC",
pvcNamespace: "awesome-ns",
pvcName: "csi-pvc1",
expectedPodCount: 1,
},
{
name: "should find 0 pods using the PVC",
pvcNamespace: "default",
pvcName: "unused-pvc",
expectedPodCount: 0,
},
{
name: "should find 0 pods in non-existent namespace",
pvcNamespace: "does-not-exist",
pvcName: "csi-pvc1",
expectedPodCount: 0,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualPods, err := GetPodsUsingPVC(tc.pvcNamespace, tc.pvcName, fakeClient)
require.NoErrorf(t, err, "Want error=nil; Got error=%v", err)
assert.Lenf(t, actualPods, tc.expectedPodCount, "unexpected number of pods in result; Want: %d; Got: %d", tc.expectedPodCount, len(actualPods))
})
}
}
func TestGetVolumesToProcess(t *testing.T) {
testCases := []struct {
name string