Add option to disable discovery of attached volumes from instance metadata

[jsafrane]

Is this a bug fix or adding new feature?
/kind bug

What is this PR about? / Why do we need it?
Add --disable-volume-attach-limit-from-metadata to disable counting of attached volumes from instance metadata.

The option name and its description is not great, I know. Feel free to suggest a better one.

Instance metadata contains block volumes that were attached at the time the instance was started and then it's never updated. In case a node was rebooted with CSI volumes attached, the metadata includes these CSI volumes and the CSI driver "reserves" an attachment slot for them forever, even when they are detached later.

The option disables counting of attached volumes when computing the attach limit, because it's not able to distinguish a CSI volume (should not have a reserved attachment, because scheduler will count it on its own) and a root disk / volume attached by the system admin (should have a reserved attachment).

What testing is done?
Manually I tested that --disable-volume-attach-limit-from-metadata ignored a second volume attached to a node.

Fixes: #1808

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jsafrane. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[github-actions]

Code Coverage Diff

File	Old Coverage	New Coverage	Delta
github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/driver.go	34.7%	37.3%	2.6

[gnufied]

I am just trying to capture and summarize discussions so far.

@zerkms You mentioned does this count ENIs - #1843 (comment) and it does. ENIs are counted separately so I am not sure if https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/1843/files#diff-4f4d9c2e0ec5c4d3b4ac2d79abd70ee21bacff65ba05c059396921996dbf6607 broke anything there.

@kon-angelo - is it typical for gardner users to attach same number of additional volumes to all nodes in the cluster or the number of additional volumes is different on each node?

FWIW - the default behaviour of driver is incorrect and causes nodes to report far lower number than actually available on the node. We are kinda working with imperfect tools here, so we are stuck between rock and a hard place.

I wonder if a configuration option that allows users to configure number of additional volumes(out-of-band) they have attached to a node is the right solution, because that would allow other clusters which attach bunch of volumes out-of-band to specify a number and that number will be appropriately deducted.

[jsafrane]

Yeah, something like --out-of-band-volumes=6 looks usable. Still, it's hard for me to describe interaction between --volume-attach-limit and --out-of-band-volumes. (and it may need a better name)

[gnufied]

How about --attached-volume-count ? That perhaps explains the interaction more clearly. and we can then describe that, it will subtract from volumeLimits ?

[jsafrane]

/hold
I created #1919 with --reserved-volume-attachments=X.

[torredil]

Merging in #1919
/close

[k8s-ci-robot]

@torredil: Closed this PR.

In response to this:

Merging in #1919
/close

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.