diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index f6e52473fb7..2c6b2d4b63a 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/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} } diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index 40d09e15c26..2c13cdb5111 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/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 - pvc *v1.PersistentVolumeClaim - pv *v1.PersistentVolume + name string + pvc *v1.PersistentVolumeClaim + 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", - pvc: getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed), - pv: getTestPV("test-vol0", "2G"), + 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, @@ -78,9 +80,11 @@ func TestNodeExpander(t *testing.T) { expectedStatusSize: resource.MustParse("1G"), }, { - name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending", - pvc: getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending), - pv: getTestPV("test-vol0", "2G"), + 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, @@ -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", - pvc: getTestPVC(volumetesting.InfeasibleNodeExpansion, "2G", "1G", "2G", &nodeResizePending), - pv: getTestPV(volumetesting.InfeasibleNodeExpansion, "2G"), - expectError: true, - expectedResizeStatus: nodeResizeFailed, - expectResizeCall: true, - assumeResizeOpAsFinished: true, - expectFinalErrors: true, - expectedStatusSize: resource.MustParse("1G"), + 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, + assumeResizeOpAsFinished: true, + expectFinalErrors: true, + expectedStatusSize: resource.MustParse("1G"), }, { - 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"), - expectError: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeInProgress, - expectResizeCall: true, - assumeResizeOpAsFinished: true, - expectFinalErrors: true, - expectedStatusSize: resource.MustParse("1G"), + 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, + assumeResizeOpAsFinished: true, + expectFinalErrors: true, + expectedStatusSize: resource.MustParse("1G"), }, { - name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", - pvc: getTestPVC("test-vol0", "2G", "2G", "2G", nil), - pv: getTestPV("test-vol0", "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, @@ -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 diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index b450a8f0ddb..d4e4d555ce4 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/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 } diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 0c267222e89..6bd7f51e04c 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/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{ diff --git a/pkg/volume/util/resize_util.go b/pkg/volume/util/resize_util.go index 2bf54b4b85c..599f220976e 100644 --- a/pkg/volume/util/resize_util.go +++ b/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) {