cli: add monitor flag to deployment status #10661
Conversation
|
I'd love to see something like that added to |
|
Hi @apollo13! This PR is associated with @isabeldepapel's work on #6818 which is intended to implement just that. |
isabeldepapel
force-pushed
the
f-deploy-status-monitor
branch
from
f8c6672
to
9433fbe
Compare
isabeldepapel
force-pushed
the
f-deploy-status-monitor
branch
from
9433fbe
to
8c828e8
Compare
isabeldepapel
force-pushed
the
f-deploy-status-monitor
branch
from
199a5c5
to
e76650b
Compare
isabeldepapel
force-pushed
the
f-deploy-status-monitor
branch
from
e76650b
to
9005829
Compare
There was a problem hiding this comment.
This looks great @isabeldepapel! I've left a few suggestions / questions but nothing that should be a show-stopper. If you want to do the docs updates in a separate PR that's fine too, so long as they land in the same release.
|
|
||
| switch status { | ||
| case structs.DeploymentStatusFailed: | ||
| if hasAutoRevert(deploy) { |
There was a problem hiding this comment.
👍 This is a nice usability addition so that folks don't have to go rummaging around for it themselves.
| return | ||
| } | ||
|
|
||
| // Check for noop/no target rollbacks |
There was a problem hiding this comment.
We slept before calling LatestDeployment above, which I think will work most of the time.
But if we have a cluster that's maybe in a troubled state with slow scheduling, can that LatestDeployment call potentially get the same deployment that just failed even though there eventually will be a rollback deployment? I think it's probably okay to bail out in that situation just because we probably don't have a good way of determining that, but let's flesh out this comment a bit to make sure that's clear to developers in case we want to revisit that decision later.
There was a problem hiding this comment.
Yeah, I wasn't sure about sleeping since it didn't seem like the most robust way of getting the rollback but I couldn't think of anything better. Great suggestion on the comment though, will do
Adding '-verbose' will print out the allocation information for the deployment. This also changes the job run command so that it now blocks until deployment is complete and adds timestamps to the output so that it's more in line with the output of node drain. This uses glint to print in place in running in a tty. Because glint doesn't yet support cmd/powershell, Windows workflows use a different library to print in place, which results in slightly different formatting: 1) different margins, and 2) no spinner indicating deployment in progress.
Glint pulled in an updated version of mitchellh/go-testing-interface which broke some existing tests because the update added a Parallel() method to testing.T. This switches to the standard library testing.TB which doesn't have a Parallel() method.
isabeldepapel
force-pushed
the
f-deploy-status-monitor
branch
from
ff093d8
to
bfbaef0
Compare
|
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. |
Fixes #6818
Adding '-verbose' will print out the allocation information for the
deployment. This also changes the job run command so that it now blocks
until deployment is complete and adds timestamps to the output so that
it's more in line with the output of node drain.
This uses glint to print in place in running in a tty. Because glint
doesn't yet support cmd/powershell, Windows workflows use a different
library to print in place, which results in slightly different
formatting: 1) different margins, and 2) no spinner indicating
deployment in progress.