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

cli: Fix a panic in deployment status when scheduling is slow #16011

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented Feb 1, 2023

Fixes #15235

If a deployment fails, the deployment status command can get a nil deployment when it checks for a rollback deployment if there isn't one (or at least not one at the time of the query). Fix the panic.

Note that I'm intentionally punting on solving the old TODO here, as it'd just take too long for me to page all this code in. Dropping a quick panic fix was a small win between other tasks 😁

@tgross tgross added type/bug theme/cli theme/crash backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line labels Feb 1, 2023
@tgross tgross added this to the 1.5.0 milestone Feb 1, 2023
@angrycub
Copy link
Contributor

angrycub commented Feb 1, 2023

Does this need to happen at line 444 also?

if rollback.ID == deploy.ID {
return
}

If a deployment fails, the `deployment status` command can get a nil deployment
when it checks for a rollback deployment if there isn't one (or at least not one
at the time of the query). Fix the panic.
@tgross
Copy link
Member Author

tgross commented Feb 1, 2023

Does this need to happen at line 444 also?

Good catch. Fixed! (Plus changelog!)

@tgross tgross force-pushed the b-deployment-status-panic-fix branch from 6963142 to 5c5a7cd Compare February 2, 2023 17:10
Copy link
Contributor

@angrycub angrycub left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line theme/cli theme/crash type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nil pointer dereference in deployment monitor
3 participants