-
Notifications
You must be signed in to change notification settings - Fork 2k
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
CLI: Return non-zero exit code when deployment fails in nomad run
#11550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lukas-w! This is looking good! Once you've addressed the comments here, if you'd like to add a changelog item for the improvement that'd be great. If that's a confusing mess (understandable), let me know and I'll get that added.
Co-authored-by: Tim Gross <tgross@hashicorp.com>
Thanks for the review @tgross! I added a changelog entry via e78fd43 but included the |
LGTM. The changelog entry is perfect. Thanks @lukas-w! This will ship in the next planned patch release. |
PR #11550 changed the job stop exit behaviour when monitoring the deployment. When stopping a job, the deployment becomes cancelled and therefore the CLI now exits with status code 1 as it see this as an error. This change adds a new utility e2e function that accounts for this behaviour.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR changes the
nomad run
command (and other commands using themonitor.monitor
function) to return a non-zero exit code when monitoring is enabled and deployment fails just likelevant deploy
does. This enables CI scripts to easily detect deployment failures. I chose return code1
to match the existing documentation stating0
on success,2
on scheduling problems, and1
for any other failures. A dedicated return code for deployment failures may makes sense though.I also noticed and fixed a supposed typo from #10661 via 24fbb5e: The monitoring code was using
DeploymentStatusDescriptionBlocked
instead ofDeploymentStatusBlocked
.Fixes #11082, fixes #10994.