rename volumeid to volumeID

Author: gnufied

pkg/volume/csi/csi_block_test.go

@@ -277,7 +277,7 @@ func TestBlockMapperSetupDeviceError(t *testing.T) {
 	}
 	t.Log("created attachement ", attachID)
 
-	err = csiMapper.SetUpDevice()
+	stagingPath, err := csiMapper.SetUpDevice()
 	if err == nil {
 		t.Fatal("mapper unexpectedly succeeded")
 	}
@@ -292,7 +292,7 @@ func TestBlockMapperSetupDeviceError(t *testing.T) {
 	if _, err := os.Stat(devDir); err == nil {
 		t.Errorf("volume publish device directory %s was not deleted", devDir)
 	}
-	stagingPath := csiMapper.getStagingPath()
+
 	if _, err := os.Stat(stagingPath); err == nil {
 		t.Errorf("volume staging path %s was not deleted", stagingPath)
 	}
@@ -474,12 +474,11 @@ func TestVolumeSetupTeardown(t *testing.T) {
 	}
 	t.Log("created attachement ", attachID)
 
-	err = csiMapper.SetUpDevice()
+	stagingPath, err := csiMapper.SetUpDevice()
 	if err != nil {
 		t.Fatalf("mapper failed to SetupDevice: %v", err)
 	}
 	// Check if NodeStageVolume staged to the right path
-	stagingPath := csiMapper.getStagingPath()
 	svols := csiMapper.csiClient.(*fakeCsiDriverClient).nodeClient.GetNodeStagedVolumes()
 	svol, ok := svols[csiMapper.volumeID]
 	if !ok {

pkg/volume/csi/csi_client.go

@@ -96,7 +96,11 @@ type csiDriverClient struct {
 }
 
 type csiResizeOptions struct {
-	volumeid          string
+	volumeID string
+	// volumePath is path where volume is available. It could be:
+	//   - path where node is staged if NodeExpandVolume is called after NodeStageVolume
+	//   - path where volume is published if NodeExpandVolume is called after NodePublishVolume
+	// DEPRECATION NOTICE: in future NodeExpandVolume will be always called after NodePublish
 	volumePath        string
 	stagingTargetPath string
 	fsType            string
@@ -260,10 +264,10 @@ func (c *csiDriverClient) NodeExpandVolume(ctx context.Context, opts csiResizeOp
 		return opts.newSize, fmt.Errorf("version of CSI driver does not support volume expansion")
 	}
 
-	if opts.volumeid == "" {
+	if opts.volumeID == "" {
 		return opts.newSize, errors.New("missing volume id")
 	}
-	if opts.volumeid == "" {
+	if opts.volumePath == "" {
 		return opts.newSize, errors.New("missing volume path")
 	}
 
@@ -278,7 +282,7 @@ func (c *csiDriverClient) NodeExpandVolume(ctx context.Context, opts csiResizeOp
 	defer closer.Close()
 
 	req := &csipbv1.NodeExpandVolumeRequest{
-		VolumeId:          opts.volumeid,
+		VolumeId:          opts.volumeID,
 		VolumePath:        opts.volumePath,
 		StagingTargetPath: opts.stagingTargetPath,
 		CapacityRange:     &csipbv1.CapacityRange{RequiredBytes: opts.newSize.Value()},

pkg/volume/csi/csi_client_test.go

@@ -287,7 +287,7 @@ func (c *fakeCsiDriverClient) NodeSupportsStageUnstage(ctx context.Context) (boo
 func (c *fakeCsiDriverClient) NodeExpandVolume(ctx context.Context, opts csiResizeOptions) (resource.Quantity, error) {
 	c.t.Log("calling fake.NodeExpandVolume")
 	req := &csipbv1.NodeExpandVolumeRequest{
-		VolumeId:          opts.volumeid,
+		VolumeId:          opts.volumeID,
 		VolumePath:        opts.volumePath,
 		StagingTargetPath: opts.stagingTargetPath,
 		CapacityRange:     &csipbv1.CapacityRange{RequiredBytes: opts.newSize.Value()},
@@ -653,7 +653,7 @@ func TestNodeExpandVolume(t *testing.T) {
 				return nodeClient, fakeCloser, nil
 			},
 		}
-		opts := csiResizeOptions{volumeid: tc.volID, volumePath: tc.volumePath, newSize: tc.newSize}
+		opts := csiResizeOptions{volumeID: tc.volID, volumePath: tc.volumePath, newSize: tc.newSize}
 		_, err := client.NodeExpandVolume(context.Background(), opts)
 		checkErr(t, tc.mustFail, err)
 		if !tc.mustFail {

pkg/volume/csi/expander.go

@@ -102,7 +102,7 @@ func (c *csiPlugin) nodeExpandWithClient(
 	opts := csiResizeOptions{
 		volumePath:        resizeOptions.DeviceMountPath,
 		stagingTargetPath: resizeOptions.DeviceStagePath,
-		volumeid:          csiSource.VolumeHandle,
+		volumeID:          csiSource.VolumeHandle,
 		newSize:           resizeOptions.NewSize,
 		fsType:            csiSource.FSType,
 		accessMode:        api.ReadWriteOnce,

pkg/volume/local/local_test.go

@@ -375,7 +375,7 @@ func TestMapUnmap(t *testing.T) {
 	var devPath string
 
 	if customMapper, ok := mapper.(volume.CustomBlockVolumeMapper); ok {
-		err = customMapper.SetUpDevice()
+		_, err = customMapper.SetUpDevice()
 		if err != nil {
 			t.Errorf("Failed to SetUpDevice, err: %v", err)
 		}

pkg/volume/testing/testing.go

@@ -956,46 +956,46 @@ func (fv *FakeVolume) TearDownAt(dir string) error {
 }
 
 // Block volume support
-func (fv *FakeVolume) SetUpDevice() error {
+func (fv *FakeVolume) SetUpDevice() (string, error) {
 	fv.Lock()
 	defer fv.Unlock()
 	if fv.VolName == TimeoutOnMountDeviceVolumeName {
 		fv.DeviceMountState[fv.VolName] = deviceMountUncertain
-		return volumetypes.NewUncertainProgressError("mount failed")
+		return "", volumetypes.NewUncertainProgressError("mount failed")
 	}
 	if fv.VolName == FailMountDeviceVolumeName {
 		fv.DeviceMountState[fv.VolName] = deviceNotMounted
-		return fmt.Errorf("error mapping disk: %s", fv.VolName)
+		return "", fmt.Errorf("error mapping disk: %s", fv.VolName)
 	}
 
 	if fv.VolName == TimeoutAndFailOnMountDeviceVolumeName {
 		_, ok := fv.DeviceMountState[fv.VolName]
 		if !ok {
 			fv.DeviceMountState[fv.VolName] = deviceMountUncertain
-			return volumetypes.NewUncertainProgressError("timed out mounting error")
+			return "", volumetypes.NewUncertainProgressError("timed out mounting error")
 		}
 		fv.DeviceMountState[fv.VolName] = deviceNotMounted
-		return fmt.Errorf("error mapping disk: %s", fv.VolName)
+		return "", fmt.Errorf("error mapping disk: %s", fv.VolName)
 	}
 
 	if fv.VolName == SuccessAndTimeoutDeviceName {
 		_, ok := fv.DeviceMountState[fv.VolName]
 		if ok {
 			fv.DeviceMountState[fv.VolName] = deviceMountUncertain
-			return volumetypes.NewUncertainProgressError("error mounting state")
+			return "", volumetypes.NewUncertainProgressError("error mounting state")
 		}
 	}
 	if fv.VolName == SuccessAndFailOnMountDeviceName {
 		_, ok := fv.DeviceMountState[fv.VolName]
 		if ok {
-			return fmt.Errorf("error mapping disk: %s", fv.VolName)
+			return "", fmt.Errorf("error mapping disk: %s", fv.VolName)
 		}
 	}
 
 	fv.DeviceMountState[fv.VolName] = deviceMounted
 	fv.SetUpDeviceCallCount++
 
-	return nil
+	return "", nil
 }
 
 // Block volume support

pkg/volume/util/operationexecutor/operation_generator.go

@@ -603,6 +603,10 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 				return volumeToMount.GenerateError("MountVolume.MarkDeviceAsMounted failed", markDeviceMountedErr)
 			}
 
+			// If volume expansion is performed after MountDevice but before SetUp then
+			// deviceMountPath and deviceStagePath is going to be the same.
+			// Deprecation: Calling NodeExpandVolume after NodeStage/MountDevice will be deprecated
+			// in a future version of k8s.
 			resizeOptions.DeviceMountPath = deviceMountPath
 			resizeOptions.DeviceStagePath = deviceMountPath
 			resizeOptions.CSIVolumePhase = volume.CSIVolumeStaged

pkg/volume/volume.go

@@ -174,14 +174,15 @@ type CustomBlockVolumeMapper interface {
 	// For most in-tree plugins, attacher.Attach() and attacher.WaitForAttach()
 	// will do necessary works.
 	// This may be called more than once, so implementations must be idempotent.
-	SetUpDevice() (string, error)
+	// SetUpDevice returns stagingPath if device setup was successful
+	SetUpDevice() (stagingPath string, err error)
 
 	// MapPodDevice maps the block device to a path and return the path.
 	// Unique device path across kubelet node reboot is required to avoid
 	// unexpected block volume destruction.
 	// If empty string is returned, the path retuned by attacher.Attach() and
 	// attacher.WaitForAttach() will be used.
-	MapPodDevice() (string, error)
+	MapPodDevice() (publishPath string, err error)
 }
 
 // BlockVolumeUnmapper interface is an unmapper interface for block volume.