From 5878a528435d8d7ef308b39fe1542d7c7d7d670b Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Thu, 11 Nov 2021 14:54:31 +0800 Subject: [PATCH] Read Availability zone from nodeAffinity requirements Velero to read the AZ info from `NodeAffinity` of a PV when it's taking the snapshot. Fixes #4332 Signed-off-by: Daniel Jiang --- changelogs/unreleased/4350-reasonerjt | 1 + pkg/backup/item_backupper.go | 35 +++++++++- pkg/backup/item_backupper_test.go | 82 ++++++++++++++++++++++++ pkg/builder/node_selector_builder.go | 64 ++++++++++++++++++ pkg/builder/persistent_volume_builder.go | 8 +++ 5 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/4350-reasonerjt create mode 100644 pkg/builder/node_selector_builder.go diff --git a/changelogs/unreleased/4350-reasonerjt b/changelogs/unreleased/4350-reasonerjt new file mode 100644 index 000000000..7fb91ec87 --- /dev/null +++ b/changelogs/unreleased/4350-reasonerjt @@ -0,0 +1 @@ +Read Availability zone from nodeAffinity requirements \ No newline at end of file diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 4f5c7ca8b..72be1bd7f 100644 --- a/pkg/backup/item_backupper.go +++ b/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 "", "" +} diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 192b0c585..580207a0f 100644 --- a/pkg/backup/item_backupper_test.go +++ b/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) + }) + } +} diff --git a/pkg/builder/node_selector_builder.go b/pkg/builder/node_selector_builder.go new file mode 100644 index 000000000..11ce306b5 --- /dev/null +++ b/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 +} diff --git a/pkg/builder/persistent_volume_builder.go b/pkg/builder/persistent_volume_builder.go index 648778bae..5fee88c19 100644 --- a/pkg/builder/persistent_volume_builder.go +++ b/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 +}