Skip to content

Add simple options for enabling Gremlin GPU access - #147

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

Add simple options for enabling Gremlin GPU access#147
ZachNo wants to merge 8 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

@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.

{{- if .Values.gremlin.gpu.enabled -}}
{{- $runtimeClassName := include "gremlinGpuRuntimeClassName" . -}}
{{- if and $runtimeClassName .Values.gremlin.gpu.createRuntimeClass -}}
{{- if not (lookup "node.k8s.io/v1" "RuntimeClass" "" $runtimeClassName) }}

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.

Unfortunately, I think the use of lookup here is going to present an issue on subsequent upgrades, as the next time an upgrade lands (after the first), lookup will find something and omit this document. The net effect is behavior like helm/helm#10281, where the resource gets deleted after the first upgrade/install.

I think the problem of preventing collisions with an existing runtimeClass is solved by gremlin.gpu.createRuntimeClass. That is, if a user already has a runtime class, they should not be setting gremlin.gpu.createRuntimeClass=true.

If we want a better error message for someone that has the class already, we can use lookup to exit the chart installation with an error, e.g. "a RuntimeClass named X already exists; set gremlin.gpu.createRuntimeClass=false if you intend to reuse it"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the normal use case will be that a customer already has the runtimeclass existing for their normal application, but unfortunately not having the runtime makes the gremlin pod unschedulable, so it's a hard call honestly.

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
# Any advanced option you set explicitly OVERRIDES the corresponding value from the vendor
# preset, so you can start from a preset and tweak a single field, or set vendor to "" and
# configure everything by hand.
gpu:

@philgebhardt philgebhardt Jul 22, 2026

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.

The presence of env and hostMounts here make sense but they start to rub up against the other fields we have for generic insertion (E.g. gremlin.extraEnv). Further, I think we could make the configuration a bit more explicit to the end user by formatting it closer to what the end-yaml would look like. Thoughts on a structure like this?

gpu:
  enabled: false
  vendor: nvidia          # nvidia | amd | custom
  createRuntimeClass: false
  cdiDevice: ""
  projectOpenclIcd: true   # project the vendor's openclIcd file below, when set

  nvidia:
    runtimeClassName: nvidia
    env:
      - name: NVIDIA_VISIBLE_DEVICES
        value: "all"
      - name: NVIDIA_DRIVER_CAPABILITIES
        value: "all"
    volumes: []
    volumeMounts: []
    openclIcd:
      filename: nvidia.icd
      library: libnvidia-opencl.so.1

  amd:
    runtimeClassName: ""
    env: []
    volumes:
      - name: kfd
        hostPath: { path: /dev/kfd, type: CharDevice }
      - name: dri
        hostPath: { path: /dev/dri, type: Directory }
      - name: opencl-vendors
        hostPath: { path: /etc/OpenCL/vendors, type: DirectoryOrCreate }
    volumeMounts:
      - name: kfd
        mountPath: /dev/kfd
        readOnly: false
      - name: dri
        mountPath: /dev/dri
        readOnly: false
      - name: opencl-vendors
        mountPath: /etc/OpenCL/vendors
        readOnly: true
    # no openclIcd here: amd already mounts the host's real /etc/OpenCL/vendors
    # above, so projecting one would collide with it.

  custom:
    runtimeClassName: ""
    env: []
    volumes: []
    volumeMounts: []

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One of my main goals was to try to provide a very "one click" approach to setting up the access, while still giving the option for power users or non-standard installs to change things. This would still give that simplicity, but just more explicitly expose what values are being set, correct?

(btw cdiDevice is nvidia only as well)

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