Should fail on non-zero exit

[aldy505]

Is your feature request related to a problem? Please describe

Currently sake does not exit with non-zero code (or, simply, fail) when the command we're executing exited with non-zero code.

It'd be nice if it would fail on non-zero code.

Describe the solution you'd like

I think the feature should be optional to remain backward compatible with existing users. So, to have sake fail on non-zero exit code, the user should incite another CLI options like --fail.

Additional context

The sake command that I use to produce this issue was sake run <command> --servers <server> --known-host-file <file>

Let me know if I was wrong about the exit behavior or any needed reproductions.

[alajmo]

There's the flag any-errors-fatal and config spec setting any_errors_fatal that you can enable to stop task execution on all servers when exit code is non-zero. But perhaps, you mean the output of sake is sent to stdout instead of stderr?

Could you copy your command's actual and wanted output and relevant config settings/tasks?

[aldy505]

I can't find the exact issue as I was searching on the CD pipeline, maybe I executed it from my local machine. As I remembered it, it's just a normal npm install (Nodejs stuff) that got a timeout error, and I also remembered that I put the --any-errors-fatal flag.

But I will surely create a repro over the weekend if that's okay.

[alajmo]

Just to clarify, it should fail on non-zero exit already, if the setting any-errors-fatal is set. So, could be there's a bug, but I tried with a simple script and it seemed to work as intended:

$ go run ../../main.go run lala -t pihole --any-errors-fatal=false

TASK [lala]

172.24.2.3 | 123
172.24.2.3 | Process exited with status 1

TASK [lala]

172.24.2.3 | 123
172.24.2.3 | Process exited with status 1

$ go run ../../main.go run lala -t pihole --any-errors-fatal=true

TASK [lala]

172.24.2.3 | 123
172.24.2.3 | Process exited with status 1

  lala:
    cmd: |
      echo 123 
      exit 1

[aldy505]

Sorry it took a while. I ran it on the CD server with the sake configuration as:

pull_master:
    desc: Pull from the master branch
    cmd: |
      git remote set-url origin https://${GIT_USERNAME}:${GIT_PASSWORD}@github.com/some-organization/some-repository.git
      git pull
      git remote set-url origin https://github.com/some-organization/some-repository.git
    work_dir: /home/user/some-repository
    env:
      GIT_USERNAME: $(echo $GIT_USERNAME)
      GIT_PASSWORD: $(echo $GIT_PASSWORD)
      DEBIAN_FRONTEND: noninteractive

The CD results is (the first line is the command I executed):

Run yes | sake run pull_master --servers onpremise -i /root/.ssh/id_ed25519 --known-hosts-file /root/.ssh/known_hosts --any-errors-fatal
Unknown Host: ***:22 
Fingerprint: <redacted>
Would you like to add it? type (y)es or (n)o: 
TASK [pull_master: Pull from the master branch] 
*** | error: No such remote 'origin'
*** | There is no tracking information for the current branch.
*** | Please specify which branch you want to merge with.
*** | See git-pull(1) for details.
*** | 
*** |     git pull <remote> <branch>
*** | 
*** | If you wish to set tracking information for this branch you can do so with:
*** | 
*** |     git branch --set-upstream-to=<remote>/<branch> master
*** | 
*** | Process exited with status 2
*** | error: No such remote 'origin'

[alajmo]

Thanks for finding the bug, the issue is that the exit code for sake is 0, even if the tasks have a different exit code.
The simplest solution is to exit with code 1 if there's an error with the task execution, which I'll add.

[aldy505]

Or maybe you should add an option whether to exit with code 1 or equal to the error code returned by the task. I guess providing both options would be a better way to put things.

[aldy505]

Hi @alajmo

I want to know if there are any progress to this particular issue. If you don't have time to work on this one, I might be able to submit a PR later this weekend.

[alajmo]

I was on vacation but I got back yesterday so will most likely solve it tonight (I did find the responsible code and it should be an easy fix).

[aldy505]

Oh alright. Thank you! Sent from Proton Mail for iOS

…

On Thu, Jul 28, 2022 at 19:08, Samir Alajmovic ***@***.***> wrote: I was on vacation but I got back yesterday so will most likely solve it tonight (I did find the responsible code and it should be an easy fix). — Reply to this email directly, [view it on GitHub](#23 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ABXP6VTNCMU53QGHIBCJK6TVWJZ4FANCNFSM523REJNA). You are receiving this because you authored the thread.Message ID: ***@***.***>

[alajmo]

Fix should be working now for release v0.10.2, the following changes are included:

• If a task fails, then return the last exit code from said task iff --any-errors-fatal=true, otherwise the exit code will be 0
• If a host is unreachable and --ignore-unreachable=false, return error code 4

Note, it only outputs correct exit code for serial host execution (since there's some additional work needed for parallel execution, like what if multiple hosts have different exit codes).

[aldy505]

Thank you! I will test it right away.