From cd42fdd7b80ae157e84e59cf1e27f7ba9485675b Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 7 Nov 2018 16:02:48 -0800 Subject: [PATCH] Use strongerrors in provider implementations. (#403) This ensures that we can catch certain types of errors from providers and handle accordingly in the core. There is still more to do here to improve that but this resolves an immediate need to know why a Delete failed. vic provider was not updated since I could not figure out where to get this information. --- providers/alicloud/eci.go | 9 +++--- providers/alicloud/errors.go | 26 +++++++++++++++++ providers/aws/fargate/cluster.go | 3 +- providers/azure/aci.go | 9 +++--- providers/azure/client/api/api.go | 4 ++- providers/azure/errors.go | 26 +++++++++++++++++ providers/azurebatch/batch.go | 2 +- providers/azurebatch/errors.go | 48 +++++++++++++++++++++++++++++++ providers/cri/cri.go | 11 +++---- providers/huawei/cci.go | 25 ++++++++++++++-- providers/hypersh/hypersh.go | 4 +++ providers/mock/mock.go | 5 ++++ providers/sfmesh/errors.go | 48 +++++++++++++++++++++++++++++++ providers/sfmesh/sfmesh.go | 2 +- providers/web/broker.go | 8 +++++- 15 files changed, 210 insertions(+), 20 deletions(-) create mode 100644 providers/alicloud/errors.go create mode 100644 providers/azure/errors.go create mode 100644 providers/azurebatch/errors.go create mode 100644 providers/sfmesh/errors.go diff --git a/providers/alicloud/eci.go b/providers/alicloud/eci.go index a6a83f4da..e090f1a89 100644 --- a/providers/alicloud/eci.go +++ b/providers/alicloud/eci.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/virtual-kubelet/virtual-kubelet/log" "io" "os" "strconv" @@ -17,6 +16,8 @@ import ( "time" "github.com/aliyun/alibaba-cloud-sdk-go/sdk/requests" + "github.com/cpuguy83/strongerrors" + "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/manager" "github.com/virtual-kubelet/virtual-kubelet/providers/alicloud/eci" "k8s.io/api/core/v1" @@ -246,13 +247,13 @@ func (p *ECIProvider) DeletePod(ctx context.Context, pod *v1.Pod) error { } } if eciId == "" { - return fmt.Errorf("DeletePod cann't find Pod %s-%s", pod.Namespace, pod.Name) + return strongerrors.NotFound(fmt.Errorf("DeletePod can't find Pod %s-%s", pod.Namespace, pod.Name)) } request := eci.CreateDeleteContainerGroupRequest() request.ContainerGroupId = eciId _, err := p.eciClient.DeleteContainerGroup(request) - return err + return wrapError(err) } // GetPod returns a pod by name that is running inside ECI @@ -280,7 +281,7 @@ func (p *ECIProvider) GetContainerLogs(ctx context.Context, namespace, podName, } } if eciId == "" { - return "", errors.New(fmt.Sprintf("GetContainerLogs cann't find Pod %s-%s", namespace, podName)) + return "", errors.New(fmt.Sprintf("GetContainerLogs can't find Pod %s-%s", namespace, podName)) } request := eci.CreateDescribeContainerLogRequest() diff --git a/providers/alicloud/errors.go b/providers/alicloud/errors.go new file mode 100644 index 000000000..f3d9ea23c --- /dev/null +++ b/providers/alicloud/errors.go @@ -0,0 +1,26 @@ +package alicloud + +import ( + "net/http" + + "github.com/aliyun/alibaba-cloud-sdk-go/sdk/errors" + "github.com/cpuguy83/strongerrors" +) + +func wrapError(err error) error { + if err == nil { + return nil + } + + se, ok := err.(*errors.ServerError) + if !ok { + return err + } + + switch se.HttpStatus() { + case http.StatusNotFound: + return strongerrors.NotFound(err) + default: + return err + } +} diff --git a/providers/aws/fargate/cluster.go b/providers/aws/fargate/cluster.go index 49e013997..89e778f3b 100644 --- a/providers/aws/fargate/cluster.go +++ b/providers/aws/fargate/cluster.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatchlogs" "github.com/aws/aws-sdk-go/service/ecs" + "github.com/cpuguy83/strongerrors" k8sTypes "k8s.io/apimachinery/pkg/types" ) @@ -272,7 +273,7 @@ func (c *Cluster) GetPod(namespace string, name string) (*Pod, error) { tag := buildTaskDefinitionTag(c.name, namespace, name) pod, ok := c.pods[tag] if !ok { - return nil, fmt.Errorf("pod %s/%s is not found", namespace, name) + return nil, strongerrors.NotFound(fmt.Errorf("pod %s/%s is not found", namespace, name)) } return pod, nil diff --git a/providers/azure/aci.go b/providers/azure/aci.go index f3da86d28..05d308c2d 100644 --- a/providers/azure/aci.go +++ b/providers/azure/aci.go @@ -342,8 +342,8 @@ func (p *ACIProvider) setupNetworkProfile(auth *client.Authentication) error { return fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance since it references the network security group '%s'.", p.subnetName, *subnet.SubnetPropertiesFormat.NetworkSecurityGroup.ID) } if subnet.SubnetPropertiesFormat.RouteTable != nil { - return fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance since it references the route table '%s'.", p.subnetName, *subnet.SubnetPropertiesFormat.RouteTable.ID) - } + return fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance since it references the route table '%s'.", p.subnetName, *subnet.SubnetPropertiesFormat.RouteTable.ID) + } if subnet.SubnetPropertiesFormat.ServiceAssociationLinks != nil { for _, l := range *subnet.SubnetPropertiesFormat.ServiceAssociationLinks { if l.ServiceAssociationLinkPropertiesFormat != nil && *l.ServiceAssociationLinkPropertiesFormat.LinkedResourceType == subnetDelegationService { @@ -427,7 +427,7 @@ func getKubeProxyExtension(secretPath, masterURI, clusterCIDR string) (*aci.Exte clientcmdv1.NamedCluster{ Name: name, Cluster: clientcmdv1.Cluster{ - Server: masterURI, + Server: masterURI, CertificateAuthorityData: ca, }, }, @@ -691,7 +691,8 @@ func (p *ACIProvider) DeletePod(ctx context.Context, pod *v1.Pod) error { defer span.End() addAzureAttributes(span, p) - return p.aciClient.DeleteContainerGroup(ctx, p.resourceGroup, fmt.Sprintf("%s-%s", pod.Namespace, pod.Name)) + err := p.aciClient.DeleteContainerGroup(ctx, p.resourceGroup, fmt.Sprintf("%s-%s", pod.Namespace, pod.Name)) + return wrapError(err) } // GetPod returns a pod by name that is running inside ACI diff --git a/providers/azure/client/api/api.go b/providers/azure/client/api/api.go index 2c4ccf3b5..fe4654865 100644 --- a/providers/azure/client/api/api.go +++ b/providers/azure/client/api/api.go @@ -46,6 +46,7 @@ func CheckResponse(res *http.Response) error { if res.StatusCode >= 200 && res.StatusCode <= 299 { return nil } + slurp, err := ioutil.ReadAll(res.Body) if err == nil { jerr := new(errorReply) @@ -59,9 +60,10 @@ func CheckResponse(res *http.Response) error { return jerr.Error } } + return &Error{ StatusCode: res.StatusCode, - Body: string(slurp), + Body: res.Status, Header: res.Header, } } diff --git a/providers/azure/errors.go b/providers/azure/errors.go new file mode 100644 index 000000000..33118f266 --- /dev/null +++ b/providers/azure/errors.go @@ -0,0 +1,26 @@ +package azure + +import ( + "net/http" + + "github.com/cpuguy83/strongerrors" + "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client/api" +) + +func wrapError(err error) error { + if err == nil { + return nil + } + + e, ok := err.(*api.Error) + if !ok { + return err + } + + switch e.StatusCode { + case http.StatusNotFound: + return strongerrors.NotFound(err) + default: + return err + } +} diff --git a/providers/azurebatch/batch.go b/providers/azurebatch/batch.go index cbf9f2f53..5577fa389 100644 --- a/providers/azurebatch/batch.go +++ b/providers/azurebatch/batch.go @@ -227,7 +227,7 @@ func (p *Provider) DeletePod(ctx context.Context, pod *v1.Pod) error { if err != nil { log.Println(task) log.Println(err) - return err + return wrapError(err) } log.Printf(fmt.Sprintf("Deleting task: %v", taskID)) diff --git a/providers/azurebatch/errors.go b/providers/azurebatch/errors.go new file mode 100644 index 000000000..811484cca --- /dev/null +++ b/providers/azurebatch/errors.go @@ -0,0 +1,48 @@ +package azurebatch + +import ( + "net/http" + + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/cpuguy83/strongerrors" +) + +func wrapError(err error) error { + if err == nil { + return nil + } + switch { + case isStatus(err, http.StatusNotFound): + return strongerrors.NotFound(err) + default: + return err + } +} + +type causal interface { + Cause() error +} + +func isStatus(err error, status int) bool { + if err == nil { + return false + } + + switch e := err.(type) { + case *azure.RequestError: + if e.StatusCode != 0 { + return e.StatusCode == status + } + return isStatus(e.Original, status) + case autorest.DetailedError: + if e.StatusCode != 0 { + return e.StatusCode == status + } + return isStatus(e.Original, status) + case causal: + return isStatus(e.Cause(), status) + } + + return false +} diff --git a/providers/cri/cri.go b/providers/cri/cri.go index d14422b24..d838a8163 100644 --- a/providers/cri/cri.go +++ b/providers/cri/cri.go @@ -15,6 +15,7 @@ import ( "time" log "github.com/Sirupsen/logrus" + "github.com/cpuguy83/strongerrors" "github.com/virtual-kubelet/virtual-kubelet/manager" "github.com/virtual-kubelet/virtual-kubelet/providers" "google.golang.org/grpc" @@ -566,7 +567,7 @@ func (p *CRIProvider) DeletePod(ctx context.Context, pod *v1.Pod) error { ps, ok := p.podStatus[pod.UID] if !ok { - return fmt.Errorf("Pod %s not found", pod.UID) + return strongerrors.NotFound(fmt.Errorf("Pod %s not found", pod.UID)) } // TODO: Check pod status for running state @@ -598,7 +599,7 @@ func (p *CRIProvider) GetPod(ctx context.Context, namespace, name string) (*v1.P pod := p.findPodByName(namespace, name) if pod == nil { - return nil, fmt.Errorf("Pod %s in namespace %s could not be found on the node", name, namespace) + return nil, strongerrors.NotFound(fmt.Errorf("Pod %s in namespace %s could not be found on the node", name, namespace)) } return createPodSpecFromCRI(pod, p.nodeName), nil @@ -635,11 +636,11 @@ func (p *CRIProvider) GetContainerLogs(ctx context.Context, namespace, podName, pod := p.findPodByName(namespace, podName) if pod == nil { - return "", fmt.Errorf("Pod %s in namespace %s not found", podName, namespace) + return "", strongerrors.NotFound(fmt.Errorf("Pod %s in namespace %s not found", podName, namespace)) } container := pod.containers[containerName] if container == nil { - return "", fmt.Errorf("Cannot find container %s in pod %s namespace %s", containerName, podName, namespace) + return "", strongerrors.NotFound(fmt.Errorf("Cannot find container %s in pod %s namespace %s", containerName, podName, namespace)) } return readLogFile(container.LogPath, tail) @@ -683,7 +684,7 @@ func (p *CRIProvider) GetPodStatus(ctx context.Context, namespace, name string) pod := p.findPodByName(namespace, name) if pod == nil { - return nil, fmt.Errorf("Pod %s in namespace %s could not be found on the node", name, namespace) + return nil, strongerrors.NotFound(fmt.Errorf("Pod %s in namespace %s could not be found on the node", name, namespace)) } return createPodStatusFromCRI(pod), nil diff --git a/providers/huawei/cci.go b/providers/huawei/cci.go index 3f6dacbbe..da6e4ac4d 100644 --- a/providers/huawei/cci.go +++ b/providers/huawei/cci.go @@ -14,6 +14,7 @@ import ( "os" "time" + "github.com/cpuguy83/strongerrors" "github.com/virtual-kubelet/virtual-kubelet/manager" "github.com/virtual-kubelet/virtual-kubelet/providers/huawei/auth" "k8s.io/api/core/v1" @@ -237,8 +238,28 @@ func (p *CCIProvider) DeletePod(ctx context.Context, pod *v1.Pod) error { if err = p.signRequest(r); err != nil { return fmt.Errorf("Sign the request failed: %v", err) } - _, err = p.client.HTTPClient.Do(r) - return err + resp, err := p.client.HTTPClient.Do(r) + if err != nil { + return err + } + + return errorFromResponse(resp) +} + +func errorFromResponse(resp *http.Response) error { + if resp.StatusCode < 400 { + return nil + } + + body, _ := ioutil.ReadAll(io.LimitReader(resp.Body, 16*1024)) + err := fmt.Errorf("error during http request, status=%d: %q", resp.StatusCode, string(body)) + + switch resp.StatusCode { + case http.StatusNotFound: + return strongerrors.NotFound(err) + default: + return err + } } // GetPod retrieves a pod by name from the huawei CCI provider. diff --git a/providers/hypersh/hypersh.go b/providers/hypersh/hypersh.go index b312e4c5b..ebf6140bf 100755 --- a/providers/hypersh/hypersh.go +++ b/providers/hypersh/hypersh.go @@ -10,6 +10,7 @@ import ( "runtime" "time" + "github.com/cpuguy83/strongerrors" "github.com/virtual-kubelet/virtual-kubelet/manager" "github.com/virtual-kubelet/virtual-kubelet/providers" @@ -268,6 +269,9 @@ func (p *HyperProvider) DeletePod(ctx context.Context, pod *v1.Pod) (err error) // Inspect hyper container container, err = p.hyperClient.ContainerInspect(context.Background(), containerName) if err != nil { + if hyper.IsErrContainerNotFound(err) { + return strongerrors.NotFound(err) + } return err } // Check container label diff --git a/providers/mock/mock.go b/providers/mock/mock.go index 58803119a..efb8582ee 100644 --- a/providers/mock/mock.go +++ b/providers/mock/mock.go @@ -9,6 +9,7 @@ import ( "log" "time" + "github.com/cpuguy83/strongerrors" "github.com/virtual-kubelet/virtual-kubelet/providers" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -133,6 +134,10 @@ func (p *MockProvider) DeletePod(ctx context.Context, pod *v1.Pod) (err error) { return err } + if _, exists := p.pods[key]; !exists { + return strongerrors.NotFound(fmt.Errorf("pod not found")) + } + delete(p.pods, key) return nil diff --git a/providers/sfmesh/errors.go b/providers/sfmesh/errors.go new file mode 100644 index 000000000..4f67fddbb --- /dev/null +++ b/providers/sfmesh/errors.go @@ -0,0 +1,48 @@ +package sfmesh + +import ( + "net/http" + + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/cpuguy83/strongerrors" +) + +func wrapError(err error) error { + if err == nil { + return nil + } + switch { + case isStatus(err, http.StatusNotFound): + return strongerrors.NotFound(err) + default: + return err + } +} + +type causal interface { + Cause() error +} + +func isStatus(err error, status int) bool { + if err == nil { + return false + } + + switch e := err.(type) { + case *azure.RequestError: + if e.StatusCode != 0 { + return e.StatusCode == status + } + return isStatus(e.Original, status) + case autorest.DetailedError: + if e.StatusCode != 0 { + return e.StatusCode == status + } + return isStatus(e.Original, status) + case causal: + return isStatus(e.Cause(), status) + } + + return false +} diff --git a/providers/sfmesh/sfmesh.go b/providers/sfmesh/sfmesh.go index 3e590a246..9c47939ed 100644 --- a/providers/sfmesh/sfmesh.go +++ b/providers/sfmesh/sfmesh.go @@ -462,7 +462,7 @@ func (p *SFMeshProvider) DeletePod(ctx context.Context, pod *v1.Pod) (err error) _, err = p.appClient.Delete(ctx, p.resourceGroup, pod.Name) if err != nil { - return err + return wrapError(err) } return nil diff --git a/providers/web/broker.go b/providers/web/broker.go index 2db56334b..6b9220f94 100644 --- a/providers/web/broker.go +++ b/providers/web/broker.go @@ -29,6 +29,7 @@ import ( "time" "github.com/cenkalti/backoff" + "github.com/cpuguy83/strongerrors" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/remotecommand" @@ -283,7 +284,12 @@ func (p *BrokerProvider) doRequest(method string, urlPath *url.URL, body []byte, defer response.Body.Close() if response.StatusCode < 200 || response.StatusCode > 299 { - return nil, errors.New(response.Status) + switch response.StatusCode { + case http.StatusNotFound: + return nil, strongerrors.NotFound(errors.New(response.Status)) + default: + return nil, errors.New(response.Status) + } } // read response body if asked to