ebs.csi.aws.com/throughput annotation for volumemodifier sidecar

[jclegras]

/kind bug

What happened?

When I add the annotation ebs.csi.aws.com/throughput: 127 to a PVC (you have to enable the volumemodifier sidecar first), I receive an error from ebs-csi controller:

ebs-csi-controller-85dd68f7b7-2kg26 ebs-plugin ebs-csi-controller-85dd68f7b7-2kg26      rpc error: code = Internal desc = Could not modify volume "<volume-id>": unable to modify AWS volume "<volume-id>": InvalidParameterValue: Invalid input: Must specify at least one of size, type, iops, throughput or multi-attach.
ebs-csi-controller-85dd68f7b7-2kg26 volumemodifier ebs-plugin I0619 14:21:19.361322       1 connection.go:201] GRPC error: rpc error: code = Internal desc = Could not modify volume "<volume-id>": unable to modify AWS volume "<volume-id>": InvalidParameterValue: Invalid input: Must specify at least one of size, type, iops, throughput or multi-attach.
                status code: 400, request id: <request-id>

What you expected to happen?

No error and the volume has been modified accordingly to reflect the new value for throughput

How to reproduce it (as minimally and precisely as possible)?

• EKS cluster and driver installed as an add-on
• Install controller with volumemodifier enabled
  {"controller":{"volumeModificationFeature":{"enabled":true}}}
• k annotate pvc <my-pvc> 'ebs.csi.aws.com/throughput'=126
• Then you can kubectl describe the pvc to see the error or ebs-csi-controller logs (you can even increase the log level for this sidecar only)

Anything else we need to know?:

We managed to make it work thanks to the code
Actually you're expecting two annotations when we want to change the throughput value:

'ebs.csi.aws.com/throughput'=<my-value-int>
AND
'ebs.csi.aws.com/volumeType'=gp3

The condition is here: rdpsin@1560d54#diff-d7ef58b21290a74508c38ca5fdcd104ca5d098820cfac0681ba5415c9820c91dR477

Without this second annotation, throughput is not set and AWS side, the request received is as such:

 "errorCode": "Client.InvalidParameterValue",

    "errorMessage": "Invalid input: Must specify at least one of size, type, iops, throughput or multi-attach.",

    "requestParameters": {

        "ModifyVolumeRequest": {

            "VolumeId": "<volume-id>"

        }

    },

Instead of:

"requestParameters": {

        "ModifyVolumeRequest": {

            "VolumeId": "<volume-id>",

            "Throughput": 126

        }

    },

To sum up, we don't know what you had in mind:
If it's on purpose (two annotations to change the throughput), then the doc should be modified accordingly
Otherwise, we may get rid of the condition that checks volume type and let AWS SDK raises an error if we are not in GP3 volume type, that way we only need one annotation (throughput)
Or since we have the volumeId, we could issue an api call to describe the volume and checks the type but it's one additional API call

Environment

• Kubernetes version (use kubectl version): Server Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.1-eks-2f008fe", GitCommit:"abfec7d7e55d56346a5259c9379dea9f56ba2926", GitTreeState:"clean", BuildDate:"2023-04-14T20:40:28Z", GoVersion:"go1.20.3", Compiler:"gc", Platform:"linux/amd64"}

• Driver version: v1.19.0-eksbuild.2

[hanyuel]

Hi @jclegras , the condition checking volume type should not be necessary. So we can remove it.

[Indresh2410]

Hi @jclegras / @hanyuel . Would like to work on the above issue. Please let me know if you have any concerns

[hanyuel]

@Indresh2410 sounds good. Please tag me when you have a PR up.

[Indresh2410]

Sure. Thanks @hanyuel

[Indresh2410]

@hanyuel can you please check once ?