Remove external-attacher from the repo.

[humblec]

Describe the bug

The presence of external-attacher adds on overhead and issues wrt attachment in various scenarios. One of them would be the lack of performance on syncing volumeattachment from api server..etc. More or less we don't have controller publish and unpublish capabilities , so we should not make use of this sidecar and cause unnecessary addon here thus other issues.

The solution:

Make use of skip attach field in CSI Driver object.

apiVersion: storage.k8s.io/v1
kind: CSIDriver
metadata:
  name: mycsidriver.example.com
spec:
  attachRequired: false

[humblec]

/me on it.

[Madhu-1]

This needs extensive testing to make sure there won't be an issue for RWO PVC

[Madhu-1]

this is already tried in https://github.com/rook/rook/pull/4172 and reverted back https://github.com/rook/rook/pull/4332 due to the issue we have seen in RWO PVC

[humblec]

This needs extensive testing to make sure there won't be an issue for RWO PVC

Yeah, rather this need extensive testing for all types of PVCs :).

[humblec]

this is already tried in rook/rook#4172 and reverted back rook/rook#4332 due to the issue we have seen in RWO PVC

The difference here is, this is GA in 1.18 compared to previous versions. So ideally it should work without issues. Lets try out with 1.18 version and see.

[ShyamsundarR]

Tagging @gnufied

On a prior discussion regarding removal of the attacher, as it improved performance of overall pod startup times, it was pointed out by @gnufied that this essentially means double attach is detected and prevented by the storage plugin, and the basic AD controller will not ensure that the older node running the pod has actually detached etc. (this is even when the csi plugin ControllerPublish/Unpublish are NOPs) (unable to find a link to the discussion as yet)

Also, as @Madhu-1 points out, the prior attempt led to the related problems.

A subsequent speed up of the attach-detach operation was attempted here and was not fully successful.

HTH, in taking this forward as appropriate.

[gnufied]

@ShyamsundarR thank you for tagging me. So unless ceph itself can provide strong guarantees about preventing volume attach to multiple nodes for block volumes, we should not disable external-attacher.

Evidence so far suggests that such iron clad guarantees don't exist. And hence I would not disable external-attacher just yet because it could lead to file system corruption and what not.

[Madhu-1]

@humblec are you working on the fix? would you like to track this one for v3.0.0 if not please move this out of v3.0.0 release milestone

[Madhu-1]

Moving it to release-v3.1.0

[Madhu-1]

@humblec can this be done in 3.1.0? or do we need to move it out to the next release?

[humblec]

Yeah, we have to move this from 3.1.0. Done.. thanks :+1:

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[humblec]

Considering skipAttach has been GAd with 1.18 and take care of the burden on unncessary operation (round-trip) in the preparation of a volume for a container, and requires CSI Drivers to deploy an unnecessary sidecar container (external-attacher), this is a good time to take it out.