fix bug preventing PV snapshots from v0.10 backups from restoring

Signed-off-by: Steve Kriss <steve@heptio.com>
vmware-tanzu · Oct 23, 2018 · 195e6aa · 195e6aa
1 parent 9cda7ea
commit 195e6aa
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 8 deletions.
diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
@@ -259,6 +259,7 @@ func (kr *kubernetesRestorer) Restore(
 		resticRestorer:       resticRestorer,
 		pvsToProvision:       sets.NewString(),
 		pvRestorer:           pvRestorer,
+		volumeSnapshots:      volumeSnapshots,
 	}
 
 	return restoreCtx.execute()
@@ -343,6 +344,7 @@ type context struct {
 	resourceWatches      []watch.Interface
 	pvsToProvision       sets.String
 	pvRestorer           PVRestorer
+	volumeSnapshots      []*volume.Snapshot
 }
 
 func (ctx *context) execute() (api.RestoreResult, api.RestoreResult) {
@@ -671,13 +673,24 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a
 		}
 
 		if groupResource == kuberesource.PersistentVolumes {
-			_, found := ctx.backup.Status.VolumeBackups[name]
-			reclaimPolicy, err := collections.GetString(obj.Object, "spec.persistentVolumeReclaimPolicy")
-			if err == nil && !found && reclaimPolicy == "Delete" {
-				ctx.log.Infof("Not restoring PV because it doesn't have a snapshot and its reclaim policy is Delete.")
+			var hasSnapshot bool
 
-				ctx.pvsToProvision.Insert(name)
+			if len(ctx.backup.Status.VolumeBackups) > 0 {
+				// pre-v0.10 backup
+				_, hasSnapshot = ctx.backup.Status.VolumeBackups[name]
+			} else {
+				// v0.10+ backup
+				for _, snapshot := range ctx.volumeSnapshots {
+					if snapshot.Spec.PersistentVolumeName == name {
+						hasSnapshot = true
+						break
+					}
+				}
+			}
 
+			if !hasSnapshot && hasDeleteReclaimPolicy(obj.Object) {
+				ctx.log.Infof("Not restoring PV because it doesn't have a snapshot and its reclaim policy is Delete.")
+				ctx.pvsToProvision.Insert(name)
 				continue
 			}
 
@@ -859,6 +872,15 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a
 	return warnings, errs
 }
 
+func hasDeleteReclaimPolicy(obj map[string]interface{}) bool {
+	reclaimPolicy, err := collections.GetString(obj, "spec.persistentVolumeReclaimPolicy")
+	if err != nil {
+		return false
+	}
+
+	return reclaimPolicy == "Delete"
+}
+
 func waitForReady(
 	watchChan <-chan watch.Event,
 	name string,

diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
@@ -813,21 +813,40 @@ status:
 	tests := []struct {
 		name                          string
 		haveSnapshot                  bool
+		legacyBackup                  bool
 		reclaimPolicy                 string
 		expectPVCVolumeName           bool
 		expectedPVCAnnotationsMissing sets.String
 		expectPVCreation              bool
 	}{
 		{
-			name:                "have snapshot, reclaim policy delete",
+			name:                "legacy backup, have snapshot, reclaim policy delete",
 			haveSnapshot:        true,
+			legacyBackup:        true,
 			reclaimPolicy:       "Delete",
 			expectPVCVolumeName: true,
 			expectPVCreation:    true,
 		},
 		{
-			name:                "have snapshot, reclaim policy retain",
+			name:                "non-legacy backup, have snapshot, reclaim policy delete",
 			haveSnapshot:        true,
+			legacyBackup:        false,
+			reclaimPolicy:       "Delete",
+			expectPVCVolumeName: true,
+			expectPVCreation:    true,
+		},
+		{
+			name:                "legacy backup, have snapshot, reclaim policy retain",
+			haveSnapshot:        true,
+			legacyBackup:        true,
+			reclaimPolicy:       "Retain",
+			expectPVCVolumeName: true,
+			expectPVCreation:    true,
+		},
+		{
+			name:                "non-legacy backup, have snapshot, reclaim policy retain",
+			haveSnapshot:        true,
+			legacyBackup:        false,
 			reclaimPolicy:       "Retain",
 			expectPVCVolumeName: true,
 			expectPVCreation:    true,
@@ -880,7 +899,7 @@ status:
 			require.NoError(t, err)
 
 			backup := &api.Backup{}
-			if test.haveSnapshot {
+			if test.haveSnapshot && test.legacyBackup {
 				backup.Status.VolumeBackups = map[string]*api.VolumeBackupInfo{
 					"pvc-6a74b5af-78a5-11e8-a0d8-e2ad1e9734ce": {
 						SnapshotID: "snap",
@@ -914,6 +933,17 @@ status:
 				pvRestorer:     pvRestorer,
 			}
 
+			if test.haveSnapshot && !test.legacyBackup {
+				ctx.volumeSnapshots = append(ctx.volumeSnapshots, &volume.Snapshot{
+					Spec: volume.SnapshotSpec{
+						PersistentVolumeName: "pvc-6a74b5af-78a5-11e8-a0d8-e2ad1e9734ce",
+					},
+					Status: volume.SnapshotStatus{
+						ProviderSnapshotID: "snap",
+					},
+				})
+			}
+
 			pvWatch := new(mockWatch)
 			defer pvWatch.AssertExpectations(t)