Fix race condition between external-resizer and kubelet

[gnufied]

This fixes the race condition that could happen because resize controller just finished volume expansion and has only finished marking PV and yet to mark PVC.

The workaround proposed here should not be necessary once RecoverVolumeExpansionFailure goes GA/beta.

#117871

Fix error when trying to expand a volume that does not require node expansion

[k8s-ci-robot]

[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/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]

/sig storage
/triage accepted

[gnufied]

/kind bug

[xing-yang]

cc @akankshapanse

[nixpanic]

The PR looks good to me, but I wonder why there is no check (I might have missed that) on the CSI spec NodeServiceCapability EXPAND_VOLUME type. I expect that such a check nicely prevents #117871 from happening, although probably not the race from #115294...

[akankshapanse]

@gnufied It seems the fix done here is not resolving the exact issue #115294. In current fix, we are not checking for node_expansion_required value returned from the response of ControllerExpandVolume operation. Please point me to the code if this is not the case.

In case of #115294, we have ControllerExpand as well as NodeExpand capability supported at CSI driver but for raw block volumes, NodeExpand is not required and hence the call to it needs to be skipped.
@xing-yang Please correct me if wrong.

[gnufied]

@akankshapanse https://github.com/kubernetes-csi/external-resizer/blob/master/pkg/controller/controller.go#L461 Is where we are checking for return value from ControllerExpandVolume, but the problem is - there could be race between kubelet and external-resizer and hence kubelet can decide to node-expand the volume even though, external-resizer is going to mark entire expansion operation as finished.

A proper fix for this issue requires additional state on PVC and hence we will need RecoverFromVolumeExpansionFailure feature.

If your driver has both ControllerExpandVolume and NodeExpandVolume capability and it is indeed true, your driver may still receive NodeExpandVolume calls for raw block volumes. But once RecoverFromVolumeExpansionFailure feature is enabled, then that should not happen. For now - I will recommend, handling that extraneous NodeExpandVolume call in your driver and returning success immediately because nothing needs to be done on the node side.

[gnufied]

The PR looks good to me, but I wonder why there is no check (I might have missed that) on the CSI spec NodeServiceCapability EXPAND_VOLUME type.

That is the same capability we are using in this PR to return early, so we are using that feature.

[gnufied]

Updated the wording about #115294 not getting really fixed here.

[jsafrane]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: b7c9024540bb807ff762bbe6fbc3010c4773df72

[xing-yang]

@akankshapanse I see that @gnufied has answered your question. It is due to the race between kubelet and external-resizer.

[gnufied]

/cherry-pick release-1.29

[saschagrunert]

We do not need a CP for 1.27, right?