added in a "monitor" subcommand to monitor jobs to completion #141
Conversation
command/monitor.go
Outdated
| ) | ||
|
|
||
| // MonitorCommand is the command implementation that allows users to monitor | ||
| // a deploy until it completes |
There was a problem hiding this comment.
is it not monitoring the job until it completes?
There was a problem hiding this comment.
correct. will fix.
levant/monitor.go
Outdated
| time.Sleep(time.Second * 5) | ||
| continue | ||
| } | ||
| //logging.Info("lastIndex: %v", meta.LastIndex) |
There was a problem hiding this comment.
can we remove this commented out line?
|
|
||
| Arguments: | ||
|
|
||
| EVAL_ID nomad job evaluation |
There was a problem hiding this comment.
More of a design question and I am happy to be wrong, but would it not be better if this was the JobID and then Levant would discover the allocs from this, rather than the eval ID?
There was a problem hiding this comment.
I completely agree - that would be both better and more user friendly. The trouble is that if you are just monitoring the job via the job id, you aren't really guaranteed to be monitoring a particular invocation of levant deploy ....
As an example, if you run levant deploy job_1, and then a different user runs levant deploy job_1 after you do and before you run the monitor command, then, if you run levant monitor job_1 and we use the job_id to pull the most recent evaluation, you will wind up monitoring the other users invocation of the job rather than your own. I agree this is somewhat unlikely, however I can certainly see automation tools running into this. It seemed like the need to guarantee that the command monitors what you think it is monitoring was more important.
Having said that, I do think that you are right and many people will not care. We can put a ticket in for that, and I can add that in as an additional feature if that works for you.
command/monitor.go
Outdated
| Number of seconds to allow until we exit with an error. | ||
|
|
||
| -log-level=<level> | ||
| Specify the verbosity level of Levant's logs. Valid values include DEBUG, |
command/monitor.go
Outdated
| General Options: | ||
|
|
||
| -timeout=<int> | ||
| Number of seconds to allow until we exit with an error. |
There was a problem hiding this comment.
please use four spaces for this indent
|
@jrasell I fixed the formatting and comment issues. |
|
I think the check failures are an error in the checks. It seems to say that the MonitorCommand struct is not being used, but it is used in the commands.go file. Am I understanding that incorrectly? |
|
@dansteen the check is pointing the the |
|
@jrasell ok. fixed up the args issue, and also fixed up things to use new logging stuff. |
|
thanks @dansteen; I am going to push out a release and then get this in to 0.2.0 if that is OK for you? |
|
sure! |
|
@jrasell any chance of getting this in for the 0.2.0 that is being worked on now? |
|
yes sorry my bad; I have been a bit slack the past few weeks. are you able to resolve the conflict and I will get it in today |
|
pretty sure it's just our conversation above from the 26th. I don't think there are any actual conflicts. |
|
the conflict as in the merge conflict; if not I will fix it up when I have a moment later/tomorrow morning |
|
This seems pretty great and our team could definitely use this -- we're shipping database migrations prior to deploying new code and would love levant to verify that the tasks completed correctly or exit non-zero |
|
If given commit bit, I'd be happy to fix the merge conflicts (and merge once @jrasell gives second approval) :) |
|
|
|
This is very useful feature, @jrasell is there a way to move it forward? |
|
@eugengarkusha and @jrasell I signed the CLA just in case that was the only thing holding it back. |
Hi @jrasell,
I've finally managed to circle back around and get this sorted out. I've refactored the monitor function to be its own subcommand as we had discussed. I named it "monitor" (instead of "batch-monitor" or similar) because it can technically be used to monitor services or system jobs as well as batch jobs. I've also updating things to match how you are doing the CLI components.
I opened a new pull request for this since its so radically different from what I had before.
Meant to resolve #77
Thanks!