From 4ff4079041c7e3d911796a9998ef1f255b232e98 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Fri, 7 Jun 2024 11:01:34 -0700 Subject: [PATCH] cloudprovider-specific nodegroupset Signed-off-by: Jack Francis --- .../aws}/aws_nodegroups.go | 13 +-- .../aws}/aws_nodegroups_test.go | 15 +-- .../azure}/azure_nodegroups.go | 13 +-- .../azure}/azure_nodegroups_test.go | 43 ++++---- .../gce}/gce_nodegroups.go | 11 ++- cluster-autoscaler/main.go | 9 +- .../nodegroupset/balancing_processor_test.go | 65 ++---------- .../nodegroupset/compare_nodegroups_test.go | 57 ++++------- .../compare_nodegroups_test_helpers.go | 98 +++++++++++++++++++ .../nodegroupset/label_nodegroups_test.go | 2 +- 10 files changed, 182 insertions(+), 144 deletions(-) rename cluster-autoscaler/{processors/nodegroupset => cloudprovider/aws}/aws_nodegroups.go (77%) rename cluster-autoscaler/{processors/nodegroupset => cloudprovider/aws}/aws_nodegroups_test.go (89%) rename cluster-autoscaler/{processors/nodegroupset => cloudprovider/azure}/azure_nodegroups.go (86%) rename cluster-autoscaler/{processors/nodegroupset => cloudprovider/azure}/azure_nodegroups_test.go (77%) rename cluster-autoscaler/{processors/nodegroupset => cloudprovider/gce}/gce_nodegroups.go (78%) create mode 100644 cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test_helpers.go diff --git a/cluster-autoscaler/processors/nodegroupset/aws_nodegroups.go b/cluster-autoscaler/cloudprovider/aws/aws_nodegroups.go similarity index 77% rename from cluster-autoscaler/processors/nodegroupset/aws_nodegroups.go rename to cluster-autoscaler/cloudprovider/aws/aws_nodegroups.go index 2c22f828052..ef759d5c91a 100644 --- a/cluster-autoscaler/processors/nodegroupset/aws_nodegroups.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_nodegroups.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2024 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. @@ -14,17 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodegroupset +package aws import ( "k8s.io/autoscaler/cluster-autoscaler/config" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" ) -// CreateAwsNodeInfoComparator returns a comparator that checks if two nodes should be considered +// CreateNodeInfoComparator returns a comparator that checks if two nodes should be considered // part of the same NodeGroupSet. This is true if they match usual conditions checked by IsCloudProviderNodeInfoSimilar, // even if they have different AWS-specific labels. -func CreateAwsNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) NodeInfoComparator { +func CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) nodegroupset.NodeInfoComparator { awsIgnoredLabels := map[string]bool{ "alpha.eksctl.io/instance-id": true, // this is a label used by eksctl to identify instances. "alpha.eksctl.io/nodegroup-name": true, // this is a label used by eksctl to identify "node group" names. @@ -34,7 +35,7 @@ func CreateAwsNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.N "topology.ebs.csi.aws.com/zone": true, // this is a label used by the AWS EBS CSI driver as a target for Persistent Volume Node Affinity } - for k, v := range BasicIgnoredLabels { + for k, v := range nodegroupset.BasicIgnoredLabels { awsIgnoredLabels[k] = v } @@ -43,6 +44,6 @@ func CreateAwsNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.N } return func(n1, n2 *schedulerframework.NodeInfo) bool { - return IsCloudProviderNodeInfoSimilar(n1, n2, awsIgnoredLabels, ratioOpts) + return nodegroupset.IsCloudProviderNodeInfoSimilar(n1, n2, awsIgnoredLabels, ratioOpts) } } diff --git a/cluster-autoscaler/processors/nodegroupset/aws_nodegroups_test.go b/cluster-autoscaler/cloudprovider/aws/aws_nodegroups_test.go similarity index 89% rename from cluster-autoscaler/processors/nodegroupset/aws_nodegroups_test.go rename to cluster-autoscaler/cloudprovider/aws/aws_nodegroups_test.go index 334d9dc8d62..9b291fb0fa0 100644 --- a/cluster-autoscaler/processors/nodegroupset/aws_nodegroups_test.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_nodegroups_test.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Kubernetes Authors. +Copyright 2024 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. @@ -14,18 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodegroupset +package aws import ( "testing" "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/context" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" ) func TestIsAwsNodeInfoSimilar(t *testing.T) { - comparator := CreateAwsNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{}) + comparator := CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{}) node1 := BuildTestNode("node1", 1000, 2000) node2 := BuildTestNode("node2", 1000, 2000) @@ -169,14 +170,14 @@ func TestIsAwsNodeInfoSimilar(t *testing.T) { if tc.removeOneLabel { delete(node2.ObjectMeta.Labels, tc.label) } - checkNodesSimilar(t, node1, node2, comparator, true) + nodegroupset.CheckNodesSimilar(t, node1, node2, comparator, true) }) } } func TestFindSimilarNodeGroupsAwsBasic(t *testing.T) { context := &context.AutoscalingContext{} - ni1, ni2, ni3 := buildBasicNodeGroups(context) - processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAwsNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} - basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) + ni1, ni2, ni3 := nodegroupset.BuildBasicNodeGroups(context) + processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} + nodegroupset.BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) } diff --git a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go b/cluster-autoscaler/cloudprovider/azure/azure_nodegroups.go similarity index 86% rename from cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go rename to cluster-autoscaler/cloudprovider/azure/azure_nodegroups.go index 3b615a62067..1fda177a135 100644 --- a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_nodegroups.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2024 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. @@ -14,10 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodegroupset +package azure import ( "k8s.io/autoscaler/cluster-autoscaler/config" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" ) @@ -52,12 +53,12 @@ func nodesFromSameAzureNodePoolLegacy(n1, n2 *schedulerframework.NodeInfo) bool return n1AzureNodePool != "" && n1AzureNodePool == n2AzureNodePool } -// CreateAzureNodeInfoComparator returns a comparator that checks if two nodes should be considered +// CreateNodeInfoComparator returns a comparator that checks if two nodes should be considered // part of the same NodeGroupSet. This is true if they either belong to the same Azure agentpool // or match usual conditions checked by IsCloudProviderNodeInfoSimilar, even if they have different agentpool labels. -func CreateAzureNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) NodeInfoComparator { +func CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) nodegroupset.NodeInfoComparator { azureIgnoredLabels := make(map[string]bool) - for k, v := range BasicIgnoredLabels { + for k, v := range nodegroupset.BasicIgnoredLabels { azureIgnoredLabels[k] = v } azureIgnoredLabels[AzureNodepoolLegacyLabel] = true @@ -78,6 +79,6 @@ func CreateAzureNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config if nodesFromSameAzureNodePool(n1, n2) { return true } - return IsCloudProviderNodeInfoSimilar(n1, n2, azureIgnoredLabels, ratioOpts) + return nodegroupset.IsCloudProviderNodeInfoSimilar(n1, n2, azureIgnoredLabels, ratioOpts) } } diff --git a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go b/cluster-autoscaler/cloudprovider/azure/azure_nodegroups_test.go similarity index 77% rename from cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go rename to cluster-autoscaler/cloudprovider/azure/azure_nodegroups_test.go index 8b73f36111a..9b7e892d9d8 100644 --- a/cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_nodegroups_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2024 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. @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodegroupset +package azure import ( "testing" @@ -23,6 +23,7 @@ import ( testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/context" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" @@ -30,75 +31,75 @@ import ( ) func TestIsAzureNodeInfoSimilar(t *testing.T) { - comparator := CreateAzureNodeInfoComparator([]string{"example.com/ready"}, config.NodeGroupDifferenceRatios{}) + comparator := CreateNodeInfoComparator([]string{"example.com/ready"}, config.NodeGroupDifferenceRatios{}) n1 := BuildTestNode("node1", 1000, 2000) n1.ObjectMeta.Labels["test-label"] = "test-value" n1.ObjectMeta.Labels["character"] = "thing" n2 := BuildTestNode("node2", 1000, 2000) n2.ObjectMeta.Labels["test-label"] = "test-value" // No node-pool labels. - checkNodesSimilar(t, n1, n2, comparator, false) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false) // Empty agentpool labels n1.ObjectMeta.Labels["agentpool"] = "" n2.ObjectMeta.Labels["agentpool"] = "" - checkNodesSimilar(t, n1, n2, comparator, false) + nodegroupset.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) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false) // Only one non empty n1.ObjectMeta.Labels["agentpool"] = "" n2.ObjectMeta.Labels["agentpool"] = "foo" - checkNodesSimilar(t, n1, n2, comparator, false) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false) // Only one present delete(n1.ObjectMeta.Labels, "agentpool") n2.ObjectMeta.Labels["agentpool"] = "foo" - checkNodesSimilar(t, n1, n2, comparator, false) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false) // Different vales n1.ObjectMeta.Labels["agentpool"] = "foo1" n2.ObjectMeta.Labels["agentpool"] = "foo2" - checkNodesSimilar(t, n1, n2, comparator, false) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false) // Same values n1.ObjectMeta.Labels["agentpool"] = "foo" n2.ObjectMeta.Labels["agentpool"] = "foo" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Same labels except for agentpool delete(n1.ObjectMeta.Labels, "character") n1.ObjectMeta.Labels["agentpool"] = "foo" n2.ObjectMeta.Labels["agentpool"] = "bar" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Different creationSource n1.ObjectMeta.Labels["creationSource"] = "aks-aks-nodepool2-vmss" n2.ObjectMeta.Labels["creationSource"] = "aks-aks-nodepool3-vmss" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Different node image version n1.ObjectMeta.Labels["kubernetes.azure.com/node-image-version"] = "AKSUbuntu-1804gen2-2021.01.28" n2.ObjectMeta.Labels["kubernetes.azure.com/node-image-version"] = "AKSUbuntu-1804gen2-2022.01.30" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Custom label n1.ObjectMeta.Labels["example.com/ready"] = "true" n2.ObjectMeta.Labels["example.com/ready"] = "false" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // One node with aksConsolidatedAdditionalProperties label n1.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "foo" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Same aksConsolidatedAdditionalProperties n2.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "foo" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) // Different aksConsolidatedAdditionalProperties label n2.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "bar" - checkNodesSimilar(t, n1, n2, comparator, true) + nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true) } func TestFindSimilarNodeGroupsAzureBasic(t *testing.T) { context := &context.AutoscalingContext{} - ni1, ni2, ni3 := buildBasicNodeGroups(context) - processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} - basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) + ni1, ni2, ni3 := nodegroupset.BuildBasicNodeGroups(context) + processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} + nodegroupset.BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) } func TestFindSimilarNodeGroupsAzureByLabel(t *testing.T) { - processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} + processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})} context := &context.AutoscalingContext{} n1 := BuildTestNode("n1", 1000, 1000) diff --git a/cluster-autoscaler/processors/nodegroupset/gce_nodegroups.go b/cluster-autoscaler/cloudprovider/gce/gce_nodegroups.go similarity index 78% rename from cluster-autoscaler/processors/nodegroupset/gce_nodegroups.go rename to cluster-autoscaler/cloudprovider/gce/gce_nodegroups.go index 483f6f04f39..0667b95a154 100644 --- a/cluster-autoscaler/processors/nodegroupset/gce_nodegroups.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_nodegroups.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2024 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. @@ -14,22 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodegroupset +package gce import ( "k8s.io/autoscaler/cluster-autoscaler/config" + "k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" ) // CreateGceNodeInfoComparator returns a comparator that checks if two nodes should be considered // part of the same NodeGroupSet. This is true if they match usual conditions checked by IsCloudProviderNodeInfoSimilar, // even if they have different GCE-specific labels. -func CreateGceNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) NodeInfoComparator { +func CreateGceNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) nodegroupset.NodeInfoComparator { gceIgnoredLabels := map[string]bool{ "topology.gke.io/zone": true, } - for k, v := range BasicIgnoredLabels { + for k, v := range nodegroupset.BasicIgnoredLabels { gceIgnoredLabels[k] = v } @@ -38,6 +39,6 @@ func CreateGceNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.N } return func(n1, n2 *schedulerframework.NodeInfo) bool { - return IsCloudProviderNodeInfoSimilar(n1, n2, gceIgnoredLabels, ratioOpts) + return nodegroupset.IsCloudProviderNodeInfoSimilar(n1, n2, gceIgnoredLabels, ratioOpts) } } diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index efa3042e0c2..dbfd0cd1bcd 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -47,7 +47,10 @@ import ( "k8s.io/apiserver/pkg/server/routes" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/aws" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/azure" cloudBuilder "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/builder" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gce" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gce/localssdsize" "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/core" @@ -568,12 +571,12 @@ func buildAutoscaler(debuggingSnapshotter debuggingsnapshot.DebuggingSnapshotter } else { nodeInfoComparatorBuilder := nodegroupset.CreateGenericNodeInfoComparator if autoscalingOptions.CloudProviderName == cloudprovider.AzureProviderName { - nodeInfoComparatorBuilder = nodegroupset.CreateAzureNodeInfoComparator + nodeInfoComparatorBuilder = azure.CreateNodeInfoComparator } else if autoscalingOptions.CloudProviderName == cloudprovider.AwsProviderName { - nodeInfoComparatorBuilder = nodegroupset.CreateAwsNodeInfoComparator + nodeInfoComparatorBuilder = aws.CreateNodeInfoComparator opts.Processors.TemplateNodeInfoProvider = nodeinfosprovider.NewAsgTagResourceNodeInfoProvider(nodeInfoCacheExpireTime, *forceDaemonSets) } else if autoscalingOptions.CloudProviderName == cloudprovider.GceProviderName { - nodeInfoComparatorBuilder = nodegroupset.CreateGceNodeInfoComparator + nodeInfoComparatorBuilder = gce.CreateGceNodeInfoComparator opts.Processors.TemplateNodeInfoProvider = nodeinfosprovider.NewAnnotationNodeInfoProvider(nodeInfoCacheExpireTime, *forceDaemonSets) } nodeInfoComparator = nodeInfoComparatorBuilder(autoscalingOptions.BalancingExtraIgnoredLabels, autoscalingOptions.NodeGroupSetRatios) diff --git a/cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go b/cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go index 9645cdb3997..6def3bd4f9d 100644 --- a/cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go +++ b/cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go @@ -25,83 +25,30 @@ import ( testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/context" - . "k8s.io/autoscaler/cluster-autoscaler/utils/test" "github.com/stretchr/testify/assert" ) -func buildBasicNodeGroups(context *context.AutoscalingContext) (*schedulerframework.NodeInfo, *schedulerframework.NodeInfo, *schedulerframework.NodeInfo) { - n1 := BuildTestNode("n1", 1000, 1000) - n2 := BuildTestNode("n2", 1000, 1000) - n3 := BuildTestNode("n3", 2000, 2000) - provider := testprovider.NewTestCloudProvider(nil, nil) - provider.AddNodeGroup("ng1", 1, 10, 1) - provider.AddNodeGroup("ng2", 1, 10, 1) - provider.AddNodeGroup("ng3", 1, 10, 1) - provider.AddNode("ng1", n1) - provider.AddNode("ng2", n2) - provider.AddNode("ng3", n3) - - ni1 := schedulerframework.NewNodeInfo() - ni1.SetNode(n1) - ni2 := schedulerframework.NewNodeInfo() - ni2.SetNode(n2) - ni3 := schedulerframework.NewNodeInfo() - ni3.SetNode(n3) - - context.CloudProvider = provider - return ni1, ni2, ni3 -} - -func basicSimilarNodeGroupsTest( - t *testing.T, - context *context.AutoscalingContext, - processor NodeGroupSetProcessor, - ni1 *schedulerframework.NodeInfo, - ni2 *schedulerframework.NodeInfo, - ni3 *schedulerframework.NodeInfo, -) { - nodeInfosForGroups := map[string]*schedulerframework.NodeInfo{ - "ng1": ni1, "ng2": ni2, "ng3": ni3, - } - - ng1, _ := context.CloudProvider.NodeGroupForNode(ni1.Node()) - ng2, _ := context.CloudProvider.NodeGroupForNode(ni2.Node()) - ng3, _ := context.CloudProvider.NodeGroupForNode(ni3.Node()) - - similar, err := processor.FindSimilarNodeGroups(context, ng1, nodeInfosForGroups) - assert.NoError(t, err) - assert.Equal(t, []cloudprovider.NodeGroup{ng2}, similar) - - similar, err = processor.FindSimilarNodeGroups(context, ng2, nodeInfosForGroups) - assert.NoError(t, err) - assert.Equal(t, []cloudprovider.NodeGroup{ng1}, similar) - - similar, err = processor.FindSimilarNodeGroups(context, ng3, nodeInfosForGroups) - assert.NoError(t, err) - assert.Equal(t, []cloudprovider.NodeGroup{}, similar) -} - func TestFindSimilarNodeGroups(t *testing.T) { context := &context.AutoscalingContext{} - ni1, ni2, ni3 := buildBasicNodeGroups(context) + ni1, ni2, ni3 := BuildBasicNodeGroups(context) processor := NewDefaultNodeGroupSetProcessor([]string{}, config.NodeGroupDifferenceRatios{}) - basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) + BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) } func TestFindSimilarNodeGroupsCustomLabels(t *testing.T) { context := &context.AutoscalingContext{} - ni1, ni2, ni3 := buildBasicNodeGroups(context) + ni1, ni2, ni3 := BuildBasicNodeGroups(context) ni1.Node().Labels["example.com/ready"] = "true" ni2.Node().Labels["example.com/ready"] = "false" processor := NewDefaultNodeGroupSetProcessor([]string{"example.com/ready"}, config.NodeGroupDifferenceRatios{}) - basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) + BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) } func TestFindSimilarNodeGroupsCustomComparator(t *testing.T) { context := &context.AutoscalingContext{} - ni1, ni2, ni3 := buildBasicNodeGroups(context) + ni1, ni2, ni3 := BuildBasicNodeGroups(context) processor := &BalancingNodeGroupSetProcessor{ Comparator: func(n1, n2 *schedulerframework.NodeInfo) bool { @@ -109,7 +56,7 @@ func TestFindSimilarNodeGroupsCustomComparator(t *testing.T) { (n1.Node().Name == "n2" && n2.Node().Name == "n1") }, } - basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) + BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3) } func TestBalanceSingleGroup(t *testing.T) { diff --git a/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go b/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go index eae8a67090e..990c90edd8c 100644 --- a/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go +++ b/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go @@ -24,28 +24,13 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/config" "k8s.io/autoscaler/cluster-autoscaler/utils/gpu" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" - schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" - - "github.com/stretchr/testify/assert" ) -func checkNodesSimilar(t *testing.T, n1, n2 *apiv1.Node, comparator NodeInfoComparator, shouldEqual bool) { - checkNodesSimilarWithPods(t, n1, n2, []*apiv1.Pod{}, []*apiv1.Pod{}, comparator, shouldEqual) -} - -func checkNodesSimilarWithPods(t *testing.T, n1, n2 *apiv1.Node, pods1, pods2 []*apiv1.Pod, comparator NodeInfoComparator, shouldEqual bool) { - ni1 := schedulerframework.NewNodeInfo(pods1...) - ni1.SetNode(n1) - ni2 := schedulerframework.NewNodeInfo(pods2...) - ni2.SetNode(n2) - assert.Equal(t, shouldEqual, comparator(ni1, ni2)) -} - func TestIdenticalNodesSimilar(t *testing.T) { comparator := CreateGenericNodeInfoComparator([]string{}, config.NewDefaultNodeGroupDifferenceRatios()) n1 := BuildTestNode("node1", 1000, 2000) n2 := BuildTestNode("node2", 1000, 2000) - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) } func TestNodesSimilarVariousRequirements(t *testing.T) { @@ -55,23 +40,23 @@ func TestNodesSimilarVariousRequirements(t *testing.T) { // Different CPU capacity n2 := BuildTestNode("node2", 1000, 2000) n2.Status.Capacity[apiv1.ResourceCPU] = *resource.NewMilliQuantity(1001, resource.DecimalSI) - checkNodesSimilar(t, n1, n2, comparator, false) + CheckNodesSimilar(t, n1, n2, comparator, false) // Same CPU capacity, but slightly different allocatable n3 := BuildTestNode("node3", 1000, 2000) n3.Status.Allocatable[apiv1.ResourceCPU] = *resource.NewMilliQuantity(999, resource.DecimalSI) - checkNodesSimilar(t, n1, n3, comparator, true) + CheckNodesSimilar(t, n1, n3, comparator, true) // Same CPU capacity, significantly different allocatable n4 := BuildTestNode("node4", 1000, 2000) n4.Status.Allocatable[apiv1.ResourceCPU] = *resource.NewMilliQuantity(500, resource.DecimalSI) - checkNodesSimilar(t, n1, n4, comparator, false) + CheckNodesSimilar(t, n1, n4, comparator, false) // One with GPU, one without n5 := BuildTestNode("node5", 1000, 2000) n5.Status.Capacity[gpu.ResourceNvidiaGPU] = *resource.NewQuantity(1, resource.DecimalSI) n5.Status.Allocatable[gpu.ResourceNvidiaGPU] = n5.Status.Capacity[gpu.ResourceNvidiaGPU] - checkNodesSimilar(t, n1, n5, comparator, false) + CheckNodesSimilar(t, n1, n5, comparator, false) } func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) { @@ -84,20 +69,20 @@ func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) { n2 := BuildTestNode("node2", 1000, 2000) n2.Status.Allocatable[apiv1.ResourceCPU] = *resource.NewMilliQuantity(500, resource.DecimalSI) n2.Status.Allocatable[apiv1.ResourceMemory] = *resource.NewQuantity(1000, resource.DecimalSI) - checkNodesSimilarWithPods(t, n1, n2, []*apiv1.Pod{p1}, []*apiv1.Pod{}, comparator, false) + CheckNodesSimilarWithPods(t, n1, n2, []*apiv1.Pod{p1}, []*apiv1.Pod{}, comparator, false) // Same requests of pods n3 := BuildTestNode("node3", 1000, 2000) p3 := BuildTestPod("pod3", 500, 1000) p3.Spec.NodeName = "node3" - checkNodesSimilarWithPods(t, n1, n3, []*apiv1.Pod{p1}, []*apiv1.Pod{p3}, comparator, true) + CheckNodesSimilarWithPods(t, n1, n3, []*apiv1.Pod{p1}, []*apiv1.Pod{p3}, comparator, true) // Similar allocatable, similar pods n4 := BuildTestNode("node4", 1000, 2000) n4.Status.Allocatable[apiv1.ResourceCPU] = *resource.NewMilliQuantity(999, resource.DecimalSI) p4 := BuildTestPod("pod4", 501, 1001) p4.Spec.NodeName = "node4" - checkNodesSimilarWithPods(t, n1, n4, []*apiv1.Pod{p1}, []*apiv1.Pod{p4}, comparator, true) + CheckNodesSimilarWithPods(t, n1, n4, []*apiv1.Pod{p1}, []*apiv1.Pod{p4}, comparator, true) } func TestNodesSimilarVariousMemoryRequirements(t *testing.T) { @@ -107,12 +92,12 @@ func TestNodesSimilarVariousMemoryRequirements(t *testing.T) { // Different memory capacity within tolerance n2 := BuildTestNode("node2", 1000, 1000) n2.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(1000-(1000*config.DefaultMaxCapacityMemoryDifferenceRatio)+1, resource.DecimalSI) - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different memory capacity exceeds tolerance n3 := BuildTestNode("node3", 1000, 1000) n3.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(1000-(1000*config.DefaultMaxCapacityMemoryDifferenceRatio)-1, resource.DecimalSI) - checkNodesSimilar(t, n1, n3, comparator, false) + CheckNodesSimilar(t, n1, n3, comparator, false) } func TestNodesSimilarVariousLargeMemoryRequirementsM5XLarge(t *testing.T) { @@ -128,13 +113,13 @@ func TestNodesSimilarVariousLargeMemoryRequirementsM5XLarge(t *testing.T) { // Different memory capacity within tolerance // Value taken from another m5.xLarge in a different zone n2 := BuildTestNode("node2", 1000, q2.Value()) - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different memory capacity exceeds tolerance // Value of q1 * 1.02 q3 := resource.MustParse("16438475Ki") n3 := BuildTestNode("node3", 1000, q3.Value()) - checkNodesSimilar(t, n1, n3, comparator, false) + CheckNodesSimilar(t, n1, n3, comparator, false) } func TestNodesSimilarVariousLargeMemoryRequirementsM516XLarge(t *testing.T) { @@ -150,13 +135,13 @@ func TestNodesSimilarVariousLargeMemoryRequirementsM516XLarge(t *testing.T) { // Different memory capacity within tolerance // Value taken from another m5.xLarge in a different zone n2 := BuildTestNode("node2", 1000, q2.Value()) - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different memory capacity exceeds tolerance // Value of q1 * 1.02 q3 := resource.MustParse("265169453Ki") n3 := BuildTestNode("node3", 1000, q3.Value()) - checkNodesSimilar(t, n1, n3, comparator, false) + CheckNodesSimilar(t, n1, n3, comparator, false) } func TestNodesSimilarVariousLabels(t *testing.T) { @@ -169,32 +154,32 @@ func TestNodesSimilarVariousLabels(t *testing.T) { n2.ObjectMeta.Labels["test-label"] = "test-value" // Missing character label - checkNodesSimilar(t, n1, n2, comparator, false) + CheckNodesSimilar(t, n1, n2, comparator, false) n2.ObjectMeta.Labels["character"] = "winnie the pooh" - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different hostname labels shouldn't matter n1.ObjectMeta.Labels[apiv1.LabelHostname] = "node1" n2.ObjectMeta.Labels[apiv1.LabelHostname] = "node2" - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different zone shouldn't matter either n1.ObjectMeta.Labels[apiv1.LabelZoneFailureDomain] = "mars-olympus-mons1-b" n2.ObjectMeta.Labels[apiv1.LabelZoneFailureDomain] = "us-houston1-a" - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different beta.kubernetes.io/fluentd-ds-ready should not matter n1.ObjectMeta.Labels["beta.kubernetes.io/fluentd-ds-ready"] = "true" n2.ObjectMeta.Labels["beta.kubernetes.io/fluentd-ds-ready"] = "false" - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) n1.ObjectMeta.Labels["beta.kubernetes.io/fluentd-ds-ready"] = "true" delete(n2.ObjectMeta.Labels, "beta.kubernetes.io/fluentd-ds-ready") - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) // Different custom labels should not matter n1.ObjectMeta.Labels["example.com/ready"] = "true" n2.ObjectMeta.Labels["example.com/ready"] = "false" - checkNodesSimilar(t, n1, n2, comparator, true) + CheckNodesSimilar(t, n1, n2, comparator, true) } diff --git a/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test_helpers.go b/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test_helpers.go new file mode 100644 index 00000000000..4819569676f --- /dev/null +++ b/cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test_helpers.go @@ -0,0 +1,98 @@ +/* +Copyright 2024 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 nodegroupset + +import ( + "testing" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" + "k8s.io/autoscaler/cluster-autoscaler/context" + testutils "k8s.io/autoscaler/cluster-autoscaler/utils/test" + schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" + + "github.com/stretchr/testify/assert" +) + +// CheckNodesSimilar is a helper func for tests to validate node comparison outcomes +func CheckNodesSimilar(t *testing.T, n1, n2 *apiv1.Node, comparator NodeInfoComparator, shouldEqual bool) { + CheckNodesSimilarWithPods(t, n1, n2, []*apiv1.Pod{}, []*apiv1.Pod{}, comparator, shouldEqual) +} + +// CheckNodesSimilarWithPods is a helper func for tests to validate nodes with pods comparison outcomes +func CheckNodesSimilarWithPods(t *testing.T, n1, n2 *apiv1.Node, pods1, pods2 []*apiv1.Pod, comparator NodeInfoComparator, shouldEqual bool) { + ni1 := schedulerframework.NewNodeInfo(pods1...) + ni1.SetNode(n1) + ni2 := schedulerframework.NewNodeInfo(pods2...) + ni2.SetNode(n2) + assert.Equal(t, shouldEqual, comparator(ni1, ni2)) +} + +// BuildBasicNodeGroups is a helper func for tests to get a set of NodeInfo objects +func BuildBasicNodeGroups(context *context.AutoscalingContext) (*schedulerframework.NodeInfo, *schedulerframework.NodeInfo, *schedulerframework.NodeInfo) { + n1 := testutils.BuildTestNode("n1", 1000, 1000) + n2 := testutils.BuildTestNode("n2", 1000, 1000) + n3 := testutils.BuildTestNode("n3", 2000, 2000) + provider := testprovider.NewTestCloudProvider(nil, nil) + provider.AddNodeGroup("ng1", 1, 10, 1) + provider.AddNodeGroup("ng2", 1, 10, 1) + provider.AddNodeGroup("ng3", 1, 10, 1) + provider.AddNode("ng1", n1) + provider.AddNode("ng2", n2) + provider.AddNode("ng3", n3) + + ni1 := schedulerframework.NewNodeInfo() + ni1.SetNode(n1) + ni2 := schedulerframework.NewNodeInfo() + ni2.SetNode(n2) + ni3 := schedulerframework.NewNodeInfo() + ni3.SetNode(n3) + + context.CloudProvider = provider + return ni1, ni2, ni3 +} + +// BasicSimilarNodeGroupsTest is a helper func for tests to assert node group similarity +func BasicSimilarNodeGroupsTest( + t *testing.T, + context *context.AutoscalingContext, + processor NodeGroupSetProcessor, + ni1 *schedulerframework.NodeInfo, + ni2 *schedulerframework.NodeInfo, + ni3 *schedulerframework.NodeInfo, +) { + nodeInfosForGroups := map[string]*schedulerframework.NodeInfo{ + "ng1": ni1, "ng2": ni2, "ng3": ni3, + } + + ng1, _ := context.CloudProvider.NodeGroupForNode(ni1.Node()) + ng2, _ := context.CloudProvider.NodeGroupForNode(ni2.Node()) + ng3, _ := context.CloudProvider.NodeGroupForNode(ni3.Node()) + + similar, err := processor.FindSimilarNodeGroups(context, ng1, nodeInfosForGroups) + assert.NoError(t, err) + assert.Equal(t, []cloudprovider.NodeGroup{ng2}, similar) + + similar, err = processor.FindSimilarNodeGroups(context, ng2, nodeInfosForGroups) + assert.NoError(t, err) + assert.Equal(t, []cloudprovider.NodeGroup{ng1}, similar) + + similar, err = processor.FindSimilarNodeGroups(context, ng3, nodeInfosForGroups) + assert.NoError(t, err) + assert.Equal(t, []cloudprovider.NodeGroup{}, similar) +} diff --git a/cluster-autoscaler/processors/nodegroupset/label_nodegroups_test.go b/cluster-autoscaler/processors/nodegroupset/label_nodegroups_test.go index aa3f7722ccb..afee2542f47 100644 --- a/cluster-autoscaler/processors/nodegroupset/label_nodegroups_test.go +++ b/cluster-autoscaler/processors/nodegroupset/label_nodegroups_test.go @@ -86,7 +86,7 @@ func TestNodeLabelComparison(t *testing.T) { t.Run(tc.description, func(t *testing.T) { node1.ObjectMeta.Labels = tc.labels1 node2.ObjectMeta.Labels = tc.labels2 - checkNodesSimilar(t, node1, node2, comparator, tc.isSimilar) + CheckNodesSimilar(t, node1, node2, comparator, tc.isSimilar) }) } }