Template emits events explaining why it is blocked #3001
Conversation
This PR does the following: * Adds a mechanism to emit events in the TaskRunner * Vendors a new version of Consul-Template that allows extraction of missing dependencies * Adds logic to our consul_template.go to determine missing events and emit them in a batched fashion. * Refactors the consul_template code to split the run method and take in a config struct rather than many parameters. Fixes #2578
dadgar
force-pushed
the
f-template-events
branch
from
aad9efc
to
1e7ae91
Compare
|
nice! |
| envBuilder *env.Builder) (*TaskTemplateManager, error) { | ||
| // TaskTemplateManagerConfig is used to configure an instance of the | ||
| // TaskTemplateManager | ||
| type TaskTemplateManagerConfig struct { |
client/consul_template.go
Outdated
| @@ -173,7 +260,7 @@ WAIT: | |||
| continue | |||
| } | |||
|
|
|||
| tm.hook.Kill("consul-template", err.Error(), true) | |||
| tm.config.Hooks.Kill("template", err.Error(), true) | |||
There was a problem hiding this comment.
This is "Template" elsewhere. Perhaps use a const?
client/consul_template.go
Outdated
| eventTimerCh = eventTimer.C | ||
| defer eventTimer.Stop() | ||
| outstandingEvent = true | ||
| continue |
There was a problem hiding this comment.
If you create a stopped timer outside of this loop could you kill this block and just let the if !outstandEvent { block handle resetting the timer?
There was a problem hiding this comment.
Yeah I did it this way to avoid creating the timer if it wasn't needed. But its not worth the extra code complexity.
| @@ -1318,7 +1344,7 @@ func (r *TaskRunner) killTask(killingEvent *structs.TaskEvent) { | |||
| r.runningLock.Unlock() | |||
|
|
|||
| // Store that the task has been destroyed and any associated error. | |||
| r.setState("", structs.NewTaskEvent(structs.TaskKilled).SetKillError(err)) | |||
| r.setState("", structs.NewTaskEvent(structs.TaskKilled).SetKillError(err), true) | |||
There was a problem hiding this comment.
Hm, why can this be lazy sync'd?
There was a problem hiding this comment.
If you look at the callers they all sync immediately after
| @@ -643,10 +644,18 @@ func (r *AllocRunner) setTaskState(taskName, state string, event *structs.TaskEv | |||
| r.appendTaskEvent(taskState, event) | |||
| } | |||
|
|
|||
| if state == "" { | |||
| if lazySync { | |||
There was a problem hiding this comment.
Why do we need a new flag for this? Can't we skip sync'ing if new state == old state (meaning just a local event has occurred)?
There was a problem hiding this comment.
No because we want to push the events. The lazy sync is useful when the event will be immediately followed up so that you can avoid a networked sync.
|
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. |
This PR does the following:
missing dependencies
them in a batched fashion.
config struct rather than many parameters.
Fixes #2578