From 5acabf2f55cc80fc56ef9efe7aae00c1b9b4351a Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 14 Aug 2019 14:37:22 -0400 Subject: [PATCH 1/5] core/v1: update well known labels for zones/regions to topology.kubernetes.io/zone and topology.kubernetes.io/region, mark beta labels as deprecated Signed-off-by: Andrew Sy Kim --- staging/src/k8s.io/api/core/v1/well_known_labels.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/api/core/v1/well_known_labels.go b/staging/src/k8s.io/api/core/v1/well_known_labels.go index 3287fb51fc2..9057bfa5bef 100644 --- a/staging/src/k8s.io/api/core/v1/well_known_labels.go +++ b/staging/src/k8s.io/api/core/v1/well_known_labels.go @@ -17,9 +17,12 @@ limitations under the License. package v1 const ( - LabelHostname = "kubernetes.io/hostname" - LabelZoneFailureDomain = "failure-domain.beta.kubernetes.io/zone" - LabelZoneRegion = "failure-domain.beta.kubernetes.io/region" + LabelHostname = "kubernetes.io/hostname" + + LabelZoneFailureDomain = "failure-domain.beta.kubernetes.io/zone" + LabelZoneRegion = "failure-domain.beta.kubernetes.io/region" + LabelZoneFailureDomainStable = "topology.kubernetes.io/zone" + LabelZoneRegionStable = "topology.kubernetes.io/region" LabelInstanceType = "beta.kubernetes.io/instance-type" From 3032c811875c3edc2472ebd35384c07f32494603 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 14 Aug 2019 14:34:47 -0400 Subject: [PATCH 2/5] node controller: set both deprecated Beta and GA labels for Zone/Region topology Signed-off-by: Andrew Sy Kim --- pkg/controller/cloud/BUILD | 1 + pkg/controller/cloud/node_controller.go | 91 +++++++++++++++ pkg/controller/cloud/node_controller_test.go | 106 +++++++++++++++++- .../k8s.io/cloud-provider/node/helpers/BUILD | 3 + .../cloud-provider/node/helpers/labels.go | 102 +++++++++++++++++ 5 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 staging/src/k8s.io/cloud-provider/node/helpers/labels.go diff --git a/pkg/controller/cloud/BUILD b/pkg/controller/cloud/BUILD index d3c48293f0f..2dbef978894 100644 --- a/pkg/controller/cloud/BUILD +++ b/pkg/controller/cloud/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/scheduler/api:go_default_library", "//pkg/util/node:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/controller/cloud/node_controller.go b/pkg/controller/cloud/node_controller.go index c833b57e56e..17fedb72b13 100644 --- a/pkg/controller/cloud/node_controller.go +++ b/pkg/controller/cloud/node_controller.go @@ -23,6 +23,7 @@ import ( "time" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -42,6 +43,35 @@ import ( nodeutil "k8s.io/kubernetes/pkg/util/node" ) +// labelReconcileInfo lists Node labels to reconcile, and how to reconcile them. +// primaryKey and secondaryKey are keys of labels to reconcile. +// - If both keys exist, but their values don't match. Use the value from the +// primaryKey as the source of truth to reconcile. +// - If ensureSecondaryExists is true, and the secondaryKey does not +// exist, secondaryKey will be added with the value of the primaryKey. +var labelReconcileInfo = []struct { + primaryKey string + secondaryKey string + ensureSecondaryExists bool +}{ + { + // Reconcile the beta and the GA zone label using the beta label as + // the source of truth + // TODO: switch the primary key to GA labels in v1.21 + primaryKey: v1.LabelZoneFailureDomain, + secondaryKey: v1.LabelZoneFailureDomainStable, + ensureSecondaryExists: true, + }, + { + // Reconcile the beta and the stable region label using the beta label as + // the source of truth + // TODO: switch the primary key to GA labels in v1.21 + primaryKey: v1.LabelZoneRegion, + secondaryKey: v1.LabelZoneRegionStable, + ensureSecondaryExists: true, + }, +} + var UpdateNodeSpecBackoff = wait.Backoff{ Steps: 20, Duration: 50 * time.Millisecond, @@ -125,6 +155,63 @@ func (cnc *CloudNodeController) UpdateNodeStatus(ctx context.Context) { for i := range nodes.Items { cnc.updateNodeAddress(ctx, &nodes.Items[i], instances) } + + for _, node := range nodes.Items { + err = cnc.reconcileNodeLabels(node.Name) + if err != nil { + klog.Errorf("Error reconciling node labels for node %q, err: %v", node.Name, err) + } + } +} + +// reconcileNodeLabels reconciles node labels transitioning from beta to GA +func (cnc *CloudNodeController) reconcileNodeLabels(nodeName string) error { + node, err := cnc.nodeInformer.Lister().Get(nodeName) + if err != nil { + // If node not found, just ignore it. + if apierrors.IsNotFound(err) { + return nil + } + + return err + } + + if node.Labels == nil { + // Nothing to reconcile. + return nil + } + + labelsToUpdate := map[string]string{} + for _, r := range labelReconcileInfo { + primaryValue, primaryExists := node.Labels[r.primaryKey] + secondaryValue, secondaryExists := node.Labels[r.secondaryKey] + + if !primaryExists { + // The primary label key does not exist. This should not happen + // within our supported version skew range, when no external + // components/factors modifying the node object. Ignore this case. + continue + } + if secondaryExists && primaryValue != secondaryValue { + // Secondary label exists, but not consistent with the primary + // label. Need to reconcile. + labelsToUpdate[r.secondaryKey] = primaryValue + + } else if !secondaryExists && r.ensureSecondaryExists { + // Apply secondary label based on primary label. + labelsToUpdate[r.secondaryKey] = primaryValue + } + } + + if len(labelsToUpdate) == 0 { + return nil + } + + if !cloudnodeutil.AddOrUpdateLabelsOnNode(cnc.kubeClient, labelsToUpdate, node) { + return fmt.Errorf("failed update labels for node %+v", node) + } + + return nil } // UpdateNodeAddress updates the nodeAddress of a single node @@ -298,10 +385,14 @@ func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Nod if zone.FailureDomain != "" { klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneFailureDomain, zone.FailureDomain) curNode.ObjectMeta.Labels[v1.LabelZoneFailureDomain] = zone.FailureDomain + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneFailureDomainStable, zone.FailureDomain) + curNode.ObjectMeta.Labels[v1.LabelZoneFailureDomainStable] = zone.FailureDomain } if zone.Region != "" { klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneRegion, zone.Region) curNode.ObjectMeta.Labels[v1.LabelZoneRegion] = zone.Region + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelZoneRegionStable, zone.Region) + curNode.ObjectMeta.Labels[v1.LabelZoneRegionStable] = zone.Region } } diff --git a/pkg/controller/cloud/node_controller_test.go b/pkg/controller/cloud/node_controller_test.go index 6271e3ab492..5e10eb9236a 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -19,6 +19,7 @@ package cloud import ( "context" "errors" + "reflect" "testing" "time" @@ -460,8 +461,12 @@ func TestZoneInitialized(t *testing.T) { assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated") assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated") - assert.Equal(t, 2, len(fnh.UpdatedNodes[0].ObjectMeta.Labels), + assert.Equal(t, 4, len(fnh.UpdatedNodes[0].ObjectMeta.Labels), "Node label for Region and Zone were not set") + assert.Equal(t, "us-west", fnh.UpdatedNodes[0].ObjectMeta.Labels[v1.LabelZoneRegionStable], + "Node Region not correctly updated") + assert.Equal(t, "us-west-1a", fnh.UpdatedNodes[0].ObjectMeta.Labels[v1.LabelZoneFailureDomainStable], + "Node FailureDomain not correctly updated") assert.Equal(t, "us-west", fnh.UpdatedNodes[0].ObjectMeta.Labels[v1.LabelZoneRegion], "Node Region not correctly updated") assert.Equal(t, "us-west-1a", fnh.UpdatedNodes[0].ObjectMeta.Labels[v1.LabelZoneFailureDomain], @@ -672,6 +677,105 @@ func TestNodeProvidedIPAddresses(t *testing.T) { assert.Equal(t, "10.0.0.1", updatedNodes[0].Status.Addresses[0].Address, "Node Addresses not correctly updated") } +func Test_reconcileNodeLabels(t *testing.T) { + testcases := []struct { + name string + labels map[string]string + expectedLabels map[string]string + expectedErr error + }{ + { + name: "requires reconcile", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + }, + expectedLabels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + v1.LabelZoneFailureDomainStable: "foo", + v1.LabelZoneRegionStable: "bar", + }, + expectedErr: nil, + }, + { + name: "doesn't require reconcile", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + v1.LabelZoneFailureDomainStable: "foo", + v1.LabelZoneRegionStable: "bar", + }, + expectedLabels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + v1.LabelZoneFailureDomainStable: "foo", + v1.LabelZoneRegionStable: "bar", + }, + expectedErr: nil, + }, + { + name: "require reconcile -- secondary labels are different from primary", + labels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + v1.LabelZoneFailureDomainStable: "wrongfoo", + v1.LabelZoneRegionStable: "wrongbar", + }, + expectedLabels: map[string]string{ + v1.LabelZoneFailureDomain: "foo", + v1.LabelZoneRegion: "bar", + v1.LabelZoneFailureDomainStable: "foo", + v1.LabelZoneRegionStable: "bar", + }, + expectedErr: nil, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + testNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node01", + Labels: test.labels, + }, + } + + clientset := fake.NewSimpleClientset(testNode) + factory := informers.NewSharedInformerFactory(clientset, 0) + + cnc := &CloudNodeController{ + kubeClient: clientset, + nodeInformer: factory.Core().V1().Nodes(), + } + + // activate node informer + factory.Core().V1().Nodes().Informer() + factory.Start(nil) + factory.WaitForCacheSync(nil) + + err := cnc.reconcileNodeLabels("node01") + if err != test.expectedErr { + t.Logf("actual err: %v", err) + t.Logf("expected err: %v", test.expectedErr) + t.Errorf("unexpected error") + } + + actualNode, err := clientset.CoreV1().Nodes().Get("node01", metav1.GetOptions{}) + if err != nil { + t.Fatalf("error getting updated node: %v", err) + } + + if !reflect.DeepEqual(actualNode.Labels, test.expectedLabels) { + t.Logf("actual node labels: %v", actualNode.Labels) + t.Logf("expected node labels: %v", test.expectedLabels) + t.Errorf("updated node did not match expected node") + } + }) + } + +} + // Tests that node address changes are detected correctly func TestNodeAddressesChangeDetected(t *testing.T) { addressSet1 := []v1.NodeAddress{ diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/BUILD b/staging/src/k8s.io/cloud-provider/node/helpers/BUILD index e6c77e354a9..a1fc3a33de7 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/BUILD +++ b/staging/src/k8s.io/cloud-provider/node/helpers/BUILD @@ -5,6 +5,7 @@ go_library( srcs = [ "address.go", "conditions.go", + "labels.go", "taints.go", ], importmap = "k8s.io/kubernetes/vendor/k8s.io/cloud-provider/node/helpers", @@ -15,10 +16,12 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/util/retry:go_default_library", + "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/labels.go b/staging/src/k8s.io/cloud-provider/node/helpers/labels.go new file mode 100644 index 00000000000..80e77bb145c --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/node/helpers/labels.go @@ -0,0 +1,102 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helpers + +import ( + "encoding/json" + "fmt" + "time" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/wait" + clientset "k8s.io/client-go/kubernetes" + clientretry "k8s.io/client-go/util/retry" + "k8s.io/klog" +) + +var updateLabelBackoff = wait.Backoff{ + Steps: 5, + Duration: 100 * time.Millisecond, + Jitter: 1.0, +} + +// AddOrUpdateLabelsOnNode updates the labels on the node and returns true on +// success and false on failure. +func AddOrUpdateLabelsOnNode(kubeClient clientset.Interface, labelsToUpdate map[string]string, node *v1.Node) bool { + err := addOrUpdateLabelsOnNode(kubeClient, node.Name, labelsToUpdate) + if err != nil { + utilruntime.HandleError( + fmt.Errorf( + "unable to update labels %+v for Node %q: %v", + labelsToUpdate, + node.Name, + err)) + return false + } + + klog.V(4).Infof("Updated labels %+v to Node %v", labelsToUpdate, node.Name) + return true +} + +func addOrUpdateLabelsOnNode(kubeClient clientset.Interface, nodeName string, labelsToUpdate map[string]string) error { + firstTry := true + return clientretry.RetryOnConflict(updateLabelBackoff, func() error { + var err error + var node *v1.Node + // First we try getting node from the API server cache, as it's cheaper. If it fails + // we get it from etcd to be sure to have fresh data. + if firstTry { + node, err = kubeClient.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"}) + firstTry = false + } else { + node, err = kubeClient.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{}) + } + if err != nil { + return err + } + + // Make a copy of the node and update the labels. + newNode := node.DeepCopy() + if newNode.Labels == nil { + newNode.Labels = make(map[string]string) + } + for key, value := range labelsToUpdate { + newNode.Labels[key] = value + } + + oldData, err := json.Marshal(node) + if err != nil { + return fmt.Errorf("failed to marshal the existing node %#v: %v", node, err) + } + newData, err := json.Marshal(newNode) + if err != nil { + return fmt.Errorf("failed to marshal the new node %#v: %v", newNode, err) + } + patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, &v1.Node{}) + if err != nil { + return fmt.Errorf("failed to create a two-way merge patch: %v", err) + } + if _, err := kubeClient.CoreV1().Nodes().Patch(node.Name, types.StrategicMergePatchType, patchBytes); err != nil { + return fmt.Errorf("failed to patch the node: %v", err) + } + return nil + }) +} From 4c194d52da26fc99a02e16515312713118610e47 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 14 Aug 2019 14:35:42 -0400 Subject: [PATCH 3/5] 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.") } From 07229d6c51491fd7e60e8699691e19353e1daf3b Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 28 Aug 2019 12:59:09 -0400 Subject: [PATCH 4/5] pkg/util/node: update GetZoneKey to check both beta and GA labels Signed-off-by: Andrew Sy Kim --- .../node_lifecycle_controller_test.go | 244 ++++++++++++------ .../internal/cache/node_tree_test.go | 35 ++- pkg/util/node/node.go | 22 +- pkg/util/node/node_test.go | 81 ++++++ 4 files changed, 294 insertions(+), 88 deletions(-) diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go index 7d8a9e2eb99..ea0f75eaec2 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -197,8 +197,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { fakeNow := metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC) evictionTimeout := 10 * time.Minute labels := map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", } // Because of the logic that prevents NC from evicting anything when all Nodes are NotReady @@ -234,8 +236,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: fakeNow, Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, }, @@ -244,8 +248,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -314,8 +320,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -334,8 +342,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -378,8 +388,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -398,8 +410,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -469,8 +483,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -489,8 +505,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -533,8 +551,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -553,8 +573,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -597,8 +619,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -617,8 +641,10 @@ func TestMonitorNodeHealthEvictPods(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -762,8 +788,10 @@ func TestPodStatusChange(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -930,8 +958,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -950,8 +980,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -985,8 +1017,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1005,8 +1039,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region2", - v1.LabelZoneFailureDomain: "zone2", + v1.LabelZoneRegionStable: "region2", + v1.LabelZoneFailureDomainStable: "zone2", + v1.LabelZoneRegion: "region2", + v1.LabelZoneFailureDomain: "zone2", }, }, Status: v1.NodeStatus{ @@ -1047,8 +1083,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1067,8 +1105,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone2", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone2", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone2", }, }, Status: v1.NodeStatus{ @@ -1108,8 +1148,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1128,8 +1170,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node-master", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1167,8 +1211,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1187,8 +1233,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone2", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone2", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone2", }, }, Status: v1.NodeStatus{ @@ -1229,8 +1277,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1249,8 +1299,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1269,8 +1321,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node2", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1289,8 +1343,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node3", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -1309,8 +1365,10 @@ func TestMonitorNodeHealthEvictPodsWithDisruption(t *testing.T) { Name: "node4", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2584,8 +2642,10 @@ func TestApplyNoExecuteTaints(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2606,8 +2666,10 @@ func TestApplyNoExecuteTaints(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2627,8 +2689,10 @@ func TestApplyNoExecuteTaints(t *testing.T) { Name: "node2", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2731,8 +2795,10 @@ func TestSwapUnreachableNotReadyTaints(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2754,8 +2820,10 @@ func TestSwapUnreachableNotReadyTaints(t *testing.T) { Name: "node1", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2880,8 +2948,10 @@ func TestTaintsNodeByCondition(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2939,8 +3009,10 @@ func TestTaintsNodeByCondition(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2969,8 +3041,10 @@ func TestTaintsNodeByCondition(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -2999,8 +3073,10 @@ func TestTaintsNodeByCondition(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -3023,8 +3099,10 @@ func TestTaintsNodeByCondition(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -3145,8 +3223,10 @@ func TestReconcileNodeLabels(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegion: "region1", + v1.LabelZoneFailureDomain: "zone1", }, }, Status: v1.NodeStatus{ @@ -3201,12 +3281,12 @@ func TestReconcileNodeLabels(t *testing.T) { Name: "node0", CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), Labels: map[string]string{ - v1.LabelZoneRegion: "region1", + v1.LabelZoneRegionStable: "region1", }, }, }, ExpectedLabels: map[string]string{ - v1.LabelZoneRegion: "region1", + v1.LabelZoneRegionStable: "region1", }, }, { diff --git a/pkg/scheduler/internal/cache/node_tree_test.go b/pkg/scheduler/internal/cache/node_tree_test.go index 726e3158a24..34d2d3af2e8 100644 --- a/pkg/scheduler/internal/cache/node_tree_test.go +++ b/pkg/scheduler/internal/cache/node_tree_test.go @@ -108,7 +108,30 @@ var allNodes = []*v1.Node{ v1.LabelZoneFailureDomain: "zone-2", }, }, - }} + }, + // Node 9: a node with zone + region label and the deprecated zone + region label + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-9", + Labels: map[string]string{ + v1.LabelZoneRegionStable: "region-2", + v1.LabelZoneFailureDomainStable: "zone-2", + v1.LabelZoneRegion: "region-2", + v1.LabelZoneFailureDomain: "zone-2", + }, + }, + }, + // Node 10: a node with only the deprecated zone + region labels + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-10", + Labels: map[string]string{ + v1.LabelZoneRegion: "region-2", + v1.LabelZoneFailureDomain: "zone-3", + }, + }, + }, +} func verifyNodeTree(t *testing.T, nt *nodeTree, expectedTree map[string]*nodeArray) { expectedNumNodes := int(0) @@ -164,6 +187,14 @@ func TestNodeTree_AddNode(t *testing.T) { "region-2:\x00:zone-2": {[]string{"node-6"}, 0}, }, }, + { + name: "nodes also using deprecated zone/region label", + nodesToAdd: allNodes[9:], + expectedTree: map[string]*nodeArray{ + "region-2:\x00:zone-2": {[]string{"node-9"}, 0}, + "region-2:\x00:zone-3": {[]string{"node-10"}, 0}, + }, + }, } for _, test := range tests { @@ -400,7 +431,7 @@ func TestNodeTreeMultiOperations(t *testing.T) { nodesToAdd: append(allNodes[4:9], allNodes[3]), nodesToRemove: nil, operations: []string{"add", "add", "add", "add", "add", "next", "next", "next", "next", "add", "next", "next", "next"}, - expectedOutput: []string{"node-4", "node-5", "node-6", "node-7", "node-3", "node-8", "node-4"}, + expectedOutput: []string{"node-4", "node-6", "node-7", "node-8", "node-3", "node-4", "node-6"}, }, { name: "remove zone and add new to ensure exhausted is reset correctly", diff --git a/pkg/util/node/node.go b/pkg/util/node/node.go index c31ad647513..9984380db81 100644 --- a/pkg/util/node/node.go +++ b/pkg/util/node/node.go @@ -139,23 +139,37 @@ func GetNodeIP(client clientset.Interface, hostname string) net.IP { // GetZoneKey is a helper function that builds a string identifier that is unique per failure-zone; // it returns empty-string for no zone. +// Since there are currently two separate zone keys: +// * "failure-domain.beta.kubernetes.io/zone" +// * "topology.kubernetes.io/zone" +// GetZoneKey will first check failure-domain.beta.kubernetes.io/zone and if not exists, will then check +// topology.kubernetes.io/zone func GetZoneKey(node *v1.Node) string { labels := node.Labels if labels == nil { return "" } - region, _ := labels[v1.LabelZoneRegion] - failureDomain, _ := labels[v1.LabelZoneFailureDomain] + // TODO: prefer stable labels for zone in v1.18 + zone, ok := labels[v1.LabelZoneFailureDomain] + if !ok { + zone, _ = labels[v1.LabelZoneFailureDomainStable] + } - if region == "" && failureDomain == "" { + // TODO: prefer stable labels for region in v1.18 + region, ok := labels[v1.LabelZoneRegion] + if !ok { + region, _ = labels[v1.LabelZoneRegionStable] + } + + if region == "" && zone == "" { return "" } // We include the null character just in case region or failureDomain has a colon // (We do assume there's no null characters in a region or failureDomain) // As a nice side-benefit, the null character is not printed by fmt.Print or glog - return region + ":\x00:" + failureDomain + return region + ":\x00:" + zone } type nodeForConditionPatch struct { diff --git a/pkg/util/node/node_test.go b/pkg/util/node/node_test.go index 2d7d2d62775..c02a8679a85 100644 --- a/pkg/util/node/node_test.go +++ b/pkg/util/node/node_test.go @@ -120,3 +120,84 @@ func TestGetHostname(t *testing.T) { } } + +func Test_GetZoneKey(t *testing.T) { + tests := []struct { + name string + node *v1.Node + zone string + }{ + { + name: "has no zone or region keys", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + zone: "", + }, + { + name: "has beta zone and region keys", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegion: "region1", + }, + }, + }, + zone: "region1:\x00:zone1", + }, + { + name: "has GA zone and region keys", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegionStable: "region1", + }, + }, + }, + zone: "region1:\x00:zone1", + }, + { + name: "has both beta and GA zone and region keys", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomain: "zone1", + v1.LabelZoneRegion: "region1", + }, + }, + }, + zone: "region1:\x00:zone1", + }, + { + name: "has both beta and GA zone and region keys, beta labels take precedent", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + v1.LabelZoneFailureDomainStable: "zone1", + v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomain: "zone2", + v1.LabelZoneRegion: "region2", + }, + }, + }, + zone: "region2:\x00:zone2", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + zone := GetZoneKey(test.node) + if zone != test.zone { + t.Logf("actual zone key: %q", zone) + t.Logf("expected zone key: %q", test.zone) + t.Errorf("unexpected zone key") + } + }) + } +} From 349749644fb40f737c44e27b5b749bd18a3efaae Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Wed, 28 Aug 2019 13:14:02 -0400 Subject: [PATCH 5/5] test/e2e: check both beta and zone label for getting cluster zone Signed-off-by: Andrew Sy Kim --- 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 +- .../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 - test/e2e/framework/providers/gce/util.go | 16 +- test/e2e/framework/util.go | 4 + .../storage/vsphere/vsphere_zone_support.go | 2 +- 32 files changed, 201 insertions(+), 1352 deletions(-) diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index f2cc3582725..4885d991cef 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/pkg/volume/awsebs/aws_ebs.go @@ -35,7 +35,6 @@ 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" @@ -520,7 +519,7 @@ func (c *awsElasticBlockStoreProvisioner) Provision(selectedNode *v1.Node, allow pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: c.options.PVName, - Labels: labels, + Labels: map[string]string{}, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "aws-ebs-dynamic-provisioner", }, @@ -548,9 +547,21 @@ 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 = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements return pv, nil } diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index fa8387f85ce..e10f5273362 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 != 2 { + if n != 1 { t.Errorf("Provision() returned unexpected number of NodeSelectorTerms %d. Expected %d", n, 1) } @@ -228,25 +228,6 @@ 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 038dbec4281..6936e40428d 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -322,16 +322,22 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie if zoned { // Set node affinity labels based on availability zone labels. if len(labels) > 0 { - nodeSelectorTerms = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) - } + 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, + }) + } } 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++ { - deprecatedTopologyReqs := []v1.NodeSelectorRequirement{ + requirements := []v1.NodeSelectorRequirement{ { Key: v1.LabelZoneRegion, Operator: v1.NodeSelectorOpIn, @@ -343,25 +349,8 @@ 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: deprecatedTopologyReqs, - }) - nodeSelectorTerms = append(nodeSelectorTerms, v1.NodeSelectorTerm{ - MatchExpressions: topologyReqs, + MatchExpressions: requirements, }) } } diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index c71e6c951be..b95929a82c2 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -31,7 +31,6 @@ 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" @@ -619,9 +618,18 @@ func (c *cinderVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTopolo pv.Spec.AccessModes = c.plugin.GetAccessModes() } - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = volumehelpers.TranslateZoneRegionLabelsToNodeSelectorTerms(labels) + 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 + } return pv, nil } diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index 1c81ab45ebb..7926ed4a3b3 100644 --- a/pkg/volume/cinder/cinder_test.go +++ b/pkg/volume/cinder/cinder_test.go @@ -123,7 +123,6 @@ 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 } @@ -230,7 +229,7 @@ func TestPlugin(t *testing.T) { } n := len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) - if n != 2 { + if n != 1 { t.Errorf("Provision() returned unexpected number of NodeSelectorTerms %d. Expected %d", n, 1) } @@ -253,25 +252,6 @@ 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 fc99299e611..60092dfc52a 100644 --- a/pkg/volume/cinder/cinder_util.go +++ b/pkg/volume/cinder/cinder_util.go @@ -157,12 +157,6 @@ 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) @@ -235,12 +229,9 @@ 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 3ed3c8295ec..43a7126ae74 100644 --- a/pkg/volume/gcepd/attacher_test.go +++ b/pkg/volume/gcepd/attacher_test.go @@ -457,8 +457,7 @@ 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.LabelZoneFailureDomainStable: zonesLabel, + v1.LabelZoneFailureDomain: zonesLabel, } } diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index 738eea6a57c..9f770cdf69d 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: labels, + Labels: map[string]string{}, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "gce-pd-dynamic-provisioner", }, @@ -542,15 +542,32 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT pv.Spec.AccessModes = c.plugin.GetAccessModes() } - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - - nodeSelectorTerms, err := volumehelpers.TranslateZoneLabelsToNodeSelectorTerms(labels) - if err != nil { - return nil, err + 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.Required.NodeSelectorTerms = nodeSelectorTerms + 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 + } return pv, nil } diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index 8cdcedc6b23..e2bd930d555 100644 --- a/pkg/volume/gcepd/gce_pd_test.go +++ b/pkg/volume/gcepd/gce_pd_test.go @@ -86,7 +86,6 @@ 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 } @@ -204,14 +203,10 @@ 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) != 2 { + if len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) != 1 { t.Errorf("Unexpected number of NodeSelectorTerms") } term := persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[0] @@ -232,24 +227,6 @@ 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 074f7ba84ef..146b208001d 100644 --- a/pkg/volume/gcepd/gce_util.go +++ b/pkg/volume/gcepd/gce_util.go @@ -349,11 +349,7 @@ 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, ok := spec.PersistentVolume.Labels[v1.LabelZoneFailureDomain] - if !ok { - zonesLabel = spec.PersistentVolume.Labels[v1.LabelZoneFailureDomainStable] - } - + zonesLabel := spec.PersistentVolume.Labels[v1.LabelZoneFailureDomain] 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 dd3980af2f3..14066bffe55 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -223,231 +223,6 @@ 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 7301da0f8d5..b6de7fba54a 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: volSpec.Labels, + Labels: map[string]string{}, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "vsphere-volume-dynamic-provisioner", }, @@ -430,15 +430,33 @@ func (v *vsphereVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTopol pv.Spec.AccessModes = v.plugin.GetAccessModes() } - 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 + 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.Required.NodeSelectorTerms = nodeSelectorTerms + 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 + } return pv, nil } diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go index a18135a87d4..62fe2b3a5fe 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission.go @@ -22,7 +22,6 @@ import ( "errors" "fmt" "io" - "reflect" "sync" v1 "k8s.io/api/core/v1" @@ -118,13 +117,11 @@ 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 @@ -132,11 +129,8 @@ 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 || k == v1.LabelZoneFailureDomainStable { + if k == v1.LabelZoneFailureDomain { 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)) @@ -146,17 +140,7 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute } else { values = []string{v} } - - // 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}) - } + requirements = append(requirements, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values}) } if volume.Spec.NodeAffinity == nil { @@ -169,44 +153,16 @@ 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) } - - 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) + 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) + } } } - - 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 @@ -215,17 +171,15 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) { existingLabels := volume.Labels - // deprecated zone/region labels are still the source of truth + // All cloud providers set only these two labels. 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.LabelZoneFailureDomainStable: domain, - v1.LabelZoneRegionStable: region, + v1.LabelZoneFailureDomain: domain, + v1.LabelZoneRegion: 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 06756132804..ae890c8daa0 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go @@ -66,10 +66,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"}, @@ -175,10 +174,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"}, @@ -195,10 +193,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -229,25 +226,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -259,19 +237,15 @@ 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.LabelZoneFailureDomainStable: "domain1", - v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "existingDomain", - v1.LabelZoneRegion: "existingRegion", - v1.LabelZoneFailureDomainStable: "existingDomain", - v1.LabelZoneRegionStable: "existingRegion", + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", }, Annotations: map[string]string{ persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", @@ -290,10 +264,8 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "existingDomain", - v1.LabelZoneRegion: "existingRegion", - v1.LabelZoneFailureDomainStable: "existingDomain", - v1.LabelZoneRegionStable: "existingRegion", + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", }, Annotations: map[string]string{ persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", @@ -322,20 +294,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -347,10 +305,8 @@ 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.LabelZoneFailureDomainStable: "domain1", - v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -373,10 +329,8 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelZoneFailureDomain: "domain1", - v1.LabelZoneRegion: "region1", - v1.LabelZoneFailureDomainStable: "domain1", - v1.LabelZoneRegionStable: "region1", + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", }, }, Spec: api.PersistentVolumeSpec{ @@ -402,20 +356,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -427,10 +367,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "gcepd", Namespace: "myns"}, @@ -447,10 +386,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "gcepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -481,25 +419,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -511,10 +430,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -534,10 +452,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "azurepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -568,25 +485,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -598,10 +496,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -621,10 +518,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "azurepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -655,25 +551,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -685,10 +562,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -711,10 +587,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -745,25 +620,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -963,10 +819,9 @@ 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", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -986,10 +841,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "vSpherePV", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelZoneFailureDomain: "1__2__3", - v1.LabelZoneFailureDomainStable: "1__2__3", + "a": "1", + "b": "2", + v1.LabelZoneFailureDomain: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -1020,25 +874,6 @@ 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"}, - }, - }, - }, }, }, }, @@ -1065,8 +900,6 @@ 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") } @@ -1094,12 +927,10 @@ func sortMatchExpressions(pv *api.PersistentVolume) { return } - 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 - }) + match := pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions + sort.Slice(match, func(i, j int) bool { + return match[i].Key < match[j].Key + }) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[t].MatchExpressions = match - } + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].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 edd7b4d3275..ec9c5d99158 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go @@ -120,11 +120,7 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone var ok bool zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] if !ok { - 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) + return nil, fmt.Errorf("%s Label for node missing", v1.LabelZoneFailureDomain) } // if single replica volume and node with zone found, return immediately if numReplicas == 1 { @@ -139,8 +135,7 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone } if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) { - 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) + return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomain) } if allowedZones.Len() > 0 { @@ -190,7 +185,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 || exp.Key == v1.LabelZoneFailureDomainStable { + if exp.Key == v1.LabelZoneFailureDomain { for _, value := range exp.Values { zones.Insert(value) } @@ -313,103 +308,3 @@ 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 2f5f92f03ec..0a3b156f95a 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,10 +17,7 @@ limitations under the License. package helpers import ( - "fmt" "hash/fnv" - "reflect" - "sort" "testing" "k8s.io/api/core/v1" @@ -420,7 +417,7 @@ func TestChooseZonesForVolume(t *testing.T) { func TestSelectZoneForVolume(t *testing.T) { nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX", v1.LabelZoneFailureDomainStable: "zoneX"} + nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX"} nodeWithNoLabels := &v1.Node{} @@ -661,10 +658,6 @@ func TestSelectZoneForVolume(t *testing.T) { Key: v1.LabelZoneFailureDomain, Values: []string{"zoneX", "zoneY"}, }, - { - Key: v1.LabelZoneFailureDomainStable, - Values: []string{"zoneX", "zoneY"}, - }, }, }, }, @@ -696,22 +689,6 @@ 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", @@ -1133,7 +1110,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, both beta and GA zone labels + // [3] AllowedTopologies with single term with multiple values specified // [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 @@ -1149,10 +1126,6 @@ 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"}, - }, }, }, }, @@ -1165,35 +1138,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, 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 + // [3] AllowedTopologies with single term with multiple values specified // [4] ReplicaCount specified // [5] ZonesWithNodes irrelevant { @@ -1207,10 +1152,6 @@ func TestSelectZonesForVolume(t *testing.T) { Key: v1.LabelZoneFailureDomain, Values: []string{"zoneX", "zoneY"}, }, - { - Key: v1.LabelZoneFailureDomainStable, - Values: []string{"zoneX", "zoneY"}, - }, }, }, }, @@ -1334,7 +1275,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, has both beta/GA zone labels + // [3] AllowedTopologies with multiple terms specified // [4] ReplicaCount specified // [5] ZonesWithNodes irrelevant { @@ -1358,22 +1299,6 @@ 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, @@ -1712,396 +1637,3 @@ 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 160d14afdce..7f07c814bb4 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,16 +73,13 @@ 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 !zoneFound && (exp.Key == v1.LabelZoneFailureDomain || exp.Key == v1.LabelZoneFailureDomainStable) { + if exp.Key == v1.LabelZoneFailureDomain { newExp = v1.TopologySelectorLabelRequirement{ Key: GCEPDTopologyKey, Values: exp.Values, } - - zoneFound = true } else if exp.Key == GCEPDTopologyKey { newExp = exp } else { @@ -243,11 +240,7 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten return nil, fmt.Errorf("pv is nil or GCE Persistent Disk source not defined on pv") } - zonesLabel, ok := pv.Labels[v1.LabelZoneFailureDomain] - if !ok { - zonesLabel = pv.Labels[v1.LabelZoneFailureDomainStable] - } - + zonesLabel := pv.Labels[v1.LabelZoneFailureDomain] 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 93c644cb328..be3d4f195cc 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 @@ -71,14 +71,9 @@ func TestTranslatePDInTreeStorageClassToCSI(t *testing.T) { options: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})), }, - { - 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"})), + options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelZoneFailureDomain, []string{"foo"})), expOptions: NewStorageClass(map[string]string{}, 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 846f8fe90f6..4320d8861cf 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -2639,15 +2639,11 @@ 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 ccf062c1bd3..4da723a6760 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,11 +1262,8 @@ 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", - v1.LabelZoneFailureDomainStable: "us-east-1a", - v1.LabelZoneRegionStable: "us-east-1", - }, labels) + v1.LabelZoneFailureDomain: "us-east-1a", + v1.LabelZoneRegion: "us-east-1"}, labels) awsServices.ec2.(*MockedFakeEC2).AssertExpectations(t) } @@ -1339,10 +1336,8 @@ 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.LabelZoneFailureDomainStable: "us-east-1a", - v1.LabelZoneRegionStable: "us-east-1", + v1.LabelZoneFailureDomain: "us-east-1a", + v1.LabelZoneRegion: "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 5b6aeba7c95..5dd572fcf0b 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -636,9 +636,7 @@ func (az *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) { prevNode := prev.(*v1.Node) newNode := obj.(*v1.Node) if newNode.Labels[v1.LabelZoneFailureDomain] == - prevNode.Labels[v1.LabelZoneFailureDomain] && - newNode.Labels[v1.LabelZoneFailureDomainStable] == - prevNode.Labels[v1.LabelZoneFailureDomainStable] { + prevNode.Labels[v1.LabelZoneFailureDomain] { return } az.updateNodeCaches(prevNode, newNode) @@ -673,10 +671,6 @@ 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 { @@ -700,10 +694,6 @@ 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 96d90b6668e..847da0237c2 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,10 +333,8 @@ 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.LabelZoneRegionStable: c.Location, - v1.LabelZoneFailureDomainStable: zone, + v1.LabelZoneRegion: c.Location, + v1.LabelZoneFailureDomain: 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 f39cb7ef00b..1de777909c0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go @@ -694,9 +694,7 @@ func (g *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) { prevNode := prev.(*v1.Node) newNode := obj.(*v1.Node) if newNode.Labels[v1.LabelZoneFailureDomain] == - prevNode.Labels[v1.LabelZoneFailureDomain] && - newNode.Labels[v1.LabelZoneFailureDomainStable] == - prevNode.Labels[v1.LabelZoneFailureDomainStable] { + prevNode.Labels[v1.LabelZoneFailureDomain] { return } g.updateNodeZones(prevNode, newNode) @@ -728,10 +726,6 @@ 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 { @@ -741,10 +735,6 @@ 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 4a81953a1aa..ba18584fea8 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,10 +499,7 @@ func (g *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) } // If the zone is already labeled, honor the hint - zone, ok := pv.Labels[v1.LabelZoneFailureDomain] - if !ok { - zone = pv.Labels[v1.LabelZoneFailureDomainStable] - } + zone := pv.Labels[v1.LabelZoneFailureDomain] labels, err := g.GetAutoLabelsForPD(pv.Spec.GCEPersistentDisk.PDName, zone) if err != nil { @@ -859,8 +856,6 @@ 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 @@ -869,9 +864,6 @@ 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 78957deee01..413c9d32529 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,23 +465,13 @@ 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) { @@ -518,14 +508,6 @@ 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) { @@ -612,14 +594,6 @@ 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 56f9de3929e..15d7b6f2e3f 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,20 +60,12 @@ 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 zone + if !ok { + return defaultZone } - - zone, ok = n.Labels[v1.LabelZoneFailureDomainStable] - if ok { - return zone - } - - return defaultZone + return zone } 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 90b450831df..95e38854912 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,8 +736,6 @@ 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 4e3a43836d2..bf2f1cd4eaf 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 := getNodeZoneFailureDomain(node) - nodeRegion := getNodeRegion(node) + nodeFd := node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + nodeRegion := node.ObjectMeta.Labels[v1.LabelZoneRegion] 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,34 +230,6 @@ 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 dcfc3709004..305e88be6a1 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -1649,8 +1649,6 @@ 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/framework/providers/gce/util.go b/test/e2e/framework/providers/gce/util.go index 34e3172f070..e44169ee723 100644 --- a/test/e2e/framework/providers/gce/util.go +++ b/test/e2e/framework/providers/gce/util.go @@ -34,11 +34,19 @@ func RecreateNodes(c clientset.Interface, nodes []v1.Node) error { nodeNamesByZone := make(map[string][]string) for i := range nodes { node := &nodes[i] - zone := framework.TestContext.CloudConfig.Zone - if z, ok := node.Labels[v1.LabelZoneFailureDomain]; ok { - zone = z + + if zone, ok := node.Labels[v1.LabelZoneFailureDomain]; ok { + nodeNamesByZone[zone] = append(nodeNamesByZone[zone], node.Name) + continue } - nodeNamesByZone[zone] = append(nodeNamesByZone[zone], node.Name) + + if zone, ok := node.Labels[v1.LabelZoneFailureDomainStable]; ok { + nodeNamesByZone[zone] = append(nodeNamesByZone[zone], node.Name) + continue + } + + defaultZone := framework.TestContext.CloudConfig.Zone + nodeNamesByZone[defaultZone] = append(nodeNamesByZone[defaultZone], node.Name) } // Find the sole managed instance group name diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 19f4b1d652a..30c31470ce7 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -2587,6 +2587,10 @@ func GetClusterZones(c clientset.Interface) (sets.String, error) { if zone, found := node.Labels[v1.LabelZoneFailureDomain]; found { zones.Insert(zone) } + + if zone, found := node.Labels[v1.LabelZoneFailureDomainStable]; found { + zones.Insert(zone) + } } return zones, nil } diff --git a/test/e2e/storage/vsphere/vsphere_zone_support.go b/test/e2e/storage/vsphere/vsphere_zone_support.go index 19df404e290..8f06e119f9f 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[v1.LabelZoneFailureDomain], "__") + pvZoneLabels := strings.Split(pv.ObjectMeta.Labels["failure-domain.beta.kubernetes.io/zone"], "__") for _, zone := range zones { gomega.Expect(pvZoneLabels).Should(gomega.ContainElement(zone), "Incorrect or missing zone labels in pv.") }