Track the skipped PV when SnapshotVolumes set as false

This commit makes sure if a PV is not taken snapshot b/c the flag
SnapshotVolumes is set to false in a backup CR, the PV is also also
tracked as skipped in the tracker.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>

changelogs/unreleased/7152-reasonerjt

@@ -0,0 +1 @@
+Track the skipped PV when SnapshotVolumes set as false

pkg/backup/backup_test.go

@@ -1368,6 +1368,7 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
 							"any": "whatever reason",
 						},
 					},
+					includedPVs: map[string]struct{}{},
 				},
 			},
 			apiResources: []*test.APIResource{

pkg/backup/item_backupper.go

@@ -509,6 +509,7 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 
 	if boolptr.IsSetToFalse(ib.backupRequest.Spec.SnapshotVolumes) {
 		log.Info("Backup has volume snapshots disabled; skipping volume snapshot action.")
+		ib.trackSkippedPV(obj, kuberesource.PersistentVolumes, volumeSnapshotApproach, "backup has volume snapshots disabled", log)
 		return nil
 	}
 

pkg/backup/pv_skip_tracker.go

@@ -29,6 +29,8 @@ type skipPVTracker struct {
 	// 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 (
@@ -40,8 +42,9 @@ const (
 
 func NewSkipPVTracker() *skipPVTracker {
 	return &skipPVTracker{
-		RWMutex: &sync.RWMutex{},
-		pvs:     make(map[string]map[string]string),
+		RWMutex:     &sync.RWMutex{},
+		pvs:         make(map[string]map[string]string),
+		includedPVs: make(map[string]struct{}),
 	}
 }
 
@@ -52,9 +55,12 @@ func (pt *skipPVTracker) Track(name, approach, reason string) {
 	if name == "" || reason == "" {
 		return
 	}
+	if _, ok := pt.includedPVs[name]; ok {
+		return
+	}
 	skipReasons := pt.pvs[name]
 	if skipReasons == nil {
-		skipReasons = make(map[string]string, 0)
+		skipReasons = make(map[string]string)
 		pt.pvs[name] = skipReasons
 	}
 	if approach == "" {
@@ -64,9 +70,12 @@ func (pt *skipPVTracker) Track(name, approach, reason string) {
 }
 
 // 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)
 }
 

pkg/backup/pv_skip_tracker_test.go

@@ -53,3 +53,12 @@ func TestSerializeSkipReasons(t *testing.T) {
 		require.Equal(t, "csiSnapshot: not applicable for CSI ;podvolume: it's set to opt-out;", skippedPV.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 ")
+	assert.Equal(t, 0, len(tracker.Summary()))
+}