From 81cad55d405794afd22f51760ccd7c66d2eef09c Mon Sep 17 00:00:00 2001
From: Jasmine Dahilig <jasmine@dahilig.com>
Date: Mon, 31 Aug 2020 13:22:41 -0700
Subject: [PATCH] task lifecycle poststart: code review fixes

---
 client/allocrunner/alloc_runner_test.go | 24 +++++++++++++++---------
 e2e/lifecycle/lifecycle.go              |  8 ++------
 nomad/structs/structs.go                |  2 +-
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/client/allocrunner/alloc_runner_test.go b/client/allocrunner/alloc_runner_test.go
index 24344669a28..b64990e0497 100644
--- a/client/allocrunner/alloc_runner_test.go
+++ b/client/allocrunner/alloc_runner_test.go
@@ -247,16 +247,16 @@ func TestAllocRunner_TaskMain_KillTG(t *testing.T) {
 	alloc.Job.TaskGroups[0].Tasks[0].RestartPolicy.Attempts = 0
 
 	// Create four tasks in the task group
-	sidecar := alloc.Job.TaskGroups[0].Tasks[0].Copy()
-	sidecar.Name = "prestart-sidecar"
-	sidecar.Driver = "mock_driver"
-	sidecar.KillTimeout = 10 * time.Millisecond
-	sidecar.Lifecycle = &structs.TaskLifecycleConfig{
+	prestart := alloc.Job.TaskGroups[0].Tasks[0].Copy()
+	prestart.Name = "prestart-sidecar"
+	prestart.Driver = "mock_driver"
+	prestart.KillTimeout = 10 * time.Millisecond
+	prestart.Lifecycle = &structs.TaskLifecycleConfig{
 		Hook:    structs.TaskLifecycleHookPrestart,
 		Sidecar: true,
 	}
 
-	sidecar.Config = map[string]interface{}{
+	prestart.Config = map[string]interface{}{
 		"run_for": "100s",
 	}
 
@@ -288,9 +288,9 @@ func TestAllocRunner_TaskMain_KillTG(t *testing.T) {
 		"run_for": "2s",
 	}
 
-	alloc.Job.TaskGroups[0].Tasks = []*structs.Task{sidecar, main1, main2}
+	alloc.Job.TaskGroups[0].Tasks = []*structs.Task{prestart, poststart, main1, main2}
 	alloc.AllocatedResources.Tasks = map[string]*structs.AllocatedTaskResources{
-		sidecar.Name:   tr,
+		prestart.Name:  tr,
 		poststart.Name: tr,
 		main1.Name:     tr,
 		main2.Name:     tr,
@@ -327,7 +327,10 @@ func TestAllocRunner_TaskMain_KillTG(t *testing.T) {
 		var state *structs.TaskState
 
 		// both sidecars should be killed because Task2 exited
-		state = last.TaskStates[sidecar.Name]
+		state = last.TaskStates[prestart.Name]
+		if state == nil {
+			return false, fmt.Errorf("could not find state for task %s", prestart.Name)
+		}
 		if state.State != structs.TaskStateDead {
 			return false, fmt.Errorf("got state %v; want %v", state.State, structs.TaskStateDead)
 		}
@@ -344,6 +347,9 @@ func TestAllocRunner_TaskMain_KillTG(t *testing.T) {
 		}
 
 		state = last.TaskStates[poststart.Name]
+		if state == nil {
+			return false, fmt.Errorf("could not find state for task %s", poststart.Name)
+		}
 		if state.State != structs.TaskStateDead {
 			return false, fmt.Errorf("got state %v; want %v", state.State, structs.TaskStateDead)
 		}
diff --git a/e2e/lifecycle/lifecycle.go b/e2e/lifecycle/lifecycle.go
index 075612d652f..89a42ff16cc 100644
--- a/e2e/lifecycle/lifecycle.go
+++ b/e2e/lifecycle/lifecycle.go
@@ -2,6 +2,7 @@ package lifecycle
 
 import (
 	"fmt"
+
 	"github.com/hashicorp/nomad/api"
 	"github.com/hashicorp/nomad/e2e/e2eutil"
 	"github.com/hashicorp/nomad/e2e/framework"
@@ -18,8 +19,7 @@ type LifecycleE2ETest struct {
 
 func init() {
 	framework.AddSuites(&framework.TestSuite{
-		Component: "Lifecycle",
-		// YOU COULD RUN THIS LOCALLY BC DIS FLAG
+		Component:   "Lifecycle",
 		CanRunLocal: true,
 		Cases:       []framework.TestCase{new(LifecycleE2ETest)},
 	})
@@ -61,10 +61,6 @@ func (tc *LifecycleE2ETest) TestBatchJob(f *framework.F) {
 	require.Equal(expected, got)
 }
 
-// TODO: cleanup == poststop
-//       q: what is a good example for a poststart?
-//       a: notify(-slack)
-
 // TestServiceJob runs a service job with prestart and poststop hooks
 func (tc *LifecycleE2ETest) TestServiceJob(f *framework.F) {
 	t := f.T()
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 437af243cd8..4c4aefdb633 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -4905,7 +4905,7 @@ func (d *DispatchPayloadConfig) Validate() error {
 }
 
 const (
-	TaskLifecycleHookPrestart = "prestart"
+	TaskLifecycleHookPrestart  = "prestart"
 	TaskLifecycleHookPoststart = "poststart"
 )