diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index 9665d83a431e..dc8d50093221 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -648,8 +648,9 @@ func (csr *ClusterStateRegistry) updateIncorrectNodeGroupSizes(currentTime time. } continue } - if len(readiness.Registered) > acceptableRange.MaxNodes || - len(readiness.Registered) < acceptableRange.MinNodes { + unregisteredNodes := len(readiness.Unregistered) + len(readiness.LongUnregistered) + if len(readiness.Registered) > acceptableRange.CurrentTarget || + len(readiness.Registered) < acceptableRange.CurrentTarget-unregisteredNodes { incorrect := IncorrectNodeGroupSize{ CurrentSize: len(readiness.Registered), ExpectedSize: acceptableRange.CurrentTarget, diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index d0147009f126..9ff4eea5006f 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -1064,3 +1064,319 @@ func newBackoff() backoff.Backoff { return backoff.NewIdBasedExponentialBackoff(5*time.Minute, /*InitialNodeGroupBackoffDuration*/ 30*time.Minute /*MaxNodeGroupBackoffDuration*/, 3*time.Hour /*NodeGroupBackoffResetTimeout*/) } + +func TestUpdateAcceptableRanges(t *testing.T) { + testCases := []struct { + name string + + targetSizes map[string]int + readiness map[string]Readiness + scaleUpRequests map[string]*ScaleUpRequest + scaledDownGroups []string + + wantAcceptableRanges map[string]AcceptableRange + }{ + { + name: "No scale-ups/scale-downs", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 10)}, + "ng2": {Ready: make([]string, 20)}, + }, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 10, MaxNodes: 10, CurrentTarget: 10}, + "ng2": {MinNodes: 20, MaxNodes: 20, CurrentTarget: 20}, + }, + }, + { + name: "Ongoing scale-ups", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 10)}, + "ng2": {Ready: make([]string, 20)}, + }, + scaleUpRequests: map[string]*ScaleUpRequest{ + "ng1": {Increase: 3}, + "ng2": {Increase: 5}, + }, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 7, MaxNodes: 10, CurrentTarget: 10}, + "ng2": {MinNodes: 15, MaxNodes: 20, CurrentTarget: 20}, + }, + }, + { + name: "Ongoing scale-downs", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 10)}, + "ng2": {Ready: make([]string, 20)}, + }, + scaledDownGroups: []string{"ng1", "ng1", "ng2", "ng2", "ng2"}, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 10, MaxNodes: 12, CurrentTarget: 10}, + "ng2": {MinNodes: 20, MaxNodes: 23, CurrentTarget: 20}, + }, + }, + { + name: "Some short unregistered nodes", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 8), Unregistered: make([]string, 2)}, + "ng2": {Ready: make([]string, 17), Unregistered: make([]string, 3)}, + }, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 10, MaxNodes: 10, CurrentTarget: 10}, + "ng2": {MinNodes: 20, MaxNodes: 20, CurrentTarget: 20}, + }, + }, + { + name: "Some long unregistered nodes", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 8), LongUnregistered: make([]string, 2)}, + "ng2": {Ready: make([]string, 17), LongUnregistered: make([]string, 3)}, + }, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 8, MaxNodes: 10, CurrentTarget: 10}, + "ng2": {MinNodes: 17, MaxNodes: 20, CurrentTarget: 20}, + }, + }, + { + name: "Everything together", + targetSizes: map[string]int{ + "ng1": 10, + "ng2": 20, + }, + readiness: map[string]Readiness{ + "ng1": {Ready: make([]string, 8), Unregistered: make([]string, 1), LongUnregistered: make([]string, 2)}, + "ng2": {Ready: make([]string, 17), Unregistered: make([]string, 3), LongUnregistered: make([]string, 4)}, + }, + scaleUpRequests: map[string]*ScaleUpRequest{ + "ng1": {Increase: 3}, + "ng2": {Increase: 5}, + }, + scaledDownGroups: []string{"ng1", "ng1", "ng2", "ng2", "ng2"}, + wantAcceptableRanges: map[string]AcceptableRange{ + "ng1": {MinNodes: 5, MaxNodes: 12, CurrentTarget: 10}, + "ng2": {MinNodes: 11, MaxNodes: 23, CurrentTarget: 20}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + provider := testprovider.NewTestCloudProvider(nil, nil) + for nodeGroupName, targetSize := range tc.targetSizes { + provider.AddNodeGroup(nodeGroupName, 0, 1000, targetSize) + } + var scaleDownRequests []*ScaleDownRequest + for _, nodeGroupName := range tc.scaledDownGroups { + scaleDownRequests = append(scaleDownRequests, &ScaleDownRequest{ + NodeGroup: provider.GetNodeGroup(nodeGroupName), + }) + } + + clusterState := &ClusterStateRegistry{ + cloudProvider: provider, + perNodeGroupReadiness: tc.readiness, + scaleUpRequests: tc.scaleUpRequests, + scaleDownRequests: scaleDownRequests, + } + + clusterState.updateAcceptableRanges(tc.targetSizes) + assert.Equal(t, tc.wantAcceptableRanges, clusterState.acceptableRanges) + }) + } +} + +func TestUpdateIncorrectNodeGroupSizes(t *testing.T) { + timeNow := time.Now() + testCases := []struct { + name string + + acceptableRanges map[string]AcceptableRange + readiness map[string]Readiness + incorrectSizes map[string]IncorrectNodeGroupSize + + wantIncorrectSizes map[string]IncorrectNodeGroupSize + }{ + { + name: "node groups with correct sizes", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 10)}, + "ng2": {Registered: make([]string, 20)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{}, + }, + { + name: "node groups with correct sizes after not being correct sized", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 10)}, + "ng2": {Registered: make([]string, 20)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 8, ExpectedSize: 10, FirstObserved: timeNow.Add(-time.Hour)}, + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow.Add(-time.Minute)}, + }, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{}, + }, + { + name: "node groups below the target size", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8)}, + "ng2": {Registered: make([]string, 15)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 8, ExpectedSize: 10, FirstObserved: timeNow}, + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + { + name: "node groups above the target size", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 12)}, + "ng2": {Registered: make([]string, 25)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 12, ExpectedSize: 10, FirstObserved: timeNow}, + "ng2": {CurrentSize: 25, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + { + name: "node groups below the target size with changed delta", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8)}, + "ng2": {Registered: make([]string, 15)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 7, ExpectedSize: 10, FirstObserved: timeNow.Add(-time.Hour)}, + "ng2": {CurrentSize: 14, ExpectedSize: 20, FirstObserved: timeNow.Add(-time.Minute)}, + }, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 8, ExpectedSize: 10, FirstObserved: timeNow}, + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + { + name: "node groups below the target size with the same delta", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8)}, + "ng2": {Registered: make([]string, 15)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 8, ExpectedSize: 10, FirstObserved: timeNow.Add(-time.Hour)}, + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow.Add(-time.Minute)}, + }, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng1": {CurrentSize: 8, ExpectedSize: 10, FirstObserved: timeNow.Add(-time.Hour)}, + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow.Add(-time.Minute)}, + }, + }, + { + name: "node groups below the target size with short unregistered nodes", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8), Unregistered: make([]string, 2)}, + "ng2": {Registered: make([]string, 15), Unregistered: make([]string, 3)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + { + name: "node groups below the target size with long unregistered nodes", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8), LongUnregistered: make([]string, 2)}, + "ng2": {Registered: make([]string, 15), LongUnregistered: make([]string, 3)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + { + name: "node groups below the target size with various unregistered nodes", + acceptableRanges: map[string]AcceptableRange{ + "ng1": {CurrentTarget: 10}, + "ng2": {CurrentTarget: 20}, + }, + readiness: map[string]Readiness{ + "ng1": {Registered: make([]string, 8), Unregistered: make([]string, 1), LongUnregistered: make([]string, 1)}, + "ng2": {Registered: make([]string, 15), Unregistered: make([]string, 2), LongUnregistered: make([]string, 2)}, + }, + incorrectSizes: map[string]IncorrectNodeGroupSize{}, + wantIncorrectSizes: map[string]IncorrectNodeGroupSize{ + "ng2": {CurrentSize: 15, ExpectedSize: 20, FirstObserved: timeNow}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + provider := testprovider.NewTestCloudProvider(nil, nil) + for nodeGroupName, acceptableRange := range tc.acceptableRanges { + provider.AddNodeGroup(nodeGroupName, 0, 1000, acceptableRange.CurrentTarget) + } + + clusterState := &ClusterStateRegistry{ + cloudProvider: provider, + acceptableRanges: tc.acceptableRanges, + perNodeGroupReadiness: tc.readiness, + incorrectNodeGroupSizes: tc.incorrectSizes, + } + + clusterState.updateIncorrectNodeGroupSizes(timeNow) + assert.Equal(t, tc.wantIncorrectSizes, clusterState.incorrectNodeGroupSizes) + }) + } +}