Skip to content

Commit

Permalink
fix: avoid not found error mishandled (#23) (#25)
Browse files Browse the repository at this point in the history
Signed-off-by: Patrik Cyvoct <patrik@ptrk.io>
  • Loading branch information
Sh4d1 authored Sep 3, 2020
1 parent e751fad commit 334c624
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 128 deletions.
32 changes: 14 additions & 18 deletions scaleway/baremetal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions scaleway/baremetal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
23 changes: 10 additions & 13 deletions scaleway/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion scaleway/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
125 changes: 31 additions & 94 deletions scaleway/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@ package scaleway
import (
"context"
"fmt"
"net/http"
"os"
"strconv"
"strings"
"time"

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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down

0 comments on commit 334c624

Please sign in to comment.