Merge pull request #131438 from gnufied/automated-cherry-pick-of-#131418-upstream-release-1.32

Automated cherry pick of #131418: Check for newer fields when deciding expansion recovery feature status

pkg/volume/util/operationexecutor/node_expander.go

@@ -184,7 +184,7 @@ func (ne *NodeExpander) expandOnPlugin() (bool, resource.Quantity, error) {
 	}
 
 	// 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)
+	ne.pvc, err = util.MarkNodeExpansionFinishedWithRecovery(ne.pvc, ne.pluginResizeOpts.NewSize, ne.kubeClient)
 	if err != nil {
 		return true, ne.pluginResizeOpts.NewSize, fmt.Errorf("mountVolume.NodeExpandVolume update pvc status failed: %w", err)
 	}

pkg/volume/util/operationexecutor/node_expander_test.go

@@ -49,9 +49,10 @@ func TestNodeExpander(t *testing.T) {
 
 	nodeResizePending := v1.PersistentVolumeClaimNodeResizePending
 	var tests = []struct {
-		name string
+		name                          string
-		pvc  *v1.PersistentVolumeClaim
+		pvc                           *v1.PersistentVolumeClaim
-		pv   *v1.PersistentVolume
+		pv                            *v1.PersistentVolume
+		recoverVolumeExpansionFailure bool
 
 		// desired size, defaults to pv.Spec.Capacity
 		desiredSize *resource.Quantity
@@ -67,9 +68,10 @@ func TestNodeExpander(t *testing.T) {
 		expectError              bool
 	}{
 		{
-			name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed",
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed",
-			pvc:  getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed),
+			pvc:                           getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed),
-			pv:   getTestPV("test-vol0", "2G"),
+			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: true,
 
 			expectedResizeStatus:     nodeResizeFailed,
 			expectResizeCall:         false,
@@ -78,9 +80,11 @@ func TestNodeExpander(t *testing.T) {
 			expectedStatusSize:       resource.MustParse("1G"),
 		},
 		{
-			name:                     "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending",
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending",
-			pvc:                      getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending),
+			pvc:                           getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending),
-			pv:                       getTestPV("test-vol0", "2G"),
+			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: true,
+
 			expectedResizeStatus:     "",
 			expectResizeCall:         true,
 			assumeResizeOpAsFinished: true,
@@ -88,31 +92,34 @@ func TestNodeExpander(t *testing.T) {
 			expectedStatusSize:       resource.MustParse("2G"),
 		},
 		{
-			name:                     "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=infeasible",
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=infeasible",
-			pvc:                      getTestPVC(volumetesting.InfeasibleNodeExpansion, "2G", "1G", "2G", &nodeResizePending),
+			pvc:                           getTestPVC(volumetesting.InfeasibleNodeExpansion, "2G", "1G", "2G", &nodeResizePending),
-			pv:                       getTestPV(volumetesting.InfeasibleNodeExpansion, "2G"),
+			pv:                            getTestPV(volumetesting.InfeasibleNodeExpansion, "2G"),
-			expectError:              true,
+			recoverVolumeExpansionFailure: false,
-			expectedResizeStatus:     nodeResizeFailed,
+			expectError:                   true,
-			expectResizeCall:         true,
+			expectedResizeStatus:          nodeResizeFailed,
-			assumeResizeOpAsFinished: true,
+			expectResizeCall:              true,
-			expectFinalErrors:        true,
+			assumeResizeOpAsFinished:      true,
-			expectedStatusSize:       resource.MustParse("1G"),
+			expectFinalErrors:             true,
+			expectedStatusSize:            resource.MustParse("1G"),
 		},
 		{
-			name:                     "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing",
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing",
-			pvc:                      getTestPVC(volumetesting.OtherFinalNodeExpansionError, "2G", "1G", "2G", &nodeResizePending),
+			pvc:                           getTestPVC(volumetesting.OtherFinalNodeExpansionError, "2G", "1G", "2G", &nodeResizePending),
-			pv:                       getTestPV(volumetesting.OtherFinalNodeExpansionError, "2G"),
+			pv:                            getTestPV(volumetesting.OtherFinalNodeExpansionError, "2G"),
-			expectError:              true,
+			recoverVolumeExpansionFailure: true,
-			expectedResizeStatus:     v1.PersistentVolumeClaimNodeResizeInProgress,
+			expectError:                   true,
-			expectResizeCall:         true,
+			expectedResizeStatus:          v1.PersistentVolumeClaimNodeResizeInProgress,
-			assumeResizeOpAsFinished: true,
+			expectResizeCall:              true,
-			expectFinalErrors:        true,
+			assumeResizeOpAsFinished:      true,
-			expectedStatusSize:       resource.MustParse("1G"),
+			expectFinalErrors:             true,
+			expectedStatusSize:            resource.MustParse("1G"),
 		},
 		{
-			name: "RWO volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
+			name:                          "RWO volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
-			pvc:  getTestPVC("test-vol0", "2G", "2G", "2G", nil),
+			pvc:                           getTestPVC("test-vol0", "2G", "2G", "2G", nil),
-			pv:   getTestPV("test-vol0", "2G"),
+			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: false,
 
 			expectedResizeStatus:     "",
 			expectResizeCall:         false,
@@ -121,9 +128,22 @@ func TestNodeExpander(t *testing.T) {
 			expectedStatusSize:       resource.MustParse("2G"),
 		},
 		{
-			name: "RWX volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
+			name:                          "RWX volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
-			pvc:  addAccessMode(getTestPVC("test-vol0", "2G", "2G", "2G", nil), v1.ReadWriteMany),
+			pvc:                           addAccessMode(getTestPVC("test-vol0", "2G", "2G", "2G", nil), v1.ReadWriteMany),
-			pv:   getTestPV("test-vol0", "2G"),
+			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: true,
+
+			expectedResizeStatus:     "",
+			expectResizeCall:         true,
+			assumeResizeOpAsFinished: true,
+			expectFinalErrors:        false,
+			expectedStatusSize:       resource.MustParse("2G"),
+		},
+		{
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, featuregate=disabled",
+			pvc:                           getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending),
+			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: false,
 
 			expectedResizeStatus:     "",
 			expectResizeCall:         true,
@@ -136,7 +156,7 @@ func TestNodeExpander(t *testing.T) {
 	for i := range tests {
 		test := tests[i]
 		t.Run(test.name, func(t *testing.T) {
-			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, true)
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.recoverVolumeExpansionFailure)
 			volumePluginMgr, fakePlugin := volumetesting.GetTestKubeletVolumePluginMgr(t)
 
 			pvc := test.pvc

pkg/volume/util/operationexecutor/operation_generator.go

@@ -2075,6 +2075,11 @@ func (og *operationGenerator) checkForRecoveryFromExpansion(pvc *v1.PersistentVo
 	featureGateStatus := utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure)
 
 	if !featureGateStatus {
+		// even though RecoverVolumeExpansionFailure feature-gate is disabled, we should consider it enabled
+		// if resizeStatus is not empty or allocatedresources is set
+		if resizeStatus != "" || allocatedResource != nil {
+			return true
+		}
 		return false
 	}
 

pkg/volume/util/operationexecutor/operation_generator_test.go

@@ -402,6 +402,109 @@ func TestExpandDuringMount(t *testing.T) {
 		})
 	}
 }
+func TestCheckForRecoveryFromExpansion(t *testing.T) {
+	tests := []struct {
+		name                  string
+		pvc                   *v1.PersistentVolumeClaim
+		featureGateEnabled    bool
+		expectedRecoveryCheck bool
+	}{
+		{
+			name: "feature gate disabled, no resize status or allocated resources",
+			pvc: &v1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pvc-1",
+				},
+				Status: v1.PersistentVolumeClaimStatus{
+					AllocatedResourceStatuses: nil,
+					AllocatedResources:        nil,
+				},
+			},
+			featureGateEnabled:    false,
+			expectedRecoveryCheck: false,
+		},
+		{
+			name: "feature gate disabled, resize status set",
+			pvc: &v1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pvc-2",
+				},
+				Status: v1.PersistentVolumeClaimStatus{
+					AllocatedResourceStatuses: map[v1.ResourceName]v1.ClaimResourceStatus{
+						v1.ResourceStorage: v1.PersistentVolumeClaimNodeResizePending,
+					},
+				},
+			},
+			featureGateEnabled:    false,
+			expectedRecoveryCheck: true,
+		},
+		{
+			name: "feature gate enabled, resize status and allocated resources set",
+			pvc: &v1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pvc-3",
+				},
+				Status: v1.PersistentVolumeClaimStatus{
+					AllocatedResourceStatuses: map[v1.ResourceName]v1.ClaimResourceStatus{
+						v1.ResourceStorage: v1.PersistentVolumeClaimNodeResizePending,
+					},
+					AllocatedResources: v1.ResourceList{
+						v1.ResourceStorage: resource.MustParse("10Gi"),
+					},
+				},
+			},
+			featureGateEnabled:    true,
+			expectedRecoveryCheck: true,
+		},
+		{
+			name: "feature gate enabled, no resize status or allocated resources",
+			pvc: &v1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pvc-4",
+				},
+				Status: v1.PersistentVolumeClaimStatus{
+					AllocatedResourceStatuses: nil,
+					AllocatedResources:        nil,
+				},
+			},
+			featureGateEnabled:    true,
+			expectedRecoveryCheck: false,
+		},
+		{
+			name: "feature gate enabled, older external resize controller",
+			pvc: &v1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pvc-5",
+				},
+				Status: v1.PersistentVolumeClaimStatus{
+					AllocatedResourceStatuses: nil,
+					AllocatedResources:        nil,
+				},
+			},
+			featureGateEnabled:    true,
+			expectedRecoveryCheck: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.featureGateEnabled)
+
+			pod := getTestPod("test-pod", test.pvc.Name)
+			pv := getTestPV("test-vol0", "2G")
+			og := &operationGenerator{}
+
+			vmt := VolumeToMount{
+				Pod:        pod,
+				VolumeName: v1.UniqueVolumeName(pv.Name),
+				VolumeSpec: volume.NewSpecFromPersistentVolume(pv, false),
+			}
+			result := og.checkForRecoveryFromExpansion(test.pvc, vmt)
+
+			assert.Equal(t, test.expectedRecoveryCheck, result, "unexpected recovery check result for test: %s", test.name)
+		})
+	}
+}
 
 func getTestPod(podName, pvcName string) *v1.Pod {
 	return &v1.Pod{

pkg/volume/util/resize_util.go

@@ -236,6 +236,27 @@ func MarkFSResizeFinished(
 	return updatedPVC, err
 }
 
+func MarkNodeExpansionFinishedWithRecovery(
+	pvc *v1.PersistentVolumeClaim,
+	newSize resource.Quantity,
+	kubeClient clientset.Interface) (*v1.PersistentVolumeClaim, error) {
+	newPVC := pvc.DeepCopy()
+
+	newPVC.Status.Capacity[v1.ResourceStorage] = newSize
+
+	allocatedResourceStatusMap := newPVC.Status.AllocatedResourceStatuses
+	delete(allocatedResourceStatusMap, v1.ResourceStorage)
+	if len(allocatedResourceStatusMap) == 0 {
+		newPVC.Status.AllocatedResourceStatuses = nil
+	} else {
+		newPVC.Status.AllocatedResourceStatuses = allocatedResourceStatusMap
+	}
+
+	newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{}, false /* keepOldResizeConditions */)
+	updatedPVC, err := PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient)
+	return updatedPVC, err
+}
+
 // MarkNodeExpansionInfeasible marks a PVC for node expansion as failed. Kubelet should not retry expansion
 // of volumes which are in failed state.
 func MarkNodeExpansionInfeasible(pvc *v1.PersistentVolumeClaim, kubeClient clientset.Interface, err error) (*v1.PersistentVolumeClaim, error) {