Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver
Pull request #1559 (2024-01-02 20:51:43)

Filter multiattach errors
/kind bug

**What this PR does / why we need it**:
User misconfiguration causing multiattach errors clouds up our SLO.

```release-note
Filter user misconfigured multiattach errors.
```

/assign @msau42 

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/pull/1559#" title="Author self-approved">mattcary</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fgcp-compute-persistent-disk-csi-driver).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/master/OWNERS)~~ [mattcary]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @mattcary: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-gcp-compute-persistent-disk-csi-driver-verify | 3d3c8f032e78f156aeff8be7629e0057ef772d6d | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_gcp-compute-persistent-disk-csi-driver/1559/pull-gcp-compute-persistent-disk-csi-driver-verify/1742287588430974976) | true | `/test pull-gcp-compute-persistent-disk-csi-driver-verify`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=gcp-compute-persistent-disk-csi-driver&pr=1559). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Amattcary). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/common/utils.go b/pkg/common/utils.go
index 5f35abfa..b004f9d6 100644
--- a/pkg/common/utils.go
+++ b/pkg/common/utils.go
@@ -337,6 +337,9 @@ func CodeForError(sourceError error) codes.Code {
 		return codes.Internal
 	}
 
+	if code, err := isUserMultiAttachError(sourceError); err == nil {
+		return code
+	}
 	if code, err := existingErrorCode(sourceError); err == nil {
 		return code
 	}
@@ -374,6 +377,17 @@ func isContextError(err error) (codes.Code, error) {
 	return codes.Unknown, fmt.Errorf("Not a context error: %w", err)
 }
 
+// isUserMultiAttachError returns an InvalidArgument if the error is
+// multi-attach detected from the API server. If we get this error from the API
+// server, it means that the kubelet doesn't know about the multiattch so it is
+// due to user configuration.
+func isUserMultiAttachError(err error) (codes.Code, error) {
+	if strings.Contains(err.Error(), "The disk resource") && strings.Contains(err.Error(), "is already being used") {
+		return codes.InvalidArgument, nil
+	}
+	return codes.Unknown, fmt.Errorf("Not a user multiattach error: %w", err)
+}
+
 func existingErrorCode(err error) (codes.Code, error) {
 	if err == nil {
 		return codes.Unknown, fmt.Errorf("null error")


Test patch:
diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go
index 5d7d541d..de7d27ba 100644
--- a/pkg/common/utils_test.go
+++ b/pkg/common/utils_test.go
@@ -1016,6 +1016,11 @@ func TestCodeForError(t *testing.T) {
 			inputErr: nil,
 			expCode:  codes.Internal,
 		},
+		{
+			name:     "user multiattach error",
+			inputErr: fmt.Errorf("The disk resource 'projects/foo/disk/bar' is already being used by 'projects/foo/instances/1'"),
+			expCode:  codes.InvalidArgument,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1077,6 +1082,34 @@ func TestIsContextError(t *testing.T) {
 	}
 }
 
+func TestIsUserMultiAttachError(t *testing.T) {
+	cases := []struct {
+		errorString  string
+		expectedCode codes.Code
+		expectCode   bool
+	}{
+		{
+			errorString:  "The disk resource 'projects/foo/disk/bar' is already being used by 'projects/foo/instance/biz'",
+			expectedCode: codes.InvalidArgument,
+			expectCode:   true,
+		},
+		{
+			errorString: "The disk resource is ok!",
+			expectCode:  false,
+		},
+	}
+	for _, test := range cases {
+		code, err := isUserMultiAttachError(fmt.Errorf(test.errorString))
+		if test.expectCode {
+			if err != nil || code != test.expectedCode {
+				t.Errorf("Failed with non-nil error %v or bad code %v: %s", err, code, test.errorString)
+			}
+		} else if err == nil {
+			t.Errorf("Expected error for test but got none: %s", test.errorString)
+		}
+	}
+}
+
 func TestIsValidDiskEncryptionKmsKey(t *testing.T) {
 	cases := []struct {
 		diskEncryptionKmsKey string