diff --git a/changelogs/unreleased/7152-reasonerjt b/changelogs/unreleased/7152-reasonerjt new file mode 100644 index 000000000..809921f33 --- /dev/null +++ b/changelogs/unreleased/7152-reasonerjt @@ -0,0 +1 @@ +Track the skipped PV when SnapshotVolumes set as false \ No newline at end of file diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 6760d627b..286156755 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -1368,6 +1368,7 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) { "any": "whatever reason", }, }, + includedPVs: map[string]struct{}{}, }, }, apiResources: []*test.APIResource{ diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 01258a4aa..ef1ff79d2 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -510,6 +510,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 } diff --git a/pkg/backup/pv_skip_tracker.go b/pkg/backup/pv_skip_tracker.go index 64241a240..03f1a719d 100644 --- a/pkg/backup/pv_skip_tracker.go +++ b/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) } diff --git a/pkg/backup/pv_skip_tracker_test.go b/pkg/backup/pv_skip_tracker_test.go index 16de8f555..8e75a808c 100644 --- a/pkg/backup/pv_skip_tracker_test.go +++ b/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())) +}