Print machineType, rather than just the machine family when encountering an incompatible disk attach

Author: pwschuurman

pkg/common/utils.go

@@ -277,14 +277,14 @@ func ConvertMiStringToInt64(str string) (int64, error) {
 	return volumehelpers.RoundUpToMiB(quantity)
 }
 
-// ParseMachineTypeFromUrl returns an extracted machineType from a URL, or empty if not found.
+// ParseMachineType 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) {
+func ParseMachineType(machineTypeUrl string) (string, error) {
 	machineType := machineTypeRegex.FindStringSubmatch(machineTypeUrl)
 	if machineType == nil {
-		return "", fmt.Errorf("failed to parse machineTypeUrl. expected suffix zones/{zone}/machineTypes/{machine-type}. Got: %s", machineTypeUrl)
+		return "", fmt.Errorf("failed to parse machineTypeUrl. Expected suffix: zones/{zone}/machineTypes/{machine-type}. Got: %s", machineTypeUrl)
 	}
-	return strings.Split(machineType[1], "-")[0], nil
+	return machineType[1], nil
 }

pkg/common/utils_test.go

@@ -800,27 +800,27 @@ func TestConvertMiStringToInt64(t *testing.T) {
 	}
 }
 
-func TestParseMachineFamily(t *testing.T) {
+func TestParseMachineType(t *testing.T) {
 	tests := []struct {
-		desc                  string
-		inputMachineTypeUrl   string
-		expectedMachineFamily string
-		expectError           bool
+		desc                string
+		inputMachineTypeUrl string
+		expectedMachineType 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:                "full URL machine type",
+			inputMachineTypeUrl: "https://www.googleapis.com/compute/v1/projects/my-project/zones/us-central1-c/machineTypes/c3-highcpu-4",
+			expectedMachineType: "c3-highcpu-4",
 		},
 		{
-			desc:                  "partial URL machine family",
-			inputMachineTypeUrl:   "zones/us-central1-c/machineTypes/n2-standard-4",
-			expectedMachineFamily: "n2",
+			desc:                "partial URL machine type",
+			inputMachineTypeUrl: "zones/us-central1-c/machineTypes/n2-standard-4",
+			expectedMachineType: "n2-standard-4",
 		},
 		{
-			desc:                  "custom partial URL machine family",
-			inputMachineTypeUrl:   "zones/us-central1-c/machineTypes/e2-custom-2-4096",
-			expectedMachineFamily: "e2",
+			desc:                "custom partial URL machine type",
+			inputMachineTypeUrl: "zones/us-central1-c/machineTypes/e2-custom-2-4096",
+			expectedMachineType: "e2-custom-2-4096",
 		},
 		{
 			desc:                "incorrect URL",
@@ -840,15 +840,15 @@ func TestParseMachineFamily(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
-			actualMachineFamily, err := ParseMachineFamily(tc.inputMachineTypeUrl)
+			actualMachineFamily, err := ParseMachineType(tc.inputMachineTypeUrl)
 			if err != nil && !tc.expectError {
-				t.Errorf("Got error %v parsing machine family %s; expect no error", err, tc.inputMachineTypeUrl)
+				t.Errorf("Got error %v parsing machine type %s; expect no error", err, tc.inputMachineTypeUrl)
 			}
 			if err == nil && tc.expectError {
-				t.Errorf("Got no error parsing machine family %s; expect an error", tc.inputMachineTypeUrl)
+				t.Errorf("Got no error parsing machine type %s; expect an error", tc.inputMachineTypeUrl)
 			}
-			if err == nil && actualMachineFamily != tc.expectedMachineFamily {
-				t.Errorf("Got %s parsing machine family; expect %s", actualMachineFamily, tc.expectedMachineFamily)
+			if err == nil && actualMachineFamily != tc.expectedMachineType {
+				t.Errorf("Got %s parsing machine type; expect %s", actualMachineFamily, tc.expectedMachineType)
 			}
 		})
 	}

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

@@ -507,13 +507,13 @@ func (gceCS *GCEControllerServer) validateControllerPublishVolumeRequest(ctx con
 	return project, volKey, nil
 }
 
-func parseMachineFamily(machineTypeUrl string) string {
-	machineFamily, parseErr := common.ParseMachineFamily(machineTypeUrl)
+func parseMachineType(machineTypeUrl string) string {
+	machineType, parseErr := common.ParseMachineType(machineTypeUrl)
 	if parseErr != nil {
 		// Parse errors represent an unexpected API change with instance.MachineType; log a warning.
-		klog.Warningf("ParseMachineFamily(%v): %v", machineTypeUrl, parseErr)
+		klog.Warningf("ParseMachineType(%v): %v", machineTypeUrl, parseErr)
 	}
-	return machineFamily
+	return machineType
 }
 
 func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
@@ -596,8 +596,8 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
 		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.InvalidArgument, "'%s' is not a compatible disk type with the machine family '%s', please review the GCP online documentation for available persistent disk options", udErr.DiskType, machineFamily)
+			machineType := parseMachineType(instance.MachineType)
+			return nil, status.Errorf(codes.InvalidArgument, "'%s' is not a compatible disk type with the machine type %s, please review the GCP online documentation for available persistent disk options", udErr.DiskType, machineType)
 		}
 		return nil, status.Errorf(codes.Internal, "unknown Attach error: %v", err.Error())
 	}