diff --git a/pkg/volume/csi/csi_block_test.go b/pkg/volume/csi/csi_block_test.go index 47be8f3303a..09985747435 100644 --- a/pkg/volume/csi/csi_block_test.go +++ b/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 { diff --git a/pkg/volume/csi/csi_client.go b/pkg/volume/csi/csi_client.go index b076e15ec46..aa7aa37f68d 100644 --- a/pkg/volume/csi/csi_client.go +++ b/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()}, diff --git a/pkg/volume/csi/csi_client_test.go b/pkg/volume/csi/csi_client_test.go index 7d1052bd9a7..714c2181ad8 100644 --- a/pkg/volume/csi/csi_client_test.go +++ b/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 { diff --git a/pkg/volume/csi/expander.go b/pkg/volume/csi/expander.go index 7af4d4dff25..650c94b2427 100644 --- a/pkg/volume/csi/expander.go +++ b/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, diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index 301cff5f1dd..f76564af33f 100644 --- a/pkg/volume/local/local_test.go +++ b/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) } diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 4ce7962328b..d9c2f628398 100644 --- a/pkg/volume/testing/testing.go +++ b/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 diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 6ee6e865570..2bcbb8bdbfd 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/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 diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 1f36d1e17de..23949a64034 100644 --- a/pkg/volume/volume.go +++ b/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.