client GC: separate allocrunner from allocdir management
- Dominant language
- Go
- Stars
- 17k
- Forks
- 2.1k
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 105
Description
After an allocation is terminal, the Nomad client needs to keep the allocation directory around so that users can read their logs to debug failed allocations. But to do so, Nomad keeps the entire [`AllocRunner`](https://github.com/hashicorp/nomad/tree/main/client/allocrunner) alive, which uses up memory resources we should be freeing for the user's workloads (sometimes substantially, ex. https://github.com/hashicorp/nomad/issues/25269), and complicates doing the actual GC (ex. https://github.com/hashicorp/nomad/pull/25123). At the end of the day users want the alloc dir to persist as long as they don't otherwise need the disk space, but shouldn't have to pay for all this overhead.
Nomad should instead split the allocrunner and the alloc directory into separate objects, so that we can abandon the allocrunner once the allocation is terminal, and keep the alloc directory around until GC so we can serve `/v1/client/fs` APIs like [Stream Logs](https://developer.hashicorp.com/nomad/api-docs/client#stream-logs).
There are several subtasks to figure out, mostly around the cleanup operations in allocrunner hooks:
* The `csi_hook` has a `Destroy` hook method (triggered on GC). It looks like this only cancels any in-flight requests. Can we safely move this to a `Postrun` hook?
* The `consul_hook` has a `Destroy` hook method. This exactly copies the `Postrun` hook method. Why do we feel like we need both here? Do we have a resource cleanup problem that we're trying to paper over?
* The `identity_hook` has a `Destroy` hook method. This exactly copies the `PreKill` and `Shutdown` hook methods. Why do we feel like we need all three here? Why isn't there a `Postrun` hook?
* The `allocdir_hook` has a `Destroy` hook method which is used to cleanup on GC. We would need to move this out of the allocrunner entirely and into whatever "alloc directory proxy" we build.
* The alloc FS RPCs defined in `fs_endpoint.go` will need reworking to use some new client object as a proxy to the allocdir. It looks like there are a small number of methods to implement (ex. we need the allocation ID to find the allocdir and its namespace to check auth).
* There will be a bunch of refactoring to do in [`client/client.go`](https://github.com/hashicorp/nomad/blob/main/client/client.go) and [`client/gc.go`](https://github.com/hashicorp/nomad/blob/main/client/gc.go) so that we're ensuring the allocrunner is completely dead before cleaning up the disk for GC.
(internal ref https://hashicorp.atlassian.net/browse/NET-12306)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing allocation cleanup through client/allocrunner, its hook implementations, client/fs_endpoint.go, client/client.go, and client/gc.go. Review the csi_hook, consul_hook, identity_hook, and allocdir_hook cleanup paths first. Done means the allocrunner can be released after an allocation becomes terminal while a separate alloc-directory object continues serving the client FS APIs until GC.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, infrastructure
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100