Script exiting with non-zero exist status even if successful

[laszlof]

Description

I'm using this action to deploy some DNS configurations to some powerdns servers. However, when it attempts to run, the action bails out because of these lines returning a non-zero exit status:

https://github.com/solvaholic/octodns-sync/blob/main/scripts/run.sh#L31-L36

The sync itself actually works, its just failing the job in Actions.

Expected Behavior

Action completes successfully

Actual Behavior

Action fails a task with a non-zero exit status

Possible Fix

It appears that the problem is due to how you're determining if there is an error state. It could have something to do with the octodns version I'm using (0.9.19). If we break down that conditional that triggers the error state, here is what we have:

scout@runner01$ cat requirements.txt 
octodns==0.9.19
octodns_powerdns==0.0.1
requests==2.28.1
scout@runner01$ tail --lines=1 octodns-sync.log
Script done on 2022-09-22 13:52:49+0000
scout@runner01$ if tail --lines=1 octodns-sync.log | grep --quiet --fixed-string --invert-match '[COMMAND_EXIT_CODE="0"]'; then echo "FAIL"; fi
FAIL

[solvaholic]

Thank you for reporting this @laszlof 🙇 Please forgive my delay in responding.

scout@runner01$ tail --lines=1 octodns-sync.log
Script done on 2022-09-22 13:52:49+0000

If that's the last line in $_logfile then I think script didn't store the exit status of the command in the file as expected.
https://manpages.debian.org/bullseye/bsdutils/script.1.en.html#e

Could you share details about your runner's operating environment and the version of script it provides?

[solvaholic]

Could you share details about your runner's operating environment and the version of script it provides?

Looking around, I think this Action can't assume macOS and self-hosted runners provide a script that supports the options run.sh currently uses. I tried to make sense of the variations. I think it looks like all current script came from early BSD but Linux and the subsequent BSDs went different directions with it.

I stuffed the octodns-sync run under the script call in ee5a4b5. I don't recall why, and don't find notes about it in #53. Boo.

I'll look for alternative ways to populate octodns-sync.log AND check the result of octodns-sync. Suggestions welcome 🙇

In the meantime, I think Debian-based runners (like GitHub's hosted Ubuntu runners) should remain unaffected.

[laszlof]

@solvaholic apologies for the delay. My runner was on Ubuntu 18.04. What you mention about script makes sense. I'll mess around with it a bit on Monday and see if I can come up with a more cross-platform solution.

Question: Can we not rely on exit codes directly from octodns-sync? That would seem like the most reliable way.

I appreciate the work you put into this, saved me from having to write it all myself :)

[solvaholic]

Question: Can we not rely on exit codes directly from octodns-sync? That would seem like the most reliable way.

Definitely it should be possible, yes. The challenge I see is how to notice that exit code AND output the plan to a file AND output everything octodns-sync says (including plan and stderr) to a separate log. I feel like tee should help, here. Or maybe octodns-sync provides a way to direct the outputs.

I'll keep looking, tho it'll probably be 09 Oct before I can put much time into it.

I feel like the solution could be super simple, just need to research and test. For example, something like this might work:

if ! octodns-sync >./plan.txt 2>./log.txt; then exit $?; fi

[laszlof]

@solvaholic The -e option exists for script. However, I don't think its cross-platform compatible. It exists on CentOS 7 and Ubuntu 18.04, but doesnt appear to exist on a mac.

       -e, --return
              Return the exit code of the child process.  Uses the same format as bash termination on signal termination exit code is 128+n.

shell redirection may be the answer, i dont think you'll be able to use tee simply because trying to check $? will likely result in getting the exit code from tee itself rather than the preceding command.

I'll mess around with what you posted and give it a shot, If it works out, I'll submit a PR for it.

[solvaholic]

Starting a Docker container this way:

_image=python:3-bullseye

docker run --interactive --tty --rm \
--env-file .env \
--volume "$(realpath .)":/data \
--workdir /data \
"$_image" /bin/bash

And running octodns-sync this way:

python3 -m pip install -r requirements.txt
octodns-sync --config-file public.yaml 1>plan.htm 2>log.txt; echo $?

I find the redirects do what's expected, and the exit code survives, with these images:
python:3-bullseye, python:3-buster, python:3-alpine, and fedora:36

With Alpine I tested /bin/sh and /bin/ash, and used Dennis Joel suggestion in:
https://stackoverflow.com/questions/68094955/building-wheel-for-cffi-setup-py-error-while-installing-the-packages-from

With Fedora I did dnf install python3-pip to get pip.

[solvaholic]

@laszlof When you have some minutes for it, will you try using the @issue92 version of this action and let us know how it goes?

      - uses: solvaholic/octodns-sync@issue92

My own testing says the change in afdbff0 should work fine. One side effect is the plan will no longer be captured in $_logfile. Apart from that I think the behavior seems similar, minus dependency on screen.

[laszlof]

@solvaholic Will do. I'll try to get it today and report back. Thanks!

[laszlof]

@solvaholic That seemed to fix the issue, but we lost the script output from the sync. Not a huge issue, but it may be for some who regularly look at the actions console to see their job runs.

[solvaholic]

Thank you for confirming, @laszlof 🙇

but we lost the script output from the sync

Thanks also for pointing out this change. The stdout of octodns sync is the plan text, and the stderr is everything else. As of afdbff0 stderr is logged to $_logfile and it is not output to the workflow run log.

Somewhat related .. while looking for context I see this project used to use tee (#23) and after moving to screen (#24) it was still/again possible for this action to silently fail (#40).