From 89d1de9eb991cb328d6c1167a38ab0aa2d39d04d Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 28 Feb 2019 12:07:16 -0500 Subject: [PATCH] Rename ExandFS to NodeExpand Handle resize error in online resizing Use NodeExpandable plugin to mark volumes that require node expansion --- .../cache/actual_state_of_world.go | 2 +- pkg/volume/awsebs/aws_ebs.go | 9 ++-- pkg/volume/azure_dd/azure_dd.go | 9 ++-- pkg/volume/cinder/cinder.go | 9 ++-- pkg/volume/flexvolume/expander-defaults.go | 9 ++-- pkg/volume/flexvolume/expander.go | 14 +++--- pkg/volume/flexvolume/plugin.go | 2 +- pkg/volume/gcepd/gce_pd.go | 9 ++-- pkg/volume/plugins.go | 18 ++++---- pkg/volume/rbd/rbd.go | 9 ++-- pkg/volume/testing/testing.go | 6 +-- .../operationexecutor/operation_generator.go | 45 ++++++++++++------- 12 files changed, 89 insertions(+), 52 deletions(-) diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index 360d3ab0b7..5ecec2342d 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/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( diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index 8a98699473..b335984053 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/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{} diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index e9a850f901..328716b7d1 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/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()) diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index 7240614c78..0d6add65e9 100644 --- a/pkg/volume/cinder/cinder.go +++ b/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 diff --git a/pkg/volume/flexvolume/expander-defaults.go b/pkg/volume/flexvolume/expander-defaults.go index 4a33e184e3..27de51d483 100644 --- a/pkg/volume/flexvolume/expander-defaults.go +++ b/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 } diff --git a/pkg/volume/flexvolume/expander.go b/pkg/volume/flexvolume/expander.go index 345bad26d7..89375238c7 100644 --- a/pkg/volume/flexvolume/expander.go +++ b/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 } diff --git a/pkg/volume/flexvolume/plugin.go b/pkg/volume/flexvolume/plugin.go index e907485a0f..c0a95af96b 100644 --- a/pkg/volume/flexvolume/plugin.go +++ b/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{} diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index 01a66a2d3c..79f9fac642 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/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()) diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 76780a5f6f..91a803a739 100644 --- a/pkg/volume/plugins.go +++ b/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 -func (pm *VolumePluginMgr) FindFSResizablePluginBySpec(spec *Spec) (FSResizableVolumePlugin, error) { +// FindNodeExpandablePluginBySpec fetches a persistent volume plugin by spec +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 } diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index d62dcbc174..5b7cb3f011 100644 --- a/pkg/volume/rbd/rbd.go +++ b/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) diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 0d3a6f66ca..e958e6ee2d 100644 --- a/pkg/volume/testing/testing.go +++ b/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 { - return nil +func (plugin *FakeVolumePlugin) NodeExpand(spec *Spec, devicePath, deviceMountPath string, _, _ resource.Quantity) (bool, error) { + return true, nil } func (plugin *FakeVolumePlugin) GetVolumeLimits() (map[string]int64, error) { diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index d2ed9edc69..7570cf75d9 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/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 { - return resizeSimpleError, resizeDetailedError + if resizeError != nil { + 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 { - return volumeToMount.GenerateError("MountVolume.resizeFileSystem failed", resizeErr) + resizeDone, resizeErr := expandableVolumePlugin.NodeExpand(volumeToMount.VolumeSpec, devicePath, deviceMountPath, pvSpecCap, pvcStatusCap) + 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 { - return resizeSimpleError, resizeDetailedError + if resizeError != nil { + return volumeToMount.GenerateError("VolumeFSResize.resizeFileSystem failed", resizeError) } markFSResizedErr := actualStateOfWorld.MarkVolumeAsResized(volumeToMount.PodName, volumeToMount.VolumeName) if markFSResizedErr != nil {