Rename ExandFS to NodeExpand

Handle resize error in online resizing
Use NodeExpandable plugin to mark volumes that require node expansion

pkg/kubelet/volumemanager/cache/actual_state_of_world.go

@@ -542,7 +542,7 @@ func (asw *actualStateOfWorld) MarkFSResizeRequired(
 	}
 
 	volumePlugin, err :=
-		asw.volumePluginMgr.FindExpandablePluginBySpec(podObj.volumeSpec)
+		asw.volumePluginMgr.FindNodeExpandablePluginBySpec(podObj.volumeSpec)
 	if err != nil || volumePlugin == nil {
 		// Log and continue processing
 		klog.Errorf(

pkg/volume/awsebs/aws_ebs.go

@@ -317,12 +317,15 @@ func (plugin *awsElasticBlockStorePlugin) ExpandVolumeDevice(
 	return awsVolume.ResizeDisk(volumeID, oldSize, newSize)
 }
 
-func (plugin *awsElasticBlockStorePlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *awsElasticBlockStorePlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var _ volume.FSResizableVolumePlugin = &awsElasticBlockStorePlugin{}
+var _ volume.NodeExpandableVolumePlugin = &awsElasticBlockStorePlugin{}
 var _ volume.ExpandableVolumePlugin = &awsElasticBlockStorePlugin{}
 var _ volume.VolumePluginWithAttachLimits = &awsElasticBlockStorePlugin{}
 

pkg/volume/azure_dd/azure_dd.go

@@ -312,12 +312,15 @@ func (plugin *azureDataDiskPlugin) ExpandVolumeDevice(
 	return diskController.ResizeDisk(spec.PersistentVolume.Spec.AzureDisk.DataDiskURI, oldSize, newSize)
 }
 
-func (plugin *azureDataDiskPlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *azureDataDiskPlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var _ volume.FSResizableVolumePlugin = &azureDataDiskPlugin{}
+var _ volume.NodeExpandableVolumePlugin = &azureDataDiskPlugin{}
 
 func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
 	mounter := plugin.host.GetMounter(plugin.GetPluginName())

pkg/volume/cinder/cinder.go

@@ -304,12 +304,15 @@ func (plugin *cinderPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resour
 	return expandedSize, nil
 }
 
-func (plugin *cinderPlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *cinderPlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var _ volume.FSResizableVolumePlugin = &cinderPlugin{}
+var _ volume.NodeExpandableVolumePlugin = &cinderPlugin{}
 
 func (plugin *cinderPlugin) RequiresFSResize() bool {
 	return true

pkg/volume/flexvolume/expander-defaults.go

@@ -36,10 +36,13 @@ func (e *expanderDefaults) ExpandVolumeDevice(spec *volume.Spec, newSize resourc
 	return newSize, nil
 }
 
-// the defaults for ExpandFS return a generic resize indicator that will trigger the operation executor to go ahead with
+// the defaults for NodeExpand return a generic resize indicator that will trigger the operation executor to go ahead with
 // generic filesystem resize
-func (e *expanderDefaults) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (e *expanderDefaults) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	klog.Warning(logPrefix(e.plugin), "using default filesystem resize for volume ", spec.Name(), ", at ", devicePath)
 	_, err := util.GenericResizeFS(e.plugin.host, e.plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }

pkg/volume/flexvolume/expander.go

@@ -18,9 +18,10 @@ package flexvolume
 
 import (
 	"fmt"
+	"strconv"
+
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/volume"
-	"strconv"
 )
 
 func (plugin *flexVolumePlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resource.Quantity, oldSize resource.Quantity) (resource.Quantity, error) {
@@ -42,14 +43,14 @@ func (plugin *flexVolumePlugin) ExpandVolumeDevice(spec *volume.Spec, newSize re
 	return newSize, err
 }
 
-func (plugin *flexVolumePlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, newSize, oldSize resource.Quantity) error {
+func (plugin *flexVolumePlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, newSize, oldSize resource.Quantity) (bool, error) {
 	// This method is called after we spec.PersistentVolume.Spec.Capacity
 	// has been updated to the new size. The underlying driver thus sees
 	// the _new_ (requested) size and can find out the _current_ size from
 	// its underlying storage implementation
 
 	if spec.PersistentVolume == nil {
-		return fmt.Errorf("PersistentVolume not found for spec: %s", spec.Name())
+		return false, fmt.Errorf("PersistentVolume not found for spec: %s", spec.Name())
 	}
 
 	call := plugin.NewDriverCall(expandFSCmd)
@@ -61,7 +62,10 @@ func (plugin *flexVolumePlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMo
 
 	_, err := call.Run()
 	if isCmdNotSupportedErr(err) {
-		return newExpanderDefaults(plugin).ExpandFS(spec, devicePath, deviceMountPath, newSize, oldSize)
+		return newExpanderDefaults(plugin).NodeExpand(spec, devicePath, deviceMountPath, newSize, oldSize)
 	}
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }

pkg/volume/flexvolume/plugin.go

@@ -56,7 +56,7 @@ type flexVolumeAttachablePlugin struct {
 
 var _ volume.AttachableVolumePlugin = &flexVolumeAttachablePlugin{}
 var _ volume.PersistentVolumePlugin = &flexVolumePlugin{}
-var _ volume.FSResizableVolumePlugin = &flexVolumePlugin{}
+var _ volume.NodeExpandableVolumePlugin = &flexVolumePlugin{}
 var _ volume.ExpandableVolumePlugin = &flexVolumePlugin{}
 
 var _ volume.DeviceMountableVolumePlugin = &flexVolumeAttachablePlugin{}

pkg/volume/gcepd/gce_pd.go

@@ -290,12 +290,15 @@ func (plugin *gcePersistentDiskPlugin) ExpandVolumeDevice(
 	return updatedQuantity, nil
 }
 
-func (plugin *gcePersistentDiskPlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *gcePersistentDiskPlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var _ volume.FSResizableVolumePlugin = &gcePersistentDiskPlugin{}
+var _ volume.NodeExpandableVolumePlugin = &gcePersistentDiskPlugin{}
 
 func (plugin *gcePersistentDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
 	mounter := plugin.host.GetMounter(plugin.GetPluginName())

pkg/volume/plugins.go

@@ -226,11 +226,13 @@ type ExpandableVolumePlugin interface {
 	RequiresFSResize() bool
 }
 
-// FSResizableVolumePlugin is an extension of ExpandableVolumePlugin and is used for volumes (flex)
+// NodeExpandableVolumePlugin is an extension of ExpandableVolumePlugin and is used for volumes (flex)
 // that require extra steps on nodes for expansion to complete
-type FSResizableVolumePlugin interface {
+type NodeExpandableVolumePlugin interface {
 	ExpandableVolumePlugin
-	ExpandFS(spec *Spec, devicePath, deviceMountPath string, newSize, oldSize resource.Quantity) error
+	// NodeExpand expands volume on given deviceMountPath and returns true if resize is successful.
+	// devicePath can be set to empty string if unavailable.
+	NodeExpand(spec *Spec, devicePath, deviceMountPath string, newSize, oldSize resource.Quantity) (bool, error)
 }
 
 // VolumePluginWithAttachLimits is an extended interface of VolumePlugin that restricts number of
@@ -951,26 +953,26 @@ func (pm *VolumePluginMgr) FindMapperPluginByName(name string) (BlockVolumePlugi
 	return nil, nil
 }
 
-// FindFSResizablePluginBySpec fetches a persistent volume plugin by spec
+// FindNodeExpandablePluginBySpec fetches a persistent volume plugin by spec
-func (pm *VolumePluginMgr) FindFSResizablePluginBySpec(spec *Spec) (FSResizableVolumePlugin, error) {
+func (pm *VolumePluginMgr) FindNodeExpandablePluginBySpec(spec *Spec) (NodeExpandableVolumePlugin, error) {
 	volumePlugin, err := pm.FindPluginBySpec(spec)
 	if err != nil {
 		return nil, err
 	}
-	if fsResizablePlugin, ok := volumePlugin.(FSResizableVolumePlugin); ok {
+	if fsResizablePlugin, ok := volumePlugin.(NodeExpandableVolumePlugin); ok {
 		return fsResizablePlugin, nil
 	}
 	return nil, nil
 }
 
 // FindFSResizablePluginByName fetches a persistent volume plugin by name
-func (pm *VolumePluginMgr) FindFSResizablePluginByName(name string) (FSResizableVolumePlugin, error) {
+func (pm *VolumePluginMgr) FindFSResizablePluginByName(name string) (NodeExpandableVolumePlugin, error) {
 	volumePlugin, err := pm.FindPluginByName(name)
 	if err != nil {
 		return nil, err
 	}
 
-	if fsResizablePlugin, ok := volumePlugin.(FSResizableVolumePlugin); ok {
+	if fsResizablePlugin, ok := volumePlugin.(NodeExpandableVolumePlugin); ok {
 		return fsResizablePlugin, nil
 	}
 

pkg/volume/rbd/rbd.go

@@ -201,12 +201,15 @@ func (plugin *rbdPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resource.
 	}
 }
 
-func (plugin *rbdPlugin) ExpandFS(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *rbdPlugin) NodeExpand(spec *volume.Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
 	_, err := volutil.GenericResizeFS(plugin.host, plugin.GetPluginName(), devicePath, deviceMountPath)
-	return err
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var _ volume.FSResizableVolumePlugin = &rbdPlugin{}
+var _ volume.NodeExpandableVolumePlugin = &rbdPlugin{}
 
 func (expander *rbdVolumeExpander) ResizeImage(oldSize resource.Quantity, newSize resource.Quantity) (resource.Quantity, error) {
 	return expander.manager.ExpandImage(expander, oldSize, newSize)

pkg/volume/testing/testing.go

@@ -280,7 +280,7 @@ var _ ProvisionableVolumePlugin = &FakeVolumePlugin{}
 var _ AttachableVolumePlugin = &FakeVolumePlugin{}
 var _ VolumePluginWithAttachLimits = &FakeVolumePlugin{}
 var _ DeviceMountableVolumePlugin = &FakeVolumePlugin{}
-var _ FSResizableVolumePlugin = &FakeVolumePlugin{}
+var _ NodeExpandableVolumePlugin = &FakeVolumePlugin{}
 
 func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume {
 	volumeList := *list
@@ -537,8 +537,8 @@ func (plugin *FakeVolumePlugin) RequiresFSResize() bool {
 	return true
 }
 
-func (plugin *FakeVolumePlugin) ExpandFS(spec *Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) error {
+func (plugin *FakeVolumePlugin) NodeExpand(spec *Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) {
-	return nil
+	return true, nil
 }
 
 func (plugin *FakeVolumePlugin) GetVolumeLimits() (map[string]int64, error) {

pkg/volume/util/operationexecutor/operation_generator.go

@@ -597,6 +597,8 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 
 			klog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach succeeded", fmt.Sprintf("DevicePath %q", devicePath)))
 		}
+		var resizeDone bool
+		var resizeError error
 
 		if volumeDeviceMounter != nil {
 			deviceMountPath, err :=
@@ -628,10 +630,10 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 
 			// resizeFileSystem will resize the file system if user has requested a resize of
 			// underlying persistent volume and is allowed to do so.
-			resizeSimpleError, resizeDetailedError := og.resizeFileSystem(volumeToMount, devicePath, deviceMountPath, volumePluginName)
+			resizeDone, resizeError = og.resizeFileSystem(volumeToMount, devicePath, deviceMountPath, volumePluginName)
 
-			if resizeSimpleError != nil || resizeDetailedError != nil {
+			if resizeError != nil {
-				return resizeSimpleError, resizeDetailedError
+				return volumeToMount.GenerateError("MountVolume.MountDevice failed while expanding volume", resizeError)
 			}
 		}
 
@@ -658,6 +660,13 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 		}
 		klog.V(verbosity).Infof(detailedMsg)
 
+		if !resizeDone {
+			resizeDone, resizeError = og.resizeFileSystem(volumeToMount, "", volumeMounter.GetPath(), volumePluginName)
+			if resizeError != nil {
+				return volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError)
+			}
+		}
+
 		// Update actual state of world
 		markVolMountedErr := actualStateOfWorld.MarkVolumeAsMounted(
 			volumeToMount.PodName,
@@ -689,15 +698,15 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 	}
 }
 
-func (og *operationGenerator) resizeFileSystem(volumeToMount VolumeToMount, devicePath, deviceMountPath, pluginName string) (simpleErr, detailedErr error) {
+func (og *operationGenerator) resizeFileSystem(volumeToMount VolumeToMount, devicePath, deviceMountPath, pluginName string) (bool, error) {
 	if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) {
 		klog.V(4).Infof("Resizing is not enabled for this volume %s", volumeToMount.VolumeName)
-		return nil, nil
+		return true, nil
 	}
 
 	// Get expander, if possible
 	expandableVolumePlugin, _ :=
-		og.volumePluginMgr.FindFSResizablePluginBySpec(volumeToMount.VolumeSpec)
+		og.volumePluginMgr.FindNodeExpandablePluginBySpec(volumeToMount.VolumeSpec)
 
 	if expandableVolumePlugin != nil &&
 		expandableVolumePlugin.RequiresFSResize() &&
@@ -706,7 +715,7 @@ func (og *operationGenerator) resizeFileSystem(volumeToMount VolumeToMount, devi
 		pvc, err := og.kubeClient.CoreV1().PersistentVolumeClaims(pv.Spec.ClaimRef.Namespace).Get(pv.Spec.ClaimRef.Name, metav1.GetOptions{})
 		if err != nil {
 			// Return error rather than leave the file system un-resized, caller will log and retry
-			return volumeToMount.GenerateError("MountVolume.resizeFileSystem get PVC failed", err)
+			return false, fmt.Errorf("MountVolume.resizeFileSystem get PVC failed : %v", err)
 		}
 
 		pvcStatusCap := pvc.Status.Capacity[v1.ResourceStorage]
@@ -719,10 +728,14 @@ func (og *operationGenerator) resizeFileSystem(volumeToMount VolumeToMount, devi
 				simpleMsg, detailedMsg := volumeToMount.GenerateMsg("MountVolume.resizeFileSystem failed", "requested read-only file system")
 				klog.Warningf(detailedMsg)
 				og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FileSystemResizeFailed, simpleMsg)
-				return nil, nil
+				return true, nil
 			}
-			if resizeErr := expandableVolumePlugin.ExpandFS(volumeToMount.VolumeSpec, devicePath, deviceMountPath, pvSpecCap, pvcStatusCap); resizeErr != nil {
+			resizeDone, resizeErr := expandableVolumePlugin.NodeExpand(volumeToMount.VolumeSpec, devicePath, deviceMountPath, pvSpecCap, pvcStatusCap)
-				return volumeToMount.GenerateError("MountVolume.resizeFileSystem failed", resizeErr)
+			if resizeErr != nil {
+				return false, fmt.Errorf("MountVolume.resizeFileSystem failed : %v", resizeErr)
+			}
+			if !resizeDone {
+				return false, nil
 			}
 			simpleMsg, detailedMsg := volumeToMount.GenerateMsg("MountVolume.resizeFileSystem succeeded", "")
 			og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeNormal, kevents.FileSystemResizeSuccess, simpleMsg)
@@ -731,12 +744,12 @@ func (og *operationGenerator) resizeFileSystem(volumeToMount VolumeToMount, devi
 			err = util.MarkFSResizeFinished(pvc, pv.Spec.Capacity, og.kubeClient)
 			if err != nil {
 				// On retry, resizeFileSystem will be called again but do nothing
-				return volumeToMount.GenerateError("MountVolume.resizeFileSystem update PVC status failed", err)
+				return false, fmt.Errorf("MountVolume.resizeFileSystem update PVC status failed : %v", err)
 			}
-			return nil, nil
+			return true, nil
 		}
 	}
-	return nil, nil
+	return true, nil
 }
 
 func (og *operationGenerator) GenerateUnmountVolumeFunc(
@@ -1469,10 +1482,10 @@ func (og *operationGenerator) GenerateExpandVolumeFSWithoutUnmountingFunc(
 	}
 
 	fsResizeFunc := func() (error, error) {
-		resizeSimpleError, resizeDetailedError := og.resizeFileSystem(volumeToMount, volumeToMount.DevicePath, deviceMountPath, volumePlugin.GetPluginName())
+		_, resizeError := og.resizeFileSystem(volumeToMount, volumeToMount.DevicePath, deviceMountPath, volumePlugin.GetPluginName())
 
-		if resizeSimpleError != nil || resizeDetailedError != nil {
+		if resizeError != nil {
-			return resizeSimpleError, resizeDetailedError
+			return volumeToMount.GenerateError("VolumeFSResize.resizeFileSystem failed", resizeError)
 		}
 		markFSResizedErr := actualStateOfWorld.MarkVolumeAsResized(volumeToMount.PodName, volumeToMount.VolumeName)
 		if markFSResizedErr != nil {