core: enforce strict steps for clients reconnect

[lgfa29]

When a Nomad client that is running an allocation with max_client_disconnect set misses a heartbeat the Nomad server will update its status to disconnected.

Upon reconnecting, the client will make three main RPC calls:

• Node.UpdateStatus is used to set the client status to ready.
• Node.UpdateAlloc is used to update the client-side information about
  allocations, such as their ClientStatus, task states etc.
• Node.Register is used to upsert the entire node information,
  including its status.

These calls are made concurrently and are also running in parallel with the scheduler. Depending on the order they run the scheduler may end up with incomplete data when reconciling allocations.

#15068 already enforced clients to heartbeat before updating their allocation data, but there are still scenarios that can generate wrong results.

For example, a client disconnects and its replacement allocation cannot be placed anywhere else, so there's a pending eval waiting for resources.

When this client comes back the order of events may be:

1. Client calls Node.UpdateStatus and is now ready.
2. Scheduler reconciles allocations and places the replacement alloc to
   the client. The client is now assigned two allocations: the original
   alloc that is still unknown and the replacement that is pending.
3. Client calls Node.UpdateAlloc and updates the original alloc to
   running.
4. Scheduler notices too many allocs and stops the replacement.

This creates unnecessary placements or, in a different order of events, may leave the job without any allocations running until the whole state is updated and reconciled.

To avoid problems like this clients must update all of its relevant information before they can be considered ready and available for scheduling.

To achieve this goal the RPC endpoints mentioned above have been modified to enforce strict steps for nodes reconnecting:

• Node.Register does not set the client status anymore.
• Node.UpdateStatus sets the reconnecting client to the initializing
  status until it successfully calls Node.UpdateAlloc.

These changes are done server-side to avoid the need of additional coordination between clients and servers. Clients are kept oblivious of these changes and will keep making these calls as they normally would.

The verification of whether allocations have been updates is done by storing and comparing the Raft index of the last time the client missed a heartbeat and the last time it updated its allocations.

Closes #15483

[lgfa29]

I changed the error message in https://github.com/hashicorp/nomad/pull/15808/files#diff-86b976ad80dd125c60d191875f83e20d1e4ee971966b910665d75c3165cb65e4L1182-R1209

[lgfa29]

I'm not sure how accurate this name is. It's more like the first time the node last transition to an unresponsive status, but I couldn't think of a good name for this 😅

[tgross]

Yeah this name really implies to me that it's going to keep getting updated, whereas it's the index the node became unresponsive. It might be nice if we could clear the value once we're certain the node is live again, and that'd make things a little less confusing at the cost of a little extra logic to check for 0 in the UpdateStatus RPC.

[lgfa29]

Ah good point. Resetting it zero may help to indicate that this field is sort edge triggered. I pushed a commit do just that. Thanks!

[tgross]

This is looking really good @lgfa29. I think you boiled down the complex problem into a really nice targeted change here. I've mostly left suggestions but also one potentially nasty bug.

[tgross]

This looks great @lgfa29!

[tgross]

Yeah this name really implies to me that it's going to keep getting updated, whereas it's the index the node became unresponsive. It might be nice if we could clear the value once we're certain the node is live again, and that'd make things a little less confusing at the cost of a little extra logic to check for 0 in the UpdateStatus RPC.