Bug #17530

[arvados-client] when ARVADOS_API_HOST/TOKEN are not set, error out quickly

Added by Ward Vandewege 9 months ago. Updated 8 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Start date:
04/15/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

arvados-client then tries to connect to an empyt ARVADOS_API_HOST and times out after 30s.

What it should do is error out immediately with an appropriate message.


Subtasks

Task #17538: Review 17530-arvados-client-fastfail-take2ResolvedNico César

Associated revisions

Revision b7c424bb
Added by Nico Cesar 9 months ago

Merge branch '17530-arvados-client-fastfail-take2'

closes #17530

Arvados-DCO-1.1-Signed-off-by: Nico Cesar <>

History

#1 Updated by Ward Vandewege 9 months ago

  • Description updated (diff)

#2 Updated by Lucas Di Pentima 9 months ago

  • Assigned To set to Lucas Di Pentima

#3 Updated by Lucas Di Pentima 9 months ago

  • Assigned To changed from Lucas Di Pentima to Nico César

#4 Updated by Nico César 9 months ago

  • Status changed from New to In Progress

Is this a current ticket? I compiled arvadsos-client from e46caaf835e32106e2da5aa7f895435bd4718da6

I tried with

unset ARVADOS_API_HOST
unset ARVADOS_API_TOKEN
./arvados-client get zzzzz-somej-12345678901234
ARVADOS_API_HOST and/or ARVADOS_API_TOKEN environment variables are not set

and

export ARVADOS_API_TOKEN="" 
export ARVADOS_API_HOST="" 
./arvados-client get zzzzz-somej-12345678901234
ARVADOS_API_HOST and/or ARVADOS_API_TOKEN environment variables are not set

both of them failed fast

#5 Updated by Nico César 9 months ago

  • Assigned To changed from Nico César to Ward Vandewege
  • Status changed from In Progress to Feedback

@Ward, Can you describe how to reproduce this error?

#6 Updated by Ward Vandewege 9 months ago

Nico César wrote:

@Ward, Can you describe how to reproduce this error?

Maybe it's specific to certain subcommands, e.g. (from current head):

$ ./arvados-client costanalyzer ce8i5-xvhdp-83iel786l8dkxv4
2021/04/14 15:57:14 error reading "/etc/arvados/ca-certificates.crt": open /etc/arvados/ca-certificates.crt: permission denied
2021/04/14 15:57:26 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:35 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:44 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:53 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:58:02 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:58:11 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
error creating Keep object: timed out while getting initial list of keep services

Maybe it only happens when the keep Go sdk is used?

#7 Updated by Nico César 9 months ago

Commit:841a23635 17530-arvados-client-fastfail

https://ci.arvados.org/view/Developer/job/developer-run-tests/2415/

#8 Updated by Nico César 9 months ago

d6c8a79b073cb000c7914e3ef07bf4d95350ae4f 17530-arvados-client-fastfail-take2

#9 Updated by Ward Vandewege 9 months ago

Additional fix in e758989ebd80f3365baa56be2079d72e286c9cf5 on branch 17530-arvados-client-fastfail-take2, ready for a look.

#10 Updated by Ward Vandewege 9 months ago

  • Assigned To changed from Ward Vandewege to Nico César

#11 Updated by Tom Clegg 9 months ago

I think it would be better to make (*arvadosclient.ArvadosClient)Call() fail fast on an empty host. That will cover a lot more cases, and then the extra fail-fast in keepclient.New() will be superfluous.

Also, keep in mind it's not an error to get the discovery doc or keep_services with an empty token. We should only be checking for blank host here. Let the server decide whether "no token" is an error.

Another bit of feedback: "API server is not configured" seems like a misdirection, since it's the client we're talking about, not the server... how about "Arvados client is not configured (target API host is not set). Maybe env var [...]" or something like that?

#12 Updated by Tom Clegg 9 months ago

Sorry, I think I was unclear. When I said "better to make ... Call()" I meant "...better than the extra fail-fast in keepclient.New()" (see note 9).

We still need the fail-fast in discoverServices().

So in summary, I think we need a total of two changes:
  1. discoverServices() should return an error before adding an entry to svcListCache, starting a poll loop, etc. if host is empty
  2. CallRaw in arvadosclient should return an error instead of trying http calls if host is empty

#13 Updated by Nico César 9 months ago

Squashed at 1f1c71924 17530-arvados-client-fastfail-take2

https://ci.arvados.org/view/Developer/job/developer-run-tests/2426/

#14 Updated by Tom Clegg 9 months ago

Other than the commit message being a little confusing (it seems to use future tense to describe the old behavior of CallRaw() which is no longer true)... LGTM, thanks!

#15 Updated by Anonymous 9 months ago

  • % Done changed from 0 to 100
  • Status changed from Feedback to Resolved

#16 Updated by Peter Amstutz 8 months ago

  • Release set to 38

Also available in: Atom PDF