Add more tests for volume recovery cases

pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go

@@ -333,13 +333,9 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes(
 
 }
 
-// checkVolumeFSResize checks whether a PVC mounted by the pod requires file
+// checkVolumeFSResize records desired PVC size for a volume mounted by the pod.
-// system resize or not. If so, marks this volume as fsResizeRequired in ASW.
+// It is used for comparison with actual size(coming from pvc.Status.Capacity) and calling
-// - mountedVolumesForPod stores all mounted volumes in ASW, because online
+// volume expansion on the node if needed.
-//   volume resize only considers mounted volumes.
-// - processedVolumesForFSResize stores all volumes we have checked in current loop,
-//   because file system resize operation is a global operation for volume, so
-//   we only need to check it once if more than one pod use it.
 func (dswp *desiredStateOfWorldPopulator) checkVolumeFSResize(
 	pod *v1.Pod,
 	podVolume v1.Volume,

pkg/volume/testing/testing.go

@@ -175,6 +175,7 @@ type FakeVolumePlugin struct {
 	LastProvisionerOptions VolumeOptions
 	NewAttacherCallCount   int
 	NewDetacherCallCount   int
+	NodeExpandCallCount    int
 	VolumeLimits           map[string]int64
 	VolumeLimitsError      error
 	LimitKey               string
@@ -471,6 +472,7 @@ func (plugin *FakeVolumePlugin) RequiresFSResize() bool {
 }
 
 func (plugin *FakeVolumePlugin) NodeExpand(resizeOptions NodeResizeOptions) (bool, error) {
+	plugin.NodeExpandCallCount++
 	if resizeOptions.VolumeSpec.Name() == FailWithInUseVolumeName {
 		return false, volumetypes.NewFailedPreconditionError("volume-in-use")
 	}

pkg/volume/util/operationexecutor/node_expander_test.go

@@ -29,10 +29,14 @@ import (
 
 func TestNodeExpander(t *testing.T) {
 	var tests = []struct {
-		name               string
+		name string
-		pvc                *v1.PersistentVolumeClaim
+		pvc  *v1.PersistentVolumeClaim
-		pv                 *v1.PersistentVolume
+		pv   *v1.PersistentVolume
-		recoverFeatureGate bool
+
+		// desired size, defaults to pv.Spec.Capacity
+		desiredSize *resource.Quantity
+		// actualSize, defaults to pvc.Status.Capacity
+		actualSize *resource.Quantity
 
 		// expectations of test
 		expectedResizeStatus     v1.PersistentVolumeClaimResizeStatus
@@ -42,10 +46,9 @@ 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", "", v1.PersistentVolumeClaimNodeExpansionFailed),
+			pvc:  getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNodeExpansionFailed),
-			pv:                 getTestPV("test-vol0", "2G"),
+			pv:   getTestPV("test-vol0", "2G"),
-			recoverFeatureGate: true,
 
 			expectedResizeStatus:     v1.PersistentVolumeClaimNodeExpansionFailed,
 			expectResizeCall:         false,
@@ -56,7 +59,6 @@ func TestNodeExpander(t *testing.T) {
 			name:                     "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending",
 			pvc:                      getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending),
 			pv:                       getTestPV("test-vol0", "2G"),
-			recoverFeatureGate:       true,
 			expectedResizeStatus:     v1.PersistentVolumeClaimNoExpansionInProgress,
 			expectResizeCall:         true,
 			assumeResizeOpAsFinished: true,
@@ -66,19 +68,28 @@ func TestNodeExpander(t *testing.T) {
 			name:                     "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing",
 			pvc:                      getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending),
 			pv:                       getTestPV(volumetesting.AlwaysFailNodeExpansion, "2G"),
-			recoverFeatureGate:       true,
 			expectError:              true,
 			expectedResizeStatus:     v1.PersistentVolumeClaimNodeExpansionFailed,
 			expectResizeCall:         true,
 			assumeResizeOpAsFinished: true,
 			expectedStatusSize:       resource.MustParse("1G"),
 		},
+		{
+			name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
+			pvc:  getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress),
+			pv:   getTestPV("test-vol0", "2G"),
+
+			expectedResizeStatus:     v1.PersistentVolumeClaimNoExpansionInProgress,
+			expectResizeCall:         true,
+			assumeResizeOpAsFinished: true,
+			expectedStatusSize:       resource.MustParse("2G"),
+		},
 	}
 
 	for i := range tests {
 		test := tests[i]
 		t.Run(test.name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.recoverFeatureGate)()
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, true)()
 			volumePluginMgr, fakePlugin := volumetesting.GetTestKubeletVolumePluginMgr(t)
 
 			pvc := test.pvc
@@ -91,6 +102,14 @@ func TestNodeExpander(t *testing.T) {
 				VolumeName: v1.UniqueVolumeName(pv.Name),
 				VolumeSpec: volume.NewSpecFromPersistentVolume(pv, false),
 			}
+			desiredSize := test.desiredSize
+			if desiredSize == nil {
+				desiredSize = pv.Spec.Capacity.Storage()
+			}
+			actualSize := test.actualSize
+			if actualSize == nil {
+				actualSize = pvc.Status.Capacity.Storage()
+			}
 			resizeOp := nodeResizeOperationOpts{
 				pvc:                pvc,
 				pv:                 pv,
@@ -99,8 +118,8 @@ func TestNodeExpander(t *testing.T) {
 				actualStateOfWorld: nil,
 				pluginResizeOpts: volume.NodeResizeOptions{
 					VolumeSpec: vmt.VolumeSpec,
-					NewSize:    *pv.Spec.Capacity.Storage(),
+					NewSize:    *desiredSize,
-					OldSize:    *pvc.Status.Capacity.Storage(),
+					OldSize:    *actualSize,
 				},
 			}
 			ogInstance, _ := og.(*operationGenerator)

pkg/volume/util/operationexecutor/operation_generator.go

@@ -160,6 +160,8 @@ type OperationGenerator interface {
 	GenerateExpandAndRecoverVolumeFunc(*v1.PersistentVolumeClaim, *v1.PersistentVolume, string) (volumetypes.GeneratedOperations, error)
 
 	// Generates the volume file system resize function, which can resize volume's file system to expected size without unmounting the volume.
+	// Along with volumeToMount and actualStateOfWorld, the function expects current size of volume on the node as an argument. The current
+	// size here always refers to capacity last recorded in actualStateOfWorld from pvc.Status.Capacity
 	GenerateExpandInUseVolumeFunc(volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, currentSize resource.Quantity) (volumetypes.GeneratedOperations, error)
 }
 

pkg/volume/util/operationexecutor/operation_generator_test.go

@@ -17,9 +17,11 @@ limitations under the License.
 package operationexecutor
 
 import (
+	"fmt"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	core "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/record"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/kubernetes/pkg/features"
@@ -209,6 +211,135 @@ func TestOperationGenerator_GenerateExpandAndRecoverVolumeFunc(t *testing.T) {
 	}
 }
 
+func TestOperationGenerator_nodeExpandVolume(t *testing.T) {
+	getSizeFunc := func(size string) *resource.Quantity {
+		x := resource.MustParse(size)
+		return &x
+	}
+
+	var tests = []struct {
+		name string
+		pvc  *v1.PersistentVolumeClaim
+		pv   *v1.PersistentVolume
+
+		// desired size, defaults to pv.Spec.Capacity
+		desiredSize *resource.Quantity
+		// actualSize, defaults to pvc.Status.Capacity
+		actualSize *resource.Quantity
+
+		// expectations of test
+		expectedResizeStatus v1.PersistentVolumeClaimResizeStatus
+		expectedStatusSize   resource.Quantity
+		resizeCallCount      int
+		expectError          bool
+	}{
+		{
+			name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed",
+			pvc:  getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNodeExpansionFailed),
+			pv:   getTestPV("test-vol0", "2G"),
+
+			expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed,
+			resizeCallCount:      0,
+			expectedStatusSize:   resource.MustParse("1G"),
+		},
+		{
+			name:                 "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending",
+			pvc:                  getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending),
+			pv:                   getTestPV("test-vol0", "2G"),
+			expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress,
+			resizeCallCount:      1,
+			expectedStatusSize:   resource.MustParse("2G"),
+		},
+		{
+			name:                 "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing",
+			pvc:                  getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending),
+			pv:                   getTestPV(volumetesting.AlwaysFailNodeExpansion, "2G"),
+			expectError:          true,
+			expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed,
+			resizeCallCount:      1,
+			expectedStatusSize:   resource.MustParse("1G"),
+		},
+		{
+			name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize = actualSize",
+			pvc:  getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress),
+			pv:   getTestPV("test-vol0", "2G"),
+
+			expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress,
+			resizeCallCount:      0,
+			expectedStatusSize:   resource.MustParse("2G"),
+		},
+		{
+			name:        "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize",
+			pvc:         getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress),
+			pv:          getTestPV("test-vol0", "2G"),
+			desiredSize: getSizeFunc("2G"),
+			actualSize:  getSizeFunc("1G"),
+
+			expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress,
+			resizeCallCount:      1,
+			expectedStatusSize:   resource.MustParse("2G"),
+		},
+		{
+			name:        "pv.spec.cap = pvc.status.cap, resizeStatus=node-expansion-failed, desiredSize > actualSize",
+			pvc:         getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNodeExpansionFailed),
+			pv:          getTestPV("test-vol0", "2G"),
+			desiredSize: getSizeFunc("2G"),
+			actualSize:  getSizeFunc("1G"),
+
+			expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed,
+			resizeCallCount:      0,
+			expectedStatusSize:   resource.MustParse("2G"),
+		},
+	}
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, true)()
+			volumePluginMgr, fakePlugin := volumetesting.GetTestKubeletVolumePluginMgr(t)
+			test.pv.Spec.ClaimRef = &v1.ObjectReference{
+				Namespace: test.pvc.Namespace,
+				Name:      test.pvc.Name,
+			}
+
+			pvc := test.pvc
+			pv := test.pv
+			pod := getTestPod("test-pod", pvc.Name)
+			og := getTestOperatorGeneratorWithPVPVC(volumePluginMgr, pvc, pv)
+			vmt := VolumeToMount{
+				Pod:        pod,
+				VolumeName: v1.UniqueVolumeName(pv.Name),
+				VolumeSpec: volume.NewSpecFromPersistentVolume(pv, false),
+			}
+			desiredSize := test.desiredSize
+			if desiredSize == nil {
+				desiredSize = pv.Spec.Capacity.Storage()
+			}
+			actualSize := test.actualSize
+			if actualSize == nil {
+				actualSize = pvc.Status.Capacity.Storage()
+			}
+			pluginResizeOpts := volume.NodeResizeOptions{
+				VolumeSpec: vmt.VolumeSpec,
+				NewSize:    *desiredSize,
+				OldSize:    *actualSize,
+			}
+
+			ogInstance, _ := og.(*operationGenerator)
+			_, err := ogInstance.nodeExpandVolume(vmt, nil, pluginResizeOpts)
+
+			if !test.expectError && err != nil {
+				t.Errorf("For test %s, expected no error got: %v", test.name, err)
+			}
+			if test.expectError && err == nil {
+				t.Errorf("For test %s, expected error but got none", test.name)
+			}
+			if test.resizeCallCount != fakePlugin.NodeExpandCallCount {
+				t.Errorf("for test %s, expected node-expand call count to be %d, got %d", test.name, test.resizeCallCount, fakePlugin.NodeExpandCallCount)
+			}
+		})
+	}
+}
+
 func getTestPod(podName, pvcName string) *v1.Pod {
 	return &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
@@ -253,9 +384,7 @@ func getTestPVC(volumeName string, specSize, statusSize, allocatedSize string, r
 	if len(allocatedSize) > 0 {
 		pvc.Status.AllocatedResources = v1.ResourceList{v1.ResourceStorage: resource.MustParse(allocatedSize)}
 	}
-	if len(resizeStatus) > 0 {
+	pvc.Status.ResizeStatus = &resizeStatus
-		pvc.Status.ResizeStatus = &resizeStatus
-	}
 	return pvc
 }
 
@@ -323,6 +452,31 @@ func getTestOperationGenerator(volumePluginMgr *volume.VolumePluginMgr, objects
 	return operationGenerator
 }
 
+func getTestOperatorGeneratorWithPVPVC(volumePluginMgr *volume.VolumePluginMgr, pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) OperationGenerator {
+	fakeKubeClient := fakeclient.NewSimpleClientset(pvc, pv)
+	fakeKubeClient.AddReactor("get", "persistentvolumeclaims", func(action core.Action) (bool, runtime.Object, error) {
+		return true, pvc, nil
+	})
+	fakeKubeClient.AddReactor("get", "persistentvolumes", func(action core.Action) (bool, runtime.Object, error) {
+		return true, pv, nil
+	})
+	fakeKubeClient.AddReactor("patch", "persistentvolumeclaims", func(action core.Action) (bool, runtime.Object, error) {
+		if action.GetSubresource() == "status" {
+			return true, pvc, nil
+		}
+		return true, nil, fmt.Errorf("no reaction implemented for %s", action)
+	})
+
+	fakeRecorder := &record.FakeRecorder{}
+	fakeHandler := volumetesting.NewBlockVolumePathHandler()
+	operationGenerator := NewOperationGenerator(
+		fakeKubeClient,
+		volumePluginMgr,
+		fakeRecorder,
+		fakeHandler)
+	return operationGenerator
+}
+
 func getTestVolumeToUnmount(pod *v1.Pod, pvSpec v1.PersistentVolumeSpec, pluginName string) MountedVolume {
 	volumeSpec := &volume.Spec{
 		PersistentVolume: &v1.PersistentVolume{