From bf9a2168e74b0db587043c007e92a30c099bee26 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 17 Jan 2019 15:27:15 -0600 Subject: [PATCH 1/5] Rename TaskKillHook to TaskPreKillHook to more closely match usage Also added/fixed comments --- client/allocrunner/interfaces/task_lifecycle.go | 6 +++--- client/allocrunner/taskrunner/lifecycle.go | 4 ++-- client/allocrunner/taskrunner/service_hook.go | 2 +- client/allocrunner/taskrunner/task_runner.go | 4 ++-- client/allocrunner/taskrunner/task_runner_hooks.go | 14 ++++++++------ 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/client/allocrunner/interfaces/task_lifecycle.go b/client/allocrunner/interfaces/task_lifecycle.go index 2b3ed91d5..c95b5d059 100644 --- a/client/allocrunner/interfaces/task_lifecycle.go +++ b/client/allocrunner/interfaces/task_lifecycle.go @@ -117,11 +117,11 @@ type TaskPoststartHook interface { type TaskKillRequest struct{} type TaskKillResponse struct{} -type TaskKillHook interface { +type TaskPreKillHook interface { TaskHook - // Killing is called when a task is going to be Killed or Restarted. - Killing(context.Context, *TaskKillRequest, *TaskKillResponse) error + // PreKilling is called right before a task is going to be killed or restarted. + PreKilling(context.Context, *TaskKillRequest, *TaskKillResponse) error } type TaskExitedRequest struct{} diff --git a/client/allocrunner/taskrunner/lifecycle.go b/client/allocrunner/taskrunner/lifecycle.go index ac208e723..e4c84506e 100644 --- a/client/allocrunner/taskrunner/lifecycle.go +++ b/client/allocrunner/taskrunner/lifecycle.go @@ -22,8 +22,8 @@ func (tr *TaskRunner) Restart(ctx context.Context, event *structs.TaskEvent, fai // Emit the event since it may take a long time to kill tr.EmitEvent(event) - // Run the hooks prior to restarting the task - tr.killing() + // Run the pre-kill hooks prior to restarting the task + tr.preKill() // Tell the restart tracker that a restart triggered the exit tr.restartTracker.SetRestartTriggered(failure) diff --git a/client/allocrunner/taskrunner/service_hook.go b/client/allocrunner/taskrunner/service_hook.go index 55bd0b60e..d067b1811 100644 --- a/client/allocrunner/taskrunner/service_hook.go +++ b/client/allocrunner/taskrunner/service_hook.go @@ -200,7 +200,7 @@ func (h *serviceHook) getTaskServices() *agentconsul.TaskServices { // values from the task's environment. func interpolateServices(taskEnv *taskenv.TaskEnv, services []*structs.Service) []*structs.Service { // Guard against not having a valid taskEnv. This can be the case if the - // Killing or Exited hook is run before post-run. + // PreKilling or Exited hook is run before post-run. if taskEnv == nil || len(services) == 0 { return nil } diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index 5d22a6965..e7f2cd478 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -707,8 +707,8 @@ func (tr *TaskRunner) initDriver() error { // the handle exit result if one is available and store any error in the task // runner killErr value. func (tr *TaskRunner) handleKill() *drivers.ExitResult { - // Run the hooks prior to killing the task - tr.killing() + // Run the pre killing hooks + tr.preKill() // Tell the restart tracker that the task has been killed so it doesn't // attempt to restart it. diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 88b10cd57..71ad5d9c8 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -437,8 +437,10 @@ func (tr *TaskRunner) updateHooks() { } } -// killing is used to run the runners kill hooks. -func (tr *TaskRunner) killing() { +// preKill is used to run the runners preKill hooks +// preKill hooks contain logic that must be executed before +// a task is killed or restarted +func (tr *TaskRunner) preKill() { if tr.logger.IsTrace() { start := time.Now() tr.logger.Trace("running kill hooks", "start", start) @@ -449,24 +451,24 @@ func (tr *TaskRunner) killing() { } for _, hook := range tr.runnerHooks { - killHook, ok := hook.(interfaces.TaskKillHook) + killHook, ok := hook.(interfaces.TaskPreKillHook) if !ok { continue } name := killHook.Name() - // Time the update hook + // Time the pre kill hook var start time.Time if tr.logger.IsTrace() { start = time.Now() tr.logger.Trace("running kill hook", "name", name, "start", start) } - // Run the kill hook + // Run the pre kill hook req := interfaces.TaskKillRequest{} var resp interfaces.TaskKillResponse - if err := killHook.Killing(context.Background(), &req, &resp); err != nil { + if err := killHook.PreKilling(context.Background(), &req, &resp); err != nil { tr.emitHookError(err, name) tr.logger.Error("kill hook failed", "name", name, "error", err) } From 77c01c628331468b124fd9648f1db6ea91e567a2 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 22 Jan 2019 08:53:26 -0600 Subject: [PATCH 2/5] Fix comment Co-Authored-By: preetapan --- client/allocrunner/taskrunner/service_hook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/allocrunner/taskrunner/service_hook.go b/client/allocrunner/taskrunner/service_hook.go index d067b1811..f12ae2688 100644 --- a/client/allocrunner/taskrunner/service_hook.go +++ b/client/allocrunner/taskrunner/service_hook.go @@ -200,7 +200,7 @@ func (h *serviceHook) getTaskServices() *agentconsul.TaskServices { // values from the task's environment. func interpolateServices(taskEnv *taskenv.TaskEnv, services []*structs.Service) []*structs.Service { // Guard against not having a valid taskEnv. This can be the case if the - // PreKilling or Exited hook is run before post-run. + // PreKilling or Exited hook is run before Poststart. if taskEnv == nil || len(services) == 0 { return nil } From c3f044291d48236bfe96db27c9021f36a9cd782b Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 17 Jan 2019 15:27:15 -0600 Subject: [PATCH 3/5] Rename TaskKillHook to TaskPreKillHook to more closely match usage Also added/fixed comments --- client/allocrunner/taskrunner/task_runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index e7f2cd478..aff53994d 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -704,8 +704,8 @@ func (tr *TaskRunner) initDriver() error { } // handleKill is used to handle the a request to kill a task. It will return -// the handle exit result if one is available and store any error in the task -// runner killErr value. +//// the handle exit result if one is available and store any error in the task +//// runner killErr value. func (tr *TaskRunner) handleKill() *drivers.ExitResult { // Run the pre killing hooks tr.preKill() From 6f3e4e72b95bc0f279d250aa66a6630517f3803a Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Tue, 22 Jan 2019 09:45:58 -0600 Subject: [PATCH 4/5] Fix log comments --- client/allocrunner/taskrunner/task_runner_hooks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 71ad5d9c8..a37014698 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -443,10 +443,10 @@ func (tr *TaskRunner) updateHooks() { func (tr *TaskRunner) preKill() { if tr.logger.IsTrace() { start := time.Now() - tr.logger.Trace("running kill hooks", "start", start) + tr.logger.Trace("running pre kill hooks", "start", start) defer func() { end := time.Now() - tr.logger.Trace("finished kill hooks", "end", end, "duration", end.Sub(start)) + tr.logger.Trace("finished pre kill hooks", "end", end, "duration", end.Sub(start)) }() } From a1cc48f78d10e1789af3664856bc0c658437a114 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Tue, 22 Jan 2019 09:54:02 -0600 Subject: [PATCH 5/5] Rename TaskKillRequest/Response to TaskPreKillRequest/Response --- client/allocrunner/interfaces/task_lifecycle.go | 6 +++--- client/allocrunner/taskrunner/service_hook.go | 2 +- client/allocrunner/taskrunner/task_runner_hooks.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/allocrunner/interfaces/task_lifecycle.go b/client/allocrunner/interfaces/task_lifecycle.go index c95b5d059..01d17dfbf 100644 --- a/client/allocrunner/interfaces/task_lifecycle.go +++ b/client/allocrunner/interfaces/task_lifecycle.go @@ -114,14 +114,14 @@ type TaskPoststartHook interface { Poststart(context.Context, *TaskPoststartRequest, *TaskPoststartResponse) error } -type TaskKillRequest struct{} -type TaskKillResponse struct{} +type TaskPreKillRequest struct{} +type TaskPreKillResponse struct{} type TaskPreKillHook interface { TaskHook // PreKilling is called right before a task is going to be killed or restarted. - PreKilling(context.Context, *TaskKillRequest, *TaskKillResponse) error + PreKilling(context.Context, *TaskPreKillRequest, *TaskPreKillResponse) error } type TaskExitedRequest struct{} diff --git a/client/allocrunner/taskrunner/service_hook.go b/client/allocrunner/taskrunner/service_hook.go index f12ae2688..f9e4ebb24 100644 --- a/client/allocrunner/taskrunner/service_hook.go +++ b/client/allocrunner/taskrunner/service_hook.go @@ -140,7 +140,7 @@ func (h *serviceHook) Update(ctx context.Context, req *interfaces.TaskUpdateRequ return h.consul.UpdateTask(oldTaskServices, newTaskServices) } -func (h *serviceHook) Killing(ctx context.Context, req *interfaces.TaskKillRequest, resp *interfaces.TaskKillResponse) error { +func (h *serviceHook) Killing(ctx context.Context, req *interfaces.TaskPreKillRequest, resp *interfaces.TaskPreKillResponse) error { h.mu.Lock() defer h.mu.Unlock() diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index a37014698..f26390faf 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -466,8 +466,8 @@ func (tr *TaskRunner) preKill() { } // Run the pre kill hook - req := interfaces.TaskKillRequest{} - var resp interfaces.TaskKillResponse + req := interfaces.TaskPreKillRequest{} + var resp interfaces.TaskPreKillResponse if err := killHook.PreKilling(context.Background(), &req, &resp); err != nil { tr.emitHookError(err, name) tr.logger.Error("kill hook failed", "name", name, "error", err)