Skip to content

Commit

Permalink
Handle intentionally empty grpc BestOptions
Browse files Browse the repository at this point in the history
  • Loading branch information
rrangith committed Oct 4, 2024
1 parent 76deaaa commit 95895f0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
4 changes: 4 additions & 0 deletions cluster-autoscaler/expander/grpcplugin/grpc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ func (g *grpcclientstrategy) BestOptions(expansionOptions []expander.Option, nod
klog.V(4).Info("GRPC returned nil bestOptions, no options filtered")
return expansionOptions
}
if len(bestOptionsResponse.Options) == 0 {
// best options is intentionally empty
return nil
}
// Transform back options slice
options := transformAndSanitizeOptionsFromGRPC(bestOptionsResponse.Options, nodeGroupIDOptionMap)
if options == nil {
Expand Down
25 changes: 25 additions & 0 deletions cluster-autoscaler/expander/grpcplugin/grpc_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,31 @@ func TestBestOptionsValid(t *testing.T) {
assert.Equal(t, resp, []expander.Option{eoT3Large})
}

func TestBestOptionsEmpty(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockClient := mocks.NewMockExpanderClient(ctrl)
g := &grpcclientstrategy{mockClient}

nodeInfos := makeFakeNodeInfos()
grpcNodeInfoMap := make(map[string]*v1.Node)
for i, opt := range options {
grpcNodeInfoMap[opt.NodeGroup.Id()] = nodes[i]
}
expectedBestOptionsReq := &protos.BestOptionsRequest{
Options: []*protos.Option{&grpcEoT2Micro, &grpcEoT2Large, &grpcEoT3Large, &grpcEoM44XLarge},
NodeMap: grpcNodeInfoMap,
}

mockClient.EXPECT().BestOptions(
gomock.Any(), gomock.Eq(expectedBestOptionsReq),
).Return(&protos.BestOptionsResponse{Options: []*protos.Option{}}, nil)

resp := g.BestOptions(options, nodeInfos)

assert.Empty(t, resp)
}

// All test cases should error, and no options should be filtered
func TestBestOptionsErrors(t *testing.T) {
ctrl := gomock.NewController(t)
Expand Down

0 comments on commit 95895f0

Please sign in to comment.