csi: use a blocking initial connection with timeout #7965
Conversation
The plugin supervisor lazily connects to plugins, but this means we only get "Unavailable" back from the gRPC call in cases where the plugin can never be reached (for example, if the Nomad client has the wrong permissions for the socket). This changeset improves the operator experience by switching to a blocking `DialWithContext`. It eagerly connects so that we can validate the connection is real and get a "failed to open" error in case where Nomad can't establish the initial connection.
tgross
force-pushed
the
csi_better_error_on_bad_connect
branch
from
95daea0 to
eb2f77a
Compare
| if err != nil { | ||
| return false, fmt.Errorf("failed to create csi client: %v", err) | ||
| } | ||
| defer client.Close() |
There was a problem hiding this comment.
This fixes a panic that can't happen in 0.11.2 or earlier because we never actually connect in this block, so the error condition never happens.
There was a problem hiding this comment.
nice, I thought about it a little and talked myself out of wanting a way to change the timeout. It might be interesting to have a couple of coarse grained tuning parameters eventually where we'd have or derive an out-of-dc-connection-timeout. Being open source is a type of allowing tuning of something most folks won't need to tune, though 😀
Yeah, the timeout only impacts the initial connection and we retry it on the next fingerprint anyways, so I figured it'd be safe to have it be short. |
|
Cherry-picked to 0.11.3 branch as d31bb8b |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
For #7424 (1 of probably many) and specifically the case described in #7931 (comment)
The plugin supervisor lazily connects to plugins, but this means we only get "Unavailable" back from the gRPC call in cases where the plugin can never be reached (for example, if the Nomad client has the wrong permissions for the socket).
This changeset improves the operator experience by switching to a blocking
DialWithContext. It eagerly connects so that we can validate the connection is real and get a "failed to open" error in case where Nomad can't establish the initial connection.The new error messages in a scenario where Nomad has the wrong permissions will look like the following (repeated each fingerprint attempt):