Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use better exit codes and comparison #1708

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

fhammerl
Copy link
Contributor

Exiting with exit 1 in run-helper.sh and then reacting to it with if [[ $returnCode == 1 ]]; then will also restart on generic errors occuring in run-helper, like how we exit 1 in run-helper.sh (line 7) if it's run with sudo.

I suggest we indicate a restart request with exit 2, then react to it with if [[ $returnCode -eq 2 ]]; then

Also changes == to -eq for numerical comparison.

@fhammerl fhammerl requested a review from a team as a code owner February 22, 2022 21:02
Copy link
Collaborator

@thboop thboop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fhammerl fhammerl merged commit 6abef81 into main Feb 24, 2022
@fhammerl fhammerl deleted the fhammerl/run-sh-better-exit-codes branch February 24, 2022 20:10
TingluoHuang added a commit that referenced this pull request Apr 19, 2022
TingluoHuang added a commit that referenced this pull request Apr 19, 2022
* Revert "Use better exit codes and comparison (#1708)"

This reverts commit 6abef81.

* Revert "fix run.cmd script (#1633)"

This reverts commit 192ebfe.

* Revert "Reopen 'Make run.sh|cmd handle update without quitting so containers using them as entrypoints don't exit on update ' (#1646)"

This reverts commit 8f160bc.

* Release 2.289.3 runner.
jenseng added a commit to jenseng/runner that referenced this pull request Jul 26, 2024
- Propagate non-zero exit codes up through run.sh/run.cmd/runsvc.sh
- Ensure .cmd scripts use 2 for retries (as was done for .sh in actions#1708)

Fixes actions#2769
Supersedes actions#2719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants