Skip to content

Add simple options for enabling Gremlin GPU access - #147

Open
ZachNo wants to merge 9 commits into
masterfrom
allow-nvidia-driver
Open

Add simple options for enabling Gremlin GPU access#147
ZachNo wants to merge 9 commits into
masterfrom
allow-nvidia-driver

Conversation

@ZachNo

@ZachNo ZachNo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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.

@ZachNo
ZachNo requested review from a team as code owners July 21, 2026 19:13
Comment thread gremlin/values.yaml Outdated

@philgebhardt philgebhardt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread gremlin/templates/runtimeclass.yaml Outdated
Comment thread gremlin/templates/runtimeclass.yaml Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need unit tests for this document?

@philgebhardt philgebhardt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread gremlin/values.yaml

@philgebhardt philgebhardt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread gremlin/README.md
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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" . -}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread gremlin/README.md

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`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants