Add force attach to storage class

Change-Id: Id07dbe608613d905dffa96aa119d7deadb1dca5d

Author: mattcary

pkg/common/parameters.go

@@ -29,6 +29,7 @@ const (
 	ParameterKeyLabels                        = "labels"
 	ParameterKeyProvisionedIOPSOnCreate       = "provisioned-iops-on-create"
 	ParameterKeyProvisionedThroughputOnCreate = "provisioned-throughput-on-create"
+	ParameterForceAttach                      = "alpha-unsafe-force-attach"
 
 	// Parameters for VolumeSnapshotClass
 	ParameterKeyStorageLocations = "storage-locations"
@@ -83,6 +84,8 @@ type DiskParameters struct {
 	// Values: {int64}
 	// Default: none
 	ProvisionedThroughputOnCreate int64
+	// Default: false
+	ForceAttach bool
 }
 
 // SnapshotParameters contains normalized and defaulted parameters for snapshots
@@ -155,6 +158,12 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 				return p, fmt.Errorf("parameters contain invalid provisionedThroughputOnCreate parameter: %w", err)
 			}
 			p.ProvisionedThroughputOnCreate = paramProvisionedThroughputOnCreate
+		case ParameterForceAttach:
+			paramForceAttach, err := ConvertStringToBool(v)
+			if err != nil {
+				return p, fmt.Errorf("parameters contain invalid parameterForceAttach parameter: %w", err)
+			}
+			p.ForceAttach = paramForceAttach
 		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:       "force attach parameters",
+			parameters: map[string]string{ParameterForceAttach: "true"},
+			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{ParameterForceAttach: "false"},
+			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,17 @@ func ConvertMiStringToInt64(str string) (int64, error) {
 	return volumehelpers.RoundUpToMiB(quantity)
 }
 
+// ConvertStringToBool converts a true/false string into a bool, or error.
+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)
+}
+
 // 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,60 @@ 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 bool to int64; 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)
 
@@ -709,11 +709,12 @@ func (cloud *CloudProvider) AttachDisk(ctx context.Context, project string, volK
 		return fmt.Errorf("failed to get device name: %w", err)
 	}
 	attachedDiskV1 := &computev1.AttachedDisk{
-		DeviceName: deviceName,
-		Kind:       diskKind,
-		Mode:       readWrite,
-		Source:     source,
-		Type:       diskType,
+		DeviceName:  deviceName,
+		Kind:        diskKind,
+		Mode:        readWrite,
+		Source:      source,
+		Type:        diskType,
+		ForceAttach: forceAttach,
 	}
 
 	op, err := cloud.service.Instances.AttachDisk(project, instanceZone, instanceName, attachedDiskV1).Context(ctx).Do()

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

@@ -111,6 +111,11 @@ type locationRequirements struct {
 	cloneReplicationType string
 }
 
+// PDCSIContext is the extracted VolumeContext from controller requests.
+type PDCSIContext struct {
+	ForceAttach bool
+}
+
 var _ csi.ControllerServer = &GCEControllerServer{}
 
 const (
@@ -309,7 +314,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 
 		// If there is no validation error, immediately return success
 		klog.V(4).Infof("CreateVolume succeeded for disk %v, it already exists and was compatible", volKey)
-		return generateCreateVolumeResponse(existingDisk, zones), nil
+		return generateCreateVolumeResponse(existingDisk, zones, params), nil
 	}
 
 	snapshotID := ""
@@ -424,7 +429,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 	}
 
 	klog.V(4).Infof("CreateVolume succeeded for disk %v", volKey)
-	return generateCreateVolumeResponse(disk, zones), nil
+	return generateCreateVolumeResponse(disk, zones, params), nil
 
 }
 
@@ -483,7 +488,7 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
 		}
 	}()
 	// Only valid requests will be accepted
-	_, _, err = gceCS.validateControllerPublishVolumeRequest(ctx, req)
+	_, _, _, err = gceCS.validateControllerPublishVolumeRequest(ctx, req)
 	if err != nil {
 		return nil, err
 	}
@@ -504,32 +509,37 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
 	return resp, err
 }
 
-func (gceCS *GCEControllerServer) validateControllerPublishVolumeRequest(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (string, *meta.Key, error) {
+func (gceCS *GCEControllerServer) validateControllerPublishVolumeRequest(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (string, *meta.Key, *PDCSIContext, error) {
 	// Validate arguments
 	volumeID := req.GetVolumeId()
 	nodeID := req.GetNodeId()
 	volumeCapability := req.GetVolumeCapability()
 	if len(volumeID) == 0 {
-		return "", nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Volume ID must be provided")
+		return "", nil, nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Volume ID must be provided")
 	}
 	if len(nodeID) == 0 {
-		return "", nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Node ID must be provided")
+		return "", nil, nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Node ID must be provided")
 	}
 	if volumeCapability == nil {
-		return "", nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Volume capability must be provided")
+		return "", nil, nil, status.Error(codes.InvalidArgument, "ControllerPublishVolume Volume capability must be provided")
 	}
 
 	project, volKey, err := common.VolumeIDToKey(volumeID)
 	if err != nil {
-		return "", nil, status.Errorf(codes.InvalidArgument, "ControllerPublishVolume volume ID is invalid: %v", err.Error())
+		return "", nil, nil, status.Errorf(codes.InvalidArgument, "ControllerPublishVolume volume ID is invalid: %v", err.Error())
 	}
 
 	// TODO(#253): Check volume capability matches for ALREADY_EXISTS
 	if err = validateVolumeCapability(volumeCapability); err != nil {
-		return "", nil, status.Errorf(codes.InvalidArgument, "VolumeCapabilities is invalid: %v", err.Error())
+		return "", nil, nil, status.Errorf(codes.InvalidArgument, "VolumeCapabilities is invalid: %v", err.Error())
 	}
 
-	return project, volKey, nil
+	var pdcsiContext *PDCSIContext
+	if pdcsiContext, err = extractVolumeContext(req.VolumeContext); err != nil {
+		return "", nil, nil, status.Errorf(codes.InvalidArgument, "Invalid volume context: %v", err.Error())
+	}
+
+	return project, volKey, pdcsiContext, nil
 }
 
 func parseMachineType(machineTypeUrl string) string {
@@ -543,7 +553,7 @@ func parseMachineType(machineTypeUrl string) string {
 
 func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error, string) {
 	diskType := ""
-	project, volKey, err := gceCS.validateControllerPublishVolumeRequest(ctx, req)
+	project, volKey, pdcsiContext, err := gceCS.validateControllerPublishVolumeRequest(ctx, req)
 	if err != nil {
 		return nil, err, diskType
 	}
@@ -615,7 +625,7 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "could not split nodeID: %v", err.Error()), diskType
 	}
-	err = gceCS.CloudProvider.AttachDisk(ctx, project, volKey, readWrite, attachableDiskTypePersistent, instanceZone, instanceName)
+	err = gceCS.CloudProvider.AttachDisk(ctx, project, volKey, readWrite, attachableDiskTypePersistent, instanceZone, instanceName, pdcsiContext.ForceAttach)
 	if err != nil {
 		var udErr *gce.UnsupportedDiskError
 		if errors.As(err, &udErr) {
@@ -788,11 +798,6 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
 		return nil, common.LoggedError("Failed to getDisk: ", err)
 	}
 
-	// Check Volume Context is Empty
-	if len(req.GetVolumeContext()) != 0 {
-		return generateFailedValidationMessage("VolumeContext expected to be empty but got %v", req.GetVolumeContext()), nil
-	}
-
 	// Check volume capabilities supported by PD. These are the same for any PD
 	if err := validateVolumeCapabilities(req.GetVolumeCapabilities()); err != nil {
 		return generateFailedValidationMessage("VolumeCapabilities not valid: %v", err.Error()), nil
@@ -1667,7 +1672,35 @@ func getDefaultZonesInRegion(ctx context.Context, gceCS *GCEControllerServer, ex
 	return ret, nil
 }
 
-func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.CreateVolumeResponse {
+func paramsToVolumeContext(params common.DiskParameters) map[string]string {
+	context := map[string]string{}
+	if params.ForceAttach {
+		context[common.ParameterForceAttach] = "true"
+	}
+	if len(context) > 0 {
+		return context
+	}
+	return nil
+}
+
+func extractVolumeContext(context map[string]string) (*PDCSIContext, error) {
+	info := &PDCSIContext{}
+	for key, val := range context {
+		switch key {
+		case common.ParameterForceAttach:
+			b, err := common.ConvertStringToBool(val)
+			if err != nil {
+				return nil, fmt.Errorf("Bad volume context force attach: %v", err)
+			}
+			info.ForceAttach = b
+		default:
+			return nil, fmt.Errorf("Unknown VolumeContext key %v", key)
+		}
+	}
+	return info, nil
+}
+
+func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string, params common.DiskParameters) *csi.CreateVolumeResponse {
 	tops := []*csi.Topology{}
 	for _, zone := range zones {
 		tops = append(tops, &csi.Topology{
@@ -1679,7 +1712,7 @@ func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.Crea
 		Volume: &csi.Volume{
 			CapacityBytes:      realDiskSizeBytes,
 			VolumeId:           cleanSelfLink(disk.GetSelfLink()),
-			VolumeContext:      nil,
+			VolumeContext:      paramsToVolumeContext(params),
 			AccessibleTopology: tops,
 		},
 	}