diff --git a/cluster-autoscaler/config/autoscaling_options.go b/cluster-autoscaler/config/autoscaling_options.go index f8a346606867..4d67aa97b4b4 100644 --- a/cluster-autoscaler/config/autoscaling_options.go +++ b/cluster-autoscaler/config/autoscaling_options.go @@ -48,9 +48,9 @@ type NodeGroupAutoscalingOptions struct { // AutoscalingOptions contain various options to customize how autoscaling works type AutoscalingOptions struct { - // NodeGroupAutoscalingOptions are default values for per NodeGroup options. + // NodeGroupDefaults are default values for per NodeGroup options. // They will be used any time a specific value is not provided for a given NodeGroup. - NodeGroupAutoscalingOptions + NodeGroupDefaults NodeGroupAutoscalingOptions // MaxEmptyBulkDelete is a number of empty nodes that can be removed at the same time. MaxEmptyBulkDelete int // MaxNodesTotal sets the maximum number of nodes in the whole cluster diff --git a/cluster-autoscaler/core/scale_down.go b/cluster-autoscaler/core/scale_down.go index 67d7df73e973..a440b6d94c35 100644 --- a/cluster-autoscaler/core/scale_down.go +++ b/cluster-autoscaler/core/scale_down.go @@ -389,7 +389,7 @@ func NewScaleDown(context *context.AutoscalingContext, processors *processors.Au func (sd *ScaleDown) CleanUp(timestamp time.Time) { // Use default ScaleDownUnneededTime as in this context the value // doesn't apply to any specific NodeGroup. - sd.usageTracker.CleanUp(timestamp.Add(-sd.context.ScaleDownUnneededTime)) + sd.usageTracker.CleanUp(timestamp.Add(-sd.context.NodeGroupDefaults.ScaleDownUnneededTime)) sd.clearUnremovableNodeReasons() } @@ -431,7 +431,7 @@ func (sd *ScaleDown) checkNodeUtilization(timestamp time.Time, node *apiv1.Node, if nodeGroup == nil || reflect.ValueOf(nodeGroup).IsNil() { // We should never get here as non-autoscaled nodes should not be included in scaleDownCandidates list // (and the default PreFilteringScaleDownNodeProcessor would indeed filter them out). - klog.V(4).Infof("Skipped %s from delete considered - the node is not autoscaled", node.Name) + klog.Warningf("Skipped %s from delete consideration - the node is not autoscaled", node.Name) return simulator.NotAutoscaled, nil } diff --git a/cluster-autoscaler/core/scale_down_test.go b/cluster-autoscaler/core/scale_down_test.go index a5439c2446ff..3f56805f04d4 100644 --- a/cluster-autoscaler/core/scale_down_test.go +++ b/cluster-autoscaler/core/scale_down_test.go @@ -130,7 +130,7 @@ func TestFindUnneededNodes(t *testing.T) { provider.AddNode("ng1", n9) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, }, UnremovableNodeRecheckTimeout: 5 * time.Minute, @@ -245,7 +245,7 @@ func TestFindUnneededGPUNodes(t *testing.T) { provider.AddNode("ng1", n3) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, ScaleDownGpuUtilizationThreshold: 0.3, }, @@ -310,7 +310,7 @@ func TestFindUnneededWithPerNodeGroupThresholds(t *testing.T) { } globalOptions := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.5, ScaleDownGpuUtilizationThreshold: 0.5, }, @@ -429,7 +429,7 @@ func TestPodsWithPreemptionsFindUnneededNodes(t *testing.T) { provider.AddNode("ng1", n4) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, }, } @@ -483,7 +483,7 @@ func TestFindUnneededMaxCandidates(t *testing.T) { numCandidates := 30 options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, }, ScaleDownNonEmptyCandidatesCount: numCandidates, @@ -558,7 +558,7 @@ func TestFindUnneededEmptyNodes(t *testing.T) { numCandidates := 30 options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, }, ScaleDownNonEmptyCandidatesCount: numCandidates, @@ -608,7 +608,7 @@ func TestFindUnneededNodePool(t *testing.T) { numCandidates := 30 options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.35, }, ScaleDownNonEmptyCandidatesCount: numCandidates, @@ -1135,7 +1135,7 @@ func TestScaleDown(t *testing.T) { assert.NotNil(t, provider) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, }, @@ -1194,7 +1194,7 @@ func assertSubset(t *testing.T, a []string, b []string) { } var defaultScaleDownOptions = config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, ScaleDownGpuUtilizationThreshold: 0.5, @@ -1479,7 +1479,7 @@ func TestNoScaleDownUnready(t *testing.T) { provider.AddNode("ng1", n2) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, ScaleDownUnreadyTime: time.Hour, @@ -1589,7 +1589,7 @@ func TestScaleDownNoMove(t *testing.T) { assert.NotNil(t, provider) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUnreadyTime: time.Hour, ScaleDownUtilizationThreshold: 0.5, @@ -1839,7 +1839,7 @@ func TestSoftTaint(t *testing.T) { assert.NotNil(t, provider) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: 10 * time.Minute, ScaleDownUtilizationThreshold: 0.5, }, @@ -1960,7 +1960,7 @@ func TestSoftTaintTimeLimit(t *testing.T) { assert.NotNil(t, provider) options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: 10 * time.Minute, ScaleDownUtilizationThreshold: 0.5, }, diff --git a/cluster-autoscaler/core/static_autoscaler_test.go b/cluster-autoscaler/core/static_autoscaler_test.go index 9dc44e85da1a..9502150b1eb9 100644 --- a/cluster-autoscaler/core/static_autoscaler_test.go +++ b/cluster-autoscaler/core/static_autoscaler_test.go @@ -172,7 +172,7 @@ func TestStaticAutoscalerRunOnce(t *testing.T) { // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUnreadyTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, @@ -361,7 +361,7 @@ func TestStaticAutoscalerRunOnceWithAutoprovisionedEnabled(t *testing.T) { // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUnreadyTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, @@ -497,7 +497,7 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) { // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUnreadyTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, @@ -644,7 +644,7 @@ func TestStaticAutoscalerRunOncePodsWithPriorities(t *testing.T) { // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, ScaleDownUnreadyTime: time.Minute, @@ -774,7 +774,7 @@ func TestStaticAutoscalerRunOnceWithFilteringOnBinPackingEstimator(t *testing.T) // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.5, }, EstimatorName: estimator.BinpackingEstimatorName, @@ -870,7 +870,7 @@ func TestStaticAutoscalerRunOnceWithFilteringOnUpcomingNodesEnabledNoScaleUp(t * // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: 0.5, }, EstimatorName: estimator.BinpackingEstimatorName, @@ -930,7 +930,7 @@ func TestStaticAutoscalerInstaceCreationErrors(t *testing.T) { // Create context with mocked lister registry. options := config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUnneededTime: time.Minute, ScaleDownUnreadyTime: time.Minute, ScaleDownUtilizationThreshold: 0.5, diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index 00de60fd8565..61dc7114dbba 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -194,7 +194,7 @@ func createAutoscalingOptions() config.AutoscalingOptions { klog.Fatalf("Failed to parse flags: %v", err) } return config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: config.NodeGroupAutoscalingOptions{ + NodeGroupDefaults: config.NodeGroupAutoscalingOptions{ ScaleDownUtilizationThreshold: *scaleDownUtilizationThreshold, ScaleDownGpuUtilizationThreshold: *scaleDownGpuUtilizationThreshold, ScaleDownUnneededTime: *scaleDownUnneededTime, diff --git a/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor.go b/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor.go index 9eaef7b522d4..74ce0efbce68 100644 --- a/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor.go +++ b/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor.go @@ -45,48 +45,48 @@ type DelegatingNodeGroupConfigProcessor struct { // GetScaleDownUnneededTime returns ScaleDownUnneededTime value that should be used for a given NodeGroup. func (p *DelegatingNodeGroupConfigProcessor) GetScaleDownUnneededTime(context *context.AutoscalingContext, nodeGroup cloudprovider.NodeGroup) (time.Duration, error) { - ngConfig, err := nodeGroup.GetOptions(context.NodeGroupAutoscalingOptions) + ngConfig, err := nodeGroup.GetOptions(context.NodeGroupDefaults) if err != nil && err != cloudprovider.ErrNotImplemented { return time.Duration(0), err } if ngConfig == nil || err == cloudprovider.ErrNotImplemented { - return context.ScaleDownUnneededTime, nil + return context.NodeGroupDefaults.ScaleDownUnneededTime, nil } return ngConfig.ScaleDownUnneededTime, nil } // GetScaleDownUnreadyTime returns ScaleDownUnreadyTime value that should be used for a given NodeGroup. func (p *DelegatingNodeGroupConfigProcessor) GetScaleDownUnreadyTime(context *context.AutoscalingContext, nodeGroup cloudprovider.NodeGroup) (time.Duration, error) { - ngConfig, err := nodeGroup.GetOptions(context.NodeGroupAutoscalingOptions) + ngConfig, err := nodeGroup.GetOptions(context.NodeGroupDefaults) if err != nil && err != cloudprovider.ErrNotImplemented { return time.Duration(0), err } if ngConfig == nil || err == cloudprovider.ErrNotImplemented { - return context.ScaleDownUnreadyTime, nil + return context.NodeGroupDefaults.ScaleDownUnreadyTime, nil } return ngConfig.ScaleDownUnreadyTime, nil } // GetScaleDownUtilizationThreshold returns ScaleDownUtilizationThreshold value that should be used for a given NodeGroup. func (p *DelegatingNodeGroupConfigProcessor) GetScaleDownUtilizationThreshold(context *context.AutoscalingContext, nodeGroup cloudprovider.NodeGroup) (float64, error) { - ngConfig, err := nodeGroup.GetOptions(context.NodeGroupAutoscalingOptions) + ngConfig, err := nodeGroup.GetOptions(context.NodeGroupDefaults) if err != nil && err != cloudprovider.ErrNotImplemented { return 0.0, err } if ngConfig == nil || err == cloudprovider.ErrNotImplemented { - return context.ScaleDownUtilizationThreshold, nil + return context.NodeGroupDefaults.ScaleDownUtilizationThreshold, nil } return ngConfig.ScaleDownUtilizationThreshold, nil } // GetScaleDownGpuUtilizationThreshold returns ScaleDownGpuUtilizationThreshold value that should be used for a given NodeGroup. func (p *DelegatingNodeGroupConfigProcessor) GetScaleDownGpuUtilizationThreshold(context *context.AutoscalingContext, nodeGroup cloudprovider.NodeGroup) (float64, error) { - ngConfig, err := nodeGroup.GetOptions(context.NodeGroupAutoscalingOptions) + ngConfig, err := nodeGroup.GetOptions(context.NodeGroupDefaults) if err != nil && err != cloudprovider.ErrNotImplemented { return 0.0, err } if ngConfig == nil || err == cloudprovider.ErrNotImplemented { - return context.ScaleDownGpuUtilizationThreshold, nil + return context.NodeGroupDefaults.ScaleDownGpuUtilizationThreshold, nil } return ngConfig.ScaleDownGpuUtilizationThreshold, nil } diff --git a/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor_test.go b/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor_test.go index cd44593f95a9..aabb1838ceed 100644 --- a/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor_test.go +++ b/cluster-autoscaler/processors/nodegroupconfig/node_group_config_processor_test.go @@ -43,6 +43,19 @@ func TestDelegatingNodeGroupConfigProcessor(t *testing.T) { var GLOBAL Want = 1 var NG Want = 2 + globalOpts := config.NodeGroupAutoscalingOptions{ + ScaleDownUnneededTime: 3 * time.Minute, + ScaleDownUnreadyTime: 4 * time.Minute, + ScaleDownGpuUtilizationThreshold: 0.6, + ScaleDownUtilizationThreshold: 0.5, + } + ngOpts := &config.NodeGroupAutoscalingOptions{ + ScaleDownUnneededTime: 10 * time.Minute, + ScaleDownUnreadyTime: 11 * time.Minute, + ScaleDownGpuUtilizationThreshold: 0.85, + ScaleDownUtilizationThreshold: 0.75, + } + testUnneededTime := func(t *testing.T, p DelegatingNodeGroupConfigProcessor, c *context.AutoscalingContext, ng cloudprovider.NodeGroup, w Want, we error) { res, err := p.GetScaleDownUnneededTime(c, ng) assert.Equal(t, err, we) @@ -84,68 +97,23 @@ func TestDelegatingNodeGroupConfigProcessor(t *testing.T) { assert.Equal(t, res, results[w]) } - funcs := map[string]struct { - testFn func(*testing.T, DelegatingNodeGroupConfigProcessor, *context.AutoscalingContext, cloudprovider.NodeGroup, Want, error) - globalOpts config.NodeGroupAutoscalingOptions - ngOpts *config.NodeGroupAutoscalingOptions - }{ - "ScaleDownUnneededTime": { - testFn: testUnneededTime, - globalOpts: config.NodeGroupAutoscalingOptions{ - ScaleDownUnneededTime: 3 * time.Minute, - }, - ngOpts: &config.NodeGroupAutoscalingOptions{ - ScaleDownUnneededTime: 10 * time.Minute, - }, + funcs := map[string]func(*testing.T, DelegatingNodeGroupConfigProcessor, *context.AutoscalingContext, cloudprovider.NodeGroup, Want, error){ + "ScaleDownUnneededTime": testUnneededTime, + "ScaleDownUnreadyTime": testUnreadyTime, + "ScaleDownUtilizationThreshold": testUtilizationThreshold, + "ScaleDownGpuUtilizationThreshold": testGpuThreshold, + "MultipleOptions": func(t *testing.T, p DelegatingNodeGroupConfigProcessor, c *context.AutoscalingContext, ng cloudprovider.NodeGroup, w Want, we error) { + testUnneededTime(t, p, c, ng, w, we) + testUnreadyTime(t, p, c, ng, w, we) + testUtilizationThreshold(t, p, c, ng, w, we) + testGpuThreshold(t, p, c, ng, w, we) }, - "ScaleDownUnreadyTime": { - testFn: testUnreadyTime, - globalOpts: config.NodeGroupAutoscalingOptions{ - ScaleDownUnreadyTime: 4 * time.Minute, - }, - ngOpts: &config.NodeGroupAutoscalingOptions{ - ScaleDownUnreadyTime: 11 * time.Minute, - }, - }, - "ScaleDownUtilizationThreshold": { - testFn: testUtilizationThreshold, - globalOpts: config.NodeGroupAutoscalingOptions{ - ScaleDownUtilizationThreshold: 0.5, - }, - ngOpts: &config.NodeGroupAutoscalingOptions{ - ScaleDownUtilizationThreshold: 0.75, - }, - }, - "ScaleDownGpuUtilizationThreshold": { - testFn: testGpuThreshold, - globalOpts: config.NodeGroupAutoscalingOptions{ - ScaleDownGpuUtilizationThreshold: 0.6, - }, - ngOpts: &config.NodeGroupAutoscalingOptions{ - ScaleDownGpuUtilizationThreshold: 0.85, - }, - }, - "MultipleOptions": { - testFn: func(t *testing.T, p DelegatingNodeGroupConfigProcessor, c *context.AutoscalingContext, ng cloudprovider.NodeGroup, w Want, we error) { - testUnneededTime(t, p, c, ng, w, we) - testUnreadyTime(t, p, c, ng, w, we) - testUtilizationThreshold(t, p, c, ng, w, we) - testUnneededTime(t, p, c, ng, w, we) - testUnneededTime(t, p, c, ng, w, we) - testGpuThreshold(t, p, c, ng, w, we) - }, - globalOpts: config.NodeGroupAutoscalingOptions{ - ScaleDownUnneededTime: 3 * time.Minute, - ScaleDownUnreadyTime: 4 * time.Minute, - ScaleDownGpuUtilizationThreshold: 0.6, - ScaleDownUtilizationThreshold: 0.5, - }, - ngOpts: &config.NodeGroupAutoscalingOptions{ - ScaleDownUnneededTime: 10 * time.Minute, - ScaleDownUnreadyTime: 11 * time.Minute, - ScaleDownGpuUtilizationThreshold: 0.85, - ScaleDownUtilizationThreshold: 0.75, - }, + "RepeatingTheSameCallGivesConsistentResults": func(t *testing.T, p DelegatingNodeGroupConfigProcessor, c *context.AutoscalingContext, ng cloudprovider.NodeGroup, w Want, we error) { + testUnneededTime(t, p, c, ng, w, we) + testUnneededTime(t, p, c, ng, w, we) + // throw in a different call + testGpuThreshold(t, p, c, ng, w, we) + testUnneededTime(t, p, c, ng, w, we) }, } @@ -158,22 +126,22 @@ func TestDelegatingNodeGroupConfigProcessor(t *testing.T) { wantError error }{ "NodeGroup.GetOptions not implemented": { - globalOptions: fn.globalOpts, + globalOptions: globalOpts, ngError: cloudprovider.ErrNotImplemented, want: GLOBAL, }, "NodeGroup returns error leads to error": { - globalOptions: fn.globalOpts, + globalOptions: globalOpts, ngError: errors.New("This sentence is false."), wantError: errors.New("This sentence is false."), }, "NodeGroup returns no value fallbacks to default": { - globalOptions: fn.globalOpts, + globalOptions: globalOpts, want: GLOBAL, }, "NodeGroup option overrides global default": { - globalOptions: fn.globalOpts, - ngOptions: fn.ngOpts, + globalOptions: globalOpts, + ngOptions: ngOpts, want: NG, }, } @@ -181,13 +149,13 @@ func TestDelegatingNodeGroupConfigProcessor(t *testing.T) { t.Run(fmt.Sprintf("[%s] %s", fname, tn), func(t *testing.T) { context := &context.AutoscalingContext{ AutoscalingOptions: config.AutoscalingOptions{ - NodeGroupAutoscalingOptions: tc.globalOptions, + NodeGroupDefaults: tc.globalOptions, }, } ng := &mocks.NodeGroup{} ng.On("GetOptions", tc.globalOptions).Return(tc.ngOptions, tc.ngError) p := DelegatingNodeGroupConfigProcessor{} - fn.testFn(t, p, context, ng, tc.want, tc.wantError) + fn(t, p, context, ng, tc.want, tc.wantError) }) } }