investigating where to ignore poststop task in alloc health tracker

client/allochealth/tracker.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/consul/api"
 	hclog "github.com/hashicorp/go-hclog"
 	cconsul "github.com/hashicorp/nomad/client/consul"
@@ -118,6 +119,7 @@ func NewTracker(parentCtx context.Context, logger hclog.Logger, alloc *structs.A
 		t.taskHealth[task.Name] = &taskHealthState{task: task}
 
 		if task.Lifecycle != nil && !task.Lifecycle.Sidecar {
+			spew.Dump("ADDING LIFECYCLE TASK ", task.Name)
 			t.lifecycleTasks[task.Name] = true
 		}
 
@@ -283,7 +285,7 @@ func (t *Tracker) watchTaskEvents() {
 				return
 			}
 
-			if state.State == structs.TaskStatePending {
+			if (state.State == structs.TaskStatePending) && !t.lifecycleTasks[taskName] {
 				latestStartTime = time.Time{}
 				break
 			} else if state.StartedAt.After(latestStartTime) {
@@ -299,6 +301,7 @@ func (t *Tracker) watchTaskEvents() {
 			t.l.Lock()
 			t.allocFailed = true
 			t.l.Unlock()
+
 			t.setTaskHealth(false, true)
 			return
 		}
@@ -479,7 +482,9 @@ func (t *taskHealthState) event(deadline time.Time, minHealthyTime time.Duration
 
 		switch t.state.State {
 		case structs.TaskStatePending:
-			return "Task not running by deadline", true
+			if t.task.Lifecycle == nil || t.task.Lifecycle.Hook != structs.TaskLifecycleHookPoststop {
+				return "Task not running by deadline", true
+			}
 		case structs.TaskStateDead:
 			// hook tasks are healthy when dead successfully
 			if t.task.Lifecycle == nil || t.task.Lifecycle.Sidecar {

nomad/deploymentwatcher/deployments_watcher_test.go

@@ -1603,6 +1603,50 @@ func TestWatcher_BatchAllocUpdates(t *testing.T) {
 		func(err error) { require.Equal(2, watchersCount(w), "Should have 2 deployment") })
 }
 
+func TestWatcher_DeploymentHealth_Lifecycle(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+	w, m := defaultTestDeploymentWatcher(t)
+
+	m.On("UpdateDeploymentStatus", mocker.MatchedBy(func(args *structs.DeploymentStatusUpdateRequest) bool {
+		return true
+	})).Return(nil).Maybe()
+
+	// Create a job, alloc, and a deployment
+	j := mock.Job()
+	d := mock.Deployment()
+	d.JobID = j.ID
+	a := mock.Alloc()
+	a.DeploymentID = d.ID
+	require.Nil(m.state.UpsertJob(structs.MsgTypeTestSetup, m.nextIndex(), j), "UpsertJob")
+	require.Nil(m.state.UpsertDeployment(m.nextIndex(), d), "UpsertDeployment")
+	require.Nil(m.state.UpsertAllocs(structs.MsgTypeTestSetup, m.nextIndex(), []*structs.Allocation{a}), "UpsertAllocs")
+
+	// require that we get a call to UpsertDeploymentAllocHealth
+	matchConfig := &matchDeploymentAllocHealthRequestConfig{
+		DeploymentID: d.ID,
+		Healthy:      []string{a.ID},
+		Eval:         true,
+	}
+	matcher := matchDeploymentAllocHealthRequest(matchConfig)
+	m.On("UpdateDeploymentAllocHealth", mocker.MatchedBy(matcher)).Return(nil)
+
+	w.SetEnabled(true, m.state)
+	testutil.WaitForResult(func() (bool, error) { return 1 == watchersCount(w), nil },
+		func(err error) { require.Equal(1, watchersCount(w), "Should have 1 deployment") })
+
+	// Call SetAllocHealth
+	req := &structs.DeploymentAllocHealthRequest{
+		DeploymentID:         d.ID,
+		HealthyAllocationIDs: []string{a.ID},
+	}
+	var resp structs.DeploymentUpdateResponse
+	err := w.SetAllocHealth(req, &resp)
+	require.Nil(err, "SetAllocHealth")
+	require.Equal(1, watchersCount(w), "Deployment should still be active")
+	m.AssertCalled(t, "UpdateDeploymentAllocHealth", mocker.MatchedBy(matcher))
+
+}
 func watchersCount(w *Watcher) int {
 	w.l.Lock()
 	defer w.l.Unlock()