Bugfix: auto-promote canary taskgroups when mixed with non-canary taskgroups #11878
Conversation
…rolling deploy taskgroups that do not use the canary deployment system
kainoaseto
force-pushed
the
fix/multi-task-group-canary-deploys
branch
from
0f61a8d
to
b8db8f3
Compare
There was a problem hiding this comment.
Looks great, thanks! I added a changelog entry. Feel free to update the wording if you can think of better phrasing.
Looking back at the original PR for this code it seems likely we just forgot this case. Great work tracking it down and adjusting a test to cover it.
| a := canaryAlloc() | ||
| b := canaryAlloc() | ||
|
|
||
| // Api taskgroup (1) | ||
| c := rollingAlloc() | ||
| e := rollingAlloc() |
There was a problem hiding this comment.
Might want to give up on 1 letter variable names at this point (ca1, ca2, ra1, and ra2 perhaps), but not a blocker.
There was a problem hiding this comment.
Thank you for taking a look at this @schmichael and updating the changelog! Nice, that was feeling a bit weird and I'll make that change shortly
|
Merged! Will be release in 1.3.0 and backported to 1.1.x and 1.2.x at that time (or sooner). Thanks again. Deployments are often a source of confusion for users, so if you spot any other bugs or improvements please don't hesitate to open issues or PRs. |
lgfa29
added
backport/1.2.x
|
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. |
When using the auto promote feature with canary deployments that have task groups without canaries, the deployment will never auto promote and hang even when the canaries are all healthy for the task groups that it has been enabled for.
This PR fixes this bug by skipping task groups that have no canaries set during the auto promote validation and adds a test to catch this case specifically. To see what occurs when this fix is not implemented (as has been observed in mainstream Nomad):
L292-L294 of nomad/deploymentwatcher/deployment_watcher.goLet me know if there's anything I can do to help shepard this through into a release, currently there's manual intervention in some of our deployments that this occurs in and it would be great to alleviate that.