diff --git a/pkg/volume/csi/csi_client.go b/pkg/volume/csi/csi_client.go index 8483ca651d2..d3d474f2942 100644 --- a/pkg/volume/csi/csi_client.go +++ b/pkg/volume/csi/csi_client.go @@ -113,11 +113,7 @@ type csiDriverClient struct { } type csiResizeOptions struct { - 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 + volumeID string volumePath string stagingTargetPath string fsType string diff --git a/pkg/volume/csi/expander.go b/pkg/volume/csi/expander.go index 0d4e9b26d26..58594a5bfdc 100644 --- a/pkg/volume/csi/expander.go +++ b/pkg/volume/csi/expander.go @@ -82,20 +82,6 @@ func (c *csiPlugin) nodeExpandWithClient( return false, fmt.Errorf("Expander.NodeExpand found CSI plugin %s/%s to not support node expansion", c.GetPluginName(), driverName) } - // Check whether "STAGE_UNSTAGE_VOLUME" is set - stageUnstageSet, err := csClient.NodeSupportsStageUnstage(ctx) - if err != nil { - return false, fmt.Errorf("Expander.NodeExpand failed to check if plugins supports stage_unstage %v", err) - } - - // if plugin does not support STAGE_UNSTAGE but CSI volume path is staged - // it must mean this was placeholder staging performed by k8s and not CSI staging - // in which case we should return from here so as volume can be node published - // before we can resize - if !stageUnstageSet && resizeOptions.CSIVolumePhase == volume.CSIVolumeStaged { - return false, nil - } - pv := resizeOptions.VolumeSpec.PersistentVolume if pv == nil { return false, fmt.Errorf("Expander.NodeExpand failed to find associated PersistentVolume for plugin %s", c.GetPluginName()) diff --git a/pkg/volume/csi/expander_test.go b/pkg/volume/csi/expander_test.go index 12c10f5a892..32af09b928b 100644 --- a/pkg/volume/csi/expander_test.go +++ b/pkg/volume/csi/expander_test.go @@ -32,7 +32,6 @@ func TestNodeExpand(t *testing.T) { name string nodeExpansion bool nodeStageSet bool - volumePhase volume.CSIVolumePhaseType success bool fsVolume bool grpcError error @@ -47,37 +46,26 @@ func TestNodeExpand(t *testing.T) { name: "when nodeExpansion=on, nodeStage=on, volumePhase=staged", nodeExpansion: true, nodeStageSet: true, - volumePhase: volume.CSIVolumeStaged, success: true, fsVolume: true, deviceStagePath: "/foo/bar", }, - { - name: "when nodeExpansion=on, nodeStage=off, volumePhase=staged", - nodeExpansion: true, - volumePhase: volume.CSIVolumeStaged, - success: false, - fsVolume: true, - }, { name: "when nodeExpansion=on, nodeStage=on, volumePhase=published", nodeExpansion: true, nodeStageSet: true, - volumePhase: volume.CSIVolumePublished, success: true, fsVolume: true, }, { name: "when nodeExpansion=on, nodeStage=off, volumePhase=published", nodeExpansion: true, - volumePhase: volume.CSIVolumePublished, success: true, fsVolume: true, }, { name: "when nodeExpansion=on, nodeStage=off, volumePhase=published, fsVolume=false", nodeExpansion: true, - volumePhase: volume.CSIVolumePublished, success: true, fsVolume: false, }, @@ -85,7 +73,6 @@ func TestNodeExpand(t *testing.T) { name: "when nodeExpansion=on, nodeStage=on, volumePhase=published has grpc volume-in-use error", nodeExpansion: true, nodeStageSet: true, - volumePhase: volume.CSIVolumePublished, success: false, fsVolume: true, grpcError: status.Error(codes.FailedPrecondition, "volume-in-use"), @@ -95,7 +82,6 @@ func TestNodeExpand(t *testing.T) { name: "when nodeExpansion=on, nodeStage=on, volumePhase=published has other grpc error", nodeExpansion: true, nodeStageSet: true, - volumePhase: volume.CSIVolumePublished, success: false, fsVolume: true, grpcError: status.Error(codes.InvalidArgument, "invalid-argument"), @@ -117,7 +103,6 @@ func TestNodeExpand(t *testing.T) { DeviceMountPath: "/foo/bar", DeviceStagePath: "/foo/bar", DevicePath: "/mnt/foobar", - CSIVolumePhase: tc.volumePhase, } csiSource, _ := getCSISourceFromSpec(resizeOptions.VolumeSpec) csClient := setupClientWithExpansion(t, tc.nodeStageSet, tc.nodeExpansion) diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 56deebbcd13..47234e99de4 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -53,9 +53,6 @@ type ProbeEvent struct { Op ProbeOperation // The operation to the plugin } -// CSIVolumePhaseType stores information about CSI volume path. -type CSIVolumePhaseType string - const ( // Common parameter which can be specified in StorageClass to specify the desired FSType // Provisioners SHOULD implement support for this if they are block device based @@ -65,8 +62,6 @@ const ( ProbeAddOrUpdate ProbeOperation = 1 << iota ProbeRemove - CSIVolumeStaged CSIVolumePhaseType = "staged" - CSIVolumePublished CSIVolumePhaseType = "published" ) var ( @@ -124,9 +119,6 @@ type NodeResizeOptions struct { NewSize resource.Quantity OldSize resource.Quantity - - // CSIVolumePhase contains volume phase on the node - CSIVolumePhase CSIVolumePhaseType } type DynamicPluginProber interface { diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 6b7c5a98dbe..401a335f4b8 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -478,12 +478,7 @@ func (plugin *FakeVolumePlugin) NodeExpand(resizeOptions NodeResizeOptions) (boo return false, fmt.Errorf("Test failure: NodeExpand") } - // Set up fakeVolumePlugin not support STAGE_UNSTAGE for testing the behavior - // so as volume can be node published before we can resize - if resizeOptions.CSIVolumePhase == volume.CSIVolumeStaged { - return false, nil - } - if resizeOptions.CSIVolumePhase == volume.CSIVolumePublished && resizeOptions.VolumeSpec.Name() == FailVolumeExpansion { + if resizeOptions.VolumeSpec.Name() == FailVolumeExpansion { return false, fmt.Errorf("fail volume expansion for volume: %s", FailVolumeExpansion) } return true, nil diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index e857a67b704..265fc0f6b5a 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -634,7 +634,6 @@ func (og *operationGenerator) GenerateMountVolumeFunc( klog.InfoS(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach succeeded", fmt.Sprintf("DevicePath %q", devicePath)), "pod", klog.KObj(volumeToMount.Pod)) } - var resizeDone bool var resizeError error resizeOptions := volume.NodeResizeOptions{ DevicePath: devicePath, @@ -674,35 +673,8 @@ func (og *operationGenerator) GenerateMountVolumeFunc( eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.MarkDeviceAsMounted failed", markDeviceMountedErr) return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) } - - // 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 + // set staging path for volume expansion resizeOptions.DeviceStagePath = deviceMountPath - resizeOptions.CSIVolumePhase = volume.CSIVolumeStaged - - // NodeExpandVolume will resize the file system if user has requested a resize of - // underlying persistent volume and is allowed to do so. - resizeDone, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) - - if resizeError != nil { - klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError) - - // Resize failed. To make sure NodeExpand is re-tried again on the next attempt - // *before* SetUp(), mark the mounted device as uncertain. - markDeviceUncertainErr := actualStateOfWorld.MarkDeviceAsUncertain( - volumeToMount.VolumeName, devicePath, deviceMountPath) - if markDeviceUncertainErr != nil { - // just log, return the resizeError error instead - klog.InfoS(volumeToMount.GenerateMsgDetailed( - "MountVolume.MountDevice failed to mark volume as uncertain", - markDeviceUncertainErr.Error()), "pod", klog.KObj(volumeToMount.Pod)) - } - eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.MountDevice failed while expanding volume", resizeError) - return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) - } } // Execute mount @@ -738,27 +710,20 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } klog.V(verbosity).InfoS(detailedMsg, "pod", klog.KObj(volumeToMount.Pod)) resizeOptions.DeviceMountPath = volumeMounter.GetPath() - resizeOptions.CSIVolumePhase = volume.CSIVolumePublished - // We need to call resizing here again in case resizing was not done during device mount. There could be - // two reasons of that: - // - Volume does not support DeviceMounter interface. - // - In case of CSI the volume does not have node stage_unstage capability. - if !resizeDone { - _, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) - if resizeError != nil { - klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError) - eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError) - // At this point, MountVolume.Setup already succeeded, we should add volume into actual state - // so that reconciler can clean up volume when needed. However, volume resize failed, - // we should not mark the volume as mounted to avoid pod starts using it. - // Considering the above situations, we mark volume as uncertain here so that reconciler will tigger - // volume tear down when pod is deleted, and also makes sure pod will not start using it. - if err := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts); err != nil { - klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", err).Error()) - } - return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) + _, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) + if resizeError != nil { + klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError) + eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError) + // At this point, MountVolume.Setup already succeeded, we should add volume into actual state + // so that reconciler can clean up volume when needed. However, volume resize failed, + // we should not mark the volume as mounted to avoid pod starts using it. + // Considering the above situations, we mark volume as uncertain here so that reconciler will tigger + // volume tear down when pod is deleted, and also makes sure pod will not start using it. + if err := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts); err != nil { + klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", err).Error()) } + return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) } // record total time it takes to mount a volume. This is end to end time that includes waiting for volume to attach, node to be update @@ -1238,7 +1203,6 @@ func (og *operationGenerator) GenerateMapVolumeFunc( resizeOptions := volume.NodeResizeOptions{ DevicePath: devicePath, DeviceStagePath: stagingPath, - CSIVolumePhase: volume.CSIVolumePublished, } _, resizeError := og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) if resizeError != nil { @@ -2008,7 +1972,6 @@ func (og *operationGenerator) GenerateExpandInUseVolumeFunc( } // if we are doing online expansion then volume is already published - resizeOptions.CSIVolumePhase = volume.CSIVolumePublished resizeDone, eventErr, detailedErr = og.doOnlineExpansion(volumeToMount, actualStateOfWorld, resizeOptions) if eventErr != nil || detailedErr != nil { return volumetypes.NewOperationContext(eventErr, detailedErr, migrated)