From f6697538bc3bf87b99bd5544cd32b7d589c09bac Mon Sep 17 00:00:00 2001 From: Marwan Ahmed Date: Mon, 25 Oct 2021 13:57:11 -0700 Subject: [PATCH 1/2] add recent AKS agentpool label to ignore for similarity checks --- .../processors/nodegroupset/azure_nodegroups.go | 14 ++++++++++++-- .../nodegroupset/azure_nodegroups_test.go | 9 +++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go index 3eff7323c33a..7462f9a6ea2a 100644 --- a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go +++ b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go @@ -20,12 +20,21 @@ import ( schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" ) -// AzureNodepoolLabel is a label specifying which Azure node pool a particular node belongs to. -const AzureNodepoolLabel = "agentpool" +// AzureNodepoolLegacyLabel is a label specifying which Azure node pool a particular node belongs to. +const AzureNodepoolLegacyLabel = "agentpool" + +// AzureNodepoolLabel is an AKS label specifying which nodepool a particular node belongs to +const AzureNodepoolLabel = "kubernetes.azure.com/agentpool" func nodesFromSameAzureNodePool(n1, n2 *schedulerframework.NodeInfo) bool { n1AzureNodePool := n1.Node().Labels[AzureNodepoolLabel] n2AzureNodePool := n2.Node().Labels[AzureNodepoolLabel] + return (n1AzureNodePool != "" && n1AzureNodePool == n2AzureNodePool) || nodesFromSameAzureNodePoolLegacy(n1, n2) +} + +func nodesFromSameAzureNodePoolLegacy(n1, n2 *schedulerframework.NodeInfo) bool { + n1AzureNodePool := n1.Node().Labels[AzureNodepoolLegacyLabel] + n2AzureNodePool := n2.Node().Labels[AzureNodepoolLegacyLabel] return n1AzureNodePool != "" && n1AzureNodePool == n2AzureNodePool } @@ -37,6 +46,7 @@ func CreateAzureNodeInfoComparator(extraIgnoredLabels []string) NodeInfoComparat for k, v := range BasicIgnoredLabels { azureIgnoredLabels[k] = v } + azureIgnoredLabels[AzureNodepoolLegacyLabel] = true azureIgnoredLabels[AzureNodepoolLabel] = true for _, k := range extraIgnoredLabels { azureIgnoredLabels[k] = true diff --git a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go index 3d64b373cc0f..40bd981abe13 100644 --- a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go +++ b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go @@ -41,6 +41,10 @@ func TestIsAzureNodeInfoSimilar(t *testing.T) { n1.ObjectMeta.Labels["agentpool"] = "" n2.ObjectMeta.Labels["agentpool"] = "" checkNodesSimilar(t, n1, n2, comparator, false) + // AKS agentpool labels + n1.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foo" + n2.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "bar" + checkNodesSimilar(t, n1, n2, comparator, false) // Only one non empty n1.ObjectMeta.Labels["agentpool"] = "" n2.ObjectMeta.Labels["agentpool"] = "foo" @@ -109,6 +113,8 @@ func TestFindSimilarNodeGroupsAzureByLabel(t *testing.T) { // Unless we give them nodepool label. n1.ObjectMeta.Labels["agentpool"] = "foobar" n2.ObjectMeta.Labels["agentpool"] = "foobar" + n1.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foobar" + n2.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foobar" similar, err = processor.FindSimilarNodeGroups(context, ng1, nodeInfosForGroups) assert.NoError(t, err) assert.Equal(t, similar, []cloudprovider.NodeGroup{ng2}) @@ -125,6 +131,9 @@ func TestFindSimilarNodeGroupsAzureByLabel(t *testing.T) { n1.ObjectMeta.Labels["agentpool"] = "foobar1" n2.ObjectMeta.Labels["agentpool"] = "foobar2" n3.ObjectMeta.Labels["agentpool"] = "foobar3" + n1.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foobar1" + n2.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foobar2" + n3.ObjectMeta.Labels["kubernetes.azure.com/agentpool"] = "foobar3" similar, err = processor.FindSimilarNodeGroups(context, ng1, nodeInfosForGroups) assert.NoError(t, err) From d8df72eca1355f012dbc7c147c05421c0e7bd9cd Mon Sep 17 00:00:00 2001 From: Marwan Ahmed Date: Tue, 21 Dec 2021 20:35:24 +0200 Subject: [PATCH 2/2] ignore azure csi topology label for similarity checks and populate it for scale from zero --- cluster-autoscaler/cloudprovider/azure/azure_template.go | 4 ++++ .../processors/nodegroupset/azure_nodegroups.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_template.go b/cluster-autoscaler/cloudprovider/azure/azure_template.go index b69143bb0552..fde3a0db1f8b 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_template.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_template.go @@ -31,6 +31,8 @@ import ( "strings" ) +const azureDiskTopologyKey string = "topology.disk.csi.azure.com/zone" + func buildInstanceOS(template compute.VirtualMachineScaleSet) string { instanceOS := cloudprovider.DefaultOS if template.VirtualMachineProfile != nil && template.VirtualMachineProfile.OsProfile != nil && template.VirtualMachineProfile.OsProfile.WindowsConfiguration != nil { @@ -56,8 +58,10 @@ func buildGenericLabels(template compute.VirtualMachineScaleSet, nodeName string } result[apiv1.LabelTopologyZone] = strings.Join(failureDomains[:], cloudvolume.LabelMultiZoneDelimiter) + result[azureDiskTopologyKey] = strings.Join(failureDomains[:], cloudvolume.LabelMultiZoneDelimiter) } else { result[apiv1.LabelTopologyZone] = "0" + result[azureDiskTopologyKey] = "" } result[apiv1.LabelHostname] = nodeName diff --git a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go index 7462f9a6ea2a..2d78b137e19d 100644 --- a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go +++ b/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go @@ -26,6 +26,9 @@ const AzureNodepoolLegacyLabel = "agentpool" // AzureNodepoolLabel is an AKS label specifying which nodepool a particular node belongs to const AzureNodepoolLabel = "kubernetes.azure.com/agentpool" +// AzureDiskTopologyKey is the topology key of Azure Disk CSI driver +const AzureDiskTopologyKey = "topology.disk.csi.azure.com/zone" + func nodesFromSameAzureNodePool(n1, n2 *schedulerframework.NodeInfo) bool { n1AzureNodePool := n1.Node().Labels[AzureNodepoolLabel] n2AzureNodePool := n2.Node().Labels[AzureNodepoolLabel] @@ -48,6 +51,7 @@ func CreateAzureNodeInfoComparator(extraIgnoredLabels []string) NodeInfoComparat } azureIgnoredLabels[AzureNodepoolLegacyLabel] = true azureIgnoredLabels[AzureNodepoolLabel] = true + azureIgnoredLabels[AzureDiskTopologyKey] = true for _, k := range extraIgnoredLabels { azureIgnoredLabels[k] = true }