From 2403382ee6f6948e5f5396fc1f363dfa17664379 Mon Sep 17 00:00:00 2001 From: Patrik Cyvoct Date: Thu, 3 Sep 2020 15:26:31 +0200 Subject: [PATCH] fix: avoid not found error mishandled Signed-off-by: Patrik Cyvoct --- scaleway/baremetal.go | 32 +++++----- scaleway/baremetal_test.go | 4 +- scaleway/instances.go | 23 +++---- scaleway/instances_test.go | 2 +- scaleway/loadbalancers.go | 125 +++++++++---------------------------- 5 files changed, 58 insertions(+), 128 deletions(-) diff --git a/scaleway/baremetal.go b/scaleway/baremetal.go index bfc696b..f6282b1 100644 --- a/scaleway/baremetal.go +++ b/scaleway/baremetal.go @@ -21,7 +21,6 @@ import ( scwbaremetal "github.com/scaleway/scaleway-sdk-go/api/baremetal/v1alpha1" "github.com/scaleway/scaleway-sdk-go/scw" - "golang.org/x/xerrors" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" @@ -191,11 +190,10 @@ func (b *baremetal) getServerOfferName(server *scwbaremetal.Server) (string, err Zone: server.Zone, }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - return "UNKNOWN", nil - } + switch err.(type) { + case *scw.ResourceNotFoundError: + return "UNKNOWN", nil + default: return "", err } } @@ -217,13 +215,12 @@ func (b *baremetal) getServerByName(name string) (*scwbaremetal.Server, error) { Name: &name, }, scw.WithAllPages()) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - continue - } + switch err.(type) { + case *scw.ResourceNotFoundError: + continue + default: + return nil, err } - return nil, err } for _, srv := range resp.Servers { @@ -258,13 +255,12 @@ func (b *baremetal) getServerByProviderID(providerID string) (*scwbaremetal.Serv Zone: scw.Zone(zone), }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - return nil, cloudprovider.InstanceNotFound - } + switch err.(type) { + case *scw.ResourceNotFoundError: + return nil, cloudprovider.InstanceNotFound + default: + return nil, err } - return nil, err } return server, nil } diff --git a/scaleway/baremetal_test.go b/scaleway/baremetal_test.go index 3260b52..29a469c 100644 --- a/scaleway/baremetal_test.go +++ b/scaleway/baremetal_test.go @@ -156,7 +156,7 @@ func (f *fakeBaremetalAPI) ListServers(req *scwbaremetal.ListServersRequest, opt func (f *fakeBaremetalAPI) GetServer(req *scwbaremetal.GetServerRequest, opts ...scw.RequestOption) (*scwbaremetal.Server, error) { server, ok := f.Servers[req.ServerID] if !ok { - return nil, &scw.ResponseError{StatusCode: 404} + return nil, &scw.ResourceNotFoundError{} } server.ID = req.ServerID @@ -166,7 +166,7 @@ func (f *fakeBaremetalAPI) GetServer(req *scwbaremetal.GetServerRequest, opts .. func (f *fakeBaremetalAPI) GetOffer(req *scwbaremetal.GetOfferRequest, opts ...scw.RequestOption) (*scwbaremetal.Offer, error) { offer, ok := f.Offers[req.OfferID] if !ok { - return nil, &scw.ResponseError{StatusCode: 404} + return nil, &scw.ResourceNotFoundError{} } offer.ID = req.OfferID diff --git a/scaleway/instances.go b/scaleway/instances.go index fcaff4c..f8d3fc9 100644 --- a/scaleway/instances.go +++ b/scaleway/instances.go @@ -22,7 +22,6 @@ import ( scwinstance "github.com/scaleway/scaleway-sdk-go/api/instance/v1" "github.com/scaleway/scaleway-sdk-go/scw" - "golang.org/x/xerrors" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" @@ -203,13 +202,12 @@ func (i *instances) getServerByName(name string) (*scwinstance.Server, error) { }, scw.WithAllPages()) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - continue - } + switch err.(type) { + case *scw.ResourceNotFoundError: + continue + default: + return nil, err } - return nil, err } for _, srv := range resp.Servers { @@ -245,13 +243,12 @@ func (i *instances) getServerByProviderID(providerID string) (*scwinstance.Serve Zone: scw.Zone(zone), }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - return nil, cloudprovider.InstanceNotFound - } + switch err.(type) { + case *scw.ResourceNotFoundError: + return nil, cloudprovider.InstanceNotFound + default: + return nil, err } - return nil, err } return resp.Server, nil diff --git a/scaleway/instances_test.go b/scaleway/instances_test.go index 2e5c202..1dfb87b 100644 --- a/scaleway/instances_test.go +++ b/scaleway/instances_test.go @@ -137,7 +137,7 @@ func (f *fakeInstanceAPI) ListServers(req *scwinstance.ListServersRequest, opts func (f *fakeInstanceAPI) GetServer(req *scwinstance.GetServerRequest, opts ...scw.RequestOption) (*scwinstance.GetServerResponse, error) { server, ok := f.Servers[req.ServerID] if !ok { - return nil, &scw.ResponseError{StatusCode: 404} + return nil, &scw.ResourceNotFoundError{} } server.ID = req.ServerID diff --git a/scaleway/loadbalancers.go b/scaleway/loadbalancers.go index b39ee88..45e781c 100644 --- a/scaleway/loadbalancers.go +++ b/scaleway/loadbalancers.go @@ -19,7 +19,6 @@ package scaleway import ( "context" "fmt" - "net/http" "os" "strconv" "strings" @@ -27,7 +26,6 @@ import ( scwlb "github.com/scaleway/scaleway-sdk-go/api/lb/v1" "github.com/scaleway/scaleway-sdk-go/scw" - "golang.org/x/xerrors" v1 "k8s.io/api/core/v1" "k8s.io/klog" ) @@ -312,13 +310,8 @@ func (l *loadbalancers) deleteLoadBalancer(ctx context.Context, lb *scwlb.LB, se err := l.api.DeleteLB(request) if err != nil { - klog.Errorf("error creating load balancer: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call DeleteLb with LbID %s: error %d with message %s", lb.ID, respErr.StatusCode, respErr.Message) - } - - return err + klog.Errorf("error deleting load balancer %s: %v", lb.ID, err) + return fmt.Errorf("error deleting load balancer %s: %v", lb.ID, err) } return nil @@ -375,13 +368,13 @@ func (l *loadbalancers) fetchLoadBalancer(ctx context.Context, clusterName strin Region: region, }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) && respErr.StatusCode == http.StatusNotFound { + switch err.(type) { + case *scw.ResourceNotFoundError: return nil, LoadBalancerNotFound + default: + klog.Errorf("an error occurred while fetching loadbalancer '%s/%s' for service '%s/%s'", region, loadBalancerID, service.Namespace, service.Name) + return nil, err } - - klog.Errorf("an error occurred while fetching loadbalancer '%s/%s' for service '%s/%s'", region, loadBalancerID, service.Namespace, service.Name) - return nil, err } return resp, nil @@ -401,13 +394,12 @@ func (l *loadbalancers) getLoadbalancerByName(ctx context.Context, service *v1.S Region: region, }, scw.WithAllPages()) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - if respErr.StatusCode == 404 || respErr.StatusCode == 400 { - continue - } + switch err.(type) { + case *scw.ResourceNotFoundError: + continue + default: + return nil, err } - return nil, err } for _, lb := range resp.LBs { @@ -482,12 +474,7 @@ func (l *loadbalancers) createLoadBalancer(ctx context.Context, clusterName stri lb, err := l.api.CreateLB(&request) if err != nil { klog.Errorf("error creating load balancer for service %s: %v", service.Name, err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return nil, fmt.Errorf("error on call CreateLb with name %s: error %d with message %s", lbName, respErr.StatusCode, respErr.Message) - } - - return nil, fmt.Errorf("error on call CreateLb with name %s: %s", lbName, err.Error()) + return nil, fmt.Errorf("error creating load balancer for service %s: %v", service.Name, err) } // annotate newly created loadBalancer @@ -528,12 +515,7 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }, scw.WithAllPages()) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call ListFrontends with LbID %s: error %d with message %s", loadbalancer.ID, respErr.StatusCode, respErr.Message) - } - - return err + return fmt.Errorf("error updating load balancer %s: %v", loadbalancer.ID, err) } frontends := respFrontends.Frontends @@ -558,12 +540,7 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call DeleteFrontend with FrontendID %s: error %d with message %s", frontend.ID, respErr.StatusCode, respErr.Message) - } - - return err + return fmt.Errorf("error deleting frontend %s: %v", frontend.ID, err) } } else { portFrontends[frontend.InboundPort] = frontend @@ -576,12 +553,7 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }, scw.WithAllPages()) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call ListBackends with LoadBalancerID %s: error %d with message %s", loadbalancer.ID, respErr.StatusCode, respErr.Message) - } - - return err + return fmt.Errorf("error listing backend for load balancer %s: %v", loadbalancer.ID, err) } backends := respBackends.Backends @@ -604,12 +576,7 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }) if err != nil { - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call DeleteBackend with BackendID %s: error %d with message %s", backend.ID, respErr.StatusCode, respErr.Message) - } - - return err + return fmt.Errorf("error deleing backend %s: %v", backend.ID, err) } } else { portBackends[backend.ForwardPort] = backend @@ -629,13 +596,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc updateBackendRequest.ForwardPort = port.NodePort _, err = l.api.UpdateBackend(updateBackendRequest) if err != nil { - klog.Errorf("error updating backend: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call UpdateBackend with BackendID %s: error %d with message %s", backend.ID, respErr.StatusCode, respErr.Message) - } - - return err + klog.Errorf("error updating backend %s: %v", backend.ID, err) + return fmt.Errorf("error updating backend %s: %v", backend.ID, err) } updateHealthCheckRequest, err := l.makeUpdateHealthCheckRequest(backend, port.NodePort, service, nodes) @@ -646,13 +608,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc _, err = l.api.UpdateHealthCheck(updateHealthCheckRequest) if err != nil { - klog.Errorf("error updating healthcheck: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call UpdateHealthCheck with BackendID %s: error %d with message %s", backend.ID, respErr.StatusCode, respErr.Message) - } - - return err + klog.Errorf("error updating healthcheck for backend %s: %v", backend.ID, err) + return fmt.Errorf("error updating healthcheck for backend %s: %v", backend.ID, err) } var serverIPs []string @@ -669,12 +626,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc respBackend, err := l.api.SetBackendServers(setBackendServersRequest) if err != nil { - klog.Errorf("error setting backend servers: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call SetBackendServers with BackendID %s: error %d with message %s", backend.ID, respErr.StatusCode, respErr.Message) - } - return err + klog.Errorf("error setting backend servers for backend %s: %v", backend.ID, err) + return fmt.Errorf("error setting backend servers for backend %s: %v", backend.ID, err) } portBackends[backend.ForwardPort] = respBackend @@ -687,12 +640,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc respBackend, err := l.api.CreateBackend(request) if err != nil { - klog.Errorf("error creating backend: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call CreateBackend with LoadBalancerID %s: error %d with message %s", loadbalancer.ID, respErr.StatusCode, respErr.Message) - } - return err + klog.Errorf("error creating backend on load balancer %s: %v", loadbalancer.ID, err) + return fmt.Errorf("error creating backend on load balancer %s: %v", loadbalancer.ID, err) } portBackends[port.NodePort] = respBackend @@ -712,12 +661,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }) if err != nil { - klog.Errorf("error updating frontend: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call UpdateFrontend with FrontendID %s and Backend ID %s: error %d with message %s", frontend.ID, portBackends[port.NodePort].ID, respErr.StatusCode, respErr.Message) - } - return err + klog.Errorf("error updating frontend %s: %v", frontend.ID, err) + return fmt.Errorf("error updating frontend %s: %v", frontend.ID, err) } frontendID = frontend.ID @@ -732,12 +677,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc }) if err != nil { - klog.Errorf("error creating frontend: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("error on call CreateFronted with LbID %s and Backend ID %s: error %d with message %s", loadbalancer.ID, portBackends[port.NodePort].ID, respErr.StatusCode, respErr.Message) - } - return err + klog.Errorf("error creating frontend on load balancer %s: %v", loadbalancer.ID, err) + return fmt.Errorf("error creating frontend on load balancer %s: %v", loadbalancer.ID, err) } frontendID = resp.ID @@ -818,12 +759,8 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc Type: loadBalancerType, }) if err != nil { - klog.Errorf("error updating lb: %v", err) - var respErr *scw.ResponseError - if xerrors.As(err, &respErr) { - return fmt.Errorf("Unable to migrate loadbalancer %s error %d with message %s", loadbalancer.ID, respErr.StatusCode, respErr.Message) - } - return err + klog.Errorf("error updating load balancer %s: %v", loadbalancer.ID, err) + return fmt.Errorf("error updating load balancer %s: %v", loadbalancer.ID, err) } }