Merge pull request #4350 from reasonerjt/read-pv-az-new

Read Availability zone from nodeAffinity requirements

changelogs/unreleased/4350-reasonerjt

@@ -0,0 +1 @@
+Read Availability zone from nodeAffinity requirements 

pkg/backup/item_backupper.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	kubeerrs "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/vmware-tanzu/velero/internal/hook"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -395,7 +396,11 @@ func (ib *itemBackupper) volumeSnapshotter(snapshotLocation *velerov1api.VolumeS
 // on PVs
 const (
 	zoneLabelDeprecated = "failure-domain.beta.kubernetes.io/zone"
-	zoneLabel           = "topology.kubernetes.io/zone"
+	// this is reused for nodeAffinity requirements
+	zoneLabel = "topology.kubernetes.io/zone"
+
+	awsEbsCsiZoneKey = "topology.ebs.csi.aws.com/zone"
+	azureCsiZoneKey  = "topology.disk.csi.azure.com/zone"
 )
 
 // takePVSnapshot triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided
@@ -432,7 +437,14 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 		log.Infof("label %q is not present on PersistentVolume, checking deprecated label...", zoneLabel)
 		pvFailureDomainZone, labelFound = pv.Labels[zoneLabelDeprecated]
 		if !labelFound {
+			var k string
 			log.Infof("label %q is not present on PersistentVolume", zoneLabelDeprecated)
+			k, pvFailureDomainZone = zoneFromPVNodeAffinity(pv, awsEbsCsiZoneKey, azureCsiZoneKey, zoneLabel, zoneLabelDeprecated)
+			if pvFailureDomainZone != "" {
+				log.Infof("zone info from nodeAffinity requirements: %s, key: %s", pvFailureDomainZone, k)
+			} else {
+				log.Infof("zone info not available in nodeAffinity requirements")
+			}
 		}
 	}
 
@@ -535,3 +547,24 @@ func resourceVersion(obj runtime.Unstructured) string {
 	gvk := obj.GetObjectKind().GroupVersionKind()
 	return gvk.Version
 }
+
+// zoneFromPVNodeAffinity iterates the node affinity requirement of a PV to
+// get its availability zone, it returns the key merely for logging.
+func zoneFromPVNodeAffinity(res *corev1api.PersistentVolume, topologyKeys ...string) (string, string) {
+	nodeAffinity := res.Spec.NodeAffinity
+	if nodeAffinity == nil {
+		return "", ""
+	}
+	keySet := sets.NewString(topologyKeys...)
+	for _, term := range nodeAffinity.Required.NodeSelectorTerms {
+		if term.MatchExpressions == nil {
+			continue
+		}
+		for _, exp := range term.MatchExpressions {
+			if keySet.Has(exp.Key) && exp.Operator == "In" && len(exp.Values) > 0 {
+				return exp.Key, exp.Values[0]
+			}
+		}
+	}
+	return "", ""
+}

pkg/backup/item_backupper_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	corev1api "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -45,3 +46,84 @@ func Test_resourceKey(t *testing.T) {
 		})
 	}
 }
+
+func Test_zoneFromPVNodeAffinity(t *testing.T) {
+	keys := []string{
+		awsEbsCsiZoneKey,
+		azureCsiZoneKey,
+		zoneLabel,
+	}
+	tests := []struct {
+		name      string
+		pv        *corev1api.PersistentVolume
+		wantKey   string
+		wantValue string
+	}{
+		{
+			name: "AWS CSI Volume",
+			pv: builder.ForPersistentVolume("awscsi").NodeAffinityRequired(
+				builder.ForNodeSelector(
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.ebs.csi.aws.com/zone",
+						"In", "us-east-2c").Result(),
+				).Result(),
+			).Result(),
+			wantKey:   "topology.ebs.csi.aws.com/zone",
+			wantValue: "us-east-2c",
+		},
+		{
+			name: "Azure CSI Volume",
+			pv: builder.ForPersistentVolume("azurecsi").NodeAffinityRequired(
+				builder.ForNodeSelector(
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.disk.csi.azure.com/zone",
+						"In", "us-central").Result(),
+				).Result(),
+			).Result(),
+			wantKey:   "topology.disk.csi.azure.com/zone",
+			wantValue: "us-central",
+		},
+		{
+			name: "AWS CSI Volume with multiple zone value, returns the first",
+			pv: builder.ForPersistentVolume("awscsi").NodeAffinityRequired(
+				builder.ForNodeSelector(
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.ebs.csi.aws.com/zone",
+						"In", "us-east-2c", "us-west").Result(),
+				).Result(),
+			).Result(),
+			wantKey:   "topology.ebs.csi.aws.com/zone",
+			wantValue: "us-east-2c",
+		},
+		{
+			name: "Volume with no matching key",
+			pv: builder.ForPersistentVolume("no-matching-pv").NodeAffinityRequired(
+				builder.ForNodeSelector(
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("some-key",
+						"In", "us-west").Result(),
+				).Result(),
+			).Result(),
+			wantKey:   "",
+			wantValue: "",
+		},
+		{
+			name: "Volume with multiple valid keys, returns the first match", // it should never happen
+			pv: builder.ForPersistentVolume("multi-matching-pv").NodeAffinityRequired(
+				builder.ForNodeSelector(
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.disk.csi.azure.com/zone",
+						"In", "us-central").Result(),
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.ebs.csi.aws.com/zone",
+						"In", "us-east-2c", "us-west").Result(),
+					*builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.ebs.csi.aws.com/zone",
+						"In", "unknown").Result(),
+				).Result(),
+			).Result(),
+			wantKey:   "topology.disk.csi.azure.com/zone",
+			wantValue: "us-central",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			k, v := zoneFromPVNodeAffinity(tt.pv, keys...)
+			assert.Equal(t, tt.wantKey, k)
+			assert.Equal(t, tt.wantValue, v)
+		})
+	}
+}

pkg/builder/node_selector_builder.go

@@ -0,0 +1,64 @@
+package builder
+
+import corev1api "k8s.io/api/core/v1"
+
+// NodeSelectorBuilder builds NodeSelector objects
+type NodeSelectorBuilder struct {
+	object *corev1api.NodeSelector
+}
+
+// ForNodeSelector returns the NodeSelectorBuilder instance with given terms
+func ForNodeSelector(term ...corev1api.NodeSelectorTerm) *NodeSelectorBuilder {
+	return &NodeSelectorBuilder{
+		object: &corev1api.NodeSelector{
+			NodeSelectorTerms: term,
+		},
+	}
+}
+
+// Result returns the built NodeSelector
+func (b *NodeSelectorBuilder) Result() *corev1api.NodeSelector {
+	return b.object
+}
+
+// NodeSelectorTermBuilder builds NodeSelectorTerm objects.
+type NodeSelectorTermBuilder struct {
+	object *corev1api.NodeSelectorTerm
+}
+
+// NewNodeSelectorTermBuilder initializes an instance of NodeSelectorTermBuilder
+func NewNodeSelectorTermBuilder() *NodeSelectorTermBuilder {
+	return &NodeSelectorTermBuilder{
+		object: &corev1api.NodeSelectorTerm{
+			MatchExpressions: make([]corev1api.NodeSelectorRequirement, 0),
+			MatchFields:      make([]corev1api.NodeSelectorRequirement, 0),
+		},
+	}
+}
+
+// WithMatchExpression appends the MatchExpression to the NodeSelectorTerm
+func (ntb *NodeSelectorTermBuilder) WithMatchExpression(key string, op string, values ...string) *NodeSelectorTermBuilder {
+	req := corev1api.NodeSelectorRequirement{
+		Key:      key,
+		Operator: corev1api.NodeSelectorOperator(op),
+		Values:   values,
+	}
+	ntb.object.MatchExpressions = append(ntb.object.MatchExpressions, req)
+	return ntb
+}
+
+// WithMatchField appends the MatchField to the NodeSelectorTerm
+func (ntb *NodeSelectorTermBuilder) WithMatchField(key string, op string, values ...string) *NodeSelectorTermBuilder {
+	req := corev1api.NodeSelectorRequirement{
+		Key:      key,
+		Operator: corev1api.NodeSelectorOperator(op),
+		Values:   values,
+	}
+	ntb.object.MatchFields = append(ntb.object.MatchFields, req)
+	return ntb
+}
+
+// Result returns the built NodeSelectorTerm
+func (ntb *NodeSelectorTermBuilder) Result() *corev1api.NodeSelectorTerm {
+	return ntb.object
+}

pkg/builder/persistent_volume_builder.go

@@ -94,3 +94,11 @@ func (b *PersistentVolumeBuilder) StorageClass(name string) *PersistentVolumeBui
 	b.object.Spec.StorageClassName = name
 	return b
 }
+
+// NodeAffinityRequired sets the PersistentVolume's NodeAffinity Requirement.
+func (b *PersistentVolumeBuilder) NodeAffinityRequired(req *corev1api.NodeSelector) *PersistentVolumeBuilder {
+	b.object.Spec.NodeAffinity = &corev1api.VolumeNodeAffinity{
+		Required: req,
+	}
+	return b
+}