Support skipped PVC in VolumeInfos.

* Modify according to comments.
* Rename the pvSkipTracker and related fields to indicate both PVC and PV are supported.

Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>

changelogs/unreleased/10525-blackpiglet

@@ -0,0 +1 @@
+Support skipped PVC in VolumeInfos.

design/Implemented/pv_backup_info.md

@@ -107,7 +107,7 @@ type PVInfo struct {
 
 ### How the VolumeInfo array is generated.
 The function `persistBackup` has `backup *pkgbackup.Request` in parameters.
-From it, the `VolumeSnapshots`, `PodVolumeBackups`, `CSISnapshots`, `itemOperationsList`, and `SkippedPVTracker` can be read. All of them will be iterated and merged into the `VolumeInfo` array, and then persisted into backup repository in function `persistBackup`.
+From it, the `VolumeSnapshots`, `PodVolumeBackups`, `CSISnapshots`, `itemOperationsList`, and `SkippedVolumeTracker` can be read. All of them will be iterated and merged into the `VolumeInfo` array, and then persisted into backup repository in function `persistBackup`.
 
 Please notice that the change happened in async operations are not reflected in the new metadata file. The file only covers the volume changes happen in the Velero server process scope.
 

internal/volume/volumes_information.go

@@ -401,6 +401,13 @@ func newPVInfo(pv *corev1api.PersistentVolume) *PVInfo {
 	return info
 }
 
+type SkippedVolume struct {
+	PVName       string
+	PVCName      string
+	PVCNamespace string
+	Reasons      string
+}
+
 // BackupVolumesInformation contains the information needs by generating
 // the backup BackupVolumeInfo array.
 type BackupVolumesInformation struct {
@@ -413,7 +420,7 @@ type BackupVolumesInformation struct {
 	volumeSnapshots        []snapshotv1api.VolumeSnapshot
 	volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent
 	volumeSnapshotClasses  []snapshotv1api.VolumeSnapshotClass
-	SkippedPVs             map[string]string
+	SkippedVolumes         []SkippedVolume
 	NativeSnapshots        []*Snapshot
 	PodVolumeBackups       []*velerov1api.PodVolumeBackup
 	BackupOperations       []*itemoperation.BackupOperation
@@ -453,7 +460,7 @@ func (v *BackupVolumesInformation) Result(
 	v.volumeSnapshotContents = csiVolumeSnapshotContents
 	v.volumeSnapshotClasses = csiVolumesnapshotClasses
 
-	v.generateVolumeInfoForSkippedPV()
+	v.generateVolumeInfoForSkippedVolume()
 	v.generateVolumeInfoForVeleroNativeSnapshot()
 	v.generateVolumeInfoForCSIVolumeSnapshot()
 	v.generateVolumeInfoFromPVB()
@@ -462,26 +469,35 @@ func (v *BackupVolumesInformation) Result(
 	return v.volumeInfos
 }
 
-// generateVolumeInfoForSkippedPV generate VolumeInfos for SkippedPV.
-func (v *BackupVolumesInformation) generateVolumeInfoForSkippedPV() {
+// generateVolumeInfoForSkippedVolume generate VolumeInfos for SkippedVolume.
+func (v *BackupVolumesInformation) generateVolumeInfoForSkippedVolume() {
 	tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
 
-	for pvName, skippedReason := range v.SkippedPVs {
-		if pvcPVInfo := v.pvMap.retrieve(pvName, "", ""); pvcPVInfo != nil {
-			volumeInfo := &BackupVolumeInfo{
+	for _, skippedVolume := range v.SkippedVolumes {
+		var volumeInfo *BackupVolumeInfo
+		if pvcPVInfo := v.pvMap.retrieve(skippedVolume.PVName, skippedVolume.PVCName, skippedVolume.PVCNamespace); pvcPVInfo != nil {
+			volumeInfo = &BackupVolumeInfo{
 				PVCName:           pvcPVInfo.PVCName,
 				PVCNamespace:      pvcPVInfo.PVCNamespace,
-				PVName:            pvName,
+				PVName:            pvcPVInfo.PV.Name,
 				SnapshotDataMoved: false,
 				Skipped:           true,
-				SkippedReason:     skippedReason,
+				SkippedReason:     skippedVolume.Reasons,
 				PVInfo:            newPVInfo(&pvcPVInfo.PV),
 			}
-			tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
 		} else {
-			v.logger.Warnf("Cannot find info for PV %s", pvName)
-			continue
+			// If we cannot find it in pvMap, it might be a PVC without PV.
+			volumeInfo = &BackupVolumeInfo{
+				PVCName:           skippedVolume.PVCName,
+				PVCNamespace:      skippedVolume.PVCNamespace,
+				PVName:            skippedVolume.PVName,
+				SnapshotDataMoved: false,
+				Skipped:           true,
+				SkippedReason:     skippedVolume.Reasons,
+			}
 		}
+
+		tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
 	}
 
 	v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)

internal/volume/volumes_information_test.go

@@ -44,16 +44,16 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/util/logging"
 )
 
-func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
+func TestGenerateVolumeInfoForSkippedVolume(t *testing.T) {
 	tests := []struct {
 		name                string
-		skippedPVName       string
+		skippedVolumeName   string
 		pvMap               map[string]pvcPvInfo
 		expectedVolumeInfos []*BackupVolumeInfo
 	}{
 		{
-			name:          "Cannot find info for PV",
-			skippedPVName: "testPV",
+			name:              "Cannot find info for PV",
+			skippedVolumeName: "testPV",
 			pvMap: map[string]pvcPvInfo{
 				"velero/testPVC": {
 					PVCName:      "testPVC",
@@ -69,11 +69,17 @@ func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
 					},
 				},
 			},
-			expectedVolumeInfos: []*BackupVolumeInfo{},
+			expectedVolumeInfos: []*BackupVolumeInfo{
+				{
+					PVName:        "testPV",
+					Skipped:       true,
+					SkippedReason: "CSI: skipped for PodVolumeBackup",
+				},
+			},
 		},
 		{
-			name:          "Normal Skipped PV info",
-			skippedPVName: "testPV",
+			name:              "Normal Skipped Volume info",
+			skippedVolumeName: "testPV",
 			pvMap: map[string]pvcPvInfo{
 				"velero/testPVC": {
 					PVCName:      "testPVC",
@@ -125,9 +131,12 @@ func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
 			volumesInfo := BackupVolumesInformation{}
 			volumesInfo.Init()
 
-			if tc.skippedPVName != "" {
-				volumesInfo.SkippedPVs = map[string]string{
-					tc.skippedPVName: "CSI: skipped for PodVolumeBackup",
+			if tc.skippedVolumeName != "" {
+				volumesInfo.SkippedVolumes = []SkippedVolume{
+					{
+						PVName:  tc.skippedVolumeName,
+						Reasons: "CSI: skipped for PodVolumeBackup",
+					},
 				}
 			}
 
@@ -140,7 +149,7 @@ func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
 			}
 			volumesInfo.logger = logging.DefaultLogger(logrus.DebugLevel, logging.FormatJSON)
 
-			volumesInfo.generateVolumeInfoForSkippedPV()
+			volumesInfo.generateVolumeInfoForSkippedVolume()
 			require.Equal(t, tc.expectedVolumeInfos, volumesInfo.volumeInfos)
 		})
 	}

pkg/backup/backup.go

@@ -746,10 +746,10 @@ func (kb *kubernetesBackupper) BackupWithResolvers(
 		log.WithError(errors.WithStack((err))).Warn("Got error trying to update backup's status.progress and hook status")
 	}
 
-	if skippedPVSummary, err := json.Marshal(backupRequest.SkippedPVTracker.Summary()); err != nil {
-		log.WithError(errors.WithStack(err)).Warn("Fail to generate skipped PV summary.")
+	if skippedVolumeSummary, err := json.Marshal(backupRequest.SkippedVolumeTracker.Summary()); err != nil {
+		log.WithError(errors.WithStack(err)).Warn("Fail to generate skipped volume summary.")
 	} else {
-		log.Infof("Summary for skipped PVs: %s", skippedPVSummary)
+		log.Infof("Summary for skipped volumes: %s", skippedVolumeSummary)
 	}
 
 	backupRequest.Status.Progress = &velerov1api.BackupProgress{TotalItems: backedUpItems, ItemsBackedUp: backedUpItems}

pkg/backup/backup_test.go

@@ -80,10 +80,10 @@ func TestBackedUpItemsMatchesTarballContents(t *testing.T) {
 	defer h.itemBlockPool.Stop()
 
 	req := &Request{
-		Backup:           defaultBackup().Result(),
-		SkippedPVTracker: NewSkipPVTracker(),
-		BackedUpItems:    NewBackedUpItemsMap(),
-		WorkerPool:       &h.itemBlockPool,
+		Backup:               defaultBackup().Result(),
+		SkippedVolumeTracker: NewSkipVolumeTracker(),
+		BackedUpItems:        NewBackedUpItemsMap(),
+		WorkerPool:           &h.itemBlockPool,
 	}
 
 	backupFile := bytes.NewBuffer([]byte{})
@@ -142,10 +142,10 @@ func TestBackupProgressIsUpdated(t *testing.T) {
 	h := newHarness(t, nil)
 	defer h.itemBlockPool.Stop()
 	req := &Request{
-		Backup:           defaultBackup().Result(),
-		SkippedPVTracker: NewSkipPVTracker(),
-		BackedUpItems:    NewBackedUpItemsMap(),
-		WorkerPool:       &h.itemBlockPool,
+		Backup:               defaultBackup().Result(),
+		SkippedVolumeTracker: NewSkipVolumeTracker(),
+		BackedUpItems:        NewBackedUpItemsMap(),
+		WorkerPool:           &h.itemBlockPool,
 	}
 	backupFile := bytes.NewBuffer([]byte{})
 
@@ -882,10 +882,10 @@ func TestBackupOldResourceFiltering(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1063,10 +1063,10 @@ func TestCRDInclusion(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1162,10 +1162,10 @@ func TestBackupResourceCohabitation(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1191,10 +1191,10 @@ func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) {
 
 	// run and verify backup 1
 	backup1 := &Request{
-		Backup:           defaultBackup().Result(),
-		SkippedPVTracker: NewSkipPVTracker(),
-		BackedUpItems:    NewBackedUpItemsMap(),
-		WorkerPool:       &h.itemBlockPool,
+		Backup:               defaultBackup().Result(),
+		SkippedVolumeTracker: NewSkipVolumeTracker(),
+		BackedUpItems:        NewBackedUpItemsMap(),
+		WorkerPool:           &h.itemBlockPool,
 	}
 	backup1File := bytes.NewBuffer([]byte{})
 
@@ -1207,10 +1207,10 @@ func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) {
 
 	// run and verify backup 2
 	backup2 := &Request{
-		Backup:           defaultBackup().Result(),
-		SkippedPVTracker: NewSkipPVTracker(),
-		BackedUpItems:    NewBackedUpItemsMap(),
-		WorkerPool:       &h.itemBlockPool,
+		Backup:               defaultBackup().Result(),
+		SkippedVolumeTracker: NewSkipVolumeTracker(),
+		BackedUpItems:        NewBackedUpItemsMap(),
+		WorkerPool:           &h.itemBlockPool,
 	}
 	backup2File := bytes.NewBuffer([]byte{})
 
@@ -1261,10 +1261,10 @@ func TestBackupResourceOrdering(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1362,9 +1362,9 @@ func (a *recordResourcesAction) WithSkippedCSISnapshotFlag(flag bool) *recordRes
 	return a
 }
 
-// TestBackupItemActionsForSkippedPV runs backups with backup item actions, and
-// verifies that the data in SkippedPVTracker is updated as expected.
-func TestBackupItemActionsForSkippedPV(t *testing.T) {
+// TestBackupItemActionsForSkippedVolume runs backups with backup item actions, and
+// verifies that the data in SkippedVolumeTracker is updated as expected.
+func TestBackupItemActionsForSkippedVolume(t *testing.T) {
 	itemBlockPool := StartItemBlockWorkerPool(t.Context(), 1, logrus.StandardLogger())
 	defer itemBlockPool.Stop()
 
@@ -1376,16 +1376,16 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
 		actions          []*recordResourcesAction
 		resPolicies      *resourcepolicies.ResourcePolicies
 		// {pvName:{approach: reason}}
-		expectSkippedPVs    map[string]map[string]string
-		expectNotSkippedPVs []string
+		expectSkippedVolumes    map[string]map[string]string
+		expectNotSkippedVolumes []string
 	}{
 		{
 			name: "backup item action returns the 'not a CSI volume' error and the PV should be tracked as skippedPV",
 			backupReq: &Request{
-				Backup:           defaultBackup().SnapshotVolumes(false).Result(),
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				Backup:               defaultBackup().SnapshotVolumes(false).Result(),
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			resPolicies: &resourcepolicies.ResourcePolicies{
 				Version: "v1",
@@ -1413,24 +1413,29 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
 			actions: []*recordResourcesAction{
 				new(recordResourcesAction).WithName(csiBIAPluginName).ForNamespace("ns-1").ForResource("persistentvolumeclaims").WithSkippedCSISnapshotFlag(true),
 			},
-			expectSkippedPVs: map[string]map[string]string{
+			expectSkippedVolumes: map[string]map[string]string{
 				"pv-1": {
 					csiSnapshotApproach: "skipped b/c it's not a CSI volume",
 				},
 			},
 		},
 		{
-			name: "backup item action named as CSI plugin executed successfully and the PV will be removed from the skipped PV tracker",
+			name: "backup item action named as CSI plugin executed successfully and the PV will be removed from the skipped Volume tracker",
 			backupReq: &Request{
 				Backup: defaultBackup().Result(),
-				SkippedPVTracker: &skipPVTracker{
+				SkippedVolumeTracker: &skipVolumeTracker{
 					RWMutex: &sync.RWMutex{},
-					pvs: map[string]map[string]string{
-						"pv-1": {
+					volumes: map[string]map[string]string{
+						"pv:pv-1": {
 							"any": "whatever reason",
 						},
 					},
-					includedPVs: map[string]struct{}{},
+					includedVolumes: map[string]struct{}{},
+					volumeInfo: map[string]SkippedVolume{
+						"pv:pv-1": {
+							PVName: "pv-1",
+						},
+					},
 				},
 				BackedUpItems: NewBackedUpItemsMap(),
 				WorkerPool:    itemBlockPool,
@@ -1447,7 +1452,7 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
 			actions: []*recordResourcesAction{
 				new(recordResourcesAction).ForNamespace("ns-1").ForResource("persistentvolumeclaims").WithName(csiBIAPluginName),
 			},
-			expectNotSkippedPVs: []string{"pv-1"},
+			expectNotSkippedVolumes: []string{"pv-1"},
 		},
 	}
 	// Enable CSI feature before running the test, because Velero will check whether
@@ -1482,17 +1487,17 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
 			err := h.backupper.Backup(h.log, tc.backupReq, backupFile, actions, nil, nil)
 			require.NoError(t, err)
 
-			if tc.expectSkippedPVs != nil {
-				for pvName, reasons := range tc.expectSkippedPVs {
-					v, ok := tc.backupReq.SkippedPVTracker.pvs[pvName]
+			if tc.expectSkippedVolumes != nil {
+				for pvName, reasons := range tc.expectSkippedVolumes {
+					v, ok := tc.backupReq.SkippedVolumeTracker.volumes["pv:"+pvName]
 					assert.True(tt, ok)
 					for approach, reason := range reasons {
 						assert.Equal(tt, reason, v[approach])
 					}
 				}
 			}
-			for _, pvName := range tc.expectNotSkippedPVs {
-				_, ok := tc.backupReq.SkippedPVTracker.pvs[pvName]
+			for _, pvName := range tc.expectNotSkippedVolumes {
+				_, ok := tc.backupReq.SkippedVolumeTracker.volumes["pv:"+pvName]
 				assert.False(tt, ok)
 			}
 		})
@@ -1680,10 +1685,10 @@ func TestBackupActionsRunForCorrectItems(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1765,10 +1770,10 @@ func TestBackupWithInvalidActions(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -1919,10 +1924,10 @@ func TestBackupActionModifications(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -2179,10 +2184,10 @@ func TestBackupActionAdditionalItems(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -2440,10 +2445,10 @@ func TestItemBlockActionsRunForCorrectItems(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -2525,10 +2530,10 @@ func TestBackupWithInvalidItemBlockActions(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -2781,10 +2786,10 @@ func TestItemBlockActionRelatedItems(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -2935,13 +2940,13 @@ func TestBackupWithSnapshots(t *testing.T) {
 	itemBlockPool := StartItemBlockWorkerPool(t.Context(), 1, logrus.StandardLogger())
 	defer itemBlockPool.Stop()
 	tests := []struct {
-		name              string
-		req               *Request
-		vsls              []*velerov1.VolumeSnapshotLocation
-		apiResources      []*test.APIResource
-		snapshotterGetter volumeSnapshotterGetter
-		want              []*volume.Snapshot
-		wantSkippedPVs    []SkippedPV
+		name               string
+		req                *Request
+		vsls               []*velerov1.VolumeSnapshotLocation
+		apiResources       []*test.APIResource
+		snapshotterGetter  volumeSnapshotterGetter
+		want               []*volume.Snapshot
+		wantSkippedVolumes []SkippedVolume
 	}{
 		{
 			name: "persistent volume with no zone annotation creates a snapshot",
@@ -2950,9 +2955,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -2978,7 +2983,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 		{
 			name: "persistent volume with deprecated zone annotation creates a snapshot",
@@ -2987,9 +2992,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3016,7 +3021,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 		{
 			name: "persistent volume with GA zone annotation creates a snapshot",
@@ -3025,9 +3030,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3054,7 +3059,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 		{
 			name: "persistent volume with both GA and deprecated zone annotation creates a snapshot and should use the GA",
@@ -3063,9 +3068,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3092,7 +3097,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 		{
 			name: "error returned from CreateSnapshot results in a failed snapshot",
@@ -3101,9 +3106,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3128,7 +3133,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 		{
 			name: "backup with SnapshotVolumes=false does not create any snapshots",
@@ -3137,9 +3142,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3150,10 +3155,10 @@ func TestBackupWithSnapshots(t *testing.T) {
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 			},
 			want: nil,
-			wantSkippedPVs: []SkippedPV{
+			wantSkippedVolumes: []SkippedVolume{
 				{
-					Name: "pv-1",
-					Reasons: []PVSkipReason{
+					PVName: "pv-1",
+					Reasons: []Reason{
 						{
 							Approach: volumeSnapshotApproach,
 							Reason:   "not satisfy the criteria for VolumePolicy or the legacy snapshot way",
@@ -3165,10 +3170,10 @@ func TestBackupWithSnapshots(t *testing.T) {
 		{
 			name: "backup with no volume snapshot locations does not create any snapshots",
 			req: &Request{
-				Backup:           defaultBackup().Result(),
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				Backup:               defaultBackup().Result(),
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3179,10 +3184,10 @@ func TestBackupWithSnapshots(t *testing.T) {
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 			},
 			want: nil,
-			wantSkippedPVs: []SkippedPV{
+			wantSkippedVolumes: []SkippedVolume{
 				{
-					Name: "pv-1",
-					Reasons: []PVSkipReason{
+					PVName: "pv-1",
+					Reasons: []Reason{
 						{
 							Approach: volumeSnapshotApproach,
 							Reason:   "no applicable volumesnapshotter found",
@@ -3198,9 +3203,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3209,10 +3214,10 @@ func TestBackupWithSnapshots(t *testing.T) {
 			},
 			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{},
 			want:              nil,
-			wantSkippedPVs: []SkippedPV{
+			wantSkippedVolumes: []SkippedVolume{
 				{
-					Name: "pv-1",
-					Reasons: []PVSkipReason{
+					PVName: "pv-1",
+					Reasons: []Reason{
 						{
 							Approach: volumeSnapshotApproach,
 							Reason:   "no applicable volumesnapshotter found",
@@ -3228,9 +3233,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
 					newSnapshotLocation("velero", "default", "default"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3241,10 +3246,10 @@ func TestBackupWithSnapshots(t *testing.T) {
 				"default": new(fakeVolumeSnapshotter),
 			},
 			want: nil,
-			wantSkippedPVs: []SkippedPV{
+			wantSkippedVolumes: []SkippedVolume{
 				{
-					Name: "pv-1",
-					Reasons: []PVSkipReason{
+					PVName: "pv-1",
+					Reasons: []Reason{
 						{
 							Approach: volumeSnapshotApproach,
 							Reason:   "no applicable volumesnapshotter found",
@@ -3261,9 +3266,9 @@ func TestBackupWithSnapshots(t *testing.T) {
 					newSnapshotLocation("velero", "default", "default"),
 					newSnapshotLocation("velero", "another", "another"),
 				},
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.PVs(
@@ -3305,7 +3310,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					},
 				},
 			},
-			wantSkippedPVs: []SkippedPV{},
+			wantSkippedVolumes: []SkippedVolume{},
 		},
 	}
 
@@ -3324,7 +3329,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.want, tc.req.VolumeSnapshots.Get())
-			assert.Equal(t, tc.wantSkippedPVs, tc.req.SkippedPVTracker.Summary())
+			assert.Equal(t, tc.wantSkippedVolumes, tc.req.SkippedVolumeTracker.Summary())
 		})
 	}
 }
@@ -3396,10 +3401,10 @@ func TestBackupWithAsyncOperations(t *testing.T) {
 		{
 			name: "action that starts a short-running process records operation",
 			req: &Request{
-				Backup:           defaultBackup().Result(),
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				Backup:               defaultBackup().Result(),
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.Pods(
@@ -3428,10 +3433,10 @@ func TestBackupWithAsyncOperations(t *testing.T) {
 		{
 			name: "action that starts a long-running process records operation",
 			req: &Request{
-				Backup:           defaultBackup().Result(),
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				Backup:               defaultBackup().Result(),
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.Pods(
@@ -3460,10 +3465,10 @@ func TestBackupWithAsyncOperations(t *testing.T) {
 		{
 			name: "action that has no operation doesn't record one",
 			req: &Request{
-				Backup:           defaultBackup().Result(),
-				SkippedPVTracker: NewSkipPVTracker(),
-				BackedUpItems:    NewBackedUpItemsMap(),
-				WorkerPool:       itemBlockPool,
+				Backup:               defaultBackup().Result(),
+				SkippedVolumeTracker: NewSkipVolumeTracker(),
+				BackedUpItems:        NewBackedUpItemsMap(),
+				WorkerPool:           itemBlockPool,
 			},
 			apiResources: []*test.APIResource{
 				test.Pods(
@@ -3546,10 +3551,10 @@ func TestBackupWithInvalidHooks(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -4020,10 +4025,10 @@ func TestBackupWithHooks(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile         = bytes.NewBuffer([]byte{})
 				podCommandExecutor = new(test.MockPodCommandExecutor)
@@ -4244,11 +4249,11 @@ func TestBackupWithPodVolume(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:            tc.backup,
-					SnapshotLocations: []*velerov1.VolumeSnapshotLocation{tc.vsl},
-					SkippedPVTracker:  NewSkipPVTracker(),
-					BackedUpItems:     NewBackedUpItemsMap(),
-					WorkerPool:        itemBlockPool,
+					Backup:               tc.backup,
+					SnapshotLocations:    []*velerov1.VolumeSnapshotLocation{tc.vsl},
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -5364,10 +5369,10 @@ func TestBackupNewResourceFiltering(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -5552,10 +5557,10 @@ func TestBackupNamespaces(t *testing.T) {
 			var (
 				h   = newHarness(t, itemBlockPool)
 				req = &Request{
-					Backup:           tc.backup,
-					SkippedPVTracker: NewSkipPVTracker(),
-					BackedUpItems:    NewBackedUpItemsMap(),
-					WorkerPool:       itemBlockPool,
+					Backup:               tc.backup,
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
+					BackedUpItems:        NewBackedUpItemsMap(),
+					WorkerPool:           itemBlockPool,
 				}
 				backupFile = bytes.NewBuffer([]byte{})
 			)
@@ -6118,10 +6123,10 @@ func TestBackupWithResPoliciesLogs(t *testing.T) {
 	h.addItems(t, test.PVs(builder.ForPersistentVolume("pv-1").Result()))
 
 	backupReq := &Request{
-		Backup:           defaultBackup().ExcludedNamespaceScopedResources("pods").Result(),
-		SkippedPVTracker: NewSkipPVTracker(),
-		BackedUpItems:    NewBackedUpItemsMap(),
-		WorkerPool:       itemBlockPool,
+		Backup:               defaultBackup().ExcludedNamespaceScopedResources("pods").Result(),
+		SkippedVolumeTracker: NewSkipVolumeTracker(),
+		BackedUpItems:        NewBackedUpItemsMap(),
+		WorkerPool:           itemBlockPool,
 	}
 
 	p := new(resourcepolicies.Policies)

pkg/backup/item_backupper.go

@@ -115,7 +115,7 @@ func (ib *itemBackupper) itemInclusionChecks(log logrus.FieldLogger, mustInclude
 	} else {
 		if metadata.GetLabels()[velerov1api.ExcludeFromBackupLabel] == "true" {
 			log.Infof("Excluding item because it has label %s=true", velerov1api.ExcludeFromBackupLabel)
-			ib.trackSkippedPV(obj, groupResource, "", fmt.Sprintf("item has label %s=true", velerov1api.ExcludeFromBackupLabel), log)
+			ib.trackSkippedVolume(obj, groupResource, "", fmt.Sprintf("item has label %s=true", velerov1api.ExcludeFromBackupLabel), log)
 			return false
 		}
 		// NOTE: we have to re-check namespace & resource includes/excludes because it's possible that
@@ -228,7 +228,7 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 	)
 
 	if optedOut, podName := ib.podVolumeSnapshotTracker.OptedoutByPod(namespace, name); optedOut {
-		ib.trackSkippedPV(obj, groupResource, podVolumeApproach, fmt.Sprintf("opted out due to annotation in pod %s", podName), log)
+		ib.trackSkippedVolume(obj, groupResource, podVolumeApproach, fmt.Sprintf("opted out due to annotation in pod %s", podName), log)
 	}
 
 	if groupResource == kuberesource.Pods {
@@ -326,7 +326,7 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 				if obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(skippedPVC.PVC); err != nil {
 					backupErrs = append(backupErrs, errors.WithStack(err))
 				} else {
-					ib.trackSkippedPV(&unstructured.Unstructured{Object: obj}, kuberesource.PersistentVolumeClaims,
+					ib.trackSkippedVolume(&unstructured.Unstructured{Object: obj}, kuberesource.PersistentVolumeClaims,
 						podVolumeApproach, skippedPVC.Reason, log)
 				}
 			}
@@ -334,7 +334,7 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 				if obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pvc); err != nil {
 					backupErrs = append(backupErrs, errors.WithStack(err))
 				} else {
-					ib.unTrackSkippedPV(&unstructured.Unstructured{Object: obj}, kuberesource.PersistentVolumeClaims, log)
+					ib.unTrackSkippedVolume(&unstructured.Unstructured{Object: obj}, kuberesource.PersistentVolumeClaims, log)
 				}
 			}
 		}
@@ -419,7 +419,7 @@ func (ib *itemBackupper) executeActions(
 			return nil, itemFiles, errors.WithStack(err)
 		} else if act != nil && act.Type == resourcepolicies.Skip {
 			log.Infof("Skip executing Backup Item Action: %s of resource %s: %s/%s for the matched resource policies", actionName, groupResource, namespace, name)
-			ib.trackSkippedPV(obj, groupResource, "", "skipped due to resource policy ", log)
+			ib.trackSkippedVolume(obj, groupResource, "", "skipped due to resource policy ", log)
 			continue
 		}
 
@@ -438,7 +438,7 @@ func (ib *itemBackupper) executeActions(
 			}
 
 			if !snapshotVolume {
-				ib.trackSkippedPV(
+				ib.trackSkippedVolume(
 					obj,
 					kuberesource.PersistentVolumeClaims,
 					volumeSnapshotApproach,
@@ -466,12 +466,12 @@ func (ib *itemBackupper) executeActions(
 			if additionalItemIdentifiers == nil && u.GetAnnotations()[velerov1api.SkippedNoCSIPVAnnotation] == "true" {
 				// snapshot was skipped by CSI plugin
 				log.Infof("skip CSI snapshot for PVC %s as it's not a CSI compatible volume", namespace+"/"+name)
-				ib.trackSkippedPV(obj, groupResource, csiSnapshotApproach, "skipped b/c it's not a CSI volume", log)
+				ib.trackSkippedVolume(obj, groupResource, csiSnapshotApproach, "skipped b/c it's not a CSI volume", log)
 				delete(u.GetAnnotations(), velerov1api.SkippedNoCSIPVAnnotation)
 			} else {
 				// the snapshot has been taken by the BIA plugin
 				log.Infof("Untrack the PVC %s, because it's backed up by CSI BIA.", namespace+"/"+name)
-				ib.unTrackSkippedPV(obj, kuberesource.PersistentVolumeClaims, log)
+				ib.unTrackSkippedVolume(obj, kuberesource.PersistentVolumeClaims, log)
 			}
 		}
 
@@ -623,7 +623,7 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 	}
 
 	if !snapshotVolume {
-		ib.trackSkippedPV(
+		ib.trackSkippedVolume(
 			obj,
 			kuberesource.PersistentVolumes,
 			volumeSnapshotApproach,
@@ -669,7 +669,13 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 		} else if action != nil && action.Type == resourcepolicies.Skip {
 			log.Infof("skip snapshot of pv %s for the matched resource policies", pv.Name)
 			// at this point we are sure this object is PV therefore we'll call the tracker directly
-			ib.backupRequest.SkippedPVTracker.Track(pv.Name, volumeSnapshotApproach, "matched action is 'skip' in chosen resource policies")
+			pvcName := ""
+			pvcNamespace := ""
+			if pv.Spec.ClaimRef != nil {
+				pvcName = pv.Spec.ClaimRef.Name
+				pvcNamespace = pv.Spec.ClaimRef.Namespace
+			}
+			ib.backupRequest.SkippedVolumeTracker.Track(pv.Name, pvcName, pvcNamespace, volumeSnapshotApproach, "matched action is 'skip' in chosen resource policies")
 			return nil
 		}
 	}
@@ -724,7 +730,13 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 	if volumeSnapshotter == nil {
 		// the PV may still has change to be snapshotted by CSI plugin's `PVCBackupItemAction` in PVC backup logic
 		log.Info("Persistent volume is not a supported volume type for Velero-native volumeSnapshotter snapshot, skipping.")
-		ib.backupRequest.SkippedPVTracker.Track(pv.Name, volumeSnapshotApproach, "no applicable volumesnapshotter found")
+		pvcName := ""
+		pvcNamespace := ""
+		if pv.Spec.ClaimRef != nil {
+			pvcName = pv.Spec.ClaimRef.Name
+			pvcNamespace = pv.Spec.ClaimRef.Namespace
+		}
+		ib.backupRequest.SkippedVolumeTracker.Track(pv.Name, pvcName, pvcNamespace, volumeSnapshotApproach, "no applicable volumesnapshotter found")
 		return nil
 	}
 
@@ -748,8 +760,14 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 	snapshot := volumeSnapshot(ib.backupRequest.Backup, pv.Name, volumeID, volumeType, pvFailureDomainZone, location, iops)
 
 	var errs []error
-	log.Info("Untrack the PV %s from the skipped volumes, because it's backed by Velero native snapshot.", pv.Name)
-	ib.backupRequest.SkippedPVTracker.Untrack(pv.Name)
+	log.Infof("Untrack the PV %s from the skipped volumes, because it's backed by Velero native snapshot.", pv.Name)
+	pvcName := ""
+	pvcNamespace := ""
+	if pv.Spec.ClaimRef != nil {
+		pvcName = pv.Spec.ClaimRef.Name
+		pvcNamespace = pv.Spec.ClaimRef.Namespace
+	}
+	ib.backupRequest.SkippedVolumeTracker.Untrack(pv.Name, pvcName, pvcNamespace)
 	snapshotID, err := volumeSnapshotter.CreateSnapshot(snapshot.Spec.ProviderVolumeID, snapshot.Spec.VolumeAZ, tags)
 	if err != nil {
 		errs = append(errs, errors.Wrap(err, "error taking snapshot of volume"))
@@ -786,37 +804,35 @@ func (ib *itemBackupper) getMatchAction(obj runtime.Unstructured, groupResource
 	return nil, nil
 }
 
-// trackSkippedPV tracks the skipped PV based on the object and the given approach and reason
+// trackSkippedVolume tracks the skipped volume based on the object and the given approach and reason
 // this function will be called throughout the process of backup, it needs to handle any object
-func (ib *itemBackupper) trackSkippedPV(obj runtime.Unstructured, groupResource schema.GroupResource, approach string, reason string, log logrus.FieldLogger) {
-	if name, err := getPVName(obj, groupResource); len(name) > 0 && err == nil {
-		ib.backupRequest.SkippedPVTracker.Track(name, approach, reason)
+func (ib *itemBackupper) trackSkippedVolume(obj runtime.Unstructured, groupResource schema.GroupResource, approach string, reason string, log logrus.FieldLogger) {
+	pvName, pvcName, pvcNamespace, err := getVolumeTrackingInfo(obj, groupResource)
+	if err == nil && (len(pvName) > 0 || len(pvcName) > 0) {
+		ib.backupRequest.SkippedVolumeTracker.Track(pvName, pvcName, pvcNamespace, approach, reason)
 	} else if err != nil {
-		// Log at info level for tracking purposes. This is not an error because
-		// it's expected for some resources (e.g., PVCs in Pending or Lost phase)
-		// to not have a PV name. This occurs when volume policy skips unbound PVCs.
-		log.WithError(err).Infof("unable to get PV name, skip tracking.")
+		log.WithError(err).Info("unable to get volume tracking info, skip tracking.")
 	}
 }
 
-// unTrackSkippedPV removes skipped PV based on the object from the tracker
+// unTrackSkippedVolume removes skipped volume based on the object from the tracker
 // this function will be called throughout the process of backup, it needs to handle any object
-func (ib *itemBackupper) unTrackSkippedPV(obj runtime.Unstructured, groupResource schema.GroupResource, log logrus.FieldLogger) {
-	if name, err := getPVName(obj, groupResource); len(name) > 0 && err == nil {
-		ib.backupRequest.SkippedPVTracker.Untrack(name)
+func (ib *itemBackupper) unTrackSkippedVolume(obj runtime.Unstructured, groupResource schema.GroupResource, log logrus.FieldLogger) {
+	pvName, pvcName, pvcNamespace, err := getVolumeTrackingInfo(obj, groupResource)
+	if err == nil && (len(pvName) > 0 || len(pvcName) > 0) {
+		ib.backupRequest.SkippedVolumeTracker.Untrack(pvName, pvcName, pvcNamespace)
 	} else if err != nil {
-		// For PVCs in Pending or Lost phase, it's expected that there's no PV name.
-		// Log at debug level instead of warning to reduce noise.
 		if groupResource == kuberesource.PersistentVolumeClaims {
 			pvc := new(corev1api.PersistentVolumeClaim)
 			if convErr := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pvc); convErr == nil {
 				if pvc.Status.Phase == corev1api.ClaimPending || pvc.Status.Phase == corev1api.ClaimLost {
-					log.WithError(err).Debugf("unable to get PV name for %s PVC, skip untracking.", pvc.Status.Phase)
+					log.WithError(err).Debugf("unable to get volume tracking info for %s PVC, skip untracking.", pvc.Status.Phase)
 					return
 				}
 			}
 		}
-		log.WithError(err).Warnf("unable to get PV name, skip untracking.")
+
+		log.WithError(err).Warn("unable to get volume tracking info, skip untracking.")
 	}
 }
 
@@ -840,26 +856,29 @@ func (ib *itemBackupper) addVolumeInfo(obj runtime.Unstructured, log logrus.Fiel
 	return nil
 }
 
-// convert the input object to PV/PVC and get the PV name
-func getPVName(obj runtime.Unstructured, groupResource schema.GroupResource) (string, error) {
+// convert the input object to PV/PVC and get the PV name, PVC name and PVC namespace
+func getVolumeTrackingInfo(obj runtime.Unstructured, groupResource schema.GroupResource) (string, string, string, error) {
 	if groupResource == kuberesource.PersistentVolumes {
 		pv := new(corev1api.PersistentVolume)
 		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pv); err != nil {
-			return "", fmt.Errorf("failed to convert object to PV: %w", err)
+			return "", "", "", fmt.Errorf("failed to convert object to PV: %w", err)
 		}
-		return pv.Name, nil
+		pvcName := ""
+		pvcNamespace := ""
+		if pv.Spec.ClaimRef != nil {
+			pvcName = pv.Spec.ClaimRef.Name
+			pvcNamespace = pv.Spec.ClaimRef.Namespace
+		}
+		return pv.Name, pvcName, pvcNamespace, nil
 	}
 	if groupResource == kuberesource.PersistentVolumeClaims {
 		pvc := new(corev1api.PersistentVolumeClaim)
 		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pvc); err != nil {
-			return "", fmt.Errorf("failed to convert object to PVC: %w", err)
+			return "", "", "", fmt.Errorf("failed to convert object to PVC: %w", err)
 		}
-		if pvc.Spec.VolumeName == "" {
-			return "", fmt.Errorf("PV name is not set in PVC")
-		}
-		return pvc.Spec.VolumeName, nil
+		return pvc.Spec.VolumeName, pvc.Name, pvc.Namespace, nil
 	}
-	return "", nil
+	return "", "", "", nil
 }
 
 func volumeSnapshot(backup *velerov1api.Backup, volumeName, volumeID, volumeType, az, location string, iops *int64) *volume.Snapshot {

pkg/backup/item_backupper_test.go

@@ -180,33 +180,41 @@ func Test_zoneFromPVNodeAffinity(t *testing.T) {
 	}
 }
 
-func TestGetPVName(t *testing.T) {
+func TestGetVolumeTrackingInfo(t *testing.T) {
 	testcases := []struct {
 		name          string
 		obj           metav1.Object
 		groupResource schema.GroupResource
 		pvName        string
+		pvcName       string
+		pvcNamespace  string
 		hasErr        bool
 	}{
 		{
 			name:          "pv should return pv name",
-			obj:           builder.ForPersistentVolume("test-pv").Result(),
+			obj:           builder.ForPersistentVolume("test-pv").ClaimRef("ns", "pvc-1").Result(),
 			groupResource: kuberesource.PersistentVolumes,
 			pvName:        "test-pv",
+			pvcName:       "pvc-1",
+			pvcNamespace:  "ns",
 			hasErr:        false,
 		},
 		{
-			name:          "pvc without volumeName should return error",
+			name:          "pvc without volumeName should return pvc info",
 			obj:           builder.ForPersistentVolumeClaim("ns", "pvc-1").Result(),
 			groupResource: kuberesource.PersistentVolumeClaims,
 			pvName:        "",
-			hasErr:        true,
+			pvcName:       "pvc-1",
+			pvcNamespace:  "ns",
+			hasErr:        false,
 		},
 		{
-			name:          "pvc with volumeName should return pv name",
+			name:          "pvc with volumeName should return pv name and pvc info",
 			obj:           builder.ForPersistentVolumeClaim("ns", "pvc-1").VolumeName("test-pv-2").Result(),
 			groupResource: kuberesource.PersistentVolumeClaims,
 			pvName:        "test-pv-2",
+			pvcName:       "pvc-1",
+			pvcNamespace:  "ns",
 			hasErr:        false,
 		},
 		{
@@ -214,6 +222,8 @@ func TestGetPVName(t *testing.T) {
 			obj:           builder.ForPod("ns", "pod1").Result(),
 			groupResource: kuberesource.Pods,
 			pvName:        "",
+			pvcName:       "",
+			pvcNamespace:  "",
 			hasErr:        false,
 		},
 	}
@@ -225,8 +235,10 @@ func TestGetPVName(t *testing.T) {
 				o = &unstructured.Unstructured{Object: data}
 				require.NoError(t, err)
 			}
-			name, err2 := getPVName(o, tc.groupResource)
-			assert.Equal(t, tc.pvName, name)
+			pvName, pvcName, pvcNamespace, err2 := getVolumeTrackingInfo(o, tc.groupResource)
+			assert.Equal(t, tc.pvName, pvName)
+			assert.Equal(t, tc.pvcName, pvcName)
+			assert.Equal(t, tc.pvcNamespace, pvcNamespace)
 			assert.Equal(t, tc.hasErr, err2 != nil)
 		})
 	}
@@ -373,28 +385,36 @@ func TestGetMatchAction_PendingLostPVC(t *testing.T) {
 	}
 }
 
-func TestTrackSkippedPV_PendingLostPVC(t *testing.T) {
+func TestTrackSkippedVolume_PendingLostPVC(t *testing.T) {
 	testCases := []struct {
-		name string
-		pvc  *corev1api.PersistentVolumeClaim
+		name               string
+		pvc                *corev1api.PersistentVolumeClaim
+		expectWarningLog   bool
+		expectDebugMessage string
 	}{
 		{
 			name: "Pending PVC should log at info level",
 			pvc: builder.ForPersistentVolumeClaim("ns", "pending-pvc").
 				Phase(corev1api.ClaimPending).
 				Result(),
+			expectWarningLog:   false,
+			expectDebugMessage: "unable to get volume tracking info for ClaimPending PVC, skip tracking.",
 		},
 		{
 			name: "Lost PVC should log at info level",
 			pvc: builder.ForPersistentVolumeClaim("ns", "lost-pvc").
 				Phase(corev1api.ClaimLost).
 				Result(),
+			expectWarningLog:   false,
+			expectDebugMessage: "unable to get volume tracking info for ClaimLost PVC, skip tracking.",
 		},
 		{
 			name: "Bound PVC without VolumeName should log at info level",
 			pvc: builder.ForPersistentVolumeClaim("ns", "bound-pvc").
 				Phase(corev1api.ClaimBound).
 				Result(),
+			expectWarningLog:   false,
+			expectDebugMessage: "unable to get volume tracking info for ClaimBound PVC, skip tracking.",
 		},
 	}
 
@@ -402,7 +422,7 @@ func TestTrackSkippedPV_PendingLostPVC(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			ib := &itemBackupper{
 				backupRequest: &Request{
-					SkippedPVTracker: NewSkipPVTracker(),
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
 				},
 			}
 
@@ -411,22 +431,40 @@ func TestTrackSkippedPV_PendingLostPVC(t *testing.T) {
 			logger := logrus.New()
 			logger.SetOutput(logOutput)
 			logger.SetLevel(logrus.DebugLevel)
+			logger.SetFormatter(&logrus.TextFormatter{
+				DisableColors:    true,
+				DisableTimestamp: true,
+				DisableQuote:     true,
+			})
 
 			// Convert PVC to unstructured
 			pvcData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.pvc)
 			require.NoError(t, err)
 			obj := &unstructured.Unstructured{Object: pvcData}
 
-			ib.trackSkippedPV(obj, kuberesource.PersistentVolumeClaims, "", "test reason", logger)
+			ib.trackSkippedVolume(obj, kuberesource.PersistentVolumeClaims, "", "test reason", logger)
 
 			logStr := logOutput.String()
-			assert.Contains(t, logStr, "level=info")
-			assert.Contains(t, logStr, "unable to get PV name, skip tracking.")
+			// Since we now track Pending/Lost PVCs, there won't be an error from getVolumeTrackingInfo
+			// and therefore no debug/info log about skipping tracking.
+			// Instead, we can verify that the tracker actually contains the PVC.
+			assert.NotContains(t, logStr, "unable to get volume tracking info")
+
+			// Verify it was tracked
+			summary := ib.backupRequest.SkippedVolumeTracker.Summary()
+			found := false
+			for _, v := range summary {
+				if v.PVCName == tc.pvc.Name && v.PVCNamespace == tc.pvc.Namespace {
+					found = true
+					break
+				}
+			}
+			assert.True(t, found)
 		})
 	}
 }
 
-func TestUnTrackSkippedPV_PendingLostPVC(t *testing.T) {
+func TestUnTrackSkippedVolume_PendingLostPVC(t *testing.T) {
 	testCases := []struct {
 		name               string
 		pvc                *corev1api.PersistentVolumeClaim
@@ -439,7 +477,7 @@ func TestUnTrackSkippedPV_PendingLostPVC(t *testing.T) {
 				Phase(corev1api.ClaimPending).
 				Result(),
 			expectWarningLog:   false,
-			expectDebugMessage: "unable to get PV name for Pending PVC, skip untracking.",
+			expectDebugMessage: "unable to get volume tracking info for ClaimPending PVC, skip untracking.",
 		},
 		{
 			name: "Lost PVC should log at debug level, not warning",
@@ -447,15 +485,15 @@ func TestUnTrackSkippedPV_PendingLostPVC(t *testing.T) {
 				Phase(corev1api.ClaimLost).
 				Result(),
 			expectWarningLog:   false,
-			expectDebugMessage: "unable to get PV name for Lost PVC, skip untracking.",
+			expectDebugMessage: "unable to get volume tracking info for ClaimLost PVC, skip untracking.",
 		},
 		{
-			name: "Bound PVC without VolumeName should log warning",
+			name: "Bound PVC without VolumeName should log at debug level, not warning",
 			pvc: builder.ForPersistentVolumeClaim("ns", "bound-pvc").
 				Phase(corev1api.ClaimBound).
 				Result(),
-			expectWarningLog:   true,
-			expectDebugMessage: "",
+			expectWarningLog:   false,
+			expectDebugMessage: "unable to get volume tracking info for ClaimBound PVC, skip untracking.",
 		},
 	}
 
@@ -463,7 +501,7 @@ func TestUnTrackSkippedPV_PendingLostPVC(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			ib := &itemBackupper{
 				backupRequest: &Request{
-					SkippedPVTracker: NewSkipPVTracker(),
+					SkippedVolumeTracker: NewSkipVolumeTracker(),
 				},
 			}
 
@@ -472,25 +510,23 @@ func TestUnTrackSkippedPV_PendingLostPVC(t *testing.T) {
 			logger := logrus.New()
 			logger.SetOutput(logOutput)
 			logger.SetLevel(logrus.DebugLevel)
+			logger.SetFormatter(&logrus.TextFormatter{
+				DisableColors:    true,
+				DisableTimestamp: true,
+				DisableQuote:     true,
+			})
 
 			// Convert PVC to unstructured
 			pvcData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.pvc)
 			require.NoError(t, err)
 			obj := &unstructured.Unstructured{Object: pvcData}
 
-			ib.unTrackSkippedPV(obj, kuberesource.PersistentVolumeClaims, logger)
+			ib.unTrackSkippedVolume(obj, kuberesource.PersistentVolumeClaims, logger)
 
 			logStr := logOutput.String()
-			if tc.expectWarningLog {
-				assert.Contains(t, logStr, "level=warning")
-				assert.Contains(t, logStr, "unable to get PV name, skip untracking.")
-			} else {
-				assert.NotContains(t, logStr, "level=warning")
-				if tc.expectDebugMessage != "" {
-					assert.Contains(t, logStr, "level=debug")
-					assert.Contains(t, logStr, tc.expectDebugMessage)
-				}
-			}
+			// Since we now track Pending/Lost PVCs, there won't be an error from getVolumeTrackingInfo
+			// and therefore no debug/warning log about skipping untracking.
+			assert.NotContains(t, logStr, "unable to get volume tracking info")
 		})
 	}
 }
@@ -534,7 +570,7 @@ func baseRequest() *Request {
 		Backup:                    builder.ForBackup("velero", "test-backup").Result(),
 		NamespaceIncludesExcludes: collections.NewNamespaceIncludesExcludes().Includes("*"),
 		ResourceIncludesExcludes:  includeAllIE{},
-		SkippedPVTracker:          NewSkipPVTracker(),
+		SkippedVolumeTracker:      NewSkipVolumeTracker(),
 	}
 }
 
@@ -667,7 +703,7 @@ func TestItemInclusionChecks_GlobalExclusion_OverridesNamespaceFilter(t *testing
 		Backup:                    builder.ForBackup("velero", "test-backup").Result(),
 		NamespaceIncludesExcludes: collections.NewNamespaceIncludesExcludes().Includes("*"),
 		ResourceIncludesExcludes:  excludeSecretsIE,
-		SkippedPVTracker:          NewSkipPVTracker(),
+		SkippedVolumeTracker:      NewSkipVolumeTracker(),
 		// namespacedFilterPolicies says to back up Secrets in ns-a
 		NamespacedFilterMap: map[string]*ResolvedNamespaceFilter{
 			"ns-a": {

pkg/backup/pv_skip_tracker.go

@@ -1,130 +0,0 @@
-/*
-Copyright 2018 the Velero contributors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package backup
-
-import (
-	"sort"
-	"sync"
-)
-
-type SkippedPV struct {
-	Name    string         `json:"name"`
-	Reasons []PVSkipReason `json:"reasons"`
-}
-
-func (s *SkippedPV) SerializeSkipReasons() string {
-	ret := ""
-	for _, reason := range s.Reasons {
-		ret = ret + reason.Approach + ": " + reason.Reason + ";"
-	}
-	return ret
-}
-
-type PVSkipReason struct {
-	Approach string `json:"approach"`
-	Reason   string `json:"reason"`
-}
-
-// skipPVTracker keeps track of persistent volumes that have been skipped and the reason why they are skipped.
-type skipPVTracker struct {
-	*sync.RWMutex
-	// pvs is a map of name of the pv to the list of reasons why it is skipped.
-	// The reasons are stored in a map each key of the map is the backup approach, each approach can have one reason
-	pvs map[string]map[string]string
-	// includedPVs is a set of pv to be included in the backup, the element in this set should not be in the "pvs" map
-	includedPVs map[string]struct{}
-}
-
-const (
-	podVolumeApproach       = "podvolume"
-	csiSnapshotApproach     = "csiSnapshot"
-	volumeSnapshotApproach  = "volumeSnapshot"
-	vsphereSnapshotApproach = "vsphereSnapshot"
-	anyApproach             = "any"
-)
-
-func NewSkipPVTracker() *skipPVTracker {
-	return &skipPVTracker{
-		RWMutex:     &sync.RWMutex{},
-		pvs:         make(map[string]map[string]string),
-		includedPVs: make(map[string]struct{}),
-	}
-}
-
-// Track tracks the pv with the specified name and the reason why it is skipped
-func (pt *skipPVTracker) Track(name, approach, reason string) {
-	pt.Lock()
-	defer pt.Unlock()
-	if name == "" || reason == "" {
-		return
-	}
-	if _, ok := pt.includedPVs[name]; ok {
-		return
-	}
-	skipReasons := pt.pvs[name]
-	if skipReasons == nil {
-		skipReasons = make(map[string]string)
-		pt.pvs[name] = skipReasons
-	}
-	if approach == "" {
-		approach = anyApproach
-	}
-	skipReasons[approach] = reason
-}
-
-// Untrack removes the pvc with the specified namespace and name.
-// This func should be called when the PV is taken for snapshot, regardless native snapshot, CSI snapshot or fsb backup
-// therefore, in one backup processed if a PV is Untracked once, it will not be tracked again.
-func (pt *skipPVTracker) Untrack(name string) {
-	pt.Lock()
-	defer pt.Unlock()
-	pt.includedPVs[name] = struct{}{}
-	delete(pt.pvs, name)
-}
-
-// Summary returns the summary of the tracked pvcs.
-func (pt *skipPVTracker) Summary() []SkippedPV {
-	pt.RLock()
-	defer pt.RUnlock()
-	keys := make([]string, 0, len(pt.pvs))
-	for key := range pt.pvs {
-		keys = append(keys, key)
-	}
-	sort.Strings(keys)
-	res := make([]SkippedPV, 0, len(keys))
-	for _, key := range keys {
-		if skipReasons := pt.pvs[key]; len(skipReasons) > 0 {
-			entry := SkippedPV{
-				Name:    key,
-				Reasons: make([]PVSkipReason, 0, len(skipReasons)),
-			}
-			approaches := make([]string, 0, len(skipReasons))
-			for a := range skipReasons {
-				approaches = append(approaches, a)
-			}
-			sort.Strings(approaches)
-			for _, a := range approaches {
-				entry.Reasons = append(entry.Reasons, PVSkipReason{
-					Approach: a,
-					Reason:   skipReasons[a],
-				})
-			}
-			res = append(res, entry)
-		}
-	}
-	return res
-}

pkg/backup/request.go

@@ -90,7 +90,7 @@ type Request struct {
 	MustIncludeAdditionalItemPVCs *backedUpItemsMap
 	itemOperationsList            *[]*itemoperation.BackupOperation
 	ResPolicies                   *resourcepolicies.Policies
-	SkippedPVTracker              *skipPVTracker
+	SkippedVolumeTracker          *skipVolumeTracker
 	VolumesInformation            volume.BackupVolumesInformation
 	WorkerPool                    *ItemBlockWorkerPool
 
@@ -139,13 +139,18 @@ func (r *Request) BackupResourceList() map[string][]string {
 }
 
 func (r *Request) FillVolumesInformation() {
-	skippedPVMap := make(map[string]string)
+	var skippedVolumes []volume.SkippedVolume
 
-	for _, skippedPV := range r.SkippedPVTracker.Summary() {
-		skippedPVMap[skippedPV.Name] = skippedPV.SerializeSkipReasons()
+	for _, skippedVolume := range r.SkippedVolumeTracker.Summary() {
+		skippedVolumes = append(skippedVolumes, volume.SkippedVolume{
+			PVName:       skippedVolume.PVName,
+			PVCName:      skippedVolume.PVCName,
+			PVCNamespace: skippedVolume.PVCNamespace,
+			Reasons:      skippedVolume.SerializeSkipReasons(),
+		})
 	}
 
-	r.VolumesInformation.SkippedPVs = skippedPVMap
+	r.VolumesInformation.SkippedVolumes = skippedVolumes
 	r.VolumesInformation.NativeSnapshots = r.VolumeSnapshots.Get()
 	r.VolumesInformation.PodVolumeBackups = r.PodVolumeBackups
 	r.VolumesInformation.BackupOperations = *r.GetItemOperationsList()

pkg/backup/volume_skip_tracker.go

@@ -0,0 +1,159 @@
+/*
+Copyright 2018 the Velero contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package backup
+
+import (
+	"sort"
+	"sync"
+)
+
+type SkippedVolume struct {
+	PVName       string   `json:"pvName"`
+	PVCName      string   `json:"pvcName,omitempty"`
+	PVCNamespace string   `json:"pvcNamespace,omitempty"`
+	Reasons      []Reason `json:"reasons"`
+}
+
+func (s *SkippedVolume) SerializeSkipReasons() string {
+	ret := ""
+	for _, reason := range s.Reasons {
+		ret = ret + reason.Approach + ": " + reason.Reason + ";"
+	}
+	return ret
+}
+
+type Reason struct {
+	Approach string `json:"approach"`
+	Reason   string `json:"reason"`
+}
+
+// skipVolumeTracker keeps track of volumes (PV/PVC) that have been skipped and the reason why they are skipped.
+type skipVolumeTracker struct {
+	*sync.RWMutex
+	// volumes is a map of volume(PV/PVC) key to the list of reasons why it is skipped.
+	// The reasons are stored in a map each key of the map is the backup approach, each approach can have one reason
+	volumes map[string]map[string]string
+	// includedVolumes is a set of volume key to be included in the backup, the element in this set should not be in the "volumes" map
+	includedVolumes map[string]struct{}
+	// volumeInfo is a map of volume key to SkippedVolume info
+	volumeInfo map[string]SkippedVolume
+}
+
+const (
+	podVolumeApproach       = "podvolume"
+	csiSnapshotApproach     = "csiSnapshot"
+	volumeSnapshotApproach  = "volumeSnapshot"
+	vsphereSnapshotApproach = "vsphereSnapshot"
+	anyApproach             = "any"
+)
+
+func NewSkipVolumeTracker() *skipVolumeTracker {
+	return &skipVolumeTracker{
+		RWMutex:         &sync.RWMutex{},
+		volumes:         make(map[string]map[string]string),
+		includedVolumes: make(map[string]struct{}),
+		volumeInfo:      make(map[string]SkippedVolume),
+	}
+}
+
+func getVolumeKey(pvName, pvcName, pvcNamespace string) string {
+	if pvName != "" {
+		return "pv:" + pvName
+	}
+	if pvcName != "" && pvcNamespace != "" {
+		return "pvc:" + pvcNamespace + "/" + pvcName
+	}
+	return ""
+}
+
+// Track tracks the Volume(PV/PVC) with the specified name and the reason why it is skipped
+func (pt *skipVolumeTracker) Track(pvName, pvcName, pvcNamespace, approach, reason string) {
+	pt.Lock()
+	defer pt.Unlock()
+	key := getVolumeKey(pvName, pvcName, pvcNamespace)
+	if key == "" || reason == "" {
+		return
+	}
+	if _, ok := pt.includedVolumes[key]; ok {
+		return
+	}
+	skipReasons := pt.volumes[key]
+	if skipReasons == nil {
+		skipReasons = make(map[string]string)
+		pt.volumes[key] = skipReasons
+	}
+	if approach == "" {
+		approach = anyApproach
+	}
+	skipReasons[approach] = reason
+	pt.volumeInfo[key] = SkippedVolume{
+		PVName:       pvName,
+		PVCName:      pvcName,
+		PVCNamespace: pvcNamespace,
+	}
+}
+
+// Untrack removes the volume(pv/pvc) with the specified namespace and name.
+// This func should be called when the volume is taken for snapshot, regardless native snapshot, CSI snapshot or fsb backup
+// therefore, in one backup processed if a volume is Untracked once, it will not be tracked again.
+func (pt *skipVolumeTracker) Untrack(pvName, pvcName, pvcNamespace string) {
+	pt.Lock()
+	defer pt.Unlock()
+	key := getVolumeKey(pvName, pvcName, pvcNamespace)
+	if key == "" {
+		return
+	}
+	pt.includedVolumes[key] = struct{}{}
+	delete(pt.volumes, key)
+	delete(pt.volumeInfo, key)
+}
+
+// Summary returns the summary of the tracked volumes.
+func (pt *skipVolumeTracker) Summary() []SkippedVolume {
+	pt.RLock()
+	defer pt.RUnlock()
+	keys := make([]string, 0, len(pt.volumes))
+	for key := range pt.volumes {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	res := make([]SkippedVolume, 0, len(keys))
+	for _, key := range keys {
+		if skipReasons := pt.volumes[key]; len(skipReasons) > 0 {
+			info := pt.volumeInfo[key]
+			entry := SkippedVolume{
+				PVName:       info.PVName,
+				PVCName:      info.PVCName,
+				PVCNamespace: info.PVCNamespace,
+				Reasons:      make([]Reason, 0, len(skipReasons)),
+			}
+			approaches := make([]string, 0, len(skipReasons))
+			for a := range skipReasons {
+				approaches = append(approaches, a)
+			}
+			sort.Strings(approaches)
+			for _, a := range approaches {
+				entry.Reasons = append(entry.Reasons, Reason{
+					Approach: a,
+					Reason:   skipReasons[a],
+				})
+			}
+			res = append(res, entry)
+		}
+	}
+	return res
+}

pkg/backup/volume_skip_tracker_test.go

@@ -24,18 +24,20 @@ import (
 )
 
 func TestSummary(t *testing.T) {
-	tracker := NewSkipPVTracker()
-	tracker.Track("pv5", "", "skipped due to policy")
-	tracker.Track("pv3", podVolumeApproach, "it's set to opt-out")
-	tracker.Track("pv3", csiSnapshotApproach, "not applicable for CSI ")
+	tracker := NewSkipVolumeTracker()
+	tracker.Track("pv5", "pvc5", "ns1", "", "skipped due to policy")
+	tracker.Track("pv3", "pvc3", "ns1", podVolumeApproach, "it's set to opt-out")
+	tracker.Track("pv3", "pvc3", "ns1", csiSnapshotApproach, "not applicable for CSI ")
 	// shouldn't be added
-	tracker.Track("", podVolumeApproach, "pvc3 is set to be skipped")
-	tracker.Track("pv10", volumeSnapshotApproach, "added by mistake")
-	tracker.Untrack("pv10")
-	expected := []SkippedPV{
+	tracker.Track("", "", "", podVolumeApproach, "pvc3 is set to be skipped")
+	tracker.Track("pv10", "pvc10", "ns1", volumeSnapshotApproach, "added by mistake")
+	tracker.Untrack("pv10", "pvc10", "ns1")
+	expected := []SkippedVolume{
 		{
-			Name: "pv3",
-			Reasons: []PVSkipReason{
+			PVName:       "pv3",
+			PVCName:      "pvc3",
+			PVCNamespace: "ns1",
+			Reasons: []Reason{
 				{
 					Approach: csiSnapshotApproach,
 					Reason:   "not applicable for CSI ",
@@ -47,8 +49,10 @@ func TestSummary(t *testing.T) {
 			},
 		},
 		{
-			Name: "pv5",
-			Reasons: []PVSkipReason{
+			PVName:       "pv5",
+			PVCName:      "pvc5",
+			PVCNamespace: "ns1",
+			Reasons: []Reason{
 				{
 					Approach: anyApproach,
 					Reason:   "skipped due to policy",
@@ -60,21 +64,20 @@ func TestSummary(t *testing.T) {
 }
 
 func TestSerializeSkipReasons(t *testing.T) {
-	tracker := NewSkipPVTracker()
-	//tracker.Track("pv5", "", "skipped due to policy")
-	tracker.Track("pv3", podVolumeApproach, "it's set to opt-out")
-	tracker.Track("pv3", csiSnapshotApproach, "not applicable for CSI ")
+	tracker := NewSkipVolumeTracker()
+	tracker.Track("pv3", "pvc3", "ns1", podVolumeApproach, "it's set to opt-out")
+	tracker.Track("pv3", "pvc3", "ns1", csiSnapshotApproach, "not applicable for CSI ")
 
-	for _, skippedPV := range tracker.Summary() {
-		require.Equal(t, "csiSnapshot: not applicable for CSI ;podvolume: it's set to opt-out;", skippedPV.SerializeSkipReasons())
+	for _, skippedVolume := range tracker.Summary() {
+		require.Equal(t, "csiSnapshot: not applicable for CSI ;podvolume: it's set to opt-out;", skippedVolume.SerializeSkipReasons())
 	}
 }
 
 func TestTrackUntrack(t *testing.T) {
 	// If a pv is untracked explicitly it can't be Tracked again, b/c the pv is considered backed up already.
-	tracker := NewSkipPVTracker()
-	tracker.Track("pv3", podVolumeApproach, "it's set to opt-out")
-	tracker.Untrack("pv3")
-	tracker.Track("pv3", csiSnapshotApproach, "not applicable for CSI ")
+	tracker := NewSkipVolumeTracker()
+	tracker.Track("pv3", "pvc3", "ns1", podVolumeApproach, "it's set to opt-out")
+	tracker.Untrack("pv3", "pvc3", "ns1")
+	tracker.Track("pv3", "pvc3", "ns1", csiSnapshotApproach, "not applicable for CSI ")
 	assert.Empty(t, tracker.Summary())
 }

pkg/controller/backup_controller.go

@@ -396,7 +396,7 @@ func (b *backupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *velerov1api.Backup, logger logrus.FieldLogger) *pkgbackup.Request {
 	request := &pkgbackup.Request{
 		Backup:                        backup.DeepCopy(), // don't modify items in the cache
-		SkippedPVTracker:              pkgbackup.NewSkipPVTracker(),
+		SkippedVolumeTracker:          pkgbackup.NewSkipVolumeTracker(),
 		BackedUpItems:                 pkgbackup.NewBackedUpItemsMap(),
 		MustIncludeAdditionalItemPVCs: pkgbackup.NewBackedUpItemsMap(),
 		WorkerPool:                    pkgbackup.StartItemBlockWorkerPool(ctx, b.itemBlockWorkerCount, logger),

pkg/controller/backup_finalizer_controller.go

@@ -160,7 +160,7 @@ func (r *backupFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	backupRequest := &pkgbackup.Request{
 		Backup:                        backup,
 		StorageLocation:               location,
-		SkippedPVTracker:              pkgbackup.NewSkipPVTracker(),
+		SkippedVolumeTracker:          pkgbackup.NewSkipVolumeTracker(),
 		BackedUpItems:                 pkgbackup.NewBackedUpItemsMap(),
 		MustIncludeAdditionalItemPVCs: pkgbackup.NewBackedUpItemsMap(),
 	}