Merge pull request #2181 from k8s-infra-cherrypick-robot/cherry-pick-2118-to-release-1.21

[release-1.21] Added filtering for incorrect slo boosting logs

Author: k8s-ci-robot

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
+}

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

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)
 		}
 	}