From 7d7338968d9fa1d01afd220c700dd5b6d917a49e Mon Sep 17 00:00:00 2001
From: Matthew Cary <mattcary@google.com>
Date: Tue, 2 Jan 2024 12:50:07 -0800
Subject: [PATCH] Filter multiattach errors

---
 pkg/common/utils.go      | 14 ++++++++++++++
 pkg/common/utils_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/pkg/common/utils.go b/pkg/common/utils.go
index 368a853a4..73eb7e69b 100644
--- a/pkg/common/utils.go
+++ b/pkg/common/utils.go
@@ -335,6 +335,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
 	}
@@ -372,6 +375,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")
diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go
index 3f5993a0f..f60aace96 100644
--- a/pkg/common/utils_test.go
+++ b/pkg/common/utils_test.go
@@ -1015,6 +1015,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 {
@@ -1076,6 +1081,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