Merge pull request #131987 from gnufied/automated-cherry-pick-of-#131868-upstream-release-1.33

Automated cherry pick of #131868: Remove superflous expansion calls if controller finished expansion

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 &&
+	if ne.pvcAlreadyUpdated && storage.ContainsAccessMode(ne.pvc.Spec.AccessModes, v1.ReadWriteMany) {
-		ne.resizeStatus == "" &&
-		storage.ContainsAccessMode(ne.pvc.Spec.AccessModes, v1.ReadWriteMany) {
-		ne.pvcAlreadyUpdated = true
 		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

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
+		expectedResizeStatus v1.ClaimResourceStatus
-		expectedStatusSize       resource.Quantity
+		expectedStatusSize   resource.Quantity
-		expectResizeCall         bool
+		// whether resize call was made to the plugin
-		expectFinalErrors        bool
+		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",
+			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),
+			pvc:                           addAccessMode(getTestPVC(volumetesting.FailWithUnSupportedVolumeName, "2G", "2G", "2G", nil), v1.ReadWriteMany),
-			pv:                       getTestPV(volumetesting.FailWithUnSupportedVolumeName, "2G"),
+			pv:                            getTestPV(volumetesting.FailWithUnSupportedVolumeName, "2G"),
-			expectError:              false,
+			expectError:                   false,
-			expectedResizeStatus:     "",
+			recoverVolumeExpansionFailure: true,
-			expectResizeCall:         false,
+			expectedResizeStatus:          "",
-			assumeResizeOpAsFinished: true,
+			expectResizeCall:              false,
-			expectFinalErrors:        false,
+			expectedReturnValue:           true,
-			expectedStatusSize:       resource.MustParse("2G"),
+			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)
 			}