cli: recover from client ACL lookup failures #6423
Conversation
This fixes a bug in the CLI handling of node lookup failures when querying allocation and FS endpoints. Allocation and FS endpoint are handled by the client; one can query the relevant client directly, or query a server to have it forwarded transparently to relevant client. Querying the client directly is benefecial to avoid loading servers with IO. As an optimization, the CLI attempts to query the client directly, but then falls back to using server forwarding path if it encounters network or connection errors (e.g. clients are locked down or in a separate inaccessible network). Here, we fix a bug where if the CLI fails to find to lookup the client details because it lacks ACL capability or other unexpected reasons, the CLI will not go through fallback path.
|
this resolves #5754, correct? |
|
Yes, thanks @cgbaker for the reference! |
| errCh <- err | ||
| return nil, nil | ||
| } | ||
| nodeClient, _ := a.client.GetNodeClientWithTimeout(alloc.NodeID, ClientConnTimeout, q) |
There was a problem hiding this comment.
See other comment about logging this error
| @@ -244,6 +175,40 @@ func (a *AllocFS) Stream(alloc *Allocation, path, origin string, offset int64, | |||
| return frames, errCh | |||
| } | |||
|
|
|||
| func queryClientNode(c *Client, alloc *Allocation, reqPath string, q *QueryOptions, customizeQ func(*QueryOptions)) (io.ReadCloser, error) { | |||
| nodeClient, _ := c.GetNodeClientWithTimeout(alloc.NodeID, ClientConnTimeout, q) | |||
There was a problem hiding this comment.
Is there a way we could debug log this error? I see a future engineer trying to debug why an api call isn't able to hit the client directly and this error would probably be the key bit of knowledge.
There was a problem hiding this comment.
Not sure how to best handle it. We currently don't log anything in api package, and introducing logging might be confusing for cli tools, specially if eventually requests succeeds through server.
|
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. |
This fixes a bug in the CLI handling of node lookup failures when
querying allocation and FS endpoints.
Allocation and FS endpoint are handled by the client; one can query the
relevant client directly, or query a server to have it forwarded
transparently to relevant client. Querying the client directly is
benefecial to avoid loading servers with IO.
As an optimization, the CLI attempts to query the client directly, but
then falls back to using server forwarding path if it encounters network
or connection errors (e.g. clients are locked down or in a separate
inaccessible network).
Here, we fix a bug where if the CLI fails to find to lookup the client
details because it lacks ACL capability or other unexpected reasons, the
CLI will not go through fallback path.
Fixes #5754