From c4b5f80918a5adf9b1b58dbe0813c397b8bea0f1 Mon Sep 17 00:00:00 2001
From: Alex Dadgar <alex.dadgar@gmail.com>
Date: Thu, 6 Dec 2018 12:30:31 -0800
Subject: [PATCH] Make alloc health watcher a postrun hook rather than shutdown
 hook

---
 client/allocrunner/health_hook.go      |  6 ++--
 client/allocrunner/health_hook_test.go | 45 ++++++++++++++------------
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/client/allocrunner/health_hook.go b/client/allocrunner/health_hook.go
index 8542e171c..c7b26a717 100644
--- a/client/allocrunner/health_hook.go
+++ b/client/allocrunner/health_hook.go
@@ -175,7 +175,7 @@ func (h *allocHealthWatcherHook) Update(req *interfaces.RunnerUpdateRequest) err
 	return h.init()
 }
 
-func (h *allocHealthWatcherHook) Destroy() error {
+func (h *allocHealthWatcherHook) Postrun() error {
 	h.hookLock.Lock()
 	defer h.hookLock.Unlock()
 
@@ -189,8 +189,8 @@ func (h *allocHealthWatcherHook) Destroy() error {
 }
 
 func (h *allocHealthWatcherHook) Shutdown() {
-	// Same as Destroy
-	h.Destroy()
+	// Same as Postrun
+	h.Postrun()
 }
 
 // watchHealth watches alloc health until it is set, the alloc is stopped, or
diff --git a/client/allocrunner/health_hook_test.go b/client/allocrunner/health_hook_test.go
index 4486cc369..cb2b3d7d1 100644
--- a/client/allocrunner/health_hook_test.go
+++ b/client/allocrunner/health_hook_test.go
@@ -22,7 +22,7 @@ import (
 // statically assert health hook implements the expected interfaces
 var _ interfaces.RunnerPrerunHook = (*allocHealthWatcherHook)(nil)
 var _ interfaces.RunnerUpdateHook = (*allocHealthWatcherHook)(nil)
-var _ interfaces.RunnerDestroyHook = (*allocHealthWatcherHook)(nil)
+var _ interfaces.RunnerPostrunHook = (*allocHealthWatcherHook)(nil)
 var _ interfaces.ShutdownHook = (*allocHealthWatcherHook)(nil)
 
 // allocHealth is emitted to a chan whenever SetHealth is called
@@ -76,8 +76,9 @@ func (m *mockHealthSetter) ClearHealth() {
 	m.taskEvents = nil
 }
 
-// TestHealthHook_PrerunDestroy asserts a health hook does not error if it is run and destroyed.
-func TestHealthHook_PrerunDestroy(t *testing.T) {
+// TestHealthHook_PrerunPostrun asserts a health hook does not error if it is
+// run and postrunned.
+func TestHealthHook_PrerunPostrun(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)
 
@@ -96,7 +97,7 @@ func TestHealthHook_PrerunDestroy(t *testing.T) {
 	require.True(ok)
 	_, ok = h.(interfaces.RunnerUpdateHook)
 	require.True(ok)
-	destroyh, ok := h.(interfaces.RunnerDestroyHook)
+	postrunh, ok := h.(interfaces.RunnerPostrunHook)
 	require.True(ok)
 
 	// Prerun
@@ -109,12 +110,12 @@ func TestHealthHook_PrerunDestroy(t *testing.T) {
 	assert.False(t, ahw.isDeploy)
 	ahw.hookLock.Unlock()
 
-	// Destroy
-	require.NoError(destroyh.Destroy())
+	// Postrun
+	require.NoError(postrunh.Postrun())
 }
 
-// TestHealthHook_PrerunUpdateDestroy asserts Updates may be applied concurrently.
-func TestHealthHook_PrerunUpdateDestroy(t *testing.T) {
+// TestHealthHook_PrerunUpdatePostrun asserts Updates may be applied concurrently.
+func TestHealthHook_PrerunUpdatePostrun(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)
 
@@ -147,13 +148,13 @@ func TestHealthHook_PrerunUpdateDestroy(t *testing.T) {
 		assert.NoError(t, err)
 	}
 
-	// Destroy
-	require.NoError(h.Destroy())
+	// Postrun
+	require.NoError(h.Postrun())
 }
 
-// TestHealthHook_UpdatePrerunDestroy asserts that a hook may have Update
+// TestHealthHook_UpdatePrerunPostrun asserts that a hook may have Update
 // called before Prerun.
-func TestHealthHook_UpdatePrerunDestroy(t *testing.T) {
+func TestHealthHook_UpdatePrerunPostrun(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)
 
@@ -191,12 +192,12 @@ func TestHealthHook_UpdatePrerunDestroy(t *testing.T) {
 	assert.True(t, h.isDeploy)
 	h.hookLock.Unlock()
 
-	// Destroy
-	require.NoError(h.Destroy())
+	// Postrun
+	require.NoError(h.Postrun())
 }
 
-// TestHealthHook_Destroy asserts that a hook may have only Destroy called.
-func TestHealthHook_Destroy(t *testing.T) {
+// TestHealthHook_Postrun asserts that a hook may have only Postrun called.
+func TestHealthHook_Postrun(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)
 
@@ -209,8 +210,8 @@ func TestHealthHook_Destroy(t *testing.T) {
 
 	h := newAllocHealthWatcherHook(logger, mock.Alloc(), hs, b.Listen(), consul).(*allocHealthWatcherHook)
 
-	// Destroy
-	require.NoError(h.Destroy())
+	// Postrun
+	require.NoError(h.Postrun())
 }
 
 // TestHealthHook_SetHealth asserts SetHealth is called when health status is
@@ -290,8 +291,8 @@ func TestHealthHook_SetHealth(t *testing.T) {
 		require.Nilf(ev, "%#v", health.taskEvents)
 	}
 
-	// Destroy
-	require.NoError(h.Destroy())
+	// Postrun
+	require.NoError(h.Postrun())
 }
 
 // TestHealthHook_SystemNoop asserts that system jobs return the noop tracker.
@@ -309,7 +310,9 @@ func TestHealthHook_SystemNoop(t *testing.T) {
 	require.False(t, ok)
 	_, ok = h.(interfaces.RunnerUpdateHook)
 	require.False(t, ok)
-	_, ok = h.(interfaces.RunnerDestroyHook)
+	_, ok = h.(interfaces.RunnerPostrunHook)
+	require.False(t, ok)
+	_, ok = h.(interfaces.ShutdownHook)
 	require.False(t, ok)
 }