Merge pull request #1257 from k8s-infra-cherrypick-robot/cherry-pick-1254-to-release-1.10

[release-1.10] Add force attach to storage class

Author: k8s-ci-robot

pkg/common/parameters.go

@@ -29,6 +29,7 @@ const (
 	ParameterKeyLabels                        = "labels"
 	ParameterKeyProvisionedIOPSOnCreate       = "provisioned-iops-on-create"
 	ParameterKeyProvisionedThroughputOnCreate = "provisioned-throughput-on-create"
+	ParameterAvailabilityClass                = "availability-class"
 
 	// Parameters for VolumeSnapshotClass
 	ParameterKeyStorageLocations = "storage-locations"
@@ -38,6 +39,10 @@ const (
 	DiskImageType                = "images"
 	replicationTypeNone          = "none"
 
+	// Parameters for AvailabilityClass
+	ParameterNoAvailabilityClass       = "none"
+	ParameterRegionalHardFailoverClass = "regional-hard-failover"
+
 	// Keys for PV and PVC parameters as reported by external-provisioner
 	ParameterKeyPVCName      = "csi.storage.k8s.io/pvc/name"
 	ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
@@ -83,6 +88,8 @@ type DiskParameters struct {
 	// Values: {int64}
 	// Default: none
 	ProvisionedThroughputOnCreate int64
+	// Default: false
+	ForceAttach bool
 }
 
 // SnapshotParameters contains normalized and defaulted parameters for snapshots
@@ -155,6 +162,14 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 				return p, fmt.Errorf("parameters contain invalid provisionedThroughputOnCreate parameter: %w", err)
 			}
 			p.ProvisionedThroughputOnCreate = paramProvisionedThroughputOnCreate
+		case ParameterAvailabilityClass:
+			paramAvailabilityClass, err := ConvertStringToAvailabilityClass(v)
+			if err != nil {
+				return p, fmt.Errorf("parameters contain invalid availability class parameter: %w", err)
+			}
+			if paramAvailabilityClass == ParameterRegionalHardFailoverClass {
+				p.ForceAttach = true
+			}
 		default:
 			return p, fmt.Errorf("parameters contains invalid option %q", k)
 		}

pkg/common/parameters_test.go

@@ -178,6 +178,27 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				Labels:               map[string]string{"key1": "value1", "label-1": "value-a", "label-2": "label-value-2"},
 			},
 		},
+		{
+			name:       "availability class parameters",
+			parameters: map[string]string{ParameterAvailabilityClass: ParameterRegionalHardFailoverClass},
+			expectParams: DiskParameters{
+				DiskType:        "pd-standard",
+				ReplicationType: "none",
+				ForceAttach:     true,
+				Tags:            map[string]string{},
+				Labels:          map[string]string{},
+			},
+		},
+		{
+			name:       "no force attach parameters",
+			parameters: map[string]string{ParameterAvailabilityClass: ParameterNoAvailabilityClass},
+			expectParams: DiskParameters{
+				DiskType:        "pd-standard",
+				ReplicationType: "none",
+				Tags:            map[string]string{},
+				Labels:          map[string]string{},
+			},
+		},
 	}
 
 	for _, tc := range tests {

pkg/common/utils.go

@@ -284,6 +284,28 @@ func ConvertMiStringToInt64(str string) (int64, error) {
 	return volumehelpers.RoundUpToMiB(quantity)
 }
 
+// ConvertStringToBool converts a string to a boolean.
+func ConvertStringToBool(str string) (bool, error) {
+	switch strings.ToLower(str) {
+	case "true":
+		return true, nil
+	case "false":
+		return false, nil
+	}
+	return false, fmt.Errorf("Unexpected boolean string %s", str)
+}
+
+// ConvertStringToAvailabilityClass converts a string to an availability class string.
+func ConvertStringToAvailabilityClass(str string) (string, error) {
+	switch strings.ToLower(str) {
+	case ParameterNoAvailabilityClass:
+		return ParameterNoAvailabilityClass, nil
+	case ParameterRegionalHardFailoverClass:
+		return ParameterRegionalHardFailoverClass, nil
+	}
+	return "", fmt.Errorf("Unexpected boolean string %s", str)
+}
+
 // 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:
 //

pkg/common/utils_test.go

@@ -806,6 +806,114 @@ func TestConvertMiStringToInt64(t *testing.T) {
 	}
 }
 
+func TestConvertStringToBool(t *testing.T) {
+	tests := []struct {
+		desc        string
+		inputStr    string
+		expected    bool
+		expectError bool
+	}{
+		{
+			desc:        "valid true",
+			inputStr:    "true",
+			expected:    true,
+			expectError: false,
+		},
+		{
+			desc:        "valid mixed case true",
+			inputStr:    "True",
+			expected:    true,
+			expectError: false,
+		},
+		{
+			desc:        "valid false",
+			inputStr:    "false",
+			expected:    false,
+			expectError: false,
+		},
+		{
+			desc:        "valid mixed case false",
+			inputStr:    "False",
+			expected:    false,
+			expectError: false,
+		},
+		{
+			desc:        "invalid",
+			inputStr:    "yes",
+			expected:    false,
+			expectError: true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			got, err := ConvertStringToBool(tc.inputStr)
+			if err != nil && !tc.expectError {
+				t.Errorf("Got error %v converting string to bool %s; expect no error", err, tc.inputStr)
+			}
+			if err == nil && tc.expectError {
+				t.Errorf("Got no error converting string to bool %s; expect an error", tc.inputStr)
+			}
+			if err == nil && got != tc.expected {
+				t.Errorf("Got %v for converting string to bool; expect %v", got, tc.expected)
+			}
+		})
+	}
+}
+
+func TestConvertStringToAvailabilityClass(t *testing.T) {
+	tests := []struct {
+		desc        string
+		inputStr    string
+		expected    string
+		expectError bool
+	}{
+		{
+			desc:        "valid none",
+			inputStr:    "none",
+			expected:    ParameterNoAvailabilityClass,
+			expectError: false,
+		},
+		{
+			desc:        "valid mixed case none",
+			inputStr:    "None",
+			expected:    ParameterNoAvailabilityClass,
+			expectError: false,
+		},
+		{
+			desc:        "valid failover",
+			inputStr:    "regional-hard-failover",
+			expected:    ParameterRegionalHardFailoverClass,
+			expectError: false,
+		},
+		{
+			desc:        "valid mixed case failover",
+			inputStr:    "Regional-Hard-Failover",
+			expected:    ParameterRegionalHardFailoverClass,
+			expectError: false,
+		},
+		{
+			desc:        "invalid",
+			inputStr:    "yes",
+			expected:    "",
+			expectError: true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			got, err := ConvertStringToAvailabilityClass(tc.inputStr)
+			if err != nil && !tc.expectError {
+				t.Errorf("Got error %v converting string to availablity class %s; expect no error", err, tc.inputStr)
+			}
+			if err == nil && tc.expectError {
+				t.Errorf("Got no error converting string to availablity class %s; expect an error", tc.inputStr)
+			}
+			if err == nil && got != tc.expected {
+				t.Errorf("Got %v for converting string to availablity class; expect %v", got, tc.expected)
+			}
+		})
+	}
+}
+
 func TestParseMachineType(t *testing.T) {
 	tests := []struct {
 		desc                string

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

@@ -246,15 +246,16 @@ func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string,
 	return nil
 }
 
-func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
+func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
 	source := cloud.GetDiskSourceURI(project, volKey)
 
 	attachedDiskV1 := &computev1.AttachedDisk{
-		DeviceName: volKey.Name,
-		Kind:       diskKind,
-		Mode:       readWrite,
-		Source:     source,
-		Type:       diskType,
+		DeviceName:  volKey.Name,
+		Kind:        diskKind,
+		Mode:        readWrite,
+		Source:      source,
+		Type:        diskType,
+		ForceAttach: forceAttach,
 	}
 	instance, ok := cloud.instances[instanceName]
 	if !ok {
@@ -538,14 +539,14 @@ func (cloud *FakeBlockingCloudProvider) DetachDisk(ctx context.Context, project,
 	return cloud.FakeCloudProvider.DetachDisk(ctx, project, deviceName, instanceZone, instanceName)
 }
 
-func (cloud *FakeBlockingCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
+func (cloud *FakeBlockingCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
 	execute := make(chan Signal)
 	cloud.ReadyToExecute <- execute
 	val := <-execute
 	if val.ReportError {
 		return fmt.Errorf("force mock error for AttachDisk: volkey %s", volKey)
 	}
-	return cloud.FakeCloudProvider.AttachDisk(ctx, project, volKey, readWrite, diskType, instanceZone, instanceName)
+	return cloud.FakeCloudProvider.AttachDisk(ctx, project, volKey, readWrite, diskType, instanceZone, instanceName, forceAttach)
 }
 
 func notFoundError() *googleapi.Error {

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

@@ -92,7 +92,7 @@ type GCECompute interface {
 	ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64, multiWriter bool) error
 	InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool) error
 	DeleteDisk(ctx context.Context, project string, volumeKey *meta.Key) error
-	AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error
+	AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error
 	DetachDisk(ctx context.Context, project, deviceName, instanceZone, instanceName string) error
 	GetDiskSourceURI(project string, volKey *meta.Key) string
 	GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string
@@ -700,7 +700,7 @@ func (cloud *CloudProvider) deleteRegionalDisk(ctx context.Context, project, reg
 	return nil
 }
 
-func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
+func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string, forceAttach bool) error {
 	klog.V(5).Infof("Attaching disk %v to %s", volKey, instanceName)
 	source := cloud.GetDiskSourceURI(project, volKey)
 
@@ -714,9 +714,13 @@ func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volK
 		Mode:       readWrite,
 		Source:     source,
 		Type:       diskType,
+		// This parameter is ignored in the call, the ForceAttach decorator
+		// (query parameter) is the important one. We'll set it in both places
+		// in case that behavior changes.
+		ForceAttach: forceAttach,
 	}
 
-	op, err := cloud.service.Instances.AttachDisk(project, instanceZone, instanceName, attachedDiskV1).Context(ctx).Do()
+	op, err := cloud.service.Instances.AttachDisk(project, instanceZone, instanceName, attachedDiskV1).Context(ctx).ForceAttach(forceAttach).Do()
 	if err != nil {
 		return fmt.Errorf("failed cloud service attach disk call: %w", err)
 	}