Rewrite PD CSI error message when encountering diskType unsupported error

Author: pwschuurman

pkg/common/utils.go

@@ -59,11 +59,19 @@ const (
 	multiRegionalLocationFmt = "^[a-z]+$"
 	// Example: us-east1
 	regionalLocationFmt = "^[a-z]+-[a-z]+[0-9]$"
+
+	// Full or partial URL of the machine type resource, in the format:
+	//   zones/zone/machineTypes/machine-type
+	machineTypePattern = "zones/[^/]+/machineTypes/([^/]+)$"
 )
 
 var (
 	multiRegionalPattern = regexp.MustCompile(multiRegionalLocationFmt)
 	regionalPattern      = regexp.MustCompile(regionalLocationFmt)
+
+	// Full or partial URL of the machine type resource, in the format:
+	//   zones/zone/machineTypes/machine-type
+	machineTypeRegex = regexp.MustCompile(machineTypePattern)
 )
 
 func BytesToGbRoundDown(bytes int64) int64 {
@@ -268,3 +276,15 @@ func ConvertMiStringToInt64(str string) (int64, error) {
 	}
 	return volumehelpers.RoundUpToMiB(quantity)
 }
+
+// ParseMachineTypeFromUrl returns an extracted machineType from a URL, or empty if not found.
+// machineTypeUrl: Full or partial URL of the machine type resource, in the format:
+//
+//	zones/zone/machineTypes/machine-type
+func ParseMachineFamily(machineTypeUrl string) (string, error) {
+	machineType := machineTypeRegex.FindStringSubmatch(machineTypeUrl)
+	if machineType == nil {
+		return "", fmt.Errorf("failed to parse machineTypeUrl. expected suffix zones/{zone}/machineTypes/{name}. Got: %s", machineTypeUrl)
+	}
+	return strings.Split(machineType[1], "-")[0], nil
+}

pkg/common/utils_test.go

@@ -799,3 +799,57 @@ func TestConvertMiStringToInt64(t *testing.T) {
 		})
 	}
 }
+
+func TestParseMachineFamily(t *testing.T) {
+	tests := []struct {
+		desc                  string
+		inputMachineTypeUrl   string
+		expectedMachineFamily string
+		expectError           bool
+	}{
+		{
+			desc:                  "full URL machine family",
+			inputMachineTypeUrl:   "https://www.googleapis.com/compute/v1/projects/my-project/zones/us-central1-c/machineTypes/c3-highcpu-4",
+			expectedMachineFamily: "c3",
+		},
+		{
+			desc:                  "partial URL machine family",
+			inputMachineTypeUrl:   "zones/us-central1-c/machineTypes/n2-standard-4",
+			expectedMachineFamily: "n2",
+		},
+		{
+			desc:                  "custom partial URL machine family",
+			inputMachineTypeUrl:   "zones/us-central1-c/machineTypes/e2-custom-2-4096",
+			expectedMachineFamily: "e2",
+		},
+		{
+			desc:                "incorrect URL",
+			inputMachineTypeUrl: "https://www.googleapis.com/compute/v1/projects/psch-gke-dev/zones/us-central1-c",
+			expectError:         true,
+		},
+		{
+			desc:                "incorrect partial URL",
+			inputMachineTypeUrl: "zones/us-central1-c/machineTypes/",
+			expectError:         true,
+		},
+		{
+			desc:                "missing zone",
+			inputMachineTypeUrl: "zones//machineTypes/n2-standard-4",
+			expectError:         true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			actualMachineFamily, err := ParseMachineFamily(tc.inputMachineTypeUrl)
+			if err != nil && !tc.expectError {
+				t.Errorf("Got error %v parsing machine family %s; expect no error", err, tc.inputMachineTypeUrl)
+			}
+			if err == nil && tc.expectError {
+				t.Errorf("Got no error converting string to int64 %s; expect an error", tc.inputMachineTypeUrl)
+			}
+			if err == nil && actualMachineFamily != tc.expectedMachineFamily {
+				t.Errorf("Got %s for parsing machine family; expect %s", actualMachineFamily, tc.expectedMachineFamily)
+			}
+		})
+	}
+}

pkg/gce-cloud-provider/compute/gce-compute.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -38,9 +39,11 @@ const (
 	waitForImageCreationTimeOut    = 5 * time.Minute
 	diskKind                       = "compute#disk"
 	cryptoKeyVerDelimiter          = "/cryptoKeyVersions"
+	pdDiskTypeUnsupportedPattern   = `\[([a-z-]+)\] features are not compatible for creating instance`
 )
 
 var hyperdiskTypes = []string{"hyperdisk-extreme", "hyperdisk-throughput"}
+var pdDiskTypeUnsupportedRegex = regexp.MustCompile(pdDiskTypeUnsupportedPattern)
 
 type GCEAPIVersion string
 
@@ -69,6 +72,15 @@ var WaitForOpBackoff = wait.Backoff{
 	Steps:    100,
 	Cap:      0}
 
+// Custom error type to propagate error messages up to clients.
+type UnsupportedDiskError struct {
+	DiskType string
+}
+
+func (udErr *UnsupportedDiskError) Error() string {
+	return ""
+}
+
 type GCECompute interface {
 	// Metadata information
 	GetDefaultProject() string
@@ -838,12 +850,23 @@ func (cloud *CloudProvider) WaitForAttach(ctx context.Context, project string, v
 	})
 }
 
+func wrapOpErr(name string, opErr *computev1.OperationErrorErrors) error {
+	if opErr.Code == "UNSUPPORTED_OPERATION" {
+		if diskType := pdDiskTypeUnsupportedRegex.FindString(opErr.Message); diskType != "" {
+			return &UnsupportedDiskError{
+				DiskType: diskType,
+			}
+		}
+	}
+	return fmt.Errorf("operation %v failed (%v): %v", name, opErr.Code, opErr.Message)
+}
+
 func opIsDone(op *computev1.Operation) (bool, error) {
 	if op == nil || op.Status != operationStatusDone {
 		return false, nil
 	}
 	if op.Error != nil && len(op.Error.Errors) > 0 && op.Error.Errors[0] != nil {
-		return true, fmt.Errorf("operation %v failed (%v): %v", op.Name, op.Error.Errors[0].Code, op.Error.Errors[0].Message)
+		return true, wrapOpErr(op.Name, op.Error.Errors[0])
 	}
 	return true, nil
 }

pkg/gce-pd-csi-driver/controller.go

@@ -16,6 +16,7 @@ package gceGCEDriver
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/rand"
 	"regexp"
@@ -506,6 +507,15 @@ func (gceCS *GCEControllerServer) validateControllerPublishVolumeRequest(ctx con
 	return project, volKey, nil
 }
 
+func parseMachineFamily(machineTypeUrl string) string {
+	machineFamily, parseErr := common.ParseMachineFamily(machineTypeUrl)
+	if parseErr != nil {
+		// Parse errors represent an unexpected API change with instance.MachineType; log a warning.
+		klog.Warningf("ParseMachineFamily(%v): %v", machineTypeUrl, parseErr)
+	}
+	return machineFamily
+}
+
 func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
 	project, volKey, err := gceCS.validateControllerPublishVolumeRequest(ctx, req)
 
@@ -582,6 +592,13 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
 	}
 	err = gceCS.CloudProvider.AttachDisk(ctx, project, volKey, readWrite, attachableDiskTypePersistent, instanceZone, instanceName)
 	if err != nil {
+		var udErr *gce.UnsupportedDiskError
+		if errors.As(err, &udErr) {
+			// If we encountered an UnsupportedDiskError, rewrite the error message to be more user friendly.
+			// The error message from GCE is phrased around disk create on VM creation, not runtime attach.
+			machineFamily := parseMachineFamily(instance.MachineType)
+			return nil, status.Errorf(codes.Internal, "'%s' is not a compatible disk type with the machine family '%s', please review the GKE online documentation for available persistent disk options", udErr.DiskType, machineFamily)
+		}
 		return nil, status.Errorf(codes.Internal, "unknown Attach error: %v", err.Error())
 	}