Implement script checks #986
Conversation
| c.logger.Printf("[DEBUG] client: error removing services from non-running tasks: %v", err) | ||
| } | ||
| sync = time.After(consulSyncInterval) |
There was a problem hiding this comment.
You are creating a lot of timers. Just outside the for loop create one https://golang.org/pkg/time/#NewTimer and then use sync.Reset()
There was a problem hiding this comment.
I think a Ticker would be more appropriate.
| if res.Err != nil { | ||
| output = res.Err.Error() | ||
| } | ||
| if res.ExitCode == 0 { |
There was a problem hiding this comment.
An option that I think is cleaner
var state string
switch res.ExitCode {
case 0:
state = consul.HealthPassing
case 1:
state = consul.HealthWarning
default:
state = consul.HealthCritical
}
Otherwise change the two ifs into an if/else if
| if err = client.StartExec(exec.ID, startOpts); err != nil { | ||
| return &cstructs.CheckResult{Err: err} | ||
| } | ||
| if execRes, err = client.InspectExec(exec.ID); err != nil { |
There was a problem hiding this comment.
@dadgar StartExec starts the exec process asynchronously and inspectexec should return too.
But the daemon can be un-responsive and can block indefinitely like any other calls to docker. And the docker client doesn't have the notion of timing out if the daemon takes a really long time.
|
Looking good. The one thing I didn't like was ConsulContext being used to transfer the docker endpoint information. Seems like an abuse of that. |
|
awesome job, thanks! |
|
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. |
No description provided.