From 99fc7cc7b1773d681272e7ef1cbd9d8c2bc99e0d Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 14 Aug 2024 09:22:46 -0400 Subject: [PATCH] Use new size returned by nodeExpander for recording in ASOW --- .../cache/actual_state_of_world.go | 2 ++ .../desired_state_of_world_populator.go | 3 ++- .../util/operationexecutor/node_expander.go | 16 +++++------ .../operationexecutor/node_expander_test.go | 2 +- .../operationexecutor/operation_generator.go | 27 ++++++++++--------- .../operation_generator_test.go | 2 +- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index 38017a588f7..62dcefe6731 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -960,6 +960,8 @@ func (asw *actualStateOfWorld) volumeNeedsExpansion(volumeObj attachedVolume, de return currentSize, false } + klog.V(5).Infof("NodeExpandVolume checking size, actual size %s, desired size %s, for volume %s", volumeObj.persistentVolumeSize.String(), desiredVolumeSize.String(), volumeObj.volumeName) + if desiredVolumeSize.Cmp(*volumeObj.persistentVolumeSize) > 0 { volumePlugin, err := asw.volumePluginMgr.FindNodeExpandablePluginBySpec(volumeObj.spec) if err != nil || volumePlugin == nil { diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index 2d9b07a6021..cb9cf9d67ae 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -380,10 +380,11 @@ func (dswp *desiredStateOfWorldPopulator) checkVolumeFSResize( klog.V(5).InfoS("Skip file system resize check for the volume, as the volume is mounted as readonly", "pod", klog.KObj(pod), "volumeName", podVolume.Name) return } + pvCap := volumeSpec.PersistentVolume.Spec.Capacity.Storage() pvcStatusCap := pvc.Status.Capacity.Storage() dswp.desiredStateOfWorld.UpdatePersistentVolumeSize(uniqueVolumeName, pvCap) - + klog.V(5).Infof("NodeExpandVolume updating size, actual size %s, desired size %s, for volume %s", pvcStatusCap.String(), pvCap.String(), uniqueVolumeName) // in case the actualStateOfWorld was rebuild after kubelet restart ensure that claimSize is set to accurate value dswp.actualStateOfWorld.InitializeClaimSize(klog.TODO(), uniqueVolumeName, pvcStatusCap) } diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index fb68ad8831b..a33fafc7b98 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/pkg/volume/util/operationexecutor/node_expander.go @@ -115,11 +115,11 @@ func (ne *NodeExpander) runPreCheck() bool { return false } -func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) { +func (ne *NodeExpander) expandOnPlugin() (bool, resource.Quantity, error, testResponseData) { allowExpansion := ne.runPreCheck() if !allowExpansion { klog.V(3).Infof("NodeExpandVolume is not allowed to proceed for volume %s with resizeStatus %s", ne.vmt.VolumeName, ne.resizeStatus) - return false, nil, testResponseData{false, true} + return false, ne.pluginResizeOpts.OldSize, nil, testResponseData{false, true} } var err error @@ -131,7 +131,7 @@ func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) { if err != nil { msg := ne.vmt.GenerateErrorDetailed("MountVolume.NodeExpandVolume failed to mark node expansion in progress: %v", err) klog.Errorf(msg.Error()) - return false, err, testResponseData{} + return false, ne.pluginResizeOpts.OldSize, err, testResponseData{} } } _, resizeErr := ne.volumePlugin.NodeExpand(ne.pluginResizeOpts) @@ -158,9 +158,9 @@ func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) { if volumetypes.IsFailedPreconditionError(resizeErr) { ne.actualStateOfWorld.MarkForInUseExpansionError(ne.vmt.VolumeName) klog.Errorf(ne.vmt.GenerateErrorDetailed("MountVolume.NodeExapndVolume failed with %v", resizeErr).Error()) - return false, nil, testResponseData{assumeResizeFinished: true, resizeCalledOnPlugin: true} + return false, ne.pluginResizeOpts.OldSize, nil, testResponseData{assumeResizeFinished: true, resizeCalledOnPlugin: true} } - return false, resizeErr, testResponseData{assumeResizeFinished: true, resizeCalledOnPlugin: true} + return false, ne.pluginResizeOpts.OldSize, resizeErr, testResponseData{assumeResizeFinished: true, resizeCalledOnPlugin: true} } simpleMsg, detailedMsg := ne.vmt.GenerateMsg("MountVolume.NodeExpandVolume succeeded", nodeName) ne.recorder.Eventf(ne.vmt.Pod, v1.EventTypeNormal, kevents.FileSystemResizeSuccess, simpleMsg) @@ -169,13 +169,13 @@ func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) { // no need to update PVC object if we already updated it if ne.pvcAlreadyUpdated { - return true, nil, testResponseData{true, true} + return true, ne.pluginResizeOpts.NewSize, nil, testResponseData{true, true} } // File system resize succeeded, now update the PVC's Capacity to match the PV's ne.pvc, err = util.MarkFSResizeFinished(ne.pvc, ne.pluginResizeOpts.NewSize, ne.kubeClient) if err != nil { - return true, fmt.Errorf("mountVolume.NodeExpandVolume update pvc status failed: %v", err), testResponseData{true, true} + return true, ne.pluginResizeOpts.NewSize, fmt.Errorf("mountVolume.NodeExpandVolume update pvc status failed: %v", err), testResponseData{true, true} } - return true, nil, testResponseData{true, true} + return true, ne.pluginResizeOpts.NewSize, nil, testResponseData{true, true} } diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index 40d09e15c26..16709147295 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -162,7 +162,7 @@ func TestNodeExpander(t *testing.T) { ogInstance, _ := og.(*operationGenerator) nodeExpander := newNodeExpander(resizeOp, ogInstance.kubeClient, ogInstance.recorder) - _, err, expansionResponse := nodeExpander.expandOnPlugin() + _, _, err, expansionResponse := nodeExpander.expandOnPlugin() pvc = nodeExpander.pvc pvcStatusCap := pvc.Status.Capacity[v1.ResourceStorage] diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index d54de2cc2f0..9fda17f9547 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1957,14 +1957,14 @@ func (og *operationGenerator) doOnlineExpansion(volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, resizeOptions volume.NodeResizeOptions) (bool, error, error) { - resizeDone, err := og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) + resizeDone, newSize, err := og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions) if err != nil { e1, e2 := volumeToMount.GenerateError("NodeExpandVolume.NodeExpandVolume failed", err) klog.Errorf(e2.Error()) return false, e1, e2 } if resizeDone { - markingDone := actualStateOfWorld.MarkVolumeAsResized(volumeToMount.VolumeName, &resizeOptions.NewSize) + markingDone := actualStateOfWorld.MarkVolumeAsResized(volumeToMount.VolumeName, &newSize) if !markingDone { // On failure, return error. Caller will log and retry. genericFailureError := fmt.Errorf("unable to mark volume as resized") @@ -2013,7 +2013,7 @@ func (og *operationGenerator) expandVolumeDuringMount(volumeToMount VolumeToMoun rsOpts.NewSize = pvc.Status.AllocatedResources[v1.ResourceStorage] resizeOp.pluginResizeOpts = rsOpts nodeExpander := newNodeExpander(resizeOp, og.kubeClient, og.recorder) - resizeFinished, err, _ := nodeExpander.expandOnPlugin() + resizeFinished, _, err, _ := nodeExpander.expandOnPlugin() return resizeFinished, err } else { return og.legacyCallNodeExpandOnPlugin(resizeOp) @@ -2044,7 +2044,7 @@ func (og *operationGenerator) checkIfSupportsNodeExpansion(volumeToMount VolumeT func (og *operationGenerator) nodeExpandVolume( volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, - rsOpts volume.NodeResizeOptions) (bool, error) { + rsOpts volume.NodeResizeOptions) (bool, resource.Quantity, error) { supportsExpansion, expandableVolumePlugin := og.checkIfSupportsNodeExpansion(volumeToMount) @@ -2053,9 +2053,10 @@ func (og *operationGenerator) nodeExpandVolume( if rsOpts.NewSize.Cmp(rsOpts.OldSize) > 0 { pv := volumeToMount.VolumeSpec.PersistentVolume pvc, err := og.kubeClient.CoreV1().PersistentVolumeClaims(pv.Spec.ClaimRef.Namespace).Get(context.TODO(), pv.Spec.ClaimRef.Name, metav1.GetOptions{}) + currentSize := pvc.Status.Capacity.Storage() if err != nil { // Return error rather than leave the file system un-resized, caller will log and retry - return false, fmt.Errorf("mountVolume.NodeExpandVolume get PVC failed : %v", err) + return false, *currentSize, fmt.Errorf("mountVolume.NodeExpandVolume get PVC failed : %v", err) } if volumeToMount.VolumeSpec.ReadOnly { @@ -2063,7 +2064,7 @@ func (og *operationGenerator) nodeExpandVolume( klog.Warningf(detailedMsg) og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FileSystemResizeFailed, simpleMsg) og.recorder.Eventf(pvc, v1.EventTypeWarning, kevents.FileSystemResizeFailed, simpleMsg) - return true, nil + return true, *currentSize, nil } resizeOp := nodeResizeOperationOpts{ vmt: volumeToMount, @@ -2076,17 +2077,19 @@ func (og *operationGenerator) nodeExpandVolume( if og.checkForRecoveryFromExpansion(pvc, volumeToMount) { // if recovery feature is enabled, we can use allocated size from PVC status as new size - rsOpts.NewSize = pvc.Status.AllocatedResources[v1.ResourceStorage] - resizeOp.pluginResizeOpts = rsOpts + newSize := pvc.Status.AllocatedResources[v1.ResourceStorage] + rsOpts.NewSize = newSize + resizeOp.pluginResizeOpts.NewSize = newSize nodeExpander := newNodeExpander(resizeOp, og.kubeClient, og.recorder) - resizeFinished, err, _ := nodeExpander.expandOnPlugin() - return resizeFinished, err + resizeFinished, newSize, err, _ := nodeExpander.expandOnPlugin() + return resizeFinished, newSize, err } else { - return og.legacyCallNodeExpandOnPlugin(resizeOp) + resizeFinished, err := og.legacyCallNodeExpandOnPlugin(resizeOp) + return resizeFinished, rsOpts.NewSize, err } } } - return true, nil + return true, rsOpts.OldSize, nil } func (og *operationGenerator) checkForRecoveryFromExpansion(pvc *v1.PersistentVolumeClaim, volumeToMount VolumeToMount) bool { diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 0c267222e89..9689590e3e8 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/pkg/volume/util/operationexecutor/operation_generator_test.go @@ -305,7 +305,7 @@ func TestOperationGenerator_nodeExpandVolume(t *testing.T) { asow := newFakeActualStateOfWorld() ogInstance, _ := og.(*operationGenerator) - _, err := ogInstance.nodeExpandVolume(vmt, asow, pluginResizeOpts) + _, _, err := ogInstance.nodeExpandVolume(vmt, asow, pluginResizeOpts) if !test.expectError && err != nil { t.Errorf("For test %s, expected no error got: %v", test.name, err)