diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index 78c304b15eb..20316818018 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/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 -// system resize or not. If so, marks this volume as fsResizeRequired in ASW. -// - mountedVolumesForPod stores all mounted volumes in ASW, because online -// 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. +// checkVolumeFSResize records desired PVC size for a volume mounted by the pod. +// It is used for comparison with actual size(coming from pvc.Status.Capacity) and calling +// volume expansion on the node if needed. func (dswp *desiredStateOfWorldPopulator) checkVolumeFSResize( pod *v1.Pod, podVolume v1.Volume, diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 401a335f4b8..1a5f029982b 100644 --- a/pkg/volume/testing/testing.go +++ b/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") } diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index 135d94a78cb..bf2be4b45b5 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -29,10 +29,14 @@ import ( func TestNodeExpander(t *testing.T) { var tests = []struct { - name string - pvc *v1.PersistentVolumeClaim - pv *v1.PersistentVolume - recoverFeatureGate bool + 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 @@ -42,10 +46,9 @@ func TestNodeExpander(t *testing.T) { 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"), - recoverFeatureGate: true, + 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, 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(), - OldSize: *pvc.Status.Capacity.Storage(), + NewSize: *desiredSize, + OldSize: *actualSize, }, } ogInstance, _ := og.(*operationGenerator) diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index d4ca92fa63d..8d4815e5a36 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/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) } diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 78bc5bc6deb..1ae2994deb1 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/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{