diff --git a/cluster-autoscaler/expander/grpcplugin/grpc_client.go b/cluster-autoscaler/expander/grpcplugin/grpc_client.go index 17b4c018555c..7bcab0c8158d 100644 --- a/cluster-autoscaler/expander/grpcplugin/grpc_client.go +++ b/cluster-autoscaler/expander/grpcplugin/grpc_client.go @@ -92,9 +92,9 @@ func (g *grpcclientstrategy) BestOptions(expansionOptions []expander.Option, nod return expansionOptions } - if bestOptionsResponse == nil || bestOptionsResponse.Options == nil { - klog.V(4).Info("GRPC returned nil bestOptions, no options filtered") - return expansionOptions + if bestOptionsResponse == nil || len(bestOptionsResponse.Options) == 0 { + klog.V(4).Info("GRPC returned nil bestOptions") + return nil } // Transform back options slice options := transformAndSanitizeOptionsFromGRPC(bestOptionsResponse.Options, nodeGroupIDOptionMap) diff --git a/cluster-autoscaler/expander/grpcplugin/grpc_client_test.go b/cluster-autoscaler/expander/grpcplugin/grpc_client_test.go index 65b94a17d54b..cb6882b79fc6 100644 --- a/cluster-autoscaler/expander/grpcplugin/grpc_client_test.go +++ b/cluster-autoscaler/expander/grpcplugin/grpc_client_test.go @@ -197,6 +197,43 @@ 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} + + testCases := []struct { + desc string + mockResponse protos.BestOptionsResponse + }{ + { + desc: "empty bestOptions response", + mockResponse: protos.BestOptionsResponse{}, + }, + { + desc: "empty bestOptions response, options nil", + mockResponse: protos.BestOptionsResponse{Options: nil}, + }, + { + desc: "empty bestOptions response, empty options slice", + mockResponse: protos.BestOptionsResponse{Options: []*protos.Option{}}, + }, + } + for _, tc := range testCases { + grpcNodeInfoMap := populateNodeInfoForGRPC(makeFakeNodeInfos()) + mockClient.EXPECT().BestOptions( + gomock.Any(), gomock.Eq( + &protos.BestOptionsRequest{ + Options: []*protos.Option{&grpcEoT2Micro, &grpcEoT2Large, &grpcEoT3Large, &grpcEoM44XLarge}, + NodeMap: grpcNodeInfoMap, + })).Return(&tc.mockResponse, nil) + resp := g.BestOptions(options, makeFakeNodeInfos()) + + assert.Nil(t, resp) + } +} + // All test cases should error, and no options should be filtered func TestBestOptionsErrors(t *testing.T) { ctrl := gomock.NewController(t) @@ -232,20 +269,6 @@ func TestBestOptionsErrors(t *testing.T) { mockResponse: protos.BestOptionsResponse{}, errResponse: errors.New("timeout error"), }, - { - desc: "bad bestOptions response", - client: g, - nodeInfo: makeFakeNodeInfos(), - mockResponse: protos.BestOptionsResponse{}, - errResponse: nil, - }, - { - desc: "bad bestOptions response, options nil", - client: g, - nodeInfo: makeFakeNodeInfos(), - mockResponse: protos.BestOptionsResponse{Options: nil}, - errResponse: nil, - }, { desc: "bad bestOptions response, options invalid - nil", client: g, @@ -263,13 +286,15 @@ func TestBestOptionsErrors(t *testing.T) { } for _, tc := range testCases { grpcNodeInfoMap := populateNodeInfoForGRPC(tc.nodeInfo) - mockClient.EXPECT().BestOptions( - gomock.Any(), gomock.Eq( - &protos.BestOptionsRequest{ - Options: []*protos.Option{&grpcEoT2Micro, &grpcEoT2Large, &grpcEoT3Large, &grpcEoM44XLarge}, - NodeMap: grpcNodeInfoMap, - })).Return(&tc.mockResponse, tc.errResponse) - resp := g.BestOptions(options, tc.nodeInfo) + if tc.client.grpcClient != nil { + mockClient.EXPECT().BestOptions( + gomock.Any(), gomock.Eq( + &protos.BestOptionsRequest{ + Options: []*protos.Option{&grpcEoT2Micro, &grpcEoT2Large, &grpcEoT3Large, &grpcEoM44XLarge}, + NodeMap: grpcNodeInfoMap, + })).Return(&tc.mockResponse, tc.errResponse) + } + resp := tc.client.BestOptions(options, tc.nodeInfo) assert.Equal(t, resp, options) }