Merge pull request kubernetes#7344 from olagacek/revert-6907-nodegrou…

…pset-cloudprovider

Revert "CAS: cloudprovider-specific nodegroupset"

cluster-autoscaler/main.go

@@ -47,10 +47,7 @@ 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"
@@ -574,12 +571,12 @@ func buildAutoscaler(debuggingSnapshotter debuggingsnapshot.DebuggingSnapshotter
 	} else {
 		nodeInfoComparatorBuilder := nodegroupset.CreateGenericNodeInfoComparator
 		if autoscalingOptions.CloudProviderName == cloudprovider.AzureProviderName {
-			nodeInfoComparatorBuilder = azure.CreateNodeInfoComparator
+			nodeInfoComparatorBuilder = nodegroupset.CreateAzureNodeInfoComparator
 		} else if autoscalingOptions.CloudProviderName == cloudprovider.AwsProviderName {
-			nodeInfoComparatorBuilder = aws.CreateNodeInfoComparator
+			nodeInfoComparatorBuilder = nodegroupset.CreateAwsNodeInfoComparator
 			opts.Processors.TemplateNodeInfoProvider = nodeinfosprovider.NewAsgTagResourceNodeInfoProvider(nodeInfoCacheExpireTime, *forceDaemonSets)
 		} else if autoscalingOptions.CloudProviderName == cloudprovider.GceProviderName {
-			nodeInfoComparatorBuilder = gce.CreateGceNodeInfoComparator
+			nodeInfoComparatorBuilder = nodegroupset.CreateGceNodeInfoComparator
 			opts.Processors.TemplateNodeInfoProvider = nodeinfosprovider.NewAnnotationNodeInfoProvider(nodeInfoCacheExpireTime, *forceDaemonSets)
 		}
 		nodeInfoComparator = nodeInfoComparatorBuilder(autoscalingOptions.BalancingExtraIgnoredLabels, autoscalingOptions.NodeGroupSetRatios)

cluster-autoscaler/cloudprovider/aws/aws_nodegroups.go → cluster-autoscaler/processors/nodegroupset/aws_nodegroups.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 The Kubernetes Authors.
+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.
@@ -14,18 +14,17 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package aws
+package nodegroupset
 
 import (
 	"k8s.io/autoscaler/cluster-autoscaler/config"
-	"k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset"
 	schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
 )
 
-// CreateNodeInfoComparator returns a comparator that checks if two nodes should be considered
+// CreateAwsNodeInfoComparator 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 CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) nodegroupset.NodeInfoComparator {
+func CreateAwsNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) 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.
@@ -35,7 +34,7 @@ func CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.Node
 		"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 nodegroupset.BasicIgnoredLabels {
+	for k, v := range BasicIgnoredLabels {
 		awsIgnoredLabels[k] = v
 	}
 
@@ -44,6 +43,6 @@ func CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.Node
 	}
 
 	return func(n1, n2 *schedulerframework.NodeInfo) bool {
-		return nodegroupset.IsCloudProviderNodeInfoSimilar(n1, n2, awsIgnoredLabels, ratioOpts)
+		return IsCloudProviderNodeInfoSimilar(n1, n2, awsIgnoredLabels, ratioOpts)
 	}
 }

cluster-autoscaler/cloudprovider/aws/aws_nodegroups_test.go → cluster-autoscaler/processors/nodegroupset/aws_nodegroups_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 The Kubernetes Authors.
+Copyright 2021 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,19 +14,18 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package aws
+package nodegroupset
 
 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 := CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})
+	comparator := CreateAwsNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})
 	node1 := BuildTestNode("node1", 1000, 2000)
 	node2 := BuildTestNode("node2", 1000, 2000)
 
@@ -170,14 +169,14 @@ func TestIsAwsNodeInfoSimilar(t *testing.T) {
 			if tc.removeOneLabel {
 				delete(node2.ObjectMeta.Labels, tc.label)
 			}
-			nodegroupset.CheckNodesSimilar(t, node1, node2, comparator, true)
+			checkNodesSimilar(t, node1, node2, comparator, true)
 		})
 	}
 }
 
 func TestFindSimilarNodeGroupsAwsBasic(t *testing.T) {
 	context := &context.AutoscalingContext{}
-	ni1, ni2, ni3 := nodegroupset.BuildBasicNodeGroups(context)
-	processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
-	nodegroupset.BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
+	ni1, ni2, ni3 := buildBasicNodeGroups(context)
+	processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAwsNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
+	basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
 }

cluster-autoscaler/cloudprovider/azure/azure_nodegroups.go → cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 The Kubernetes Authors.
+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.
@@ -14,11 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package azure
+package nodegroupset
 
 import (
 	"k8s.io/autoscaler/cluster-autoscaler/config"
-	"k8s.io/autoscaler/cluster-autoscaler/processors/nodegroupset"
 	schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
 )
 
@@ -53,12 +52,12 @@ func nodesFromSameAzureNodePoolLegacy(n1, n2 *schedulerframework.NodeInfo) bool
 	return n1AzureNodePool != "" && n1AzureNodePool == n2AzureNodePool
 }
 
-// CreateNodeInfoComparator returns a comparator that checks if two nodes should be considered
+// CreateAzureNodeInfoComparator 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 CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) nodegroupset.NodeInfoComparator {
+func CreateAzureNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.NodeGroupDifferenceRatios) NodeInfoComparator {
 	azureIgnoredLabels := make(map[string]bool)
-	for k, v := range nodegroupset.BasicIgnoredLabels {
+	for k, v := range BasicIgnoredLabels {
 		azureIgnoredLabels[k] = v
 	}
 	azureIgnoredLabels[AzureNodepoolLegacyLabel] = true
@@ -79,6 +78,6 @@ func CreateNodeInfoComparator(extraIgnoredLabels []string, ratioOpts config.Node
 		if nodesFromSameAzureNodePool(n1, n2) {
 			return true
 		}
-		return nodegroupset.IsCloudProviderNodeInfoSimilar(n1, n2, azureIgnoredLabels, ratioOpts)
+		return IsCloudProviderNodeInfoSimilar(n1, n2, azureIgnoredLabels, ratioOpts)
 	}
 }

cluster-autoscaler/cloudprovider/azure/azure_nodegroups_test.go → cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 The Kubernetes Authors.
+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.
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package azure
+package nodegroupset
 
 import (
 	"testing"
@@ -23,83 +23,82 @@ 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"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestIsAzureNodeInfoSimilar(t *testing.T) {
-	comparator := CreateNodeInfoComparator([]string{"example.com/ready"}, config.NodeGroupDifferenceRatios{})
+	comparator := CreateAzureNodeInfoComparator([]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.
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	checkNodesSimilar(t, n1, n2, comparator, false)
 	// Empty agentpool labels
 	n1.ObjectMeta.Labels["agentpool"] = ""
 	n2.ObjectMeta.Labels["agentpool"] = ""
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	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"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	checkNodesSimilar(t, n1, n2, comparator, false)
 	// Only one non empty
 	n1.ObjectMeta.Labels["agentpool"] = ""
 	n2.ObjectMeta.Labels["agentpool"] = "foo"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	checkNodesSimilar(t, n1, n2, comparator, false)
 	// Only one present
 	delete(n1.ObjectMeta.Labels, "agentpool")
 	n2.ObjectMeta.Labels["agentpool"] = "foo"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	checkNodesSimilar(t, n1, n2, comparator, false)
 	// Different vales
 	n1.ObjectMeta.Labels["agentpool"] = "foo1"
 	n2.ObjectMeta.Labels["agentpool"] = "foo2"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, false)
+	checkNodesSimilar(t, n1, n2, comparator, false)
 	// Same values
 	n1.ObjectMeta.Labels["agentpool"] = "foo"
 	n2.ObjectMeta.Labels["agentpool"] = "foo"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	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"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 	// Different creationSource
 	n1.ObjectMeta.Labels["creationSource"] = "aks-aks-nodepool2-vmss"
 	n2.ObjectMeta.Labels["creationSource"] = "aks-aks-nodepool3-vmss"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	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"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 	// Custom label
 	n1.ObjectMeta.Labels["example.com/ready"] = "true"
 	n2.ObjectMeta.Labels["example.com/ready"] = "false"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 	// One node with aksConsolidatedAdditionalProperties label
 	n1.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "foo"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 	// Same aksConsolidatedAdditionalProperties
 	n2.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "foo"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 	// Different aksConsolidatedAdditionalProperties label
 	n2.ObjectMeta.Labels[aksConsolidatedAdditionalProperties] = "bar"
-	nodegroupset.CheckNodesSimilar(t, n1, n2, comparator, true)
+	checkNodesSimilar(t, n1, n2, comparator, true)
 }
 
 func TestFindSimilarNodeGroupsAzureBasic(t *testing.T) {
 	context := &context.AutoscalingContext{}
-	ni1, ni2, ni3 := nodegroupset.BuildBasicNodeGroups(context)
-	processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
-	nodegroupset.BasicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
+	ni1, ni2, ni3 := buildBasicNodeGroups(context)
+	processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
+	basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
 }
 
 func TestFindSimilarNodeGroupsAzureByLabel(t *testing.T) {
-	processor := &nodegroupset.BalancingNodeGroupSetProcessor{Comparator: CreateNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
+	processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{}, config.NodeGroupDifferenceRatios{})}
 	context := &context.AutoscalingContext{}
 
 	n1 := BuildTestNode("n1", 1000, 1000)

cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go

@@ -25,38 +25,91 @@ 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 {
 			return (n1.Node().Name == "n1" && n2.Node().Name == "n2") ||
 				(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) {