Add simple options for enabling Gremlin GPU access - #147
Conversation
philgebhardt
left a comment
There was a problem hiding this comment.
Nice work. I only took a partial first pass here. I've got some more thoughts on the organization of our vendor items, but wanted to publish these first.
There was a problem hiding this comment.
Do we need unit tests for this document?
philgebhardt
left a comment
There was a problem hiding this comment.
I've proposed an alternative layout for the vendor configuration but I'm not convinced it's complete, as it's related to one open question I have: If I have a cluster with 90% no GPU hardware and 10% nvidia, what happens to the gremlin pods on the non-GPU nodes?
philgebhardt
left a comment
There was a problem hiding this comment.
This is looking good. I think there's one more conceptual change we should make here to align address the upgrade issues (re: --force), and repeatability/testing:
When gremlin.gpu.enabled=false, we keep the existing single daemonset and name:
<release>-gremlin
When gremlin.gpu.enabled=true, we unconditionally produce two daemonsets:
<release>-gremlin-gpu-<vendor><release>-gremlin-gpu-none(matching the non-gpu hardware nodes; which may be 0 at runtime)
(on helm upgrade the old <release>-gremlin is cleanly deleted, replaced with the two new ones)
This allows our daemonset system to react to changes in the target cluster's node topology, as it relates to their chosen vendor while keeping the construction of our daemonset YAMLs a pure function. This should also give a test-time win, getting rid of lookup.
| On a cluster where only some nodes have GPUs and different nodes may have different GPU vendors, a single GPU DaemonSet cannot run cluster-wide: nodes lacking the vendor's RuntimeClass or device mounts would fail to start the Gremlin pod. So whenever `gremlin.gpu.enabled` is set, the chart inspects the cluster's Nodes at install/upgrade time and renders: | ||
|
|
||
| - one GPU DaemonSet per detected vendor (`<release>-gremlin-gpu-<vendor>`), scheduled via node affinity onto that vendor's nodes and carrying that vendor's GPU configuration, and | ||
| - one plain DaemonSet (`<release>-gremlin`) for every remaining non-GPU node. |
There was a problem hiding this comment.
Specifically for when gremlin.gpu.enabled=true, we could instead name the non-gpu daemonset something different from our existing naming scheme. I think this should remove the need for the --force based upgrade mentioned below and let the cluster upgrade the pods cleaning.
| fallback when the cluster's nodes are not visible. | ||
| */ -}} | ||
| {{- if .Values.gremlin.gpu.enabled -}} | ||
| {{- if include "gremlinGpuNodesVisible" . -}} |
There was a problem hiding this comment.
Because these *Visible functions depend on node state, the same helm upgrade on the same cluster can yield different results depending on the cluster's node topology at the time of upgrade.
Given a user has declared gremlin.gpu.enabled=true, and selected a vendor. Is there any reason we shouldn't always create a daemonset, regardless of whether nodes exist?
|
|
||
| This chart does not create RuntimeClass objects; any RuntimeClass named by a vendor block must already exist on the cluster. | ||
|
|
||
| _note_: The per-node DaemonSets carry an extra `gremlin.com/gpu` pod-selector label so each only manages its own pods. Because a DaemonSet's selector is immutable, upgrading an existing GPU-less Gremlin install to a GPU one may require deleting the old DaemonSet first (or `helm upgrade --force`). |
There was a problem hiding this comment.
I think first time upgrading from an existing cluster happens, when gremlin.gpu.enabled is flipped to true, and --force is not supplied: the old daemonset fails to be updated, and the new daemonset is created, which lands two agent pods on the same node until --force is run.
To allow customers who install gremlin via helm chart access to the host-level GPU Gremlin, additional config is needed to pull in access to files and drivers for the GPU.
Tested using an EC2 instance with Nvidia drivers, docker, minikube, and NVIDIA Container Toolkit.