From c9bc6646a567c689277225a760d934ab8f9fb11b Mon Sep 17 00:00:00 2001
From: Carlos Tadeu Panato Junior <ctadeu@gmail.com>
Date: Mon, 3 Feb 2020 19:47:18 +0100
Subject: [PATCH] use GA topology labels for PVs (#2219)

* use GA toplogy labels for PVs

Signed-off-by: Carlos Panato <ctadeu@gmail.com>
---
 pkg/backup/backup_test.go    | 70 +++++++++++++++++++++++++++++++++++-
 pkg/backup/item_backupper.go | 19 +++++++---
 2 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
index d93d85795..940fec405 100644
--- a/pkg/backup/backup_test.go
+++ b/pkg/backup/backup_test.go
@@ -1741,7 +1741,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 			},
 		},
 		{
-			name: "persistent volume with zone annotation creates a snapshot",
+			name: "persistent volume with deprecated zone annotation creates a snapshot",
 			req: &Request{
 				Backup: defaultBackup().Result(),
 				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
@@ -1774,6 +1774,74 @@ func TestBackupWithSnapshots(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "persistent volume with GA zone annotation creates a snapshot",
+			req: &Request{
+				Backup: defaultBackup().Result(),
+				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
+					newSnapshotLocation("velero", "default", "default"),
+				},
+			},
+			apiResources: []*test.APIResource{
+				test.PVs(
+					builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("topology.kubernetes.io/zone", "zone-1")).Result(),
+				),
+			},
+			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "zone-1", "type-1", 100, false),
+			},
+			want: []*volume.Snapshot{
+				{
+					Spec: volume.SnapshotSpec{
+						BackupName:           "backup-1",
+						Location:             "default",
+						PersistentVolumeName: "pv-1",
+						ProviderVolumeID:     "vol-1",
+						VolumeAZ:             "zone-1",
+						VolumeType:           "type-1",
+						VolumeIOPS:           int64Ptr(100),
+					},
+					Status: volume.SnapshotStatus{
+						Phase:              volume.SnapshotPhaseCompleted,
+						ProviderSnapshotID: "vol-1-snapshot",
+					},
+				},
+			},
+		},
+		{
+			name: "persistent volume with both GA and deprecated zone annotation creates a snapshot and should use the GA",
+			req: &Request{
+				Backup: defaultBackup().Result(),
+				SnapshotLocations: []*velerov1.VolumeSnapshotLocation{
+					newSnapshotLocation("velero", "default", "default"),
+				},
+			},
+			apiResources: []*test.APIResource{
+				test.PVs(
+					builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabelsMap(map[string]string{"failure-domain.beta.kubernetes.io/zone": "zone-1-deprecated", "topology.kubernetes.io/zone": "zone-1-ga"})).Result(),
+				),
+			},
+			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "zone-1-ga", "type-1", 100, false),
+			},
+			want: []*volume.Snapshot{
+				{
+					Spec: volume.SnapshotSpec{
+						BackupName:           "backup-1",
+						Location:             "default",
+						PersistentVolumeName: "pv-1",
+						ProviderVolumeID:     "vol-1",
+						VolumeAZ:             "zone-1-ga",
+						VolumeType:           "type-1",
+						VolumeIOPS:           int64Ptr(100),
+					},
+					Status: volume.SnapshotStatus{
+						Phase:              volume.SnapshotPhaseCompleted,
+						ProviderSnapshotID: "vol-1-snapshot",
+					},
+				},
+			},
+		},
 		{
 			name: "error returned from CreateSnapshot results in a failed snapshot",
 			req: &Request{
diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go
index d45fe5987..1f8342913 100644
--- a/pkg/backup/item_backupper.go
+++ b/pkg/backup/item_backupper.go
@@ -384,9 +384,14 @@ func (ib *defaultItemBackupper) volumeSnapshotter(snapshotLocation *api.VolumeSn
 	return bs, nil
 }
 
+// zoneLabelDeprecated is the label that stores availability-zone info
+// on PVs this is deprecated on Kubernetes >= 1.17.0
 // zoneLabel is the label that stores availability-zone info
 // on PVs
-const zoneLabel = "failure-domain.beta.kubernetes.io/zone"
+const (
+	zoneLabelDeprecated = "failure-domain.beta.kubernetes.io/zone"
+	zoneLabel           = "topology.kubernetes.io/zone"
+)
 
 // takePVSnapshot triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided
 // backup has volume snapshots enabled and the PV is of a compatible type. Also records cloud
@@ -415,9 +420,15 @@ func (ib *defaultItemBackupper) takePVSnapshot(obj runtime.Unstructured, log log
 		}
 	}
 
-	pvFailureDomainZone := pv.Labels[zoneLabel]
-	if pvFailureDomainZone == "" {
-		log.Infof("label %q is not present on PersistentVolume", zoneLabel)
+	// TODO: -- once failure-domain.beta.kubernetes.io/zone is no longer
+	// supported in any velero-supported version of Kubernetes, remove fallback checking of it
+	pvFailureDomainZone, labelFound := pv.Labels[zoneLabel]
+	if !labelFound {
+		log.Infof("label %q is not present on PersistentVolume, checking deprecated label...", zoneLabel)
+		pvFailureDomainZone, labelFound = pv.Labels[zoneLabelDeprecated]
+		if !labelFound {
+			log.Infof("label %q is not present on PersistentVolume", zoneLabelDeprecated)
+		}
 	}
 
 	var (