diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index a7c98725543..9c7767955c9 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/pkg/volume/util/operationexecutor/node_expander.go @@ -96,12 +96,13 @@ func (ne *NodeExpander) runPreCheck() bool { ne.markExpansionInfeasibleOnFailure = true } + if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && ne.resizeStatus == "" { + ne.pvcAlreadyUpdated = true + } + // PVC is already expanded but we are still trying to expand the volume because // last recorded size in ASOW is older. This can happen for RWX volume types. - if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && - ne.resizeStatus == "" && - storage.ContainsAccessMode(ne.pvc.Spec.AccessModes, v1.ReadWriteMany) { - ne.pvcAlreadyUpdated = true + if ne.pvcAlreadyUpdated && storage.ContainsAccessMode(ne.pvc.Spec.AccessModes, v1.ReadWriteMany) { return true } @@ -124,6 +125,14 @@ func (ne *NodeExpander) runPreCheck() bool { func (ne *NodeExpander) expandOnPlugin() (bool, resource.Quantity, error) { allowExpansion := ne.runPreCheck() if !allowExpansion { + if ne.pvcAlreadyUpdated { + // if pvc is already updated, then we could be here because size stored in ASOW is smaller and controller did full + // expansion and hence no node expansion is needed. + // This will stop reconciler from retrying expansion on the node. + ne.testStatus = testResponseData{assumeResizeFinished: true, resizeCalledOnPlugin: false} + return true, ne.pluginResizeOpts.NewSize, nil + } + klog.V(3).Infof("NodeExpandVolume is not allowed to proceed for volume %s with resizeStatus %s", ne.vmt.VolumeName, ne.resizeStatus) ne.testStatus = testResponseData{false /* resizeCalledOnPlugin */, true /* assumeResizeFinished */} return false, ne.pluginResizeOpts.OldSize, nil diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index 6b339d71c9b..6f8c4d83c90 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -60,10 +60,14 @@ func TestNodeExpander(t *testing.T) { actualSize *resource.Quantity // expectations of test - expectedResizeStatus v1.ClaimResourceStatus - expectedStatusSize resource.Quantity - expectResizeCall bool - expectFinalErrors bool + expectedResizeStatus v1.ClaimResourceStatus + expectedStatusSize resource.Quantity + // whether resize call was made to the plugin + expectResizeCall bool + expectFinalErrors bool + expectedReturnValue bool + + // whether resize operation was assumed as finished assumeResizeOpAsFinished bool expectError bool }{ @@ -75,6 +79,7 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: nodeResizeFailed, expectResizeCall: false, + expectedReturnValue: false, assumeResizeOpAsFinished: true, expectFinalErrors: false, expectedStatusSize: resource.MustParse("1G"), @@ -87,6 +92,7 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: "", expectResizeCall: true, + expectedReturnValue: true, assumeResizeOpAsFinished: true, expectFinalErrors: false, expectedStatusSize: resource.MustParse("2G"), @@ -100,6 +106,7 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: nodeResizeFailed, expectResizeCall: true, assumeResizeOpAsFinished: true, + expectedReturnValue: false, expectFinalErrors: true, expectedStatusSize: resource.MustParse("1G"), }, @@ -111,6 +118,7 @@ func TestNodeExpander(t *testing.T) { expectError: true, expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeInProgress, expectResizeCall: true, + expectedReturnValue: false, assumeResizeOpAsFinished: true, expectFinalErrors: true, expectedStatusSize: resource.MustParse("1G"), @@ -124,6 +132,7 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: "", expectResizeCall: false, assumeResizeOpAsFinished: true, + expectedReturnValue: true, expectFinalErrors: false, expectedStatusSize: resource.MustParse("2G"), }, @@ -136,6 +145,7 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: "", expectResizeCall: true, assumeResizeOpAsFinished: true, + expectedReturnValue: true, expectFinalErrors: false, expectedStatusSize: resource.MustParse("2G"), }, @@ -148,19 +158,22 @@ func TestNodeExpander(t *testing.T) { expectedResizeStatus: "", expectResizeCall: true, assumeResizeOpAsFinished: true, + expectedReturnValue: true, expectFinalErrors: false, expectedStatusSize: resource.MustParse("2G"), }, { - name: "RWX pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize, reize_op=unsupported", - pvc: addAccessMode(getTestPVC(volumetesting.FailWithUnSupportedVolumeName, "2G", "2G", "2G", nil), v1.ReadWriteMany), - pv: getTestPV(volumetesting.FailWithUnSupportedVolumeName, "2G"), - expectError: false, - expectedResizeStatus: "", - expectResizeCall: false, - assumeResizeOpAsFinished: true, - expectFinalErrors: false, - expectedStatusSize: resource.MustParse("2G"), + name: "RWX pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize, reize_op=unsupported", + pvc: addAccessMode(getTestPVC(volumetesting.FailWithUnSupportedVolumeName, "2G", "2G", "2G", nil), v1.ReadWriteMany), + pv: getTestPV(volumetesting.FailWithUnSupportedVolumeName, "2G"), + expectError: false, + recoverVolumeExpansionFailure: true, + expectedResizeStatus: "", + expectResizeCall: false, + expectedReturnValue: true, + assumeResizeOpAsFinished: true, + expectFinalErrors: false, + expectedStatusSize: resource.MustParse("2G"), }, } @@ -204,7 +217,7 @@ func TestNodeExpander(t *testing.T) { ogInstance, _ := og.(*operationGenerator) nodeExpander := newNodeExpander(resizeOp, ogInstance.kubeClient, ogInstance.recorder) - _, _, err := nodeExpander.expandOnPlugin() + returnValue, _, err := nodeExpander.expandOnPlugin() expansionResponse := nodeExpander.testStatus pvc = nodeExpander.pvc @@ -217,6 +230,10 @@ func TestNodeExpander(t *testing.T) { t.Errorf("For test %s, expected error but got none", test.name) } + if test.expectedReturnValue != returnValue { + t.Errorf("For test %s, expected return value %t, got %t", test.name, test.expectedReturnValue, returnValue) + } + if test.expectResizeCall != expansionResponse.resizeCalledOnPlugin { t.Errorf("For test %s, expected resize called %t, got %t", test.name, test.expectResizeCall, expansionResponse.resizeCalledOnPlugin) }