Skip to content

Commit

Permalink
Merge pull request kubernetes#5894 from BigDarkClown/fix-cs
Browse files Browse the repository at this point in the history 
Include short unregistered nodes in calculation of incorrect node group
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 29, 2023
2 parents 8f83f7e + 67d3e7e commit 973f9fd
Show file tree
Hide file tree
Showing 2 changed files with 319 additions and 2 deletions.
5 changes: 3 additions & 2 deletions cluster-autoscaler/clusterstate/clusterstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
316 changes: 316 additions & 0 deletions cluster-autoscaler/clusterstate/clusterstate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 973f9fd

Please sign in to comment.