From c3b8f242a51822b4c6a718103f6f94965a4bc573 Mon Sep 17 00:00:00 2001 From: Laila Kassar Date: Thu, 11 Feb 2021 18:35:48 +0000 Subject: [PATCH 1/2] Updated topology labels for Azure Modified to use v1 labels --- pkg/volume/azuredd/azure_provision.go | 8 ++--- .../plugins/azure_disk_test.go | 2 +- .../legacy-cloud-providers/azure/azure.go | 31 ++++++++++++------- .../azure/azure_managedDiskController.go | 4 +-- .../azure/azure_managedDiskController_test.go | 6 ++-- .../azure/azure_test.go | 9 +++--- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/pkg/volume/azuredd/azure_provision.go b/pkg/volume/azuredd/azure_provision.go index 04ba25db27d..dd523b50831 100644 --- a/pkg/volume/azuredd/azure_provision.go +++ b/pkg/volume/azuredd/azure_provision.go @@ -361,19 +361,19 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie }) } } else { - // Set node affinity labels based on fault domains. + // Set node affinity labels based on topology. // This is required because unzoned AzureDisk can't be attached to zoned nodes. - // There are at most 3 fault domains available in each region. + // There are at most 3 Availability Zones per supported Azure region. // Refer https://docs.microsoft.com/en-us/azure/virtual-machines/windows/manage-availability. for i := 0; i < 3; i++ { requirements := []v1.NodeSelectorRequirement{ { - Key: v1.LabelFailureDomainBetaRegion, + Key: v1.LabelTopologyRegion, Operator: v1.NodeSelectorOpIn, Values: []string{diskController.GetLocation()}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{strconv.Itoa(i)}, }, diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go index 9cdfb35c204..ea115e43c31 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go @@ -269,7 +269,7 @@ func TestTranslateInTreeStorageClassToCSI(t *testing.T) { }, { name: "some translated topology", - options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelFailureDomainBetaZone, []string{"foo"})), + options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelTopologyZone, []string{"foo"})), expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(AzureDiskTopologyKey, []string{"foo"})), }, { 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 3e9d13dedc3..13d13b50646 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -92,12 +92,6 @@ const ( externalResourceGroupLabel = "kubernetes.azure.com/resource-group" managedByAzureLabel = "kubernetes.azure.com/managed" - - // LabelFailureDomainBetaZone refer to https://github.com/kubernetes/api/blob/8519c5ea46199d57724725d5b969c5e8e0533692/core/v1/well_known_labels.go#L22-L23 - LabelFailureDomainBetaZone = "failure-domain.beta.kubernetes.io/zone" - - // LabelFailureDomainBetaRegion failure-domain region label - LabelFailureDomainBetaRegion = "failure-domain.beta.kubernetes.io/region" ) const ( @@ -751,8 +745,12 @@ func (az *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) { UpdateFunc: func(prev, obj interface{}) { prevNode := prev.(*v1.Node) newNode := obj.(*v1.Node) - if newNode.Labels[LabelFailureDomainBetaZone] == - prevNode.Labels[LabelFailureDomainBetaZone] { + if newNode.Labels[v1.LabelFailureDomainBetaZone] == + prevNode.Labels[v1.LabelFailureDomainBetaZone] { + return + } + if newNode.Labels[v1.LabelTopologyZone] == + prevNode.Labels[v1.LabelTopologyZone] { return } az.updateNodeCaches(prevNode, newNode) @@ -785,11 +783,13 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) { defer az.nodeCachesLock.Unlock() if prevNode != nil { + // Remove from nodeNames cache. az.nodeNames.Delete(prevNode.ObjectMeta.Name) - // Remove from nodeZones cache. - prevZone, ok := prevNode.ObjectMeta.Labels[LabelFailureDomainBetaZone] + // Remove from nodeZones cache + prevZone, ok := prevNode.ObjectMeta.Labels[v1.LabelTopologyZone] + if ok && az.isAvailabilityZone(prevZone) { az.nodeZones[prevZone].Delete(prevNode.ObjectMeta.Name) if az.nodeZones[prevZone].Len() == 0 { @@ -797,6 +797,15 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) { } } + //Remove from nodeZones cache if using depreciated LabelFailureDomainBetaZone + prevZoneFailureDomain, ok := prevNode.ObjectMeta.Labels[v1.LabelFailureDomainBetaZone] + if ok && az.isAvailabilityZone(prevZoneFailureDomain) { + az.nodeZones[prevZone].Delete(prevNode.ObjectMeta.Name) + if az.nodeZones[prevZone].Len() == 0 { + az.nodeZones[prevZone] = nil + } + } + // Remove from nodeResourceGroups cache. _, ok = prevNode.ObjectMeta.Labels[externalResourceGroupLabel] if ok { @@ -815,7 +824,7 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) { az.nodeNames.Insert(newNode.ObjectMeta.Name) // Add to nodeZones cache. - newZone, ok := newNode.ObjectMeta.Labels[LabelFailureDomainBetaZone] + newZone, ok := newNode.ObjectMeta.Labels[v1.LabelTopologyZone] 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 e93401eb504..61f6057f5e5 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 @@ -355,7 +355,7 @@ func (c *Cloud) GetAzureDiskLabels(diskURI string) (map[string]string, error) { } labels := map[string]string{ - LabelFailureDomainBetaRegion: c.Location, + v1.LabelTopologyRegion: c.Location, } // no azure credential is set, return nil if c.DisksClient == nil { @@ -384,6 +384,6 @@ 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[LabelFailureDomainBetaZone] = zone + labels[v1.LabelTopologyZone] = zone return labels, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController_test.go index 7ead4f5cf27..5cd05d062c7 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController_test.go @@ -427,8 +427,8 @@ func TestGetLabelsForVolume(t *testing.T) { }, existedDisk: compute.Disk{Name: to.StringPtr(diskName), DiskProperties: &compute.DiskProperties{DiskSizeGB: &diskSizeGB}, Zones: &[]string{"1"}}, expected: map[string]string{ - LabelFailureDomainBetaRegion: testCloud0.Location, - LabelFailureDomainBetaZone: testCloud0.makeZone(testCloud0.Location, 1), + v1.LabelTopologyRegion: testCloud0.Location, + v1.LabelTopologyZone: testCloud0.makeZone(testCloud0.Location, 1), }, expectedErr: false, }, @@ -464,7 +464,7 @@ func TestGetLabelsForVolume(t *testing.T) { }, existedDisk: compute.Disk{Name: to.StringPtr(diskName), DiskProperties: &compute.DiskProperties{DiskSizeGB: &diskSizeGB}}, expected: map[string]string{ - LabelFailureDomainBetaRegion: testCloud0.Location, + v1.LabelTopologyRegion: testCloud0.Location, }, expectedErr: false, expectedErrMsg: nil, diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go index cb6a61c08ea..42fb037751e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go @@ -3249,9 +3249,10 @@ func TestUpdateNodeCaches(t *testing.T) { prevNode := v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - LabelFailureDomainBetaZone: zone, - externalResourceGroupLabel: "true", - managedByAzureLabel: "false", + v1.LabelFailureDomainBetaZone: zone, + v1.LabelTopologyZone: zone, + externalResourceGroupLabel: "true", + managedByAzureLabel: "false", }, Name: "prevNode", }, @@ -3266,7 +3267,7 @@ func TestUpdateNodeCaches(t *testing.T) { newNode := v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - LabelFailureDomainBetaZone: zone, + v1.LabelTopologyZone: zone, externalResourceGroupLabel: "true", managedByAzureLabel: "false", }, From 33452330ac16705237d7edc5d466e26e88910d8a Mon Sep 17 00:00:00 2001 From: Laila Kassar Date: Tue, 4 May 2021 18:52:13 +0000 Subject: [PATCH 2/2] added test case for beta labels --- .../k8s.io/csi-translation-lib/plugins/azure_disk_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go index ea115e43c31..df855778773 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go @@ -272,6 +272,11 @@ func TestTranslateInTreeStorageClassToCSI(t *testing.T) { options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelTopologyZone, []string{"foo"})), expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(AzureDiskTopologyKey, []string{"foo"})), }, + { + name: "some translated topology with beta labels", + options: NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelFailureDomainBetaZone, []string{"foo"})), + expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(AzureDiskTopologyKey, []string{"foo"})), + }, { name: "zone and topology", options: NewStorageClass(map[string]string{"zone": "foo"}, generateToplogySelectors(AzureDiskTopologyKey, []string{"foo"})),