Merge pull request #131439 from gnufied/automated-cherry-pick-of-#131418-upstream-release-1.31

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

pkg/volume/util/operationexecutor/node_expander.go

@@ -174,7 +174,7 @@ func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) {
 	}
 
 	// 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, fmt.Errorf("mountVolume.NodeExpandVolume update pvc status failed: %v", err), testResponseData{true, true}
 	}

pkg/volume/util/operationexecutor/node_expander_test.go

@@ -52,6 +52,7 @@ func TestNodeExpander(t *testing.T) {
 		name                          string
 		pvc                           *v1.PersistentVolumeClaim
 		pv                            *v1.PersistentVolume
+		recoverVolumeExpansionFailure bool
 
 		// desired size, defaults to pv.Spec.Capacity
 		desiredSize *resource.Quantity
@@ -70,6 +71,7 @@ func TestNodeExpander(t *testing.T) {
 			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed",
 			pvc:                           getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed),
 			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: true,
 
 			expectedResizeStatus:     nodeResizeFailed,
 			expectResizeCall:         false,
@@ -81,6 +83,8 @@ func TestNodeExpander(t *testing.T) {
 			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending",
 			pvc:                           getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending),
 			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: true,
+
 			expectedResizeStatus:     "",
 			expectResizeCall:         true,
 			assumeResizeOpAsFinished: true,
@@ -91,6 +95,7 @@ func TestNodeExpander(t *testing.T) {
 			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=infeasible",
 			pvc:                           getTestPVC(volumetesting.InfeasibleNodeExpansion, "2G", "1G", "2G", &nodeResizePending),
 			pv:                            getTestPV(volumetesting.InfeasibleNodeExpansion, "2G"),
+			recoverVolumeExpansionFailure: false,
 			expectError:                   true,
 			expectedResizeStatus:          nodeResizeFailed,
 			expectResizeCall:              true,
@@ -102,6 +107,7 @@ func TestNodeExpander(t *testing.T) {
 			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing",
 			pvc:                           getTestPVC(volumetesting.OtherFinalNodeExpansionError, "2G", "1G", "2G", &nodeResizePending),
 			pv:                            getTestPV(volumetesting.OtherFinalNodeExpansionError, "2G"),
+			recoverVolumeExpansionFailure: true,
 			expectError:                   true,
 			expectedResizeStatus:          v1.PersistentVolumeClaimNodeResizeInProgress,
 			expectResizeCall:              true,
@@ -110,9 +116,10 @@ func TestNodeExpander(t *testing.T) {
 			expectedStatusSize:            resource.MustParse("1G"),
 		},
 		{
-			name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
+			name:                          "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, featuregate=disabled",
-			pvc:  getTestPVC("test-vol0", "2G", "2G", "2G", nil),
+			pvc:                           getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending),
 			pv:                            getTestPV("test-vol0", "2G"),
+			recoverVolumeExpansionFailure: false,
 
 			expectedResizeStatus:     "",
 			expectResizeCall:         true,
@@ -125,7 +132,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

@@ -2089,6 +2089,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) {