search

karimra / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.kmrd.dev
Apache License 2.0
217 stars 32 forks source link

Convey error in return code #307

Open plajjan opened 3 years ago

plajjan commented 3 years ago

The return code of gnmic does not appear to convey how things actually went, for example, using invalid credentials:

root@e550741b9310:/# gnmic -a vr1 -u vrnetlab -p WRONG --insecure get --path /state/system/platform       
root@e550741b9310:/# echo $?
0
root@e550741b9310:/#

Enabling debug log, we do see that we get an auth failure.

root@e550741b9310:/# gnmic -d -a vr1 -u vrnetlab -p WRONG --insecure get --path /state/system/platform 2>&1 | tail -n 1
[gnmic] 2021/02/16 10:36:32.851280 /home/runner/work/gnmic/gnmic/cmd/get.go:92: failed sending GetRequest to vr1:57400: failed sending GetRequest to 'vr1:57400': rpc error: code = Unauthenticated desc = MINOR: MGMT_CORE #2021: Authentication failed - invalid username or password
root@e550741b9310:/#

Let the return code reflect this by returning a non-0 value.

I'm not sure if this is a generic problem or specific to auth failures.

hellt commented 3 years ago

yes, we had a round of discussion about it with @karimra

I am in favour of reflecting the end result of the command intent in the return code, which would be non-zero in that case. Karim, on the other had, inclined to separate application errors from the return code of the gnmic client. I.e. gNMI client succeeded in sending the request, it just happened that request response encapsulated the application error.

iirc, we decided to hear user' voices to see which is more common and appreciated.

karimra commented 3 years ago

added for cap, get and set RPCs, subscribe is a more complicated case, hopefully not needed.