From 910e75365cda3955b8bbb45db8e722029cb230f0 Mon Sep 17 00:00:00 2001 From: Vivek Bagade Date: Tue, 12 Nov 2019 10:39:02 +0100 Subject: [PATCH] remove temporary nodes logic --- cluster-autoscaler/core/scale_down.go | 36 ++---- cluster-autoscaler/core/scale_down_test.go | 119 +++++------------- cluster-autoscaler/core/scale_test_common.go | 1 - cluster-autoscaler/core/static_autoscaler.go | 14 +-- .../nodes/pre_filtering_processor.go | 9 -- cluster-autoscaler/processors/nodes/types.go | 4 - 6 files changed, 41 insertions(+), 142 deletions(-) diff --git a/cluster-autoscaler/core/scale_down.go b/cluster-autoscaler/core/scale_down.go index b36f8943fadb..29071424b3e1 100644 --- a/cluster-autoscaler/core/scale_down.go +++ b/cluster-autoscaler/core/scale_down.go @@ -400,15 +400,13 @@ func (sd *ScaleDown) CleanUpUnneededNodes() { // * pods are the all scheduled pods. // * timestamp is the current timestamp. // * pdbs is a list of pod disruption budgets. -// * tempNodesPerNodeGroup is a map of node group id and the number of temporary nodes that node group contains. func (sd *ScaleDown) UpdateUnneededNodes( allNodes []*apiv1.Node, destinationNodes []*apiv1.Node, scaleDownCandidates []*apiv1.Node, pods []*apiv1.Pod, timestamp time.Time, - pdbs []*policyv1.PodDisruptionBudget, - tempNodesPerNodeGroup map[string]int) errors.AutoscalerError { + pdbs []*policyv1.PodDisruptionBudget) errors.AutoscalerError { currentlyUnneededNodes := make([]*apiv1.Node, 0) // Only scheduled non expendable pods and pods waiting for lower priority pods preemption can prevent node delete. @@ -473,7 +471,7 @@ func (sd *ScaleDown) UpdateUnneededNodes( emptyNodes := make(map[string]bool) - emptyNodesList := sd.getEmptyNodesNoResourceLimits(currentlyUnneededNodes, pods, len(currentlyUnneededNodes), tempNodesPerNodeGroup) + emptyNodesList := sd.getEmptyNodesNoResourceLimits(currentlyUnneededNodes, pods, len(currentlyUnneededNodes)) for _, node := range emptyNodesList { emptyNodes[node.Name] = true } @@ -690,7 +688,7 @@ func (sd *ScaleDown) SoftTaintUnneededNodes(allNodes []*apiv1.Node) (errors []er // TryToScaleDown tries to scale down the cluster. It returns a result inside a ScaleDownStatus indicating if any node was // removed and error if such occurred. func (sd *ScaleDown) TryToScaleDown(allNodes []*apiv1.Node, pods []*apiv1.Pod, pdbs []*policyv1.PodDisruptionBudget, - currentTime time.Time, tempNodes []*apiv1.Node, tempNodesPerNodeGroup map[string]int) (*status.ScaleDownStatus, errors.AutoscalerError) { + currentTime time.Time) (*status.ScaleDownStatus, errors.AutoscalerError) { scaleDownStatus := &status.ScaleDownStatus{NodeDeleteResults: sd.nodeDeletionTracker.GetAndClearNodeDeleteResults()} nodeDeletionDuration := time.Duration(0) findNodesToRemoveDuration := time.Duration(0) @@ -708,8 +706,6 @@ func (sd *ScaleDown) TryToScaleDown(allNodes []*apiv1.Node, pods []*apiv1.Pod, p return scaleDownStatus, errors.ToAutoscalerError(errors.CloudProviderError, errCP) } - nodesWithoutMaster = utils.FilterOutNodes(nodesWithoutMaster, tempNodes) - scaleDownResourcesLeft := computeScaleDownResourcesLeftLimits(nodesWithoutMaster, resourceLimiter, sd.context.CloudProvider, currentTime) nodeGroupSize := utils.GetNodeGroupSizeMap(sd.context.CloudProvider) @@ -754,9 +750,8 @@ func (sd *ScaleDown) TryToScaleDown(allNodes []*apiv1.Node, pods []*apiv1.Pod, p continue } - tempNodesForNg := tempNodesPerNodeGroup[nodeGroup.Id()] deletionsInProgress := sd.nodeDeletionTracker.GetDeletionsInProgress(nodeGroup.Id()) - if size-deletionsInProgress-tempNodesForNg <= nodeGroup.MinSize() { + if size-deletionsInProgress <= nodeGroup.MinSize() { klog.V(1).Infof("Skipping %s - node group min size reached", node.Name) continue } @@ -786,7 +781,7 @@ func (sd *ScaleDown) TryToScaleDown(allNodes []*apiv1.Node, pods []*apiv1.Pod, p // Trying to delete empty nodes in bulk. If there are no empty nodes then CA will // try to delete not-so-empty nodes, possibly killing some pods and allowing them // to recreate on other nodes. - emptyNodes := sd.getEmptyNodes(candidates, pods, sd.context.MaxEmptyBulkDelete, scaleDownResourcesLeft, tempNodesPerNodeGroup) + emptyNodes := sd.getEmptyNodes(candidates, pods, sd.context.MaxEmptyBulkDelete, scaleDownResourcesLeft) if len(emptyNodes) > 0 { nodeDeletionStart := time.Now() deletedNodes, err := sd.scheduleDeleteEmptyNodes(emptyNodes, sd.context.ClientSet, sd.context.Recorder, readinessMap, candidateNodeGroups) @@ -870,18 +865,6 @@ func (sd *ScaleDown) TryToScaleDown(allNodes []*apiv1.Node, pods []*apiv1.Pod, p return scaleDownStatus, nil } -func getTempNodesPerNodeGroup(cp cloudprovider.CloudProvider, tempNodes []*apiv1.Node) map[string]int { - tempNodesPerNg := make(map[string]int) - for _, node := range tempNodes { - ng, err := cp.NodeGroupForNode(node) - if err != nil || ng == nil { - continue - } - tempNodesPerNg[ng.Id()]++ - } - return tempNodesPerNg -} - // updateScaleDownMetrics registers duration of different parts of scale down. // Separates time spent on finding nodes to remove, deleting nodes and other operations. func updateScaleDownMetrics(scaleDownStart time.Time, findNodesToRemoveDuration *time.Duration, nodeDeletionDuration *time.Duration) { @@ -892,14 +875,14 @@ func updateScaleDownMetrics(scaleDownStart time.Time, findNodesToRemoveDuration metrics.UpdateDuration(metrics.ScaleDownMiscOperations, miscDuration) } -func (sd *ScaleDown) getEmptyNodesNoResourceLimits(candidates []*apiv1.Node, pods []*apiv1.Pod, maxEmptyBulkDelete int, tempNodesPerNodeGroup map[string]int) []*apiv1.Node { - return sd.getEmptyNodes(candidates, pods, maxEmptyBulkDelete, noScaleDownLimitsOnResources(), tempNodesPerNodeGroup) +func (sd *ScaleDown) getEmptyNodesNoResourceLimits(candidates []*apiv1.Node, pods []*apiv1.Pod, maxEmptyBulkDelete int) []*apiv1.Node { + return sd.getEmptyNodes(candidates, pods, maxEmptyBulkDelete, noScaleDownLimitsOnResources()) } // This functions finds empty nodes among passed candidates and returns a list of empty nodes // that can be deleted at the same time. func (sd *ScaleDown) getEmptyNodes(candidates []*apiv1.Node, pods []*apiv1.Pod, maxEmptyBulkDelete int, - resourcesLimits scaleDownResourcesLimits, temporaryNodesPerNodeGroup map[string]int) []*apiv1.Node { + resourcesLimits scaleDownResourcesLimits) []*apiv1.Node { emptyNodes := simulator.FindEmptyNodesToRemove(candidates, pods) availabilityMap := make(map[string]int) @@ -924,9 +907,8 @@ func (sd *ScaleDown) getEmptyNodes(candidates []*apiv1.Node, pods []*apiv1.Pod, klog.Errorf("Failed to get size for %s: %v ", nodeGroup.Id(), err) continue } - tempNodes := temporaryNodesPerNodeGroup[nodeGroup.Id()] deletionsInProgress := sd.nodeDeletionTracker.GetDeletionsInProgress(nodeGroup.Id()) - available = size - nodeGroup.MinSize() - deletionsInProgress - tempNodes + available = size - nodeGroup.MinSize() - deletionsInProgress if available < 0 { available = 0 } diff --git a/cluster-autoscaler/core/scale_down_test.go b/cluster-autoscaler/core/scale_down_test.go index 0f1d2a77ad67..9b26dabd611f 100644 --- a/cluster-autoscaler/core/scale_down_test.go +++ b/cluster-autoscaler/core/scale_down_test.go @@ -133,7 +133,7 @@ func TestFindUnneededNodes(t *testing.T) { sd := NewScaleDown(&context, clusterStateRegistry) allNodes := []*apiv1.Node{n1, n2, n3, n4, n5, n7, n8, n9} sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, - []*apiv1.Pod{p1, p2, p3, p4, p5, p6}, time.Now(), nil, nil) + []*apiv1.Pod{p1, p2, p3, p4, p5, p6}, time.Now(), nil) assert.Equal(t, 3, len(sd.unneededNodes)) _, found := sd.unneededNodes["n2"] @@ -148,7 +148,7 @@ func TestFindUnneededNodes(t *testing.T) { sd.unremovableNodes = make(map[string]time.Time) sd.unneededNodes["n1"] = time.Now() allNodes = []*apiv1.Node{n1, n2, n3, n4} - sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{p1, p2, p3, p4}, time.Now(), nil, nil) + sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{p1, p2, p3, p4}, time.Now(), nil) sd.unremovableNodes = make(map[string]time.Time) assert.Equal(t, 1, len(sd.unneededNodes)) @@ -159,18 +159,18 @@ func TestFindUnneededNodes(t *testing.T) { sd.unremovableNodes = make(map[string]time.Time) scaleDownCandidates := []*apiv1.Node{n1, n3, n4} - sd.UpdateUnneededNodes(allNodes, allNodes, scaleDownCandidates, []*apiv1.Pod{p1, p2, p3, p4}, time.Now(), nil, nil) + sd.UpdateUnneededNodes(allNodes, allNodes, scaleDownCandidates, []*apiv1.Pod{p1, p2, p3, p4}, time.Now(), nil) assert.Equal(t, 0, len(sd.unneededNodes)) // Node n1 is unneeded, but should be skipped because it has just recently been found to be unremovable allNodes = []*apiv1.Node{n1} - sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{}, time.Now(), nil, nil) + sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{}, time.Now(), nil) assert.Equal(t, 0, len(sd.unneededNodes)) // Verify that no other nodes are in unremovable map. assert.Equal(t, 1, len(sd.unremovableNodes)) // But it should be checked after timeout - sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{}, time.Now().Add(context.UnremovableNodeRecheckTimeout+time.Second), nil, nil) + sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, []*apiv1.Pod{}, time.Now().Add(context.UnremovableNodeRecheckTimeout+time.Second), nil) assert.Equal(t, 1, len(sd.unneededNodes)) // Verify that nodes that are no longer unremovable are removed. assert.Equal(t, 0, len(sd.unremovableNodes)) @@ -227,7 +227,7 @@ func TestFindUnneededGPUNodes(t *testing.T) { sd := NewScaleDown(&context, clusterStateRegistry) allNodes := []*apiv1.Node{n1, n2, n3} sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, - []*apiv1.Pod{p1, p2, p3}, time.Now(), nil, nil) + []*apiv1.Pod{p1, p2, p3}, time.Now(), nil) assert.Equal(t, 1, len(sd.unneededNodes)) _, found := sd.unneededNodes["n2"] @@ -310,7 +310,7 @@ func TestPodsWithPrioritiesFindUnneededNodes(t *testing.T) { allNodes := []*apiv1.Node{n1, n2, n3, n4} sd.UpdateUnneededNodes(allNodes, allNodes, allNodes, - []*apiv1.Pod{p1, p2, p3, p4, p5, p6, p7}, time.Now(), nil, nil) + []*apiv1.Pod{p1, p2, p3, p4, p5, p6, p7}, time.Now(), nil) assert.Equal(t, 2, len(sd.unneededNodes)) klog.Warningf("Unneeded nodes %v", sd.unneededNodes) _, found := sd.unneededNodes["n2"] @@ -359,7 +359,7 @@ func TestFindUnneededMaxCandidates(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) sd := NewScaleDown(&context, clusterStateRegistry) - sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil, nil) + sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil) assert.Equal(t, numCandidates, len(sd.unneededNodes)) // Simulate one of the unneeded nodes got deleted deleted := sd.unneededNodesList[len(sd.unneededNodesList)-1] @@ -380,7 +380,7 @@ func TestFindUnneededMaxCandidates(t *testing.T) { } } - sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil, nil) + sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil) // Check that the deleted node was replaced assert.Equal(t, numCandidates, len(sd.unneededNodes)) assert.NotContains(t, sd.unneededNodes, deleted) @@ -425,7 +425,7 @@ func TestFindUnneededEmptyNodes(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) sd := NewScaleDown(&context, clusterStateRegistry) - sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil, nil) + sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil) for _, node := range sd.unneededNodesList { t.Log(node.Name) } @@ -469,7 +469,7 @@ func TestFindUnneededNodePool(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) sd := NewScaleDown(&context, clusterStateRegistry) - sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil, nil) + sd.UpdateUnneededNodes(nodes, nodes, nodes, pods, time.Now(), nil) assert.NotEmpty(t, sd.unneededNodes) } @@ -946,8 +946,8 @@ func TestScaleDown(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) scaleDown := NewScaleDown(&context, clusterStateRegistry) - scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, []*apiv1.Pod{p1, p2, p3}, time.Now().Add(-5*time.Minute), nil, nil) - scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{p1, p2, p3}, nil, time.Now(), nil, nil) + scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, []*apiv1.Pod{p1, p2, p3}, time.Now().Add(-5*time.Minute), nil) + scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{p1, p2, p3}, nil, time.Now()) waitForDeleteToFinish(t, scaleDown) assert.NoError(t, err) assert.Equal(t, status.ScaleDownNodeDeleteStarted, scaleDownStatus.Result) @@ -1064,56 +1064,6 @@ func TestScaleDownEmptyMinMemoryLimitHit(t *testing.T) { simpleScaleDownEmpty(t, config) } -func TestScaleDownEmptyTempNodesLimits(t *testing.T) { - options := defaultScaleDownOptions - options.MinMemoryTotal = 4000 * utils.MiB - config := &scaleTestConfig{ - nodes: []nodeConfig{ - {"n1", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n2", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n3", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n4", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n5", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n6", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - - {"n7", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n8", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n9", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n10", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - }, - options: options, - expectedScaleDowns: []string{"n1", "n2", "n3", "n7"}, - tempNodeNames: []string{"n5", "n6"}, - } - simpleScaleDownEmpty(t, config) -} - -func TestScaleDownEmptyTempNodesMinSize(t *testing.T) { - options := defaultScaleDownOptions - options.MinMemoryTotal = 1000 * utils.MiB - config := &scaleTestConfig{ - nodes: []nodeConfig{ - {"n1", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n2", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n3", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - {"n4", 1000, 1000 * utils.MiB, 0, true, "ng1"}, - - {"n6", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n7", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n8", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - {"n9", 1000, 1000 * utils.MiB, 0, true, "ng2"}, - - {"n10", 1000, 1000 * utils.MiB, 0, true, "ng3"}, - {"n11", 1000, 1000 * utils.MiB, 0, true, "ng3"}, - {"n12", 1000, 1000 * utils.MiB, 0, true, "ng3"}, - }, - options: options, - expectedScaleDowns: []string{"n7", "n8", "n10", "n11"}, - tempNodeNames: []string{"n1", "n2", "n3", "n6"}, - } - simpleScaleDownEmpty(t, config) -} - func TestScaleDownEmptyMinGpuLimitHit(t *testing.T) { options := defaultScaleDownOptions options.GpuTotal = []config.GpuLimits{ @@ -1181,8 +1131,6 @@ func simpleScaleDownEmpty(t *testing.T, config *scaleTestConfig) { nodes := make([]*apiv1.Node, len(config.nodes)) nodesMap := make(map[string]*apiv1.Node) groups := make(map[string][]*apiv1.Node) - tempNodesPerGroup := make(map[string]int) - var tempNodes []*apiv1.Node provider := testprovider.NewTestCloudProvider(nil, func(nodeGroup string, node string) error { deletedNodes <- node @@ -1199,12 +1147,6 @@ func simpleScaleDownEmpty(t *testing.T, config *scaleTestConfig) { nodesMap[n.name] = node nodes[i] = node groups[n.group] = append(groups[n.group], node) - for _, tempNode := range config.tempNodeNames { - if tempNode == node.Name { - tempNodes = append(tempNodes, node) - tempNodesPerGroup[n.group]++ - } - } } fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { @@ -1249,8 +1191,8 @@ func simpleScaleDownEmpty(t *testing.T, config *scaleTestConfig) { scaleDown.nodeDeletionTracker = config.nodeDeletionTracker } - scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil, tempNodesPerGroup) - scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{}, nil, time.Now(), tempNodes, tempNodesPerGroup) + scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil) + scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{}, nil, time.Now()) assert.False(t, scaleDown.nodeDeletionTracker.IsNonEmptyNodeDeleteInProgress()) assert.NoError(t, err) @@ -1325,8 +1267,8 @@ func TestNoScaleDownUnready(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) scaleDown := NewScaleDown(&context, clusterStateRegistry) scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p2}, time.Now().Add(-5*time.Minute), nil, nil) - scaleDownStatus, err := scaleDown.TryToScaleDown([]*apiv1.Node{n1, n2}, []*apiv1.Pod{p2}, nil, time.Now(), nil, nil) + []*apiv1.Pod{p2}, time.Now().Add(-5*time.Minute), nil) + scaleDownStatus, err := scaleDown.TryToScaleDown([]*apiv1.Node{n1, n2}, []*apiv1.Pod{p2}, nil, time.Now()) waitForDeleteToFinish(t, scaleDown) assert.NoError(t, err) @@ -1347,9 +1289,8 @@ func TestNoScaleDownUnready(t *testing.T) { context.CloudProvider = provider scaleDown = NewScaleDown(&context, clusterStateRegistry) scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p2}, time.Now().Add(-2*time.Hour), nil, nil) - scaleDownStatus, err = scaleDown.TryToScaleDown([]*apiv1.Node{n1, n2}, []*apiv1.Pod{p2}, nil, - time.Now(), nil, nil) + []*apiv1.Pod{p2}, time.Now().Add(-2*time.Hour), nil) + scaleDownStatus, err = scaleDown.TryToScaleDown([]*apiv1.Node{n1, n2}, []*apiv1.Pod{p2}, nil, time.Now()) waitForDeleteToFinish(t, scaleDown) assert.NoError(t, err) @@ -1431,8 +1372,8 @@ func TestScaleDownNoMove(t *testing.T) { clusterStateRegistry := clusterstate.NewClusterStateRegistry(provider, clusterstate.ClusterStateRegistryConfig{}, context.LogRecorder, newBackoff()) scaleDown := NewScaleDown(&context, clusterStateRegistry) scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p1, p2}, time.Now().Add(5*time.Minute), nil, nil) - scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{p1, p2}, nil, time.Now(), nil, nil) + []*apiv1.Pod{p1, p2}, time.Now().Add(5*time.Minute), nil) + scaleDownStatus, err := scaleDown.TryToScaleDown(nodes, []*apiv1.Pod{p1, p2}, nil, time.Now()) waitForDeleteToFinish(t, scaleDown) assert.NoError(t, err) @@ -1665,7 +1606,7 @@ func TestSoftTaint(t *testing.T) { // Test no superfluous nodes nodes := []*apiv1.Node{n1000, n2000} scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil) errs := scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.False(t, hasDeletionCandidateTaint(t, fakeClient, n1000.Name)) @@ -1673,7 +1614,7 @@ func TestSoftTaint(t *testing.T) { // Test one unneeded node scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p500, p1200}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p500, p1200}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.True(t, hasDeletionCandidateTaint(t, fakeClient, n1000.Name)) @@ -1681,7 +1622,7 @@ func TestSoftTaint(t *testing.T) { // Test remove soft taint scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.False(t, hasDeletionCandidateTaint(t, fakeClient, n1000.Name)) @@ -1689,7 +1630,7 @@ func TestSoftTaint(t *testing.T) { // Test bulk update taint limit scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 1, countDeletionCandidateTaints(t, fakeClient)) @@ -1699,7 +1640,7 @@ func TestSoftTaint(t *testing.T) { // Test bulk update untaint limit scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p500, p700, p1200}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 1, countDeletionCandidateTaints(t, fakeClient)) @@ -1776,7 +1717,7 @@ func TestSoftTaintTimeLimit(t *testing.T) { // Test bulk taint nodes := []*apiv1.Node{n1, n2} scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil) errs := scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 2, countDeletionCandidateTaints(t, fakeClient)) @@ -1785,7 +1726,7 @@ func TestSoftTaintTimeLimit(t *testing.T) { // Test bulk untaint scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p1, p2}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p1, p2}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 0, countDeletionCandidateTaints(t, fakeClient)) @@ -1796,7 +1737,7 @@ func TestSoftTaintTimeLimit(t *testing.T) { // Test duration limit of bulk taint scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 1, countDeletionCandidateTaints(t, fakeClient)) @@ -1806,7 +1747,7 @@ func TestSoftTaintTimeLimit(t *testing.T) { // Test duration limit of bulk untaint scaleDown.UpdateUnneededNodes(nodes, nodes, nodes, - []*apiv1.Pod{p1, p2}, time.Now().Add(-5*time.Minute), nil, nil) + []*apiv1.Pod{p1, p2}, time.Now().Add(-5*time.Minute), nil) errs = scaleDown.SoftTaintUnneededNodes(getAllNodes(t, fakeClient)) assert.Empty(t, errs) assert.Equal(t, 1, countDeletionCandidateTaints(t, fakeClient)) diff --git a/cluster-autoscaler/core/scale_test_common.go b/cluster-autoscaler/core/scale_test_common.go index b2a019413fef..6b53b4c7d148 100644 --- a/cluster-autoscaler/core/scale_test_common.go +++ b/cluster-autoscaler/core/scale_test_common.go @@ -80,7 +80,6 @@ type scaleTestConfig struct { //expectedScaleUpOptions []groupSizeChange // we expect that all those options should be included in expansion options passed to expander strategy //expectedFinalScaleUp groupSizeChange // we expect this to be delivered via scale-up event expectedScaleDowns []string - tempNodeNames []string } type scaleTestResults struct { diff --git a/cluster-autoscaler/core/static_autoscaler.go b/cluster-autoscaler/core/static_autoscaler.go index a892b2d526fa..03be93908379 100644 --- a/cluster-autoscaler/core/static_autoscaler.go +++ b/cluster-autoscaler/core/static_autoscaler.go @@ -389,15 +389,12 @@ func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError var scaleDownCandidates []*apiv1.Node var podDestinations []*apiv1.Node - var temporaryNodes []*apiv1.Node if a.processors == nil || a.processors.ScaleDownNodeProcessor == nil { scaleDownCandidates = allNodes podDestinations = allNodes - temporaryNodes = []*apiv1.Node{} } else { var err errors.AutoscalerError - a.processors.ScaleDownNodeProcessor.Reset() scaleDownCandidates, err = a.processors.ScaleDownNodeProcessor.GetScaleDownCandidates( autoscalingContext, allNodes) if err != nil { @@ -409,18 +406,11 @@ func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError klog.Error(err) return err } - temporaryNodes, err = a.processors.ScaleDownNodeProcessor.GetTemporaryNodes(allNodes) - if err != nil { - klog.Error(err) - return err - } } - tempNodesPerNodeGroup := getTempNodesPerNodeGroup(a.CloudProvider, temporaryNodes) - // We use scheduledPods (not originalScheduledPods) here, so artificial scheduled pods introduced by processors // (e.g unscheduled pods with nominated node name) can block scaledown of given node. - typedErr := scaleDown.UpdateUnneededNodes(allNodes, podDestinations, scaleDownCandidates, scheduledPods, currentTime, pdbs, tempNodesPerNodeGroup) + typedErr := scaleDown.UpdateUnneededNodes(allNodes, podDestinations, scaleDownCandidates, scheduledPods, currentTime, pdbs) if typedErr != nil { scaleDownStatus.Result = status.ScaleDownError klog.Errorf("Failed to scale down: %v", typedErr) @@ -466,7 +456,7 @@ func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError scaleDownStart := time.Now() metrics.UpdateLastTime(metrics.ScaleDown, scaleDownStart) - scaleDownStatus, typedErr := scaleDown.TryToScaleDown(allNodes, originalScheduledPods, pdbs, currentTime, temporaryNodes, tempNodesPerNodeGroup) + scaleDownStatus, typedErr := scaleDown.TryToScaleDown(allNodes, originalScheduledPods, pdbs, currentTime) metrics.UpdateDurationFromStart(metrics.ScaleDown, scaleDownStart) scaleDownStatus.RemovedNodeGroups = removedNodeGroups diff --git a/cluster-autoscaler/processors/nodes/pre_filtering_processor.go b/cluster-autoscaler/processors/nodes/pre_filtering_processor.go index 909a5433a057..1dde725e5038 100644 --- a/cluster-autoscaler/processors/nodes/pre_filtering_processor.go +++ b/cluster-autoscaler/processors/nodes/pre_filtering_processor.go @@ -71,19 +71,10 @@ func (n *PreFilteringScaleDownNodeProcessor) GetScaleDownCandidates(ctx *context return result, nil } -// GetTemporaryNodes returns nodes that are temporary and will not stay in the node group -func (n *PreFilteringScaleDownNodeProcessor) GetTemporaryNodes(allNodes []*apiv1.Node) ([]*apiv1.Node, errors.AutoscalerError) { - return nil, nil -} - // CleanUp is called at CA termination. func (n *PreFilteringScaleDownNodeProcessor) CleanUp() { } -// Reset is called before the other funcs of the processors are called every CA loop. -func (n *PreFilteringScaleDownNodeProcessor) Reset() { -} - // NewPreFilteringScaleDownNodeProcessor returns a new PreFilteringScaleDownNodeProcessor. func NewPreFilteringScaleDownNodeProcessor() *PreFilteringScaleDownNodeProcessor { return &PreFilteringScaleDownNodeProcessor{} diff --git a/cluster-autoscaler/processors/nodes/types.go b/cluster-autoscaler/processors/nodes/types.go index 6301598d8e16..dedc7890f4c9 100644 --- a/cluster-autoscaler/processors/nodes/types.go +++ b/cluster-autoscaler/processors/nodes/types.go @@ -30,10 +30,6 @@ type ScaleDownNodeProcessor interface { GetPodDestinationCandidates(*context.AutoscalingContext, []*apiv1.Node) ([]*apiv1.Node, errors.AutoscalerError) // GetScaleDownCandidates returns nodes that potentially could be scaled down. GetScaleDownCandidates(*context.AutoscalingContext, []*apiv1.Node) ([]*apiv1.Node, errors.AutoscalerError) - // GetTemporaryNodes returns nodes that are temporary and will not stay in the node group - GetTemporaryNodes(allNodes []*apiv1.Node) ([]*apiv1.Node, errors.AutoscalerError) - // Reset resets the properties if ScaleDownNodeProcessor - Reset() // CleanUp is called at CA termination CleanUp() }