Enable node-expansion to be called on all nodes for RWX volumes #108693

gnufied · 2022-03-14T18:55:38Z

This PR implements support for calling NodeExpandVolume on all nodes in case of RWX volumes. It does so by storing pvSpecSize in desiredStateofWorld and pvcStatusSize in actualStateofWorld.

I tested with CSI Mock driver and RWX enabled (branch here - https://github.com/gnufied/csi-test/tree/make-rwx-possible )

and it seems to be working as expected:

Events:
  Type     Reason                      Age                From                                                                           Message
  ----     ------                      ----               ----                                                                           -------
  Normal   ExternalProvisioning        35m (x2 over 35m)  persistentvolume-controller                                                    waiting for a volume to be created, either by external provisioner "io.kubernetes.storage.mock" or manually created by system administrator
  Normal   Provisioning                35m                io.kubernetes.storage.mock_fast.home.lan_7225b6d9-9cd7-4110-a36f-7734e4cf937c  External provisioner is provisioning volume for claim "default/myclaim"
  Normal   ProvisioningSucceeded       35m                io.kubernetes.storage.mock_fast.home.lan_7225b6d9-9cd7-4110-a36f-7734e4cf937c  Successfully provisioned volume pvc-be470238-a485-4ac8-a7f9-2aeae74664b2
  Normal   Resizing                    7m55s              external-resizer io.kubernetes.storage.mock                                    External resizer is resizing volume pvc-be470238-a485-4ac8-a7f9-2aeae74664b2
  Warning  ExternalExpanding           7m55s              volume_expand                                                                  Ignoring the PVC: didn't find a plugin capable of expanding the volume; waiting for an external controller to process this PVC.
  Normal   FileSystemResizeRequired    7m55s              external-resizer io.kubernetes.storage.mock                                    Require file system resize of volume on node
  Normal   FileSystemResizeSuccessful  7m11s              kubelet                                                                        MountVolume.NodeExpandVolume succeeded for volume "pvc-be470238-a485-4ac8-a7f9-2aeae74664b2"
  Normal   FileSystemResizeSuccessful  6m42s              kubelet                                                                        MountVolume.NodeExpandVolume succeeded for volume "pvc-be470238-a485-4ac8-a7f9-2aeae74664b2"

Call NodeExpand on all nodes in case of RWX volumes

k8s-ci-robot · 2022-03-14T18:56:47Z

@gnufied: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2022-03-21T16:58:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/controller/volume/attachdetach/OWNERS~~ [gnufied]
~~pkg/kubelet/volumemanager/OWNERS~~ [gnufied]
~~pkg/volume/OWNERS~~ [gnufied]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gnufied · 2022-03-22T03:40:50Z

/retest

gnufied · 2022-03-22T03:41:12Z

/assign @jsafrane @jingxu97

jsafrane · 2022-03-22T15:52:10Z

/retest
vendor/k8s.io/client-go/util/flowcontrol TestMultithreadedThrottling is flaky !?

pkg/controller/volume/attachdetach/cache/actual_state_of_world.go

pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go

gnufied · 2022-03-28T16:59:41Z

@jsafrane Updated events to include node names:

  Type     Reason                      Age    From                                                                           Message
  ----     ------                      ----   ----                                                                           -------
  Normal   ExternalProvisioning        2m17s  persistentvolume-controller                                                    waiting for a volume to be created, either by external provisioner "io.kubernetes.storage.mock" or manually created by system administrator
  Normal   Provisioning                2m17s  io.kubernetes.storage.mock_fast.home.lan_65f913eb-bc22-4896-889a-5326a0fd9014  External provisioner is provisioning volume for claim "default/myclaim"
  Normal   ProvisioningSucceeded       2m17s  io.kubernetes.storage.mock_fast.home.lan_65f913eb-bc22-4896-889a-5326a0fd9014  Successfully provisioned volume pvc-3a17631d-3e5a-48bf-9f29-0682b726b844
  Normal   Resizing                    87s    external-resizer io.kubernetes.storage.mock                                    External resizer is resizing volume pvc-3a17631d-3e5a-48bf-9f29-0682b726b844
  Normal   FileSystemResizeRequired    87s    external-resizer io.kubernetes.storage.mock                                    Require file system resize of volume on node
  Warning  ExternalExpanding           87s    volume_expand                                                                  Ignoring the PVC: didn't find a plugin capable of expanding the volume; waiting for an external controller to process this PVC.
  Normal   FileSystemResizeSuccessful  44s    kubelet                                                                        MountVolume.NodeExpandVolume succeeded for volume "pvc-3a17631d-3e5a-48bf-9f29-0682b726b844" 192.168.121.206
  Normal   FileSystemResizeSuccessful  41s    kubelet                                                                        MountVolume.NodeExpandVolume succeeded for volume "pvc-3a17631d-3e5a-48bf-9f29-0682b726b844" 127.0.0.1

gnufied · 2022-03-28T17:02:03Z

I also tested with multiple kubelets and restarts. It is working as expected and whichever kubelet did not observe resize event while it was up, will not call NodeExpand on the volume even when the kubelet comes up. cc @jingxu97

jingxu97 · 2022-03-28T22:52:55Z

pkg/volume/util/operationexecutor/operation_generator.go

-			resizeResponse.err = msg
-			return resizeResponse
-		}
+func (og *operationGenerator) nodeExpandVolume(


where this function is called?

It gets called from doOnlineExpansion function which in turn gets called from ExpandInUsePersistentVolume function.

jingxu97 · 2022-03-28T22:58:00Z

pkg/volume/util/operationexecutor/operation_generator.go

@@ -712,7 +703,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 		klog.V(verbosity).InfoS(detailedMsg, "pod", klog.KObj(volumeToMount.Pod))
 		resizeOptions.DeviceMountPath = volumeMounter.GetPath()

-		_, resizeError = og.nodeExpandVolume(volumeToMount, actualStateOfWorld, resizeOptions)
+		_, resizeError = og.expandVolumeDuringMount(volumeToMount, actualStateOfWorld, resizeOptions)


are you changing the function name from nodeExpandVolume to expandVolumeDuringMount? The tests should also updated?

So logic for calling expansion during mount is slightly different from online expansion later on. During mount - we fetch PVC size from api-server and check if volume requires expansion, because during mount this would be the first time volume was observed and hence value in ASOW will be same as value in pvc.Status.

For online expansion while pod is running, however we strictly compare desired and actual sizes. There are tests in reconciler_test.go, which exercise this at higher level.

jsafrane · 2022-03-29T15:38:37Z

/lgtm
/hold
for @jingxu97 to have her approval too

jingxu97 · 2022-03-29T16:25:36Z

/lgtm
/cancel hold

gnufied · 2022-03-29T16:29:03Z

/hold cancel

msau42 · 2022-03-29T21:12:07Z

/milestone v1.24

liggitt · 2022-04-04T03:27:44Z

#109264 noted tests failing with nil pointer errors that might be related to this PR

gnufied · 2022-04-04T14:58:51Z

Thanks I am looking into those. posted some updates - #109264 (comment)

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet labels Mar 14, 2022

k8s-ci-robot requested review from chrishenzie and Jiawei0227 March 14, 2022 18:57

gnufied force-pushed the enable-rwx-call-all-nodes branch from 4efb040 to 2b9af72 Compare March 21, 2022 16:56

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2022

gnufied force-pushed the enable-rwx-call-all-nodes branch from 2b9af72 to 99adb3b Compare March 21, 2022 18:05

gnufied changed the title ~~{WIP} Enable node-expansion to be called on all nodes for RWX volumes~~ Enable node-expansion to be called on all nodes for RWX volumes Mar 22, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2022

k8s-ci-robot assigned jingxu97 and jsafrane Mar 22, 2022

dobsonj reviewed Mar 22, 2022

View reviewed changes

pkg/controller/volume/attachdetach/cache/actual_state_of_world.go Outdated Show resolved Hide resolved

pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go Show resolved Hide resolved

gnufied added 5 commits March 28, 2022 11:32

Remove legacyCallNodeExpandOnPlugin when RecoverVolumeExpansionFailure

4d52dbb

rename SetVolumeSize to InitializeVolumeSize

ed217f4

address review comments for rwx volume types

1809094

check existing size before querying new size from api-server

a99466c

Add more tests for volume recovery cases

dee48d3

gnufied force-pushed the enable-rwx-call-all-nodes branch from 195c666 to dee48d3 Compare March 28, 2022 15:59

Add node name to NodeExpand events

6bc8275

gnufied force-pushed the enable-rwx-call-all-nodes branch from 92dab4e to 6bc8275 Compare March 28, 2022 16:59

jingxu97 reviewed Mar 28, 2022

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 29, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 29, 2022

k8s-ci-robot added this to the v1.24 milestone Mar 29, 2022

k8s-ci-robot merged commit 1266744 into kubernetes:master Mar 30, 2022

olemarkus mentioned this pull request Apr 3, 2022

Several in-tree volumes tests started failing with nil pointer errors #109264

Closed

github-actions bot mentioned this pull request Apr 5, 2022

Week Ending April 3, 2022 dev-obs/actus#414

Open

gnufied mentioned this pull request Jul 7, 2022

Only one node extends the volume kubernetes-csi/external-resizer#212

Closed

gnufied mentioned this pull request Jul 24, 2022

CSI resize doesn't handle multi-attached volumes quite right #80918

Closed

gnufied mentioned this pull request Jan 25, 2023

NodeExpandVolume gets called, even if CSI plugin returned NodeExpansionRequired=false in ControllerExpandVolumeResponse #115294

Closed

carlory mentioned this pull request Oct 8, 2024

Fix data race in kubelet/volumemanager #127919

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable node-expansion to be called on all nodes for RWX volumes #108693

Enable node-expansion to be called on all nodes for RWX volumes #108693

gnufied commented Mar 14, 2022 •

edited

Loading

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 21, 2022

gnufied commented Mar 22, 2022

gnufied commented Mar 22, 2022

jsafrane commented Mar 22, 2022

gnufied commented Mar 28, 2022

gnufied commented Mar 28, 2022

jingxu97 Mar 28, 2022

gnufied Mar 29, 2022

jingxu97 Mar 28, 2022

gnufied Mar 29, 2022

jsafrane commented Mar 29, 2022

jingxu97 commented Mar 29, 2022

gnufied commented Mar 29, 2022

msau42 commented Mar 29, 2022

liggitt commented Apr 4, 2022

gnufied commented Apr 4, 2022

Enable node-expansion to be called on all nodes for RWX volumes #108693

Enable node-expansion to be called on all nodes for RWX volumes #108693

Conversation

gnufied commented Mar 14, 2022 • edited Loading

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 21, 2022

gnufied commented Mar 22, 2022

gnufied commented Mar 22, 2022

jsafrane commented Mar 22, 2022

gnufied commented Mar 28, 2022

gnufied commented Mar 28, 2022

jingxu97 Mar 28, 2022

Choose a reason for hiding this comment

gnufied Mar 29, 2022

Choose a reason for hiding this comment

jingxu97 Mar 28, 2022

Choose a reason for hiding this comment

gnufied Mar 29, 2022

Choose a reason for hiding this comment

jsafrane commented Mar 29, 2022

jingxu97 commented Mar 29, 2022

gnufied commented Mar 29, 2022

msau42 commented Mar 29, 2022

liggitt commented Apr 4, 2022

gnufied commented Apr 4, 2022

gnufied commented Mar 14, 2022 •

edited

Loading