disconnected clients: Deployment watcher can overwrite `ClientStatus` with stale state
- Dominant language
- Go
- Stars
- 17k
- Forks
- 2.1k
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 105
Description
### Nomad version
1.3.0-beta1
### Operating system and Environment details
Vagrant or E2E environments
### Issue
The reconciler uses the alloc name+index to stop running allocs after they have been filtered into groups by `filterByTainted`. `filterByTainted` relies on task events to determine if an allocation has reconnected. It is possible for an evaluation to be triggered by the deployment watcher that believes the alloc has a client status of `running`. Most likely, the deployment watcher has an older version of the alloc before it transitioned to `unknown`. The deployment watcher needs to check the state store for any disconnect updates and take them into account before pushing updates.
### Reproduction steps
This issue was uncovered as a result of the E2E test not waiting for the deployment to finish. To recreate this scenario, comment out the call to `e2eutil.WaitForLastDeploymentStatus` in the test. and then run the `TestDisconnectedClients` E2E test against either an E2E environment or a Vagrant cluster and it will fail frequently though not every time since it is timing based.
#### Expected Result
The deployment watcher should check for updates before publishing it's results so that it can not overwrite the `unknown` client status.
#### Actual Result
The deployment watcher publishes a state client status resulting in the alloc being filtered to the untainted set. This results in a non-deterministic ordering of allocs and the wrong alloc may get stopped.
### Job file (if appropriate)
See [test job](https://github.com/hashicorp/nomad/blob/main/e2e/disconnectedclients/input/lost_max_disconnect.nomad)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the TestDisconnectedClients E2E test and the test job at e2e/disconnectedclients/input/lost_max_disconnect.nomad; reproduce the failure after commenting out e2eutil.WaitForLastDeploymentStatus. Trace the deployment watcher and state-store updates around disconnected allocations. Done means the watcher accounts for disconnect updates before publishing and the test no longer intermittently stops the wrong allocation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100