Track Tasks State

[dadgar]

This PR:

• Adds TaskState to the allocation.
• The Client is updated to track the Task through the States: Pending, Running, Dead and records the events that caused state transitions between these.

On a side note, this PR also fixes the alloc_runner and task_runner test regressions introduced with restart policies.

[diptanu]

Are we bounding the size of Events somewhere? A service style job might potentially have a lot of events over the course of time if the node on which it is placed remains healthy for a really long time.

[dadgar]

I will in setState as suggested below

[diptanu]

@dadgar Overall looks a good step in the right direction with respect to TaskStates, and design looks fine too. Left you some questions.

[cbednarski]

How are these different from the other states defined here or here?

I think it's a bit confusing having two notions of state declared in two places. It seems like the ones in the api are intended for display, but there is a difference in granularity for failure states. Can we consolidate these somehow?

[dadgar]

Yeah I have an open issue to do this: #329. But this is not something to tackle in this PR. It is across the whole project.

[cbednarski]

But even in this file there are two blocks of const declarations for TaskState, one which describes 3 ways in which the task can be dead: https://github.com/hashicorp/nomad/blob/f-task-state/nomad/structs/structs.go#L1056-L1087

It seems like it would be clearer to do something like:

func (t *TaskState) IsAlive() bool {
  return !(t.State == TaskDriverFailure || t.State == TaskTerminated || t.State == TaskKilled)
}

if task.IsAlive() {
  // Do something
}

and remove TaskStateDead, rather than

if task.State == TaskStateDead ...
// or
state.Events[last].Type == structs.TaskTerminated

Particularly with this switch in syncStatus() it seems like the semantics are buried and could be more easily expressed via state.IsRunning(), state.IsAlive(), etc.

With untyped constants it's not clear in the code which should be used where and I think it also creates an opportunity to show end-users an ambiguous "dead" state when there is more information available.

[dadgar]

Ah there is a confusion. The State can only be pending, running, dead. However there are a lot of ways a task can enter these states. The TaskEvent are those. If you think of it as a finite state machine, the State are the nodes and the Events are the transitions.

[github-actions]

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.