fix(cloudprovider/exteranlgrpc): properly handle unimplemented methods

This commit is contained in:
Diego Bonfigli 2023-07-08 15:06:32 +02:00
parent 57f2814267
commit 436a8a0176
7 changed files with 130 additions and 16 deletions

View File

@ -20,6 +20,8 @@ import (
"context"
"fmt"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -113,6 +115,9 @@ func (w *Wrapper) PricingNodePrice(_ context.Context, req *protos.PricingNodePri
model, err := w.provider.Pricing()
if err != nil {
if err == cloudprovider.ErrNotImplemented {
return nil, status.Error(codes.Unimplemented, err.Error())
}
return nil, err
}
reqNode := req.GetNode()
@ -136,6 +141,9 @@ func (w *Wrapper) PricingPodPrice(_ context.Context, req *protos.PricingPodPrice
model, err := w.provider.Pricing()
if err != nil {
if err == cloudprovider.ErrNotImplemented {
return nil, status.Error(codes.Unimplemented, err.Error())
}
return nil, err
}
reqPod := req.GetPod()
@ -326,6 +334,9 @@ func (w *Wrapper) NodeGroupTemplateNodeInfo(_ context.Context, req *protos.NodeG
}
info, err := ng.TemplateNodeInfo()
if err != nil {
if err == cloudprovider.ErrNotImplemented {
return nil, status.Error(codes.Unimplemented, err.Error())
}
return nil, err
}
return &protos.NodeGroupTemplateNodeInfoResponse{
@ -355,6 +366,9 @@ func (w *Wrapper) NodeGroupGetOptions(_ context.Context, req *protos.NodeGroupAu
}
opts, err := ng.GetOptions(defaults)
if err != nil {
if err == cloudprovider.ErrNotImplemented {
return nil, status.Error(codes.Unimplemented, err.Error())
}
return nil, err
}
if opts == nil {

View File

@ -27,7 +27,9 @@ import (
"time"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/status"
"gopkg.in/yaml.v2"
apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
@ -158,6 +160,10 @@ func (m *pricingModel) NodePrice(node *apiv1.Node, startTime time.Time, endTime
EndTime: &end,
})
if err != nil {
st, ok := status.FromError(err)
if ok && st.Code() == codes.Unimplemented {
return 0, cloudprovider.ErrNotImplemented
}
klog.V(1).Infof("Error on gRPC call PricingNodePrice: %v", err)
return 0, err
}
@ -178,6 +184,10 @@ func (m *pricingModel) PodPrice(pod *apiv1.Pod, startTime time.Time, endTime tim
EndTime: &end,
})
if err != nil {
st, ok := status.FromError(err)
if ok && st.Code() == codes.Unimplemented {
return 0, cloudprovider.ErrNotImplemented
}
klog.V(1).Infof("Error on gRPC call PricingPodPrice: %v", err)
return 0, err
}

View File

@ -23,8 +23,11 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"
apiv1 "k8s.io/api/core/v1"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/externalgrpc/protos"
)
@ -286,6 +289,23 @@ func TestCloudProvider_Pricing(t *testing.T) {
_, err = model.NodePrice(apiv1Node3, time.Time{}, time.Time{})
assert.Error(t, err)
// test notImplemented for NodePrice
m.On(
"PricingNodePrice", mock.Anything, mock.MatchedBy(func(req *protos.PricingNodePriceRequest) bool {
return req.Node.Name == "node4"
}),
).Return(
&protos.PricingNodePriceResponse{},
status.Error(codes.Unimplemented, "mock error"),
)
apiv1Node4 := &apiv1.Node{}
apiv1Node4.Name = "node4"
_, err = model.NodePrice(apiv1Node4, time.Time{}, time.Time{})
assert.Error(t, err)
assert.Equal(t, cloudprovider.ErrNotImplemented, err)
// test correct PodPrice call
m.On(
"PricingPodPrice", mock.Anything, mock.MatchedBy(func(req *protos.PricingPodPriceRequest) bool {
@ -333,6 +353,24 @@ func TestCloudProvider_Pricing(t *testing.T) {
_, err = model.PodPrice(apiv1Pod3, time.Time{}, time.Time{})
assert.Error(t, err)
// test notImplemented for PodPrice
m.On(
"PricingPodPrice", mock.Anything, mock.MatchedBy(func(req *protos.PricingPodPriceRequest) bool {
return req.Pod.Name == "pod4"
}),
).Return(
&protos.PricingPodPriceResponse{},
status.Error(codes.Unimplemented, "mock error"),
)
apiv1Pod4 := &apiv1.Pod{}
apiv1Pod4.Name = "pod4"
_, err = model.PodPrice(apiv1Pod4, time.Time{}, time.Time{})
assert.Error(t, err)
assert.Equal(t, cloudprovider.ErrNotImplemented, err)
}
func TestCloudProvider_GPULabel(t *testing.T) {

View File

@ -20,6 +20,8 @@ import (
"context"
"sync"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
@ -206,6 +208,10 @@ func (n *NodeGroup) TemplateNodeInfo() (*schedulerframework.NodeInfo, error) {
Id: n.id,
})
if err != nil {
st, ok := status.FromError(err)
if ok && st.Code() == codes.Unimplemented {
return nil, cloudprovider.ErrNotImplemented
}
klog.V(1).Infof("Error on gRPC call NodeGroupTemplateNodeInfo: %v", err)
return nil, err
}
@ -269,6 +275,10 @@ func (n *NodeGroup) GetOptions(defaults config.NodeGroupAutoscalingOptions) (*co
},
})
if err != nil {
st, ok := status.FromError(err)
if ok && st.Code() == codes.Unimplemented {
return nil, cloudprovider.ErrNotImplemented
}
klog.V(1).Infof("Error on gRPC call NodeGroupGetOptions: %v", err)
return nil, err
}

View File

@ -23,6 +23,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
apiv1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
@ -208,6 +210,27 @@ func TestCloudProvider_TemplateNodeInfo(t *testing.T) {
_, err = ng4.TemplateNodeInfo()
assert.Error(t, err)
// test notImplemented
m.On(
"NodeGroupTemplateNodeInfo", mock.Anything, mock.MatchedBy(func(req *protos.NodeGroupTemplateNodeInfoRequest) bool {
return req.Id == "nodeGroup5"
}),
).Return(
&protos.NodeGroupTemplateNodeInfoResponse{
NodeInfo: nil,
},
status.Error(codes.Unimplemented, "mock error"),
).Once()
ng5 := NodeGroup{
id: "nodeGroup5",
client: client,
}
_, err = ng5.TemplateNodeInfo()
assert.Error(t, err)
assert.Equal(t, cloudprovider.ErrNotImplemented, err)
}
func TestCloudProvider_GetOptions(t *testing.T) {
@ -289,6 +312,26 @@ func TestCloudProvider_GetOptions(t *testing.T) {
opts, err = ng3.GetOptions(defaultsOpts)
assert.NoError(t, err)
assert.Nil(t, opts)
// test notImplemented
m.On(
"NodeGroupGetOptions", mock.Anything, mock.MatchedBy(func(req *protos.NodeGroupAutoscalingOptionsRequest) bool {
return req.Id == "nodeGroup4"
}),
).Return(
&protos.NodeGroupAutoscalingOptionsResponse{},
status.Error(codes.Unimplemented, "mock error"),
)
ng4 := NodeGroup{
id: "nodeGroup4",
client: client,
}
_, err = ng4.GetOptions(defaultsOpts)
assert.Error(t, err)
assert.Equal(t, cloudprovider.ErrNotImplemented, err)
}
func TestCloudProvider_TargetSize(t *testing.T) {

View File

@ -41,13 +41,13 @@ service CloudProvider {
// PricingNodePrice returns a theoretical minimum price of running a node for
// a given period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
rpc PricingNodePrice(PricingNodePriceRequest)
returns (PricingNodePriceResponse) {}
// PricingPodPrice returns a theoretical minimum price of running a pod for a given
// period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
rpc PricingPodPrice(PricingPodPriceRequest)
returns (PricingPodPriceResponse) {}
@ -103,13 +103,13 @@ service CloudProvider {
// NodeGroupTemplateNodeInfo returns a structure of an empty (as if just started) node,
// with all of the labels, capacity and allocatable information. This will be used in
// scale-up simulations to predict what would a new node look like if a node group was expanded.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
rpc NodeGroupTemplateNodeInfo(NodeGroupTemplateNodeInfoRequest)
returns (NodeGroupTemplateNodeInfoResponse) {}
// GetOptions returns NodeGroupAutoscalingOptions that should be used for this particular
// NodeGroup. Returning a grpc error will result in using default options.
// Implementation optional.
// NodeGroup.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
rpc NodeGroupGetOptions(NodeGroupAutoscalingOptionsRequest)
returns (NodeGroupAutoscalingOptionsResponse) {}
}
@ -371,4 +371,3 @@ message NodeGroupAutoscalingOptionsResponse {
// autoscaling options for the requested node.
NodeGroupAutoscalingOptions nodeGroupAutoscalingOptions = 1;
}

View File

@ -46,11 +46,11 @@ type CloudProviderClient interface {
NodeGroupForNode(ctx context.Context, in *NodeGroupForNodeRequest, opts ...grpc.CallOption) (*NodeGroupForNodeResponse, error)
// PricingNodePrice returns a theoretical minimum price of running a node for
// a given period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
PricingNodePrice(ctx context.Context, in *PricingNodePriceRequest, opts ...grpc.CallOption) (*PricingNodePriceResponse, error)
// PricingPodPrice returns a theoretical minimum price of running a pod for a given
// period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
PricingPodPrice(ctx context.Context, in *PricingPodPriceRequest, opts ...grpc.CallOption) (*PricingPodPriceResponse, error)
// GPULabel returns the label added to nodes with GPU resource.
GPULabel(ctx context.Context, in *GPULabelRequest, opts ...grpc.CallOption) (*GPULabelResponse, error)
@ -84,11 +84,11 @@ type CloudProviderClient interface {
// NodeGroupTemplateNodeInfo returns a structure of an empty (as if just started) node,
// with all of the labels, capacity and allocatable information. This will be used in
// scale-up simulations to predict what would a new node look like if a node group was expanded.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
NodeGroupTemplateNodeInfo(ctx context.Context, in *NodeGroupTemplateNodeInfoRequest, opts ...grpc.CallOption) (*NodeGroupTemplateNodeInfoResponse, error)
// GetOptions returns NodeGroupAutoscalingOptions that should be used for this particular
// NodeGroup. Returning a grpc error will result in using default options.
// Implementation optional.
// NodeGroup.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
NodeGroupGetOptions(ctx context.Context, in *NodeGroupAutoscalingOptionsRequest, opts ...grpc.CallOption) (*NodeGroupAutoscalingOptionsResponse, error)
}
@ -247,11 +247,11 @@ type CloudProviderServer interface {
NodeGroupForNode(context.Context, *NodeGroupForNodeRequest) (*NodeGroupForNodeResponse, error)
// PricingNodePrice returns a theoretical minimum price of running a node for
// a given period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
PricingNodePrice(context.Context, *PricingNodePriceRequest) (*PricingNodePriceResponse, error)
// PricingPodPrice returns a theoretical minimum price of running a pod for a given
// period of time on a perfectly matching machine.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
PricingPodPrice(context.Context, *PricingPodPriceRequest) (*PricingPodPriceResponse, error)
// GPULabel returns the label added to nodes with GPU resource.
GPULabel(context.Context, *GPULabelRequest) (*GPULabelResponse, error)
@ -285,11 +285,11 @@ type CloudProviderServer interface {
// NodeGroupTemplateNodeInfo returns a structure of an empty (as if just started) node,
// with all of the labels, capacity and allocatable information. This will be used in
// scale-up simulations to predict what would a new node look like if a node group was expanded.
// Implementation optional.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
NodeGroupTemplateNodeInfo(context.Context, *NodeGroupTemplateNodeInfoRequest) (*NodeGroupTemplateNodeInfoResponse, error)
// GetOptions returns NodeGroupAutoscalingOptions that should be used for this particular
// NodeGroup. Returning a grpc error will result in using default options.
// Implementation optional.
// NodeGroup.
// Implementation optional: if unimplemented return error code 12 (for `Unimplemented`)
NodeGroupGetOptions(context.Context, *NodeGroupAutoscalingOptionsRequest) (*NodeGroupAutoscalingOptionsResponse, error)
mustEmbedUnimplementedCloudProviderServer()
}