Merge pull request #108614 from gnufied/remove-support-for-volume-expansion-between-stage-publish

Remove support for previously deprecated nodeExpand call

pkg/volume/csi/csi_client.go

@@ -113,11 +113,7 @@ 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

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())

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)

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 {

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
+	if resizeOptions.VolumeSpec.Name() == FailVolumeExpansion {
-	// 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 {
 		return false, fmt.Errorf("fail volume expansion for volume: %s", FailVolumeExpansion)
 	}
 	return true, nil

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)
 			}
-
+			// set staging path for volume expansion
-			// 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
-
-			// 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
+		_, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions)
-		// two reasons of that:
+		if resizeError != nil {
-		//	- Volume does not support DeviceMounter interface.
+			klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError)
-		//	- In case of CSI the volume does not have node stage_unstage capability.
+			eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError)
-		if !resizeDone {
+			// At this point, MountVolume.Setup already succeeded, we should add volume into actual state
-			_, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions)
+			// so that reconciler can clean up volume when needed. However, volume resize failed,
-			if resizeError != nil {
+			// we should not mark the volume as mounted to avoid pod starts using it.
-				klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError)
+			// Considering the above situations, we mark volume as uncertain here so that reconciler will tigger
-				eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError)
+			// volume tear down when pod is deleted, and also makes sure pod will not start using it.
-				// At this point, MountVolume.Setup already succeeded, we should add volume into actual state
+			if err := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts); err != nil {
-				// so that reconciler can clean up volume when needed. However, volume resize failed,
+				klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", err).Error())
-				// 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)
 			}
+			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)