Cached client connections might be using an invalid socket if the provider moved its socket to another location after connection is cached

[dnlopes]

What steps did you take and what happened:
I'm running csi-driver 1.4.2, which attempts to connect to providers at /var/run as well as /etc/kubernetes. On the last release of provider-aws, the provider moved from the legacy /etc/kubernetes to /var/run.

The issue is that, during upgrades, the csi-driver might have already cached aws clients pointing to /etc/kubernetes, and after updating the provider-aws, the cached client will be unusable because the provider is now listening on /var/run. This is an issue because the only way to recover is to restart the DaemonSet, so clients are created again from scratch (now pointing to /var/run).

What did you expect to happen:
When a client connection fails with "socket not found", the client should be discarded and recreated from scratch.

Anything else you would like to add:
This analyses was done in the context of this open issue on provider-aws.

Which provider are you using:
AWS

Environment:

• Secrets Store CSI Driver version: 1.4.2
• Provider AWS version: bumping from 0.3.4 to 1.0.1
• Kubernetes version: (use kubectl version): v1.30.11-eks-bcf3d70

[dnlopes]

I noticed there is a providerHealthCheck option, that's invoking the Version method on the client. However, the health-check code simply logs an error if communication fails.

Maybe we could change this behavior to also remove the client from the cache if the health-check fails?

[aramase]

The issue is that, during upgrades, the csi-driver might have already cached aws clients pointing to /etc/kubernetes, and after updating the provider-aws, the cached client will be unusable because the provider is now listening on /var/run. This is an issue because the only way to recover is to restart the DaemonSet, so clients are created again from scratch (now pointing to /var/run).

This is a one-time upgrade step that was required to be followed for providers moving to the new path for sockets (/var/run) - requiring restart of the driver. I don't anticipate this happening in any other scenario.

Maybe we could change this behavior to also remove the client from the cache if the health-check fails?

This is a good idea. providerHealthCheck by default is false so users would need to opt-in to actually take advantage of this (which will require documentation). If we ever plan to make this true by default, we'll need to assess the overhead of keeping this flags always on, if all providers support it, etc.

[aramase]

/triage accepted

[dnlopes]

This is a good idea. providerHealthCheck by default is false so users would need to opt-in to actually take advantage of this (which will require documentation). If we ever plan to make this true by default, we'll need to assess the overhead of keeping this flags always on, if all providers support it, etc.

Alright, it looks like a small change from the code I checked. I can make a PR for it if that's ok.