Skip to content

Use errors.As so we can detect wrapped errors, and check for existing error codes in CodesForError #1232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/gce-cloud-provider/compute/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package gcecloudprovider

import (
"context"
"errors"
"fmt"
"net/http"
"os"
Expand Down Expand Up @@ -248,8 +249,8 @@ func getProjectAndZone(config *ConfigFile) (string, string, error) {
// isGCEError returns true if given error is a googleapi.Error with given
// reason (e.g. "resourceInUseByAnotherResource")
func IsGCEError(err error, reason string) bool {
apiErr, ok := err.(*googleapi.Error)
if !ok {
var apiErr *googleapi.Error
if !errors.As(err, &apiErr) {
return false
}

Expand Down
86 changes: 86 additions & 0 deletions pkg/gce-cloud-provider/compute/gce_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
Copyright 2023 The Kubernetes Authors.


Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package gcecloudprovider

import (
"errors"
"fmt"
"net/http"
"testing"

"google.golang.org/api/googleapi"
)

func TestIsGCEError(t *testing.T) {
testCases := []struct {
name string
inputErr error
reason string
expIsGCEError bool
}{
{
name: "Not googleapi.Error",
inputErr: errors.New("I am not a googleapi.Error"),
reason: "notFound",
expIsGCEError: false,
},
{
name: "googleapi.Error not found error",
inputErr: &googleapi.Error{
Code: http.StatusNotFound,
Errors: []googleapi.ErrorItem{
{
Reason: "notFound",
},
},
Message: "Not found",
},
reason: "notFound",
expIsGCEError: true,
},
{
name: "wrapped googleapi.Error",
inputErr: fmt.Errorf("encountered not found: %w", &googleapi.Error{
Code: http.StatusNotFound,
Errors: []googleapi.ErrorItem{
{
Reason: "notFound",
},
},
Message: "Not found",
},
),
reason: "notFound",
expIsGCEError: true,
},
{
name: "nil error",
inputErr: nil,
reason: "notFound",
expIsGCEError: false,
},
}

for _, tc := range testCases {
t.Logf("Running test: %v", tc.name)
isGCEError := IsGCEError(tc.inputErr, tc.reason)
if tc.expIsGCEError != isGCEError {
t.Fatalf("Got isGCEError '%t', expected '%t'", isGCEError, tc.expIsGCEError)
}
}
}
17 changes: 17 additions & 0 deletions pkg/gce-pd-csi-driver/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ func containsZone(zones []string, zone string) bool {
// (1) "context deadline exceeded", returns grpc DeadlineExceeded,
// (2) "context canceled", returns grpc Canceled
func CodeForError(err error) *codes.Code {
if err == nil {
return nil
}

if errCode := existingErrorCode(err); errCode != nil {
return errCode
}
if code := isContextError(err); code != nil {
return code
}
Expand All @@ -255,6 +262,16 @@ func CodeForError(err error) *codes.Code {
return &internalErrorCode
}

func existingErrorCode(err error) *codes.Code {
if err == nil {
return nil
}
if status, ok := status.FromError(err); ok {
return errCodePtr(status.Code())
}
return nil
}

// isContextError returns a pointer to the grpc error code DeadlineExceeded
// if the passed in error contains the "context deadline exceeded" string and returns
// the grpc error code Canceled if the error contains the "context canceled" string.
Expand Down
20 changes: 17 additions & 3 deletions pkg/gce-pd-csi-driver/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
csi "github.com/container-storage-interface/spec/lib/go/csi"
"google.golang.org/api/googleapi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

var (
Expand Down Expand Up @@ -331,13 +332,26 @@ func TestCodeForError(t *testing.T) {
inputErr: context.DeadlineExceeded,
expCode: errCodePtr(codes.DeadlineExceeded),
},
{
name: "status error with Aborted error code",
inputErr: status.Error(codes.Aborted, "aborted error"),
expCode: errCodePtr(codes.Aborted),
},
{
name: "nil error",
inputErr: nil,
expCode: nil,
},
}

for _, tc := range testCases {
t.Logf("Running test: %v", tc.name)
actualCode := *CodeForError(tc.inputErr)
if *tc.expCode != actualCode {
t.Fatalf("Expected error code '%v' but got '%v'", tc.expCode, actualCode)
errCode := CodeForError(tc.inputErr)
if (tc.expCode == nil) != (errCode == nil) {
t.Errorf("test %v failed: got %v, expected %v", tc.name, errCode, tc.expCode)
}
if tc.expCode != nil && *errCode != *tc.expCode {
t.Errorf("test %v failed: got %v, expected %v", tc.name, errCode, tc.expCode)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/remote/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ func generateMetadataWithPublicKey(pubKeyFile string) (*compute.Metadata, error)
// isGCEError returns true if given error is a googleapi.Error with given
// reason (e.g. "resourceInUseByAnotherResource")
func isGCEError(err error, reason string) bool {
apiErr, ok := err.(*googleapi.Error)
if !ok {
var apiErr *googleapi.Error
if !errors.As(err, &apiErr) {
return false
}

Expand Down