diff --git a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go index d38e60b6a38a..b2ae462951e6 100644 --- a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go +++ b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go @@ -1060,7 +1060,7 @@ func runSimpleScaleUpTest(t *testing.T, config *ScaleUpTestConfig) *ScaleUpTestR } orchestrator := New() orchestrator.Initialize(&context, processors, clusterState, newEstimatorBuilder(), taints.TaintConfig{}) - expander := NewMockRepotingStrategy(t, config.ExpansionOptionToChoose) + expander := NewMockReportingStrategy(t, config.ExpansionOptionToChoose, nil) context.ExpanderStrategy = expander // scale up @@ -1217,7 +1217,7 @@ func TestBinpackingLimiter(t *testing.T) { suOrchestrator := New() suOrchestrator.Initialize(&context, processors, clusterState, newEstimatorBuilder(), taints.TaintConfig{}) - expander := NewMockRepotingStrategy(t, nil) + expander := NewMockReportingStrategy(t, nil, nil) context.ExpanderStrategy = expander scaleUpStatus, err := suOrchestrator.ScaleUp([]*apiv1.Pod{extraPod}, nodes, []*appsv1.DaemonSet{}, nodeInfos, false) @@ -1433,48 +1433,102 @@ func TestComputeSimilarNodeGroups(t *testing.T) { } func TestScaleUpBalanceGroups(t *testing.T) { - provider := testprovider.NewTestCloudProvider(func(string, int) error { - return nil - }, nil) + // this will set the similar node groups to empty, it should be recomputed during scaleup + emptySimilarNodeGroups := []string{} + noSimilarNodeGroupsExpander := NewMockReportingStrategy(t, &GroupSizeChange{GroupName: "ng2", SizeChange: 2}, &emptySimilarNodeGroups) - type ngInfo struct { - min, max, size int - } - testCfg := map[string]ngInfo{ - "ng1": {min: 1, max: 1, size: 1}, - "ng2": {min: 1, max: 2, size: 1}, - "ng3": {min: 1, max: 5, size: 1}, - "ng4": {min: 1, max: 5, size: 3}, + testCases := []struct { + name string + expanderStrategy *MockReportingStrategy + }{ + { + name: "balance groups", + }, + { + name: "balance groups recomputes similar nodegroups", + expanderStrategy: noSimilarNodeGroupsExpander, + }, } - podList := make([]*apiv1.Pod, 0, len(testCfg)) - nodes := make([]*apiv1.Node, 0) - now := time.Now() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + provider := testprovider.NewTestCloudProvider(func(string, int) error { + return nil + }, nil) - for gid, gconf := range testCfg { - provider.AddNodeGroup(gid, gconf.min, gconf.max, gconf.size) - for i := 0; i < gconf.size; i++ { - nodeName := fmt.Sprintf("%v-node-%v", gid, i) - node := BuildTestNode(nodeName, 100, 1000) - SetNodeReadyState(node, true, now.Add(-2*time.Minute)) - nodes = append(nodes, node) + type ngInfo struct { + min, max, size int + } + testCfg := map[string]ngInfo{ + "ng1": {min: 1, max: 1, size: 1}, + "ng2": {min: 1, max: 2, size: 1}, + "ng3": {min: 1, max: 5, size: 1}, + "ng4": {min: 1, max: 5, size: 3}, + } + podList := make([]*apiv1.Pod, 0, len(testCfg)) + nodes := make([]*apiv1.Node, 0) - pod := BuildTestPod(fmt.Sprintf("%v-pod-%v", gid, i), 80, 0) - pod.Spec.NodeName = nodeName - podList = append(podList, pod) + now := time.Now() - provider.AddNode(gid, node) - } - } + for gid, gconf := range testCfg { + provider.AddNodeGroup(gid, gconf.min, gconf.max, gconf.size) + for i := 0; i < gconf.size; i++ { + nodeName := fmt.Sprintf("%v-node-%v", gid, i) + node := BuildTestNode(nodeName, 100, 1000) + SetNodeReadyState(node, true, now.Add(-2*time.Minute)) + nodes = append(nodes, node) - podLister := kube_util.NewTestPodLister(podList) - listers := kube_util.NewListerRegistry(nil, nil, podLister, nil, nil, nil, nil, nil, nil) + pod := BuildTestPod(fmt.Sprintf("%v-pod-%v", gid, i), 80, 0) + pod.Spec.NodeName = nodeName + podList = append(podList, pod) - options := config.AutoscalingOptions{ - EstimatorName: estimator.BinpackingEstimatorName, - BalanceSimilarNodeGroups: true, - MaxCoresTotal: config.DefaultMaxClusterCores, - MaxMemoryTotal: config.DefaultMaxClusterMemory, + provider.AddNode(gid, node) + } + } + + podLister := kube_util.NewTestPodLister(podList) + listers := kube_util.NewListerRegistry(nil, nil, podLister, nil, nil, nil, nil, nil, nil) + + options := config.AutoscalingOptions{ + EstimatorName: estimator.BinpackingEstimatorName, + BalanceSimilarNodeGroups: true, + MaxCoresTotal: config.DefaultMaxClusterCores, + MaxMemoryTotal: config.DefaultMaxClusterMemory, + } + context, err := NewScaleTestAutoscalingContext(options, &fake.Clientset{}, listers, provider, nil, nil) + assert.NoError(t, err) + + nodeInfos, _ := nodeinfosprovider.NewDefaultTemplateNodeInfoProvider(nil, false).Process(&context, nodes, []*appsv1.DaemonSet{}, taints.TaintConfig{}, now) + clusterState := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, NewBackoff(), nodegroupconfig.NewDefaultNodeGroupConfigProcessor(config.NodeGroupAutoscalingOptions{MaxNodeProvisionTime: 15 * time.Minute}), asyncnodegroups.NewDefaultAsyncNodeGroupStateChecker()) + clusterState.UpdateNodes(nodes, nodeInfos, time.Now()) + + pods := make([]*apiv1.Pod, 0) + for i := 0; i < 2; i++ { + pods = append(pods, BuildTestPod(fmt.Sprintf("test-pod-%v", i), 80, 0)) + } + + processors := NewTestProcessors(&context) + suOrchestrator := New() + if tc.expanderStrategy != nil { + context.ExpanderStrategy = tc.expanderStrategy + } + suOrchestrator.Initialize(&context, processors, clusterState, newEstimatorBuilder(), taints.TaintConfig{}) + scaleUpStatus, typedErr := suOrchestrator.ScaleUp(pods, nodes, []*appsv1.DaemonSet{}, nodeInfos, false) + + assert.NoError(t, typedErr) + assert.True(t, scaleUpStatus.WasSuccessful()) + groupMap := make(map[string]cloudprovider.NodeGroup, 3) + for _, group := range provider.NodeGroups() { + groupMap[group.Id()] = group + } + + ng2size, err := groupMap["ng2"].TargetSize() + assert.NoError(t, err) + ng3size, err := groupMap["ng3"].TargetSize() + assert.NoError(t, err) + assert.Equal(t, 2, ng2size) + assert.Equal(t, 2, ng3size) + }) } context, err := NewScaleTestAutoscalingContext(options, &fake.Clientset{}, listers, provider, nil, nil) assert.NoError(t, err) diff --git a/cluster-autoscaler/core/test/common.go b/cluster-autoscaler/core/test/common.go index 5abeffa27e85..cef207bf9113 100644 --- a/cluster-autoscaler/core/test/common.go +++ b/cluster-autoscaler/core/test/common.go @@ -19,6 +19,7 @@ package test import ( "fmt" "reflect" + "slices" "testing" "time" @@ -341,19 +342,21 @@ type expanderResults struct { // MockReportingStrategy implements expander.Strategy type MockReportingStrategy struct { - defaultStrategy expander.Strategy - optionToChoose *GroupSizeChange - t *testing.T - results *expanderResults + defaultStrategy expander.Strategy + optionToChoose *GroupSizeChange + similarNodeGroupsToChoose *[]string + t *testing.T + results *expanderResults } // NewMockRepotingStrategy creates an expander strategy with reporting and mocking capabilities. -func NewMockRepotingStrategy(t *testing.T, optionToChoose *GroupSizeChange) *MockReportingStrategy { +func NewMockReportingStrategy(t *testing.T, optionToChoose *GroupSizeChange, similarNodeGroupsToChoose *[]string) *MockReportingStrategy { return &MockReportingStrategy{ - defaultStrategy: random.NewStrategy(), - results: &expanderResults{}, - optionToChoose: optionToChoose, - t: t, + defaultStrategy: random.NewStrategy(), + results: &expanderResults{}, + optionToChoose: optionToChoose, + similarNodeGroupsToChoose: similarNodeGroupsToChoose, + t: t, } } @@ -373,7 +376,16 @@ func (r *MockReportingStrategy) BestOption(options []expander.Option, nodeInfo m for _, option := range options { groupSizeChange := expanderOptionToGroupSizeChange(option) if groupSizeChange == *r.optionToChoose { - return &option + bestOption := option + if r.similarNodeGroupsToChoose != nil { + bestOption.SimilarNodeGroups = []cloudprovider.NodeGroup{} + for _, nodeGroup := range options { + if slices.Contains(*r.similarNodeGroupsToChoose, nodeGroup.NodeGroup.Id()) { + bestOption.SimilarNodeGroups = append(bestOption.SimilarNodeGroups, nodeGroup.NodeGroup) + } + } + } + return &bestOption } } assert.Fail(r.t, "did not find expansionOptionToChoose %+v", r.optionToChoose)