Return non-zero exit code in case of errors

[petrutlucian94]

At the moment, k8s and k8sd return 0 even if the command fails, which is a problem especially when used inside scripts.

We'll ensure that a non-zero exit code is returned if the commands fail.

[bschimke95]

LGTM thanks
Please run go fmt to make the Go CI happy, then we are good to merge.

[bschimke95]

LGTM

[HomayoonAlimohammadi]

I think we're using Run instead of RunE which indicates that we're relying on the os.Exit(1) to occur in the individual cmds. I might be wrong, but I think the ExecuteContext here only returns an error if the RunE, PreRunE, PostRunE or similar "error-returning" methods return an error which we don't have any of them (AFAIK).
This change is a valid change but if we're addressing the fact that k8s commands does not exit with code 1 when they should, this won't solve that issue.
In other words, this change won't cause any behavior changes because of the current implementation.
If there's any k8s command that exits with code 0 while also facing with an error I think we should change the code for that specific cmd and call the os.Exit(1) there (e.g.:

k8s-snap/src/k8s/cmd/k8s/k8s_status.go

Line 32 in 5fe3c27

env.Exit(1)

).

[petrutlucian94]

@HomayoonAlimohammadi thanks for bringing this up. I did a simple check and it seemed to work fine but I'll take a closer look.

w/ fix
cloudbase@ubuntuk8s2ha:~/workspace/k8s-snap/src/k8s$ bin/static/k8sd wrong-cmd
Error: unknown command "wrong-cmd" for "k8sd"
Run 'k8sd --help' for usage.
cloudbase@ubuntuk8s2ha:~/workspace/k8s-snap/src/k8s$ echo $?
1

# w/o fix
ubuntu@hatwo:~$ /snap/k8s/1211/bin/k8sd wrong-cmd
Error: unknown command "wrong-cmd" for "k8sd"
Run 'k8sd --help' for usage.
ubuntu@hatwo:~$ echo $?
0

cloudbase@ubuntuk8s2ha:~/workspace/k8s-snap/src/k8s$ bin/static/k8sd cluster-recover
Error: k8s-dqlite state dir not specified
Usage:
  k8sd cluster-recover [flags]

...
  -v, --verbose                                   Show all information messages (default true)

cloudbase@ubuntuk8s2ha:~/workspace/k8s-snap/src/k8s$ echo $?
1

LE: I see that the commands that use Run instead of RunE do call os.Exit, so that's fine. This change will accomodate commands that use RunE as well as the case in which the argument parsing fails.

[bschimke95]

As discussed in the standup: You both are correct.
We currently use the Run command for every k8s command except the k8sd cluster recover command. I overlooked that in #618.

So,
a) @petrutlucian94 please create a PR to refactor the k8sd cluster recover command to use the environment, similar to what commands do.
b) I'd still merge this PR to be a safe-net in case we miss this again for other commands.

[bschimke95]

LGTM. Please add a note to the error check.
e.g. // Although k8s commands typically use Run instead of RunE and handle errors directly within the command execution, this acts as a safeguard in case any are overlooked.

[petrutlucian94]

a) @petrutlucian94 please create a PR to refactor the k8sd cluster recover command to use the environment, similar to what commands do.

I've added another commit here, I hope that's ok.

b) I'd still merge this PR to be a safe-net in case we miss this again for other commands.

The Run/RunE functions won't get called at all if there are argument parsing errors, so that's another reason why we need this. I've added an inline comment.

[bschimke95]

LGTM, thanks for the change!

[HomayoonAlimohammadi]

LGTM, Thanks a lot @petrutlucian94! Just some very minor comments.