Skip to content

Commit 614aa68

Browse files
BenHinthorneBenHinthorne
committed
include similar nodegroups in gRPC expander options
1 parent bf8049c commit 614aa68

File tree

5 files changed

+302
-180
lines changed

5 files changed

+302
-180
lines changed

‎cluster-autoscaler/expander/grpcplugin/grpc_client.go‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"time"
2323

2424
v1 "k8s.io/api/core/v1"
25+
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
2526
"k8s.io/autoscaler/cluster-autoscaler/expander"
2627
"k8s.io/autoscaler/cluster-autoscaler/expander/grpcplugin/protos"
2728
"k8s.io/klog/v2"
@@ -111,11 +112,20 @@ func populateOptionsForGRPC(expansionOptions []expander.Option) ([]*protos.Optio
111112
nodeGroupIDOptionMap := make(map[string]expander.Option)
112113
for _, option := range expansionOptions {
113114
nodeGroupIDOptionMap[option.NodeGroup.Id()] = option
114-
grpcOptionsSlice = append(grpcOptionsSlice, newOptionMessage(option.NodeGroup.Id(), int32(option.NodeCount), option.Debug, option.Pods))
115+
similarNodeGroupIds := getSimilarNodeGroupIds(option)
116+
grpcOptionsSlice = append(grpcOptionsSlice, newOptionMessage(option.NodeGroup.Id(), int32(option.NodeCount), option.Debug, option.Pods, similarNodeGroupIds))
115117
}
116118
return grpcOptionsSlice, nodeGroupIDOptionMap
117119
}
118120

121+
func getSimilarNodeGroupIds(option expander.Option) []string {
122+
var similarNodeGroupIds []string
123+
for _, sng := range option.SimilarNodeGroups {
124+
similarNodeGroupIds = append(similarNodeGroupIds, sng.Id())
125+
}
126+
return similarNodeGroupIds
127+
}
128+
119129
// populateNodeInfoForGRPC looks at the corresponding v1.Node object per NodeInfo object, and populates the grpcNodeInfoMap with these to pass over grpc
120130
func populateNodeInfoForGRPC(nodeInfos map[string]*schedulerframework.NodeInfo) map[string]*v1.Node {
121131
grpcNodeInfoMap := make(map[string]*v1.Node)
@@ -133,7 +143,9 @@ func transformAndSanitizeOptionsFromGRPC(bestOptionsResponseOptions []*protos.Op
133143
continue
134144
}
135145
if _, ok := nodeGroupIDOptionMap[option.NodeGroupId]; ok {
136-
options = append(options, nodeGroupIDOptionMap[option.NodeGroupId])
146+
expanderOption := nodeGroupIDOptionMap[option.NodeGroupId]
147+
expanderOption.SimilarNodeGroups = getRetainedSimilarNodegroups(option, expanderOption)
148+
options = append(options, expanderOption)
137149
} else {
138150
klog.Errorf("GRPC server returned invalid nodeGroup ID: ", option.NodeGroupId)
139151
continue
@@ -142,6 +154,25 @@ func transformAndSanitizeOptionsFromGRPC(bestOptionsResponseOptions []*protos.Op
142154
return options
143155
}
144156

145-
func newOptionMessage(nodeGroupId string, nodeCount int32, debug string, pods []*v1.Pod) *protos.Option {
146-
return &protos.Option{NodeGroupId: nodeGroupId, NodeCount: nodeCount, Debug: debug, Pod: pods}
157+
// Any similar options that were not included in the original grpcExpander request, but were added
158+
// as part of the response, will be ignored
159+
func getRetainedSimilarNodegroups(grpcOption *protos.Option, expanderOption expander.Option) []cloudprovider.NodeGroup {
160+
var retainedSimilarNodeGroups []cloudprovider.NodeGroup
161+
for _, sng := range expanderOption.SimilarNodeGroups {
162+
retained := false
163+
for _, id := range grpcOption.SimilarNodeGroupIds {
164+
if sng.Id() == id {
165+
retained = true
166+
continue
167+
}
168+
}
169+
if retained {
170+
retainedSimilarNodeGroups = append(retainedSimilarNodeGroups, sng)
171+
}
172+
}
173+
return retainedSimilarNodeGroups
174+
}
175+
176+
func newOptionMessage(nodeGroupId string, nodeCount int32, debug string, pods []*v1.Pod, similarNodeGroupIds []string) *protos.Option {
177+
return &protos.Option{NodeGroupId: nodeGroupId, NodeCount: nodeCount, Debug: debug, Pod: pods, SimilarNodeGroupIds: similarNodeGroupIds}
147178
}

‎cluster-autoscaler/expander/grpcplugin/grpc_client_test.go‎

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/golang/mock/gomock"
2424
"github.com/stretchr/testify/assert"
2525
v1 "k8s.io/api/core/v1"
26+
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
2627
"k8s.io/autoscaler/cluster-autoscaler/expander/grpcplugin/protos"
2728
"k8s.io/autoscaler/cluster-autoscaler/expander/mocks"
2829
. "k8s.io/autoscaler/cluster-autoscaler/utils/test"
@@ -58,6 +59,11 @@ var (
5859
Debug: "m4.4xlarge",
5960
NodeGroup: test.NewTestNodeGroup("my-asg.m4.4xlarge", 10, 1, 1, true, false, "m4.4xlarge", nil, nil),
6061
}
62+
eoT2MicroWithSimilar = expander.Option{
63+
Debug: "t2.micro",
64+
NodeGroup: test.NewTestNodeGroup("my-asg.t2.micro", 10, 1, 1, true, false, "t2.micro", nil, nil),
65+
SimilarNodeGroups: []cloudprovider.NodeGroup{test.NewTestNodeGroup("my-similar-asg.t2.micro", 10, 1, 1, true, false, "t2.micro", nil, nil)},
66+
}
6167
options = []expander.Option{eoT2Micro, eoT2Large, eoT3Large, eoM44XLarge}
6268

6369
grpcEoT2Micro = protos.Option{
@@ -84,6 +90,20 @@ var (
8490
Debug: eoM44XLarge.Debug,
8591
Pod: eoM44XLarge.Pods,
8692
}
93+
grpcEoT2MicroWithSimilar = protos.Option{
94+
NodeGroupId: eoT2Micro.NodeGroup.Id(),
95+
NodeCount: int32(eoT2Micro.NodeCount),
96+
Debug: eoT2Micro.Debug,
97+
Pod: eoT2Micro.Pods,
98+
SimilarNodeGroupIds: []string{eoT2MicroWithSimilar.SimilarNodeGroups[0].Id()},
99+
}
100+
grpcEoT2MicroWithSimilarWithExtraOptions = protos.Option{
101+
NodeGroupId: eoT2Micro.NodeGroup.Id(),
102+
NodeCount: int32(eoT2Micro.NodeCount),
103+
Debug: eoT2Micro.Debug,
104+
Pod: eoT2Micro.Pods,
105+
SimilarNodeGroupIds: []string{eoT2MicroWithSimilar.SimilarNodeGroups[0].Id(), "extra-ng-id"},
106+
}
87107
)
88108

89109
func TestPopulateOptionsForGrpc(t *testing.T) {
@@ -116,6 +136,12 @@ func TestPopulateOptionsForGrpc(t *testing.T) {
116136
eoM44XLarge.NodeGroup.Id(): eoM44XLarge,
117137
},
118138
},
139+
{
140+
desc: "similar nodegroups are included",
141+
opts: []expander.Option{eoT2MicroWithSimilar},
142+
expectedOpts: []*protos.Option{&grpcEoT2MicroWithSimilar},
143+
expectedMap: map[string]expander.Option{eoT2MicroWithSimilar.NodeGroup.Id(): eoT2MicroWithSimilar},
144+
},
119145
}
120146
for _, tc := range testCases {
121147
grpcOptionsSlice, nodeGroupIDOptionMap := populateOptionsForGRPC(tc.opts)
@@ -146,18 +172,44 @@ func TestPopulateNodeInfoForGRPC(t *testing.T) {
146172
}
147173

148174
func TestValidTransformAndSanitizeOptionsFromGRPC(t *testing.T) {
149-
responseOptionsSlice := []*protos.Option{&grpcEoT2Micro, &grpcEoT3Large, &grpcEoM44XLarge}
150-
nodeGroupIDOptionMap := map[string]expander.Option{
151-
eoT2Micro.NodeGroup.Id(): eoT2Micro,
152-
eoT2Large.NodeGroup.Id(): eoT2Large,
153-
eoT3Large.NodeGroup.Id(): eoT3Large,
154-
eoM44XLarge.NodeGroup.Id(): eoM44XLarge,
175+
testCases := []struct {
176+
desc string
177+
responseOptions []*protos.Option
178+
expectedOptions []expander.Option
179+
nodegroupIDOptionaMap map[string]expander.Option
180+
}{
181+
{
182+
desc: "valid transform and sanitize options",
183+
responseOptions: []*protos.Option{&grpcEoT2Micro, &grpcEoT3Large, &grpcEoM44XLarge},
184+
nodegroupIDOptionaMap: map[string]expander.Option{
185+
eoT2Micro.NodeGroup.Id(): eoT2Micro,
186+
eoT2Large.NodeGroup.Id(): eoT2Large,
187+
eoT3Large.NodeGroup.Id(): eoT3Large,
188+
eoM44XLarge.NodeGroup.Id(): eoM44XLarge,
189+
},
190+
expectedOptions: []expander.Option{eoT2Micro, eoT3Large, eoM44XLarge},
191+
},
192+
{
193+
desc: "similar ngs are retained in proto options are retained",
194+
responseOptions: []*protos.Option{&grpcEoT2MicroWithSimilar},
195+
nodegroupIDOptionaMap: map[string]expander.Option{
196+
eoT2MicroWithSimilar.NodeGroup.Id(): eoT2MicroWithSimilar,
197+
},
198+
expectedOptions: []expander.Option{eoT2MicroWithSimilar},
199+
},
200+
{
201+
desc: "extra similar ngs added to expander response are ignored",
202+
responseOptions: []*protos.Option{&grpcEoT2MicroWithSimilarWithExtraOptions},
203+
nodegroupIDOptionaMap: map[string]expander.Option{
204+
eoT2MicroWithSimilar.NodeGroup.Id(): eoT2MicroWithSimilar,
205+
},
206+
expectedOptions: []expander.Option{eoT2MicroWithSimilar},
207+
},
208+
}
209+
for _, tc := range testCases {
210+
ret := transformAndSanitizeOptionsFromGRPC(tc.responseOptions, tc.nodegroupIDOptionaMap)
211+
assert.Equal(t, tc.expectedOptions, ret)
155212
}
156-
157-
expectedOptions := []expander.Option{eoT2Micro, eoT3Large, eoM44XLarge}
158-
159-
ret := transformAndSanitizeOptionsFromGRPC(responseOptionsSlice, nodeGroupIDOptionMap)
160-
assert.Equal(t, expectedOptions, ret)
161213
}
162214

163215
func TestAnInvalidTransformAndSanitizeOptionsFromGRPC(t *testing.T) {

0 commit comments

Comments
 (0)