Skip to content

Commit

Permalink
Track the skipped PVC and print the summary in backup log
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Jiang <[email protected]>
  • Loading branch information
reasonerjt committed Jul 14, 2023
1 parent e54a8af commit 324642d
Show file tree
Hide file tree
Showing 13 changed files with 612 additions and 116 deletions.
2 changes: 1 addition & 1 deletion pkg/backup/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
if err := kube.PatchResource(backupRequest.Backup, updated, kb.kbClient); err != nil {
log.WithError(errors.WithStack((err))).Warn("Got error trying to update backup's status.progress")
}
log.Infof("Summary for skipped PVs: \n%s", backupRequest.SkippedPVTracker.Summary())
backupRequest.Status.Progress = &velerov1api.BackupProgress{TotalItems: len(backupRequest.BackedUpItems), ItemsBackedUp: len(backupRequest.BackedUpItems)}

log.WithField("progress", "").Infof("Backed up a total of %d items", len(backupRequest.BackedUpItems))

return nil
Expand Down
137 changes: 100 additions & 37 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ func TestBackedUpItemsMatchesTarballContents(t *testing.T) {
}

h := newHarness(t)
req := &Request{Backup: defaultBackup().Result()}
req := &Request{
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile := bytes.NewBuffer([]byte{})

apiResources := []*test.APIResource{
Expand Down Expand Up @@ -121,7 +124,10 @@ func TestBackedUpItemsMatchesTarballContents(t *testing.T) {
// the request's BackedUpItems field.
func TestBackupProgressIsUpdated(t *testing.T) {
h := newHarness(t)
req := &Request{Backup: defaultBackup().Result()}
req := &Request{
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile := bytes.NewBuffer([]byte{})

apiResources := []*test.APIResource{
Expand Down Expand Up @@ -853,8 +859,11 @@ func TestBackupOldResourceFiltering(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1027,8 +1036,11 @@ func TestCRDInclusion(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1119,8 +1131,11 @@ func TestBackupResourceCohabitation(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand All @@ -1144,7 +1159,8 @@ func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) {

// run and verify backup 1
backup1 := &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
}
backup1File := bytes.NewBuffer([]byte{})

Expand All @@ -1157,7 +1173,8 @@ func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) {

// run and verify backup 2
backup2 := &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
}
backup2File := bytes.NewBuffer([]byte{})

Expand Down Expand Up @@ -1204,8 +1221,11 @@ func TestBackupResourceOrdering(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1456,8 +1476,11 @@ func TestBackupActionsRunForCorrectItems(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1534,8 +1557,11 @@ func TestBackupWithInvalidActions(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1677,8 +1703,11 @@ func TestBackupActionModifications(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -1929,8 +1958,11 @@ func TestBackupActionAdditionalItems(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -2077,6 +2109,7 @@ func (*fakeVolumeSnapshotter) DeleteSnapshot(snapshotID string) error {
// looking at the backup request's VolumeSnapshots field. This test uses the fakeVolumeSnapshotter
// struct in place of real volume snapshotters.
func TestBackupWithSnapshots(t *testing.T) {
// TODO: add more verification for skippedPVTracker
tests := []struct {
name string
req *Request
Expand All @@ -2092,6 +2125,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2125,6 +2159,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2159,6 +2194,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2193,6 +2229,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2227,6 +2264,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2259,6 +2297,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand All @@ -2273,7 +2312,8 @@ func TestBackupWithSnapshots(t *testing.T) {
{
name: "backup with no volume snapshot locations does not create any snapshots",
req: &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand All @@ -2292,6 +2332,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand All @@ -2308,6 +2349,7 @@ func TestBackupWithSnapshots(t *testing.T) {
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
newSnapshotLocation("velero", "default", "default"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand All @@ -2327,6 +2369,7 @@ func TestBackupWithSnapshots(t *testing.T) {
newSnapshotLocation("velero", "default", "default"),
newSnapshotLocation("velero", "another", "another"),
},
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.PVs(
Expand Down Expand Up @@ -2455,7 +2498,8 @@ func TestBackupWithAsyncOperations(t *testing.T) {
{
name: "action that starts a short-running process records operation",
req: &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.Pods(
Expand Down Expand Up @@ -2484,7 +2528,8 @@ func TestBackupWithAsyncOperations(t *testing.T) {
{
name: "action that starts a long-running process records operation",
req: &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.Pods(
Expand Down Expand Up @@ -2513,7 +2558,8 @@ func TestBackupWithAsyncOperations(t *testing.T) {
{
name: "action that has no operation doesn't record one",
req: &Request{
Backup: defaultBackup().Result(),
Backup: defaultBackup().Result(),
SkippedPVTracker: NewSkipPVTracker(),
},
apiResources: []*test.APIResource{
test.Pods(
Expand Down Expand Up @@ -2592,8 +2638,11 @@ func TestBackupWithInvalidHooks(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -2840,8 +2889,11 @@ func TestBackupWithHooks(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
podCommandExecutor = new(test.MockPodCommandExecutor)
)
Expand Down Expand Up @@ -2881,20 +2933,21 @@ type fakePodVolumeBackupper struct{}

// BackupPodVolumes returns one pod volume backup per entry in volumes, with namespace "velero"
// and name "pvb-<pod-namespace>-<pod-name>-<volume-name>".
func (b *fakePodVolumeBackupper) BackupPodVolumes(backup *velerov1.Backup, pod *corev1.Pod, volumes []string, _ *resourcepolicies.Policies, _ logrus.FieldLogger) ([]*velerov1.PodVolumeBackup, []error) {
func (b *fakePodVolumeBackupper) BackupPodVolumes(backup *velerov1.Backup, pod *corev1.Pod, volumes []string, _ *resourcepolicies.Policies, _ logrus.FieldLogger) ([]*velerov1.PodVolumeBackup, *podvolume.PVCBackupSummary, []error) {
var res []*velerov1.PodVolumeBackup
pvcSummary := podvolume.NewPVCBackupSummary()

anno := pod.GetAnnotations()
if anno != nil && anno["backup.velero.io/bakupper-skip"] != "" {
return res, nil
return res, pvcSummary, nil
}

for _, vol := range volumes {
pvb := builder.ForPodVolumeBackup("velero", fmt.Sprintf("pvb-%s-%s-%s", pod.Namespace, pod.Name, vol)).Volume(vol).Result()
res = append(res, pvb)
}

return res, nil
return res, pvcSummary, nil
}

// TestBackupWithPodVolume runs backups of pods that are annotated for PodVolume backup,
Expand Down Expand Up @@ -3005,8 +3058,12 @@ func TestBackupWithPodVolume(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup, SnapshotLocations: []*velerov1.VolumeSnapshotLocation{tc.vsl}}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SnapshotLocations: []*velerov1.VolumeSnapshotLocation{tc.vsl},
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -4086,8 +4143,11 @@ func TestBackupNewResourceFiltering(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down Expand Up @@ -4230,8 +4290,11 @@ func TestBackupNamespaces(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var (
h = newHarness(t)
req = &Request{Backup: tc.backup}
h = newHarness(t)
req = &Request{
Backup: tc.backup,
SkippedPVTracker: NewSkipPVTracker(),
}
backupFile = bytes.NewBuffer([]byte{})
)

Expand Down
Loading

0 comments on commit 324642d

Please sign in to comment.