diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index bde9b64f0..e9c4d0920 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -21,6 +21,7 @@ import ( "net/http" "net/url" "os" + "regexp" "runtime" "sync" "time" @@ -77,6 +78,12 @@ const ( gcpTagsRequestTokenBucketSize = 8 ) +var ( + ssAlreadyExistsRegex = regexp.MustCompile("The resource [^']+ already exists") + gcpErrorCodeRegex = regexp.MustCompile(`Error (\d+)`) + orgPolicyConstraintErrorCodes = []string{"400", "412"} +) + // ResourceType indicates the type of a compute resource. type ResourceType string @@ -441,3 +448,21 @@ func IsGCENotFoundError(err error) bool { func IsGCEInvalidError(err error) bool { return IsGCEError(err, "invalid") } + +// IsSnapshotAlreadyExistsError checks if the error is a snapshot already exists error. +func IsSnapshotAlreadyExistsError(err error) bool { + return ssAlreadyExistsRegex.MatchString(err.Error()) +} + +// IsGCPOrgViolationError checks if the error is a GCP organization policy violation error. +func IsGCPOrgViolationError(err error) bool { + matches := gcpErrorCodeRegex.FindStringSubmatch(err.Error()) + if len(matches) > 1 { + errorCode := matches[1] + if slices.Contains(orgPolicyConstraintErrorCodes, errorCode) { + return true + } + } + + return false +} diff --git a/pkg/gce-cloud-provider/compute/gce_test.go b/pkg/gce-cloud-provider/compute/gce_test.go index 94bb4fd9f..3cd25addb 100644 --- a/pkg/gce-cloud-provider/compute/gce_test.go +++ b/pkg/gce-cloud-provider/compute/gce_test.go @@ -101,6 +101,70 @@ func TestIsGCEError(t *testing.T) { } } +func TestErrorIsGCPViolationRegex(t *testing.T) { + testCases := []struct { + name string + inputErr error + expectedResult bool + }{ + { + name: "is gcp org violation error, error code 400", + inputErr: errors.New("Failed to scale up: googleapi: Error 400: 'us-central1' violates constraint '`constraints/gcp.resourceLocations`' on the resource 'projects/test-project/locations/us-central1/clusters/test-cluster/nodePools/test-node-pool'"), + expectedResult: true, + }, + { + name: "is gcp org violation error, error code 412", + inputErr: errors.New("createSnapshot for content [snapcontent-xyz]: error occurred in createSnapshotWrapper: failed to take snapshot of the volume projects/test-project/regions/europe-west3/disks/pvc-test: \"rpc error: code = Internal desc = Failed to create snapshot: googleapi: Error 412: Location EU violates constraint constraints/gcp.resourceLocations on the resource projects/test-project/global/snapshots/snapshot-xyz., conditionNotMet\""), + expectedResult: true, + }, + { + name: "is not gcp org violation, error doesn't match", + inputErr: errors.New("createSnapshot for content [snapcontent-xyz]: error occurred in createSnapshotWrapper: failed to take snapshot of the volume projects/test-project/regions/europe-west3/disks/pvc-test: \"rpc error: code = Internal desc = Failed to create snapshot: googleapi: Error 500: Location EU violates constraint constraints/gcp.resourceLocations on the resource projects/test-project/global/snapshots/snapshot-xyz., conditionNotMet\""), + expectedResult: false, + }, + { + name: "is not gcp org violation error", + inputErr: errors.New("Some incorrect error message"), + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Logf("Running test: %v", tc.name) + result := IsGCPOrgViolationError(tc.inputErr) + if tc.expectedResult != result { + t.Fatalf("Got '%t', expected '%t'", result, tc.expectedResult) + } + } +} + +func TestErrorIsSnapshotExistsError(t *testing.T) { + testCases := []struct { + name string + inputErr error + expectedResult bool + }{ + { + name: "is snapshot already exists error", + inputErr: errors.New("The resource projects/test-project/global/snapshots/snapshot-xyz already exists, alreadyExists"), + expectedResult: true, + }, + { + name: "is not snapshot already exists error", + inputErr: errors.New("Some incorrect error message"), + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Logf("Running test: %v", tc.name) + result := IsSnapshotAlreadyExistsError(tc.inputErr) + if tc.expectedResult != result { + t.Fatalf("Got '%t', expected '%t'", result, tc.expectedResult) + } + } +} + func TestGetComputeVersion(t *testing.T) { testCases := []struct { name string diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index f0523d435..f77d0982b 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -1668,6 +1668,16 @@ func (gceCS *GCEControllerServer) createPDSnapshot(ctx context.Context, project if gce.IsGCEError(err, "notFound") { return nil, status.Errorf(codes.NotFound, "Could not find volume with ID %v: %v", volKey.String(), err.Error()) } + + // Identified as incorrect error handling + if gce.IsSnapshotAlreadyExistsError(err) { + return nil, status.Errorf(codes.AlreadyExists, "Snapshot already exists: %v", err.Error()) + } + + // Identified as incorrect error handling + if gce.IsGCPOrgViolationError(err) { + return nil, status.Errorf(codes.FailedPrecondition, "Violates GCP org policy: %v", err.Error()) + } return nil, common.LoggedError("Failed to create snapshot: ", err) } }