From 4c194d52da26fc99a02e16515312713118610e47 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 14 Aug 2019 14:35:42 -0400 Subject: [PATCH] kubelet: set both deprecated Beta and GA labels for zone/region topology from the cloud provider Signed-off-by: Andrew Sy Kim --- pkg/kubelet/apis/well_known_labels.go | 9 +- pkg/kubelet/kubelet_node_status.go | 6 + pkg/kubelet/kubelet_node_status_test.go | 218 ++++---- pkg/volume/awsebs/aws_ebs.go | 17 +- pkg/volume/awsebs/aws_ebs_test.go | 21 +- pkg/volume/azure_dd/azure_provision.go | 31 +- pkg/volume/cinder/cinder.go | 16 +- pkg/volume/cinder/cinder_test.go | 22 +- pkg/volume/cinder/cinder_util.go | 9 + pkg/volume/gcepd/attacher_test.go | 3 +- pkg/volume/gcepd/gce_pd.go | 33 +- pkg/volume/gcepd/gce_pd_test.go | 25 +- pkg/volume/gcepd/gce_util.go | 6 +- pkg/volume/util/util_test.go | 225 +++++++++ pkg/volume/vsphere_volume/vsphere_volume.go | 34 +- .../noderestriction/admission_test.go | 4 +- .../persistentvolume/label/admission.go | 74 ++- .../persistentvolume/label/admission_test.go | 277 ++++++++-- .../cloud-provider/volume/helpers/zones.go | 111 +++- .../volume/helpers/zones_test.go | 476 +++++++++++++++++- .../csi-translation-lib/plugins/gce_pd.go | 11 +- .../plugins/gce_pd_test.go | 7 +- .../k8s.io/legacy-cloud-providers/aws/aws.go | 4 + .../legacy-cloud-providers/aws/aws_test.go | 13 +- .../legacy-cloud-providers/azure/azure.go | 12 +- .../azure/azure_managedDiskController.go | 6 +- .../k8s.io/legacy-cloud-providers/gce/gce.go | 12 +- .../legacy-cloud-providers/gce/gce_disks.go | 10 +- .../gce/gce_disks_test.go | 26 + .../gce/gce_instances.go | 14 +- .../openstack/openstack_volumes.go | 2 + .../vsphere/nodemanager.go | 32 +- .../legacy-cloud-providers/vsphere/vsphere.go | 2 + .../storage/vsphere/vsphere_zone_support.go | 2 +- 34 files changed, 1483 insertions(+), 287 deletions(-) diff --git a/pkg/kubelet/apis/well_known_labels.go b/pkg/kubelet/apis/well_known_labels.go index b473e524956..2f29f8d3d95 100644 --- a/pkg/kubelet/apis/well_known_labels.go +++ b/pkg/kubelet/apis/well_known_labels.go @@ -35,15 +35,14 @@ const ( // TODO: stop applying the beta Arch labels in Kubernetes 1.18. LabelArch = "beta.kubernetes.io/arch" - // GA versions of the legacy beta labels. // TODO: update kubelet and controllers to set both beta and GA labels, then export these constants - labelZoneFailureDomainGA = "failure-domain.kubernetes.io/zone" - labelZoneRegionGA = "failure-domain.kubernetes.io/region" - labelInstanceTypeGA = "kubernetes.io/instance-type" + labelInstanceTypeGA = "kubernetes.io/instance-type" ) var kubeletLabels = sets.NewString( v1.LabelHostname, + v1.LabelZoneFailureDomainStable, + v1.LabelZoneRegionStable, v1.LabelZoneFailureDomain, v1.LabelZoneRegion, v1.LabelInstanceType, @@ -53,8 +52,6 @@ var kubeletLabels = sets.NewString( LabelOS, LabelArch, - labelZoneFailureDomainGA, - labelZoneRegionGA, labelInstanceTypeGA, ) diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 6f12658b96e..bdec539b837 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -150,6 +150,8 @@ func (kl *Kubelet) reconcileExtendedResource(initialNode, node *v1.Node) bool { func (kl *Kubelet) updateDefaultLabels(initialNode, existingNode *v1.Node) bool { defaultLabels := []string{ v1.LabelHostname, + v1.LabelZoneFailureDomainStable, + v1.LabelZoneRegionStable, v1.LabelZoneFailureDomain, v1.LabelZoneRegion, v1.LabelInstanceType, @@ -342,10 +344,14 @@ func (kl *Kubelet) initialNode(ctx context.Context) (*v1.Node, error) { if zone.FailureDomain != "" { klog.Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneFailureDomain, zone.FailureDomain) node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] = zone.FailureDomain + klog.Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneFailureDomainStable, zone.FailureDomain) + node.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] = zone.FailureDomain } if zone.Region != "" { klog.Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneRegion, zone.Region) node.ObjectMeta.Labels[v1.LabelZoneRegion] = zone.Region + klog.Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneRegionStable, zone.Region) + node.ObjectMeta.Labels[v1.LabelZoneRegionStable] = zone.Region } } } diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index 956f38703e2..4c66b2c00b9 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -1523,12 +1523,14 @@ func TestUpdateDefaultLabels(t *testing.T) { initialNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, @@ -1539,12 +1541,14 @@ func TestUpdateDefaultLabels(t *testing.T) { }, needsUpdate: true, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, { @@ -1552,35 +1556,41 @@ func TestUpdateDefaultLabels(t *testing.T) { initialNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, existingNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "old-hostname", - v1.LabelZoneFailureDomain: "old-zone-failure-domain", - v1.LabelZoneRegion: "old-zone-region", - v1.LabelInstanceType: "old-instance-type", - kubeletapis.LabelOS: "old-os", - kubeletapis.LabelArch: "old-arch", + v1.LabelHostname: "old-hostname", + v1.LabelZoneFailureDomainStable: "old-zone-failure-domain", + v1.LabelZoneRegionStable: "old-zone-region", + v1.LabelZoneFailureDomain: "old-zone-failure-domain", + v1.LabelZoneRegion: "old-zone-region", + v1.LabelInstanceType: "old-instance-type", + kubeletapis.LabelOS: "old-os", + kubeletapis.LabelArch: "old-arch", }, }, }, needsUpdate: true, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, { @@ -1588,37 +1598,43 @@ func TestUpdateDefaultLabels(t *testing.T) { initialNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, existingNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", - "please-persist": "foo", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", }, }, }, needsUpdate: false, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", - "please-persist": "foo", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", }, }, { @@ -1631,25 +1647,29 @@ func TestUpdateDefaultLabels(t *testing.T) { existingNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", - "please-persist": "foo", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", }, }, }, needsUpdate: false, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", - "please-persist": "foo", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", }, }, { @@ -1657,35 +1677,41 @@ func TestUpdateDefaultLabels(t *testing.T) { initialNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, existingNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, needsUpdate: false, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, { @@ -1693,12 +1719,14 @@ func TestUpdateDefaultLabels(t *testing.T) { initialNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, }, @@ -1707,12 +1735,14 @@ func TestUpdateDefaultLabels(t *testing.T) { }, needsUpdate: true, finalLabels: map[string]string{ - v1.LabelHostname: "new-hostname", - v1.LabelZoneFailureDomain: "new-zone-failure-domain", - v1.LabelZoneRegion: "new-zone-region", - v1.LabelInstanceType: "new-instance-type", - kubeletapis.LabelOS: "new-os", - kubeletapis.LabelArch: "new-arch", + v1.LabelHostname: "new-hostname", + v1.LabelZoneFailureDomainStable: "new-zone-failure-domain", + v1.LabelZoneRegionStable: "new-zone-region", + v1.LabelZoneFailureDomain: "new-zone-failure-domain", + v1.LabelZoneRegion: "new-zone-region", + v1.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", }, }, } diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index 4885d991cef..f2cc3582725 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/pkg/volume/awsebs/aws_ebs.go @@ -35,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilfeature "k8s.io/apiserver/pkg/util/feature" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" @@ -519,7 +520,7 @@ func (c *awsElasticBlockStoreProvisioner) Provision(selectedNode *v1.Node, allow pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: c.options.PVName, - Labels: map[string]string{}, + Labels: labels, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "aws-ebs-dynamic-provisioner", }, @@ -547,21 +548,9 @@ func (c *awsElasticBlockStoreProvisioner) Provision(selectedNode *v1.Node, allow pv.Spec.AccessModes = c.plugin.GetAccessModes() } - requirements := make([]v1.NodeSelectorRequirement, 0) - if len(labels) != 0 { - if pv.Labels == nil { - pv.Labels = make(map[string]string) - } - for k, v := range labels { - pv.Labels[k] = v - requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) - } - } - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) return pv, nil } diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index e10f5273362..fa8387f85ce 100644 --- a/pkg/volume/awsebs/aws_ebs_test.go +++ b/pkg/volume/awsebs/aws_ebs_test.go @@ -207,7 +207,7 @@ func TestPlugin(t *testing.T) { } n := len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) - if n != 1 { + if n != 2 { t.Errorf("Provision() returned unexpected number of NodeSelectorTerms %d. Expected %d", n, 1) } @@ -228,6 +228,25 @@ func TestPlugin(t *testing.T) { if len(req.Values) != 1 || req.Values[0] != "yes" { t.Errorf("Provision() returned unexpected requirement value in NodeAffinity %v", req.Values) } + + n = len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[1].MatchExpressions) + if n != 1 { + t.Errorf("Provision() returned unexpected number of MatchExpressions %d. Expected %d", n, 1) + } + + req = persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[1].MatchExpressions[0] + if req.Key != "fakepdmanager" { + t.Errorf("Provision() returned unexpected requirement key in NodeAffinity %v", req.Key) + } + + if req.Operator != v1.NodeSelectorOpIn { + t.Errorf("Provision() returned unexpected requirement operator in NodeAffinity %v", req.Operator) + } + + if len(req.Values) != 1 || req.Values[0] != "yes" { + t.Errorf("Provision() returned unexpected requirement value in NodeAffinity %v", req.Values) + } + // Test Deleter volSpec := &volume.Spec{ PersistentVolume: persistentSpec, diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index 6936e40428d..038dbec4281 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -322,22 +322,16 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie if zoned { // Set node affinity labels based on availability zone labels. if len(labels) > 0 { - requirements := make([]v1.NodeSelectorRequirement, 0) - for k, v := range labels { - requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) - } - - nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ - MatchExpressions: requirements, - }) + nodeSelectorTerms = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) } + } else { // Set node affinity labels based on fault domains. // This is required because unzoned AzureDisk can't be attached to zoned nodes. // There are at most 3 fault domains available in each region. // Refer https://docs.microsoft.com/en-us/azure/virtual-machines/windows/manage-availability. for i := 0; i < 3; i++ { - requirements := []v1.NodeSelectorRequirement{ + deprecatedTopologyReqs := []v1.NodeSelectorRequirement{ { Key: v1.LabelZoneRegion, Operator: v1.NodeSelectorOpIn, @@ -349,8 +343,25 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie Values: []string{strconv.Itoa(i)}, }, } + + topologyReqs := []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneRegionStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{diskController.GetLocation()}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{strconv.Itoa(i)}, + }, + } + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ - MatchExpressions: requirements, + MatchExpressions: deprecatedTopologyReqs, + }) + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ + MatchExpressions: topologyReqs, }) } } diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index b95929a82c2..c71e6c951be 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilfeature "k8s.io/apiserver/pkg/util/feature" cloudprovider "k8s.io/cloud-provider" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/klog" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/mount" @@ -618,18 +619,9 @@ func (c *cinderVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTopolo pv.Spec.AccessModes = c.plugin.GetAccessModes() } - requirements := make([]v1.NodeSelectorRequirement, 0) - for k, v := range labels { - if v != "" { - requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) - } - } - if len(requirements) > 0 { - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements - } + pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) + pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) return pv, nil } diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index 7926ed4a3b3..1c81ab45ebb 100644 --- a/pkg/volume/cinder/cinder_test.go +++ b/pkg/volume/cinder/cinder_test.go @@ -123,6 +123,7 @@ func (fake *fakePDManager) DetachDisk(c *cinderVolumeUnmounter) error { func (fake *fakePDManager) CreateVolume(c *cinderVolumeProvisioner, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm) (volumeID string, volumeSizeGB int, labels map[string]string, fstype string, err error) { labels = make(map[string]string) labels[v1.LabelZoneFailureDomain] = "nova" + labels[v1.LabelZoneFailureDomainStable] = "nova" return "test-volume-name", 1, labels, "", nil } @@ -229,7 +230,7 @@ func TestPlugin(t *testing.T) { } n := len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) - if n != 1 { + if n != 2 { t.Errorf("Provision() returned unexpected number of NodeSelectorTerms %d. Expected %d", n, 1) } @@ -252,6 +253,25 @@ func TestPlugin(t *testing.T) { t.Errorf("Provision() returned unexpected requirement value in NodeAffinity %v", req.Values) } + n = len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[1].MatchExpressions) + if n != 1 { + t.Errorf("Provision() returned unexpected number of MatchExpressions %d. Expected %d", n, 1) + } + + req = persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[1].MatchExpressions[0] + + if req.Key != v1.LabelZoneFailureDomainStable { + t.Errorf("Provision() returned unexpected requirement key in NodeAffinity %v", req.Key) + } + + if req.Operator != v1.NodeSelectorOpIn { + t.Errorf("Provision() returned unexpected requirement operator in NodeAffinity %v", req.Operator) + } + + if len(req.Values) != 1 || req.Values[0] != "nova" { + t.Errorf("Provision() returned unexpected requirement value in NodeAffinity %v", req.Values) + } + // Test Deleter volSpec := &volume.Spec{ PersistentVolume: persistentSpec, diff --git a/pkg/volume/cinder/cinder_util.go b/pkg/volume/cinder/cinder_util.go index 60092dfc52a..fc99299e611 100644 --- a/pkg/volume/cinder/cinder_util.go +++ b/pkg/volume/cinder/cinder_util.go @@ -157,6 +157,12 @@ func getZonesFromNodes(kubeClient clientset.Interface) (sets.String, error) { for _, node := range nodes.Items { if zone, ok := node.Labels[v1.LabelZoneFailureDomain]; ok { zones.Insert(zone) + // don't need to check GA label if beta label already exists + continue + } + + if zone, ok := node.Labels[v1.LabelZoneFailureDomainStable]; ok { + zones.Insert(zone) } } klog.V(4).Infof("zones found: %v", zones) @@ -229,9 +235,12 @@ func (util *DiskUtil) CreateVolume(c *cinderVolumeProvisioner, node *v1.Node, al if IgnoreVolumeAZ == false { if volumeAZ != "" { volumeLabels[v1.LabelZoneFailureDomain] = volumeAZ + volumeLabels[v1.LabelZoneFailureDomainStable] = volumeAZ } + if volumeRegion != "" { volumeLabels[v1.LabelZoneRegion] = volumeRegion + volumeLabels[v1.LabelZoneRegionStable] = volumeRegion } } return volumeID, volSizeGiB, volumeLabels, fstype, nil diff --git a/pkg/volume/gcepd/attacher_test.go b/pkg/volume/gcepd/attacher_test.go index 43a7126ae74..3ed3c8295ec 100644 --- a/pkg/volume/gcepd/attacher_test.go +++ b/pkg/volume/gcepd/attacher_test.go @@ -457,7 +457,8 @@ func createPVSpec(name string, readOnly bool, zones []string) *volume.Spec { if zones != nil { zonesLabel := strings.Join(zones, cloudvolume.LabelMultiZoneDelimiter) spec.PersistentVolume.ObjectMeta.Labels = map[string]string{ - v1.LabelZoneFailureDomain: zonesLabel, + v1.LabelZoneFailureDomain: zonesLabel, + v1.LabelZoneFailureDomainStable: zonesLabel, } } diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index 9f770cdf69d..738eea6a57c 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/pkg/volume/gcepd/gce_pd.go @@ -515,7 +515,7 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: c.options.PVName, - Labels: map[string]string{}, + Labels: labels, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "gce-pd-dynamic-provisioner", }, @@ -542,32 +542,15 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT pv.Spec.AccessModes = c.plugin.GetAccessModes() } - requirements := make([]v1.NodeSelectorRequirement, 0) - if len(labels) != 0 { - if pv.Labels == nil { - pv.Labels = make(map[string]string) - } - for k, v := range labels { - pv.Labels[k] = v - var values []string - if k == v1.LabelZoneFailureDomain { - values, err = volumehelpers.LabelZonesToList(v) - if err != nil { - return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err) - } - } else { - values = []string{v} - } - requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) - } + pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) + pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) + + nodeSelectorTerms, err := volumehelpers.TranslateZoneLabelsToNodeSelectorTerms(labels) + if err != nil { + return nil, err } - if len(requirements) > 0 { - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements - } + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = nodeSelectorTerms return pv, nil } diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index e2bd930d555..8cdcedc6b23 100644 --- a/pkg/volume/gcepd/gce_pd_test.go +++ b/pkg/volume/gcepd/gce_pd_test.go @@ -86,6 +86,7 @@ func (fake *fakePDManager) CreateVolume(c *gcePersistentDiskProvisioner, node *v labels = make(map[string]string) labels["fakepdmanager"] = "yes" labels[v1.LabelZoneFailureDomain] = "zone1__zone2" + labels[v1.LabelZoneFailureDomainStable] = "zone1__zone2" return "test-gce-volume-name", 100, labels, "", nil } @@ -203,10 +204,14 @@ func TestPlugin(t *testing.T) { t.Errorf("Provision() returned unexpected value for %s: %v", v1.LabelZoneFailureDomain, persistentSpec.Labels[v1.LabelZoneFailureDomain]) } + if persistentSpec.Labels[v1.LabelZoneFailureDomainStable] != "zone1__zone2" { + t.Errorf("Provision() returned unexpected value for %s: %v", v1.LabelZoneFailureDomainStable, persistentSpec.Labels[v1.LabelZoneFailureDomainStable]) + } + if persistentSpec.Spec.NodeAffinity == nil { t.Errorf("Unexpected nil NodeAffinity found") } - if len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) != 1 { + if len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) != 2 { t.Errorf("Unexpected number of NodeSelectorTerms") } term := persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[0] @@ -227,6 +232,24 @@ func TestPlugin(t *testing.T) { t.Errorf("ZoneFailureDomain elements %v does not match zone labels %v", r.Values, zones) } + term = persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[1] + if len(term.MatchExpressions) != 2 { + t.Errorf("Unexpected number of NodeSelectorRequirements in volume NodeAffinity: %d", len(term.MatchExpressions)) + } + r, _ = getNodeSelectorRequirementWithKey("fakepdmanager", term) + if r == nil || r.Values[0] != "yes" || r.Operator != v1.NodeSelectorOpIn { + t.Errorf("NodeSelectorRequirement fakepdmanager-in-yes not found in volume NodeAffinity") + } + zones, _ = volumehelpers.ZonesToSet("zone1,zone2") + r, _ = getNodeSelectorRequirementWithKey(v1.LabelZoneFailureDomainStable, term) + if r == nil { + t.Errorf("NodeSelectorRequirement %s-in-%v not found in volume NodeAffinity", v1.LabelZoneFailureDomainStable, zones) + } + sort.Strings(r.Values) + if !reflect.DeepEqual(r.Values, zones.List()) { + t.Errorf("ZoneFailureDomain elements %v does not match zone labels %v", r.Values, zones) + } + // Test Deleter volSpec := &volume.Spec{ PersistentVolume: persistentSpec, diff --git a/pkg/volume/gcepd/gce_util.go b/pkg/volume/gcepd/gce_util.go index 146b208001d..074f7ba84ef 100644 --- a/pkg/volume/gcepd/gce_util.go +++ b/pkg/volume/gcepd/gce_util.go @@ -349,7 +349,11 @@ func udevadmChangeToDrive(drivePath string) error { // Checks whether the given GCE PD volume spec is associated with a regional PD. func isRegionalPD(spec *volume.Spec) bool { if spec.PersistentVolume != nil { - zonesLabel := spec.PersistentVolume.Labels[v1.LabelZoneFailureDomain] + zonesLabel, ok := spec.PersistentVolume.Labels[v1.LabelZoneFailureDomain] + if !ok { + zonesLabel = spec.PersistentVolume.Labels[v1.LabelZoneFailureDomainStable] + } + zones := strings.Split(zonesLabel, cloudvolume.LabelMultiZoneDelimiter) return len(zones) > 1 } diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 14066bffe55..dd3980af2f3 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -223,6 +223,231 @@ func TestCheckVolumeNodeAffinity(t *testing.T) { } } +func Test_CheckNodeAffinityWithTopology(t *testing.T) { + tests := []struct { + name string + pv *v1.PersistentVolume + nodeLabels map[string]string + expectSuccess bool + }{ + { + name: "PV has only beta label, node has beta/GA label -- zone only", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + v1.LabelZoneFailureDomainStable: "zone", + }, + expectSuccess: true, + }, + { + name: "PV has only beta label, node has beta/GA label -- zone and region", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + v1.LabelZoneFailureDomainStable: "zone", + v1.LabelZoneRegion: "region", + v1.LabelZoneRegionStable: "region", + }, + expectSuccess: true, + }, + { + name: "PV has both beta/GA label, node has only beta -- zone only", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + }, + expectSuccess: true, + }, + { + name: "PV has both beta/GA label, node has only beta -- zone and region", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + { + Key: v1.LabelZoneRegionStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + v1.LabelZoneRegion: "region", + }, + expectSuccess: true, + }, + { + name: "PV has both beta/GA label, node has both beta/GA label -- zone only", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + v1.LabelZoneFailureDomainStable: "zone", + }, + expectSuccess: true, + }, + { + name: "PV has both beta/GA label, node has both beta/GA label -- zone and region", + pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone"}, + }, + { + Key: v1.LabelZoneRegionStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region"}, + }, + }, + }, + }, + }, + }), + nodeLabels: map[string]string{ + v1.LabelZoneFailureDomain: "zone", + v1.LabelZoneRegion: "region", + v1.LabelZoneFailureDomainStable: "zone", + v1.LabelZoneRegionStable: "region", + }, + expectSuccess: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := CheckNodeAffinity(test.pv, test.nodeLabels) + + if err != nil && test.expectSuccess { + t.Errorf("CheckTopology returned error: %v", err) + } + if err == nil && !test.expectSuccess { + t.Error("CheckTopology returned success, expected error") + } + }) + } +} + func testVolumeWithNodeAffinity(t *testing.T, affinity *v1.VolumeNodeAffinity) *v1.PersistentVolume { objMeta := metav1.ObjectMeta{Name: "test-constraints"} return &v1.PersistentVolume{ diff --git a/pkg/volume/vsphere_volume/vsphere_volume.go b/pkg/volume/vsphere_volume/vsphere_volume.go index b6de7fba54a..7301da0f8d5 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume.go +++ b/pkg/volume/vsphere_volume/vsphere_volume.go @@ -403,7 +403,7 @@ func (v *vsphereVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTopol pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: v.options.PVName, - Labels: map[string]string{}, + Labels: volSpec.Labels, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "vsphere-volume-dynamic-provisioner", }, @@ -430,33 +430,15 @@ func (v *vsphereVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTopol pv.Spec.AccessModes = v.plugin.GetAccessModes() } - labels := volSpec.Labels - requirements := make([]v1.NodeSelectorRequirement, 0) - if len(labels) != 0 { - if pv.Labels == nil { - pv.Labels = make(map[string]string) - } - for k, v := range labels { - pv.Labels[k] = v - var values []string - if k == v1.LabelZoneFailureDomain { - values, err = volumehelpers.LabelZonesToList(v) - if err != nil { - return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err) - } - } else { - values = []string{v} - } - requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) - } + pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) + pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) + + nodeSelectorTerms, err := volumehelpers.TranslateZoneLabelsToNodeSelectorTerms(volSpec.Labels) + if err != nil { + return nil, err } - if len(requirements) > 0 { - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements - } + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = nodeSelectorTerms return pv, nil } diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 3e5170ccb41..551dee24f6e 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -163,11 +163,11 @@ func setAllowedUpdateLabels(node *api.Node, value string) *api.Node { node.Labels["kubernetes.io/hostname"] = value node.Labels["failure-domain.beta.kubernetes.io/zone"] = value node.Labels["failure-domain.beta.kubernetes.io/region"] = value + node.Labels["topology.kubernetes.io/zone"] = value + node.Labels["topology.kubernetes.io/region"] = value node.Labels["beta.kubernetes.io/instance-type"] = value node.Labels["beta.kubernetes.io/os"] = value node.Labels["beta.kubernetes.io/arch"] = value - node.Labels["failure-domain.kubernetes.io/zone"] = value - node.Labels["failure-domain.kubernetes.io/region"] = value node.Labels["kubernetes.io/instance-type"] = value node.Labels["kubernetes.io/os"] = value node.Labels["kubernetes.io/arch"] = value diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go index 62fe2b3a5fe..a18135a87d4 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "reflect" "sync" v1 "k8s.io/api/core/v1" @@ -117,11 +118,13 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute return admission.NewForbidden(a, err) } - requirements := make([]api.NodeSelectorRequirement, 0) if len(volumeLabels) != 0 { if volume.Labels == nil { volume.Labels = make(map[string]string) } + + deprecatedTopologyReqs := make([]api.NodeSelectorRequirement, 0) + topologyReqs := make([]api.NodeSelectorRequirement, 0) for k, v := range volumeLabels { // We (silently) replace labels if they are provided. // This should be OK because they are in the kubernetes.io namespace @@ -129,8 +132,11 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute volume.Labels[k] = v // Set NodeSelectorRequirements based on the labels + // + // we currently set both beta (failure-domain.beta.kubernetes.io/zone) and + // GA (topology.kubernetes.io/zone) topology labels for volumes var values []string - if k == v1.LabelZoneFailureDomain { + if k == v1.LabelZoneFailureDomain || k == v1.LabelZoneFailureDomainStable { zones, err := volumehelpers.LabelZonesToSet(v) if err != nil { return admission.NewForbidden(a, fmt.Errorf("failed to convert label string for Zone: %s to a Set", v)) @@ -140,7 +146,17 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute } else { values = []string{v} } - requirements = append(requirements, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) + + // separate topology requirements based on deprecated vs stable zone/region labels + // all other labels apply to both requirements + if k == v1.LabelZoneFailureDomain || k == v1.LabelZoneRegion { + deprecatedTopologyReqs = append(deprecatedTopologyReqs, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) + } else if k == v1.LabelZoneFailureDomainStable || k == v1.LabelZoneRegionStable { + topologyReqs = append(topologyReqs, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) + } else { + deprecatedTopologyReqs = append(deprecatedTopologyReqs, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) + topologyReqs = append(topologyReqs, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) + } } if volume.Spec.NodeAffinity == nil { @@ -153,16 +169,44 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute // Need at least one term pre-allocated whose MatchExpressions can be appended to volume.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]api.NodeSelectorTerm, 1) } - if nodeSelectorRequirementKeysExistInNodeSelectorTerms(requirements, volume.Spec.NodeAffinity.Required.NodeSelectorTerms) { - klog.V(4).Infof("NodeSelectorRequirements for cloud labels %v conflict with existing NodeAffinity %v. Skipping addition of NodeSelectorRequirements for cloud labels.", - requirements, volume.Spec.NodeAffinity) - } else { - for _, req := range requirements { - for i := range volume.Spec.NodeAffinity.Required.NodeSelectorTerms { - volume.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions = append(volume.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions, req) - } + + deprecatedNodeTerms := volume.DeepCopy().Spec.NodeAffinity.Required.NodeSelectorTerms + stableNodeTerms := volume.DeepCopy().Spec.NodeAffinity.Required.NodeSelectorTerms + + // only attempt to rewrite beta/stable topology labels if there are no conflicting labels on the PV at all + if nodeSelectorRequirementKeysExistInNodeSelectorTerms(deprecatedTopologyReqs, volume.Spec.NodeAffinity.Required.NodeSelectorTerms) || + nodeSelectorRequirementKeysExistInNodeSelectorTerms(topologyReqs, volume.Spec.NodeAffinity.Required.NodeSelectorTerms) { + klog.V(4).Infof("NodeSelectorRequirements for cloud labels %v conflict with existing NodeAffinity %v or %v. Skipping addition of NodeSelectorRequirements for cloud labels.", + deprecatedTopologyReqs, topologyReqs, volume.Spec.NodeAffinity) + + return nil + } + + for _, req := range deprecatedTopologyReqs { + for i := range deprecatedNodeTerms { + deprecatedNodeTerms[i].MatchExpressions = append(deprecatedNodeTerms[i].MatchExpressions, req) } } + + for _, req := range topologyReqs { + for i := range stableNodeTerms { + stableNodeTerms[i].MatchExpressions = append(stableNodeTerms[i].MatchExpressions, req) + } + } + + // Deprecated and stable node selector terms are the same, i.e. the cloud provider + // didn't specify eithr zone/region labels. In the case set either one without + // expanding the set of selector terms + if reflect.DeepEqual(deprecatedTopologyReqs, topologyReqs) { + volume.Spec.NodeAffinity.Required.NodeSelectorTerms = stableNodeTerms + return nil + } + + // for deprecated topology labels, we overwrite existing terms directly with the deprecated topology reqs appended + volume.Spec.NodeAffinity.Required.NodeSelectorTerms = deprecatedNodeTerms + + // for new stable topology labels, we expand node selector requirements by copying existing selector terms but using stable topology labels + volume.Spec.NodeAffinity.Required.NodeSelectorTerms = append(volume.Spec.NodeAffinity.Required.NodeSelectorTerms, stableNodeTerms...) } return nil @@ -171,15 +215,17 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) { existingLabels := volume.Labels - // All cloud providers set only these two labels. + // deprecated zone/region labels are still the source of truth domain, domainOK := existingLabels[v1.LabelZoneFailureDomain] region, regionOK := existingLabels[v1.LabelZoneRegion] isDynamicallyProvisioned := metav1.HasAnnotation(volume.ObjectMeta, persistentvolume.AnnDynamicallyProvisioned) if isDynamicallyProvisioned && domainOK && regionOK { // PV already has all the labels and we can trust the dynamic provisioning that it provided correct values. return map[string]string{ - v1.LabelZoneFailureDomain: domain, - v1.LabelZoneRegion: region, + v1.LabelZoneFailureDomain: domain, + v1.LabelZoneRegion: region, + v1.LabelZoneFailureDomainStable: domain, + v1.LabelZoneRegionStable: region, }, nil } diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go index ae890c8daa0..06756132804 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go @@ -66,9 +66,10 @@ func Test_PVLAdmission(t *testing.T) { name: "non-cloud PV ignored", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"}, @@ -174,9 +175,10 @@ func Test_PVLAdmission(t *testing.T) { name: "AWS EBS PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"}, @@ -193,9 +195,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -226,6 +229,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -237,15 +259,19 @@ func Test_PVLAdmission(t *testing.T) { name: "existing labels from dynamic provisioning are not changed", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - v1.LabelZoneFailureDomain: "domain1", - v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomainStable: "domain1", + v1.LabelZoneRegionStable: "region1", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "existingDomain", - v1.LabelZoneRegion: "existingRegion", + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", + v1.LabelZoneFailureDomainStable: "existingDomain", + v1.LabelZoneRegionStable: "existingRegion", }, Annotations: map[string]string{ persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", @@ -264,8 +290,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "existingDomain", - v1.LabelZoneRegion: "existingRegion", + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", + v1.LabelZoneFailureDomainStable: "existingDomain", + v1.LabelZoneRegionStable: "existingRegion", }, Annotations: map[string]string{ persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", @@ -294,6 +322,20 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: v1.LabelZoneRegionStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingRegion"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingDomain"}, + }, + }, + }, }, }, }, @@ -305,8 +347,10 @@ func Test_PVLAdmission(t *testing.T) { name: "existing labels from user are changed", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - v1.LabelZoneFailureDomain: "domain1", - v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomainStable: "domain1", + v1.LabelZoneRegionStable: "region1", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -329,8 +373,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "domain1", - v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomainStable: "domain1", + v1.LabelZoneRegionStable: "region1", }, }, Spec: api.PersistentVolumeSpec{ @@ -356,6 +402,20 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: v1.LabelZoneRegionStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"domain1"}, + }, + }, + }, }, }, }, @@ -367,9 +427,10 @@ func Test_PVLAdmission(t *testing.T) { name: "GCE PD PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "gcepd", Namespace: "myns"}, @@ -386,9 +447,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "gcepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -419,6 +481,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -430,9 +511,10 @@ func Test_PVLAdmission(t *testing.T) { name: "Azure Disk PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -452,9 +534,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "azurepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -485,6 +568,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -496,9 +598,10 @@ func Test_PVLAdmission(t *testing.T) { name: "Cinder Disk PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -518,9 +621,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "azurepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -551,6 +655,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -562,9 +685,10 @@ func Test_PVLAdmission(t *testing.T) { name: "AWS EBS PV overrides user applied labels", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -587,9 +711,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -620,6 +745,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -819,9 +963,10 @@ func Test_PVLAdmission(t *testing.T) { name: "vSphere PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -841,9 +986,10 @@ func Test_PVLAdmission(t *testing.T) { Name: "vSpherePV", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", + v1.LabelZoneFailureDomainStable: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -874,6 +1020,25 @@ func Test_PVLAdmission(t *testing.T) { }, }, }, + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "a", + Operator: api.NodeSelectorOpIn, + Values: []string{"1"}, + }, + { + Key: "b", + Operator: api.NodeSelectorOpIn, + Values: []string{"2"}, + }, + { + Key: v1.LabelZoneFailureDomainStable, + Operator: api.NodeSelectorOpIn, + Values: []string{"1", "2", "3"}, + }, + }, + }, }, }, }, @@ -900,6 +1065,8 @@ func Test_PVLAdmission(t *testing.T) { if !reflect.DeepEqual(testcase.preAdmissionPV, testcase.postAdmissionPV) { t.Logf("expected PV: %+v", testcase.postAdmissionPV) t.Logf("actual PV: %+v", testcase.preAdmissionPV) + t.Logf("expected PV node affinity: %+v", testcase.postAdmissionPV.Spec.NodeAffinity.Required) + t.Logf("actual PV node affinity: %+v", testcase.preAdmissionPV.Spec.NodeAffinity.Required) t.Error("unexpected PV") } @@ -927,10 +1094,12 @@ func sortMatchExpressions(pv *api.PersistentVolume) { return } - match := pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions - sort.Slice(match, func(i, j int) bool { - return match[i].Key < match[j].Key - }) + for t := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms { + match := pv.Spec.NodeAffinity.Required.NodeSelectorTerms[t].MatchExpressions + sort.Slice(match, func(i, j int) bool { + return match[i].Key < match[j].Key + }) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = match + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[t].MatchExpressions = match + } } diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go index ec9c5d99158..edd7b4d3275 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go @@ -120,7 +120,11 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone var ok bool zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] if !ok { - return nil, fmt.Errorf("%s Label for node missing", v1.LabelZoneFailureDomain) + zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] + } + + if !ok { + return nil, fmt.Errorf("Label %q or %q for node is missing", v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomainStable) } // if single replica volume and node with zone found, return immediately if numReplicas == 1 { @@ -135,7 +139,8 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone } if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) { - return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomain) + return nil, fmt.Errorf("no matchLabelExpressions with keys %q,%q found in allowedTopologies. Please specify matchLabelExpressions with keys: %q,%q", + v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomainStable, v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomainStable) } if allowedZones.Len() > 0 { @@ -185,7 +190,7 @@ func ZonesFromAllowedTopologies(allowedTopologies []v1.TopologySelectorTerm) (se zones := make(sets.String) for _, term := range allowedTopologies { for _, exp := range term.MatchLabelExpressions { - if exp.Key == v1.LabelZoneFailureDomain { + if exp.Key == v1.LabelZoneFailureDomain || exp.Key == v1.LabelZoneFailureDomainStable { for _, value := range exp.Values { zones.Insert(value) } @@ -308,3 +313,103 @@ func getPVCNameHashAndIndexOffset(pvcName string) (hash uint32, index uint32) { return hash, index } + +// TranslateZoneRegionLabelsToNodeSelectorTerms translates the set of provided labels as []v1.NodeSelectorTerms. +// For zone/region topologies, a node can have either only the beta label (failure-domain.beta.kubernetes.io/{zone,region}) +// or both the beta and GA label (topology.kubernetes.io/{zone,region}). We have to put each topology label into +// separate node selector terms so PVs can be scheduled on nodes with either one or both labels +func TranslateZoneRegionLabelsToNodeSelectorTerms(labels map[string]string) []v1.NodeSelectorTerm { + nodeSelectorTerms := make([]v1.NodeSelectorTerm, 0) + deprecatedTopologyReqs := make([]v1.NodeSelectorRequirement, 0) + topologyReqs := make([]v1.NodeSelectorRequirement, 0) + + if len(labels) == 0 { + return nodeSelectorTerms + } + + for k, v := range labels { + if k == v1.LabelZoneFailureDomain || k == v1.LabelZoneRegion { + deprecatedTopologyReqs = append(deprecatedTopologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) + continue + } + + if k == v1.LabelZoneFailureDomainStable || k == v1.LabelZoneRegionStable { + topologyReqs = append(topologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) + continue + } + + // remaining labels outside known topology labels get added to both beta and GA node selectors + deprecatedTopologyReqs = append(deprecatedTopologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) + topologyReqs = append(topologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: []string{v}}) + } + + if len(deprecatedTopologyReqs) > 0 { + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ + MatchExpressions: deprecatedTopologyReqs, + }) + } + + if len(topologyReqs) > 0 { + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ + MatchExpressions: topologyReqs, + }) + } + + return nodeSelectorTerms +} + +// TranslateZoneLabelsToNodeSelectorTerms translates the set of provided labels as []v1.NodeSelectorTerms. +// For zone topologies, a node can have either only the beta label (failure-domain.beta.kubernetes.io/zone) +// or both the beta and GA label (topology.kubernetes.io/zone). We have to put each topology label into +// separate node selector terms so PVs can be scheduled on nodes with either one or both labels +func TranslateZoneLabelsToNodeSelectorTerms(labels map[string]string) ([]v1.NodeSelectorTerm, error) { + nodeSelectorTerms := make([]v1.NodeSelectorTerm, 0) + deprecatedTopologyReqs := make([]v1.NodeSelectorRequirement, 0) + topologyReqs := make([]v1.NodeSelectorRequirement, 0) + + if len(labels) == 0 { + return nodeSelectorTerms, nil + } + + for k, v := range labels { + var values []string + if k == v1.LabelZoneFailureDomain { + values, err := LabelZonesToList(v) + if err != nil { + return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err) + } + + deprecatedTopologyReqs = append(deprecatedTopologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) + continue + } + + if k == v1.LabelZoneFailureDomainStable { + values, err := LabelZonesToList(v) + if err != nil { + return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err) + } + + topologyReqs = append(topologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) + continue + } + + // remaining labels outside known topology labels get added to both beta and GA node selectors + values = []string{v} + deprecatedTopologyReqs = append(deprecatedTopologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) + topologyReqs = append(topologyReqs, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values}) + } + + if len(deprecatedTopologyReqs) > 0 { + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ + MatchExpressions: deprecatedTopologyReqs, + }) + } + + if len(topologyReqs) > 0 { + nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ + MatchExpressions: topologyReqs, + }) + } + + return nodeSelectorTerms, nil +} diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go index 0a3b156f95a..2f5f92f03ec 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go @@ -17,7 +17,10 @@ limitations under the License. package helpers import ( + "fmt" "hash/fnv" + "reflect" + "sort" "testing" "k8s.io/api/core/v1" @@ -417,7 +420,7 @@ func TestChooseZonesForVolume(t *testing.T) { func TestSelectZoneForVolume(t *testing.T) { nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX"} + nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX", v1.LabelZoneFailureDomainStable: "zoneX"} nodeWithNoLabels := &v1.Node{} @@ -658,6 +661,10 @@ func TestSelectZoneForVolume(t *testing.T) { Key: v1.LabelZoneFailureDomain, Values: []string{"zoneX", "zoneY"}, }, + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneX", "zoneY"}, + }, }, }, }, @@ -689,6 +696,22 @@ func TestSelectZoneForVolume(t *testing.T) { }, }, }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneY"}, + }, + }, + }, }, Reject: false, ExpectedZones: "zoneX,zoneY", @@ -1110,7 +1133,7 @@ func TestSelectZonesForVolume(t *testing.T) { // Select zones from node label and AllowedTopologies [Pass] // [1] Node with zone labels // [2] no Zone/Zones parameters - // [3] AllowedTopologies with single term with multiple values specified + // [3] AllowedTopologies with single term with multiple values specified, both beta and GA zone labels // [4] ReplicaCount specified // [5] ZonesWithNodes irrelevant // Note: the test Name suffix is used as the pvcname and it's suffix is important to influence ChooseZonesForVolume @@ -1126,6 +1149,10 @@ func TestSelectZonesForVolume(t *testing.T) { Key: v1.LabelZoneFailureDomain, Values: []string{"zoneV", "zoneW", "zoneX", "zoneY", "zoneZ"}, }, + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneV", "zoneW", "zoneX", "zoneY", "zoneZ"}, + }, }, }, }, @@ -1138,7 +1165,35 @@ func TestSelectZonesForVolume(t *testing.T) { // Select zones from node label and AllowedTopologies [Pass] // [1] Node with zone labels // [2] no Zone/Zones parameters - // [3] AllowedTopologies with single term with multiple values specified + // [3] AllowedTopologies with single term with multiple values specified, only GA zone labels + // [4] ReplicaCount specified + // [5] ZonesWithNodes irrelevant + // Note: the test Name suffix is used as the pvcname and it's suffix is important to influence ChooseZonesForVolume + // to NOT pick zoneX from AllowedTopologies if zoneFromNode is incorrectly set or not set at all. + { + Name: "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_Superset-1", + Node: nodeWithZoneLabels, + ReplicaCount: 2, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneV", "zoneW", "zoneX", "zoneY", "zoneZ"}, + }, + }, + }, + }, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneX", + ExpectedZones: "zoneV,zoneW,zoneX,zoneY,zoneZ", + }, + + // Select zones from node label and AllowedTopologies [Pass] + // [1] Node with zone labels + // [2] no Zone/Zones parameters + // [3] AllowedTopologies with single term with multiple values specified, has both beta/GA zone labels // [4] ReplicaCount specified // [5] ZonesWithNodes irrelevant { @@ -1152,6 +1207,10 @@ func TestSelectZonesForVolume(t *testing.T) { Key: v1.LabelZoneFailureDomain, Values: []string{"zoneX", "zoneY"}, }, + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneX", "zoneY"}, + }, }, }, }, @@ -1275,7 +1334,7 @@ func TestSelectZonesForVolume(t *testing.T) { // Select zones from node label and AllowedTopologies [Pass] // [1] Node with zone labels // [2] no Zone/Zones parametes specified - // [3] AllowedTopologies with multiple terms specified + // [3] AllowedTopologies with multiple terms specified, has both beta/GA zone labels // [4] ReplicaCount specified // [5] ZonesWithNodes irrelevant { @@ -1299,6 +1358,22 @@ func TestSelectZonesForVolume(t *testing.T) { }, }, }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Values: []string{"zoneY"}, + }, + }, + }, }, Reject: false, ExpectSpecificZone: true, @@ -1637,3 +1712,396 @@ func checkFnv32(t *testing.T, s string, expected uint32) { t.Fatalf("hash of %q was %v, expected %v", s, h.Sum32(), expected) } } + +func Test_TranslateZoneRegionLabelsToNodeSelectorTerms(t *testing.T) { + testcases := []struct { + name string + labels map[string]string + nodeSelectorTerm []v1.NodeSelectorTerm + }{ + { + name: "empty label set", + labels: map[string]string{}, + nodeSelectorTerm: []v1.NodeSelectorTerm{}, + }, + { + name: "nil label set", + labels: nil, + nodeSelectorTerm: []v1.NodeSelectorTerm{}, + }, + { + name: "with beta zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + }, + }, + { + name: "with beta and stable zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneFailureDomainStable: "zone1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + }, + }, + { + name: "with beta zone and region labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegion: "region1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + }, + }, + }, + }, + { + name: "with stable zone and region labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegion: "region1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + }, + }, + }, + }, + { + name: "with beta/stable and zone/region labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneRegionStable: "region1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegionStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + }, + }, + }, + }, + { + name: "with beta/stable and zone/region and other labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneRegionStable: "region1", + "hello": "world", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + { + Key: "hello", + Operator: v1.NodeSelectorOpIn, + Values: []string{"world"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + { + Key: v1.LabelZoneRegionStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + { + Key: "hello", + Operator: v1.NodeSelectorOpIn, + Values: []string{"world"}, + }, + }, + }, + }, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + nodeSelectorTerm := TranslateZoneRegionLabelsToNodeSelectorTerms(testcase.labels) + sortNodeSelectorTerm(nodeSelectorTerm) + sortNodeSelectorTerm(testcase.nodeSelectorTerm) + + if !reflect.DeepEqual(nodeSelectorTerm, testcase.nodeSelectorTerm) { + t.Logf("actual node selector term: %v", nodeSelectorTerm) + t.Logf("expected node selector term: %v", testcase.nodeSelectorTerm) + t.Error("unexpected node selector term") + } + }) + } +} + +func Test_TranslateZoneLabelsToNodeSelectorTerms(t *testing.T) { + testcases := []struct { + name string + labels map[string]string + nodeSelectorTerm []v1.NodeSelectorTerm + expectedErr error + }{ + { + name: "empty label set", + labels: map[string]string{}, + nodeSelectorTerm: []v1.NodeSelectorTerm{}, + }, + { + name: "nil label set", + labels: nil, + nodeSelectorTerm: []v1.NodeSelectorTerm{}, + }, + { + name: "with beta zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + }, + }, + { + name: "with beta multi-zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1__zone2", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1", "zone2"}, + }, + }, + }, + }, + }, + { + name: "with beta and stable zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneFailureDomainStable: "zone1", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1"}, + }, + }, + }, + }, + }, + { + name: "with beta and stable multi-zone labels", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1__zone2", + v1.LabelZoneFailureDomainStable: "zone1__zone2", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1", "zone2"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1", "zone2"}, + }, + }, + }, + }, + }, + { + name: "with beta/stable zones and others", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1__zone2", + v1.LabelZoneFailureDomainStable: "zone1__zone2", + "hello": "world", + }, + nodeSelectorTerm: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1", "zone2"}, + }, + { + Key: "hello", + Operator: v1.NodeSelectorOpIn, + Values: []string{"world"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelZoneFailureDomainStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{"zone1", "zone2"}, + }, + { + Key: "hello", + Operator: v1.NodeSelectorOpIn, + Values: []string{"world"}, + }, + }, + }, + }, + }, + { + name: "with invalid zone value", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1__", + }, + expectedErr: fmt.Errorf("failed to convert label string for Zone: zone1__ to a List: %q separated list (%q) must not contain an empty string", "__", "zone1__"), + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + nodeSelectorTerm, err := TranslateZoneLabelsToNodeSelectorTerms(testcase.labels) + if !reflect.DeepEqual(err, testcase.expectedErr) { + t.Logf("actual err: %v", err) + t.Logf("expected err: %v", testcase.expectedErr) + t.Fatal("unexpected error") + } + + sortNodeSelectorTerm(nodeSelectorTerm) + sortNodeSelectorTerm(testcase.nodeSelectorTerm) + + if !reflect.DeepEqual(nodeSelectorTerm, testcase.nodeSelectorTerm) { + t.Logf("actual node selector term: %v", nodeSelectorTerm) + t.Logf("expected node selector term: %v", testcase.nodeSelectorTerm) + t.Error("unexpected node selector term") + } + }) + } +} + +func sortNodeSelectorTerm(nodeSelectorTerms []v1.NodeSelectorTerm) { + for _, nodeSelectorTerm := range nodeSelectorTerms { + sort.Slice(nodeSelectorTerm.MatchExpressions, func(i, j int) bool { + return nodeSelectorTerm.MatchExpressions[i].Key < nodeSelectorTerm.MatchExpressions[j].Key + }) + } +} diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go index 7f07c814bb4..160d14afdce 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go @@ -73,13 +73,16 @@ func translateAllowedTopologies(terms []v1.TopologySelectorTerm) ([]v1.TopologyS newTopologies := []v1.TopologySelectorTerm{} for _, term := range terms { newTerm := v1.TopologySelectorTerm{} + zoneFound := false for _, exp := range term.MatchLabelExpressions { var newExp v1.TopologySelectorLabelRequirement - if exp.Key == v1.LabelZoneFailureDomain { + if !zoneFound && (exp.Key == v1.LabelZoneFailureDomain || exp.Key == v1.LabelZoneFailureDomainStable) { newExp = v1.TopologySelectorLabelRequirement{ Key: GCEPDTopologyKey, Values: exp.Values, } + + zoneFound = true } else if exp.Key == GCEPDTopologyKey { newExp = exp } else { @@ -240,7 +243,11 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten return nil, fmt.Errorf("pv is nil or GCE Persistent Disk source not defined on pv") } - zonesLabel := pv.Labels[v1.LabelZoneFailureDomain] + zonesLabel, ok := pv.Labels[v1.LabelZoneFailureDomain] + if !ok { + zonesLabel = pv.Labels[v1.LabelZoneFailureDomainStable] + } + zones := strings.Split(zonesLabel, cloudvolume.LabelMultiZoneDelimiter) if len(zones) == 1 && len(zones[0]) != 0 { // Zonal diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go index be3d4f195cc..93c644cb328 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go @@ -72,10 +72,15 @@ func TestTranslatePDInTreeStorageClassToCSI(t *testing.T) { expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), }, { - name: "some translated topology", + name: "some translated topology using deprecated zone label", options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelZoneFailureDomain, []string{"foo"})), expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), }, + { + name: "some translated topology", + options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelZoneFailureDomainStable, []string{"foo"})), + expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), + }, { name: "zone and topology", options: NewStorageClass(map[string]string{"zone": "foo"}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index 4320d8861cf..846f8fe90f6 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -2639,11 +2639,15 @@ func (c *Cloud) GetVolumeLabels(volumeName KubernetesVolumeID) (map[string]strin } labels[v1.LabelZoneFailureDomain] = az + labels[v1.LabelZoneFailureDomainStable] = az + region, err := azToRegion(az) if err != nil { return nil, err } + labels[v1.LabelZoneRegion] = region + labels[v1.LabelZoneRegionStable] = region return labels, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go index 4da723a6760..ccf062c1bd3 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go @@ -1262,8 +1262,11 @@ func TestGetVolumeLabels(t *testing.T) { assert.Nil(t, err, "Error creating Volume %v", err) assert.Equal(t, map[string]string{ - v1.LabelZoneFailureDomain: "us-east-1a", - v1.LabelZoneRegion: "us-east-1"}, labels) + v1.LabelZoneFailureDomain: "us-east-1a", + v1.LabelZoneRegion: "us-east-1", + v1.LabelZoneFailureDomainStable: "us-east-1a", + v1.LabelZoneRegionStable: "us-east-1", + }, labels) awsServices.ec2.(*MockedFakeEC2).AssertExpectations(t) } @@ -1336,8 +1339,10 @@ func TestGetLabelsForVolume(t *testing.T) { AvailabilityZone: aws.String("us-east-1a"), }}, map[string]string{ - v1.LabelZoneFailureDomain: "us-east-1a", - v1.LabelZoneRegion: "us-east-1", + v1.LabelZoneFailureDomain: "us-east-1a", + v1.LabelZoneRegion: "us-east-1", + v1.LabelZoneFailureDomainStable: "us-east-1a", + v1.LabelZoneRegionStable: "us-east-1", }, nil, }, diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index 5dd572fcf0b..5b6aeba7c95 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -636,7 +636,9 @@ func (az *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) { prevNode := prev.(*v1.Node) newNode := obj.(*v1.Node) if newNode.Labels[v1.LabelZoneFailureDomain] == - prevNode.Labels[v1.LabelZoneFailureDomain] { + prevNode.Labels[v1.LabelZoneFailureDomain] && + newNode.Labels[v1.LabelZoneFailureDomainStable] == + prevNode.Labels[v1.LabelZoneFailureDomainStable] { return } az.updateNodeCaches(prevNode, newNode) @@ -671,6 +673,10 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) { if prevNode != nil { // Remove from nodeZones cache. prevZone, ok := prevNode.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + if !ok { + prevZone, ok = prevNode.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] + } + if ok && az.isAvailabilityZone(prevZone) { az.nodeZones[prevZone].Delete(prevNode.ObjectMeta.Name) if az.nodeZones[prevZone].Len() == 0 { @@ -694,6 +700,10 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) { if newNode != nil { // Add to nodeZones cache. newZone, ok := newNode.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + if !ok { + newZone, ok = newNode.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] + } + if ok && az.isAvailabilityZone(newZone) { if az.nodeZones[newZone] == nil { az.nodeZones[newZone] = sets.NewString() diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go index 847da0237c2..96d90b6668e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go @@ -333,8 +333,10 @@ func (c *Cloud) GetAzureDiskLabels(diskURI string) (map[string]string, error) { zone := c.makeZone(c.Location, zoneID) klog.V(4).Infof("Got zone %q for Azure disk %q", zone, diskName) labels := map[string]string{ - v1.LabelZoneRegion: c.Location, - v1.LabelZoneFailureDomain: zone, + v1.LabelZoneRegion: c.Location, + v1.LabelZoneFailureDomain: zone, + v1.LabelZoneRegionStable: c.Location, + v1.LabelZoneFailureDomainStable: zone, } return labels, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go index 1de777909c0..f39cb7ef00b 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go @@ -694,7 +694,9 @@ func (g *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) { prevNode := prev.(*v1.Node) newNode := obj.(*v1.Node) if newNode.Labels[v1.LabelZoneFailureDomain] == - prevNode.Labels[v1.LabelZoneFailureDomain] { + prevNode.Labels[v1.LabelZoneFailureDomain] && + newNode.Labels[v1.LabelZoneFailureDomainStable] == + prevNode.Labels[v1.LabelZoneFailureDomainStable] { return } g.updateNodeZones(prevNode, newNode) @@ -726,6 +728,10 @@ func (g *Cloud) updateNodeZones(prevNode, newNode *v1.Node) { defer g.nodeZonesLock.Unlock() if prevNode != nil { prevZone, ok := prevNode.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + if !ok { + prevZone, ok = prevNode.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] + } + if ok { g.nodeZones[prevZone].Delete(prevNode.ObjectMeta.Name) if g.nodeZones[prevZone].Len() == 0 { @@ -735,6 +741,10 @@ func (g *Cloud) updateNodeZones(prevNode, newNode *v1.Node) { } if newNode != nil { newZone, ok := newNode.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + if !ok { + newZone, ok = newNode.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] + } + if ok { if g.nodeZones[newZone] == nil { g.nodeZones[newZone] = sets.NewString() diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go index ba18584fea8..4a81953a1aa 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go @@ -499,7 +499,10 @@ func (g *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) } // If the zone is already labeled, honor the hint - zone := pv.Labels[v1.LabelZoneFailureDomain] + zone, ok := pv.Labels[v1.LabelZoneFailureDomain] + if !ok { + zone = pv.Labels[v1.LabelZoneFailureDomainStable] + } labels, err := g.GetAutoLabelsForPD(pv.Spec.GCEPersistentDisk.PDName, zone) if err != nil { @@ -856,6 +859,8 @@ func (g *Cloud) GetAutoLabelsForPD(name string, zone string) (map[string]string, } labels[v1.LabelZoneFailureDomain] = zoneInfo.zone labels[v1.LabelZoneRegion] = disk.Region + labels[v1.LabelZoneFailureDomainStable] = zoneInfo.zone + labels[v1.LabelZoneRegionStable] = disk.Region case multiZone: if zoneInfo.replicaZones == nil || zoneInfo.replicaZones.Len() <= 0 { // Unexpected, but sanity-check @@ -864,6 +869,9 @@ func (g *Cloud) GetAutoLabelsForPD(name string, zone string) (map[string]string, labels[v1.LabelZoneFailureDomain] = volumehelpers.ZonesSetToLabelValue(zoneInfo.replicaZones) labels[v1.LabelZoneRegion] = disk.Region + labels[v1.LabelZoneFailureDomainStable] = + volumehelpers.ZonesSetToLabelValue(zoneInfo.replicaZones) + labels[v1.LabelZoneRegionStable] = disk.Region case nil: // Unexpected, but sanity-check return nil, fmt.Errorf("PD did not have ZoneInfo: %v", disk) diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks_test.go index 413c9d32529..78957deee01 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks_test.go @@ -465,13 +465,23 @@ func TestGetAutoLabelsForPD_Basic(t *testing.T) { if err != nil { t.Error(err) } + if labels[v1.LabelZoneFailureDomain] != zone { t.Errorf("Failure domain is '%v', but zone is '%v'", labels[v1.LabelZoneFailureDomain], zone) } + if labels[v1.LabelZoneRegion] != gceRegion { t.Errorf("Region is '%v', but region is 'us-central1'", labels[v1.LabelZoneRegion]) } + + if labels[v1.LabelZoneFailureDomainStable] != zone { + t.Errorf("Failure domain is '%v', but zone is '%v'", + labels[v1.LabelZoneFailureDomainStable], zone) + } + if labels[v1.LabelZoneRegionStable] != gceRegion { + t.Errorf("Region is '%v', but region is 'us-central1'", labels[v1.LabelZoneRegionStable]) + } } func TestGetAutoLabelsForPD_NoZone(t *testing.T) { @@ -508,6 +518,14 @@ func TestGetAutoLabelsForPD_NoZone(t *testing.T) { if labels[v1.LabelZoneRegion] != gceRegion { t.Errorf("Region is '%v', but region is 'europe-west1'", labels[v1.LabelZoneRegion]) } + + if labels[v1.LabelZoneFailureDomainStable] != zone { + t.Errorf("Failure domain is '%v', but zone is '%v'", + labels[v1.LabelZoneFailureDomainStable], zone) + } + if labels[v1.LabelZoneRegionStable] != gceRegion { + t.Errorf("Region is '%v', but region is 'europe-west1'", labels[v1.LabelZoneRegionStable]) + } } func TestGetAutoLabelsForPD_DiskNotFound(t *testing.T) { @@ -594,6 +612,14 @@ func TestGetAutoLabelsForPD_DupDisk(t *testing.T) { if labels[v1.LabelZoneRegion] != gceRegion { t.Errorf("Region is '%v', but region is 'us-west1'", labels[v1.LabelZoneRegion]) } + + if labels[v1.LabelZoneFailureDomainStable] != zone { + t.Errorf("Failure domain is '%v', but zone is '%v'", + labels[v1.LabelZoneFailureDomainStable], zone) + } + if labels[v1.LabelZoneRegionStable] != gceRegion { + t.Errorf("Region is '%v', but region is 'us-west1'", labels[v1.LabelZoneRegionStable]) + } } func TestGetAutoLabelsForPD_DupDiskNoZone(t *testing.T) { diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go index 15d7b6f2e3f..56f9de3929e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go @@ -60,12 +60,20 @@ func splitNodesByZone(nodes []*v1.Node) map[string][]*v1.Node { return zones } +// getZone checks the deprecated beta zone label first and falls back to GA label if the beta label does not exist +// if both don't exist, returns the default zone which is "" func getZone(n *v1.Node) string { zone, ok := n.Labels[v1.LabelZoneFailureDomain] - if !ok { - return defaultZone + if ok { + return zone } - return zone + + zone, ok = n.Labels[v1.LabelZoneFailureDomainStable] + if ok { + return zone + } + + return defaultZone } func makeHostURL(projectsAPIEndpoint, projectID, zone, host string) string { diff --git a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_volumes.go b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_volumes.go index 95e38854912..90b450831df 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_volumes.go +++ b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_volumes.go @@ -736,6 +736,8 @@ func (os *OpenStack) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVo labels := make(map[string]string) labels[v1.LabelZoneFailureDomain] = volume.AvailabilityZone labels[v1.LabelZoneRegion] = os.region + labels[v1.LabelZoneFailureDomainStable] = volume.AvailabilityZone + labels[v1.LabelZoneRegionStable] = os.region klog.V(4).Infof("The Volume %s has labels %v", pv.Spec.Cinder.VolumeID, labels) return labels, nil diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go index bf2f1cd4eaf..4e3a43836d2 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go @@ -203,8 +203,8 @@ func (nm *NodeManager) DiscoverNode(node *v1.Node) error { node.Name, vm, res.vc, res.datacenter.Name()) // Get the node zone information - nodeFd := node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] - nodeRegion := node.ObjectMeta.Labels[v1.LabelZoneRegion] + nodeFd := getNodeZoneFailureDomain(node) + nodeRegion := getNodeRegion(node) nodeZone := &cloudprovider.Zone{FailureDomain: nodeFd, Region: nodeRegion} nodeInfo := &NodeInfo{dataCenter: res.datacenter, vm: vm, vcServer: res.vc, vmUUID: nodeUUID, zone: nodeZone} nm.addNodeInfo(node.ObjectMeta.Name, nodeInfo) @@ -230,6 +230,34 @@ func (nm *NodeManager) DiscoverNode(node *v1.Node) error { return vclib.ErrNoVMFound } +func getNodeZoneFailureDomain(node *v1.Node) string { + zone, ok := node.Labels[v1.LabelZoneFailureDomain] + if ok { + return zone + } + + zone, ok = node.Labels[v1.LabelZoneFailureDomainStable] + if ok { + return zone + } + + return "" +} + +func getNodeRegion(node *v1.Node) string { + region, ok := node.Labels[v1.LabelZoneRegion] + if ok { + return region + } + + region, ok = node.Labels[v1.LabelZoneRegionStable] + if ok { + return region + } + + return "" +} + func (nm *NodeManager) RegisterNode(node *v1.Node) error { nm.addNode(node) return nm.DiscoverNode(node) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 305e88be6a1..dcfc3709004 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -1649,6 +1649,8 @@ func (vs *VSphere) GetVolumeLabels(volumePath string) (map[string]string, error) if len(dsZones) > 0 { labels[v1.LabelZoneRegion] = dsZones[0].Region labels[v1.LabelZoneFailureDomain] = dsZones[0].FailureDomain + labels[v1.LabelZoneRegionStable] = dsZones[0].Region + labels[v1.LabelZoneFailureDomainStable] = dsZones[0].FailureDomain } return labels, nil } diff --git a/test/e2e/storage/vsphere/vsphere_zone_support.go b/test/e2e/storage/vsphere/vsphere_zone_support.go index 8f06e119f9f..19df404e290 100644 --- a/test/e2e/storage/vsphere/vsphere_zone_support.go +++ b/test/e2e/storage/vsphere/vsphere_zone_support.go @@ -497,7 +497,7 @@ func verifyPVZoneLabels(client clientset.Interface, namespace string, scParamete ginkgo.By("Verify zone information is present in the volume labels") for _, pv := range persistentvolumes { // Multiple zones are separated with "__" - pvZoneLabels := strings.Split(pv.ObjectMeta.Labels["failure-domain.beta.kubernetes.io/zone"], "__") + pvZoneLabels := strings.Split(pv.ObjectMeta.Labels[v1.LabelZoneFailureDomain], "__") for _, zone := range zones { gomega.Expect(pvZoneLabels).Should(gomega.ContainElement(zone), "Incorrect or missing zone labels in pv.") }