fix: detect gpu metrics in k8 gpu delegated-executor sidecars by kaixi-wang · Pull Request #583 · voxel51/fiftyone-teams-app-deploy

kaixi-wang · 2026-06-09T00:21:05Z

Rationale

gpu k8 dos are not logging gpu metrics (https://demo.fiftyone.ai/datasets/ring-frames/runs/6a2730c4db66b264583f0794?tab=metrics&start=1780953304976.5564&end=1780953670886.813)

Review Priority

high
medium
low

Changes

add nvidia env vars just like docker already has
note this depends on accept-nvidia-visible-devices-envvar-when-unprivileged being true (which it is by default)

Checklist

This PR maintains parity between Docker Compose and Helm

Testing

coderabbitai · 2026-06-09T00:22:14Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb57878f-5ae3-4c94-8a40-59eec4fa10d7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

The PR extends the telemetry.sidecar-env Helm template to accept an optional sidecarEnv input map, enabling additional environment variables (such as NVIDIA_*) to be appended to the telemetry sidecar container. The template documentation and rendering logic are updated together.

Changes

Telemetry Sidecar Environment Variables

Layer / File(s)	Summary
Sidecar environment variables template support `helm/fiftyone-teams-app/templates/_telemetry.tpl`	Template documentation describes the new optional `sidecarEnv` map input, and template logic iterates over map entries to emit each as a quoted environment variable in the telemetry sidecar container.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A sidecar sidekick with env vars to spare,
NVIDIA whispers float through the Helm air,
One template, one map, the logic holds true,
Telemetry springs forth in emerald hue! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: adding GPU metric detection to Kubernetes GPU delegated-executor sidecars by adding NVIDIA environment variables.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description matches the template’s required sections and includes rationale, priority, changes, and checklist, with only testing left blank.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch kacey/fix-k8-gpu-sidecar-detection

_{Comment @coderabbitai help to get the list of available commands.}

kevin-dimichel · 2026-06-30T18:30:36Z

@kaixi-wang - can you please rebase this PR on main? It will make the review of #592 much easier.

kevin-dimichel · 2026-06-30T21:06:26Z

+			section + ".nvidia\\.com/gpu"
+	}
+
+	testCases := []struct {


Should we have a test case for when telemetry.enabled:false? Or is that covered somewhere else?

kevin-dimichel · 2026-06-30T21:06:36Z

+			section + ".nvidia\\.com/gpu"
+	}
+
+	testCases := []struct {


Should we have a test case for when telemetry.enabled:false? Or is that covered somewhere else?

kevin-dimichel · 2026-06-30T21:09:26Z

+// telemetry-sidecar is given the NVIDIA_* env vars needed to read GPU metrics,
+// without the sidecar requesting its own nvidia.com/gpu allocation. When no GPU
+// is requested, the sidecar receives no NVIDIA_* env vars.
+func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySidecarGpuEnv() {


In the future, let's consider the tech debt of adding functions like this. Do we need to add tests for the test functions? Being declarative in tests is often preferred to these.

moved to a shared file

kevin-dimichel · 2026-06-30T21:11:55Z

+// telemetry native-sidecar is given the NVIDIA_* env vars needed to read GPU
+// metrics, without the sidecar requesting its own nvidia.com/gpu allocation.
+// When no GPU is requested, the sidecar receives no NVIDIA_* env vars.
+func (s *doK8sConfigMapTemplateTest) TestTelemetrySidecarGpuEnv() {


If we are writing functions to be DRY, why did we duplicate the function across tests/unit/helm/delegated-operator-job-configmap_test.go and this file? The content is nearly the same, but the function names are different?

If we keep these, they should be refactored into https://github.com/voxel51/fiftyone-teams-app-deploy/blob/main/tests/unit/helm/common_test.go.

kevin-dimichel · 2026-06-30T21:12:37Z

If the intention is for this to be released in the upcoming release, we should also update the base branch from main to the release branch.

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.2.0 to 6.3.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v6.2.0...v6.3.0) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 6.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…privileged is true

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Kacey <kacey@voxel51.com>

…decar-detection' into kacey/fix-k8-gpu-sidecar-detection

…e helpers

kevin-dimichel · 2026-06-30T22:49:15Z

@kaixi-wang - docker integration tests are failing. Can you please run them locally and see if they pass?

kaixi-wang · 2026-06-30T22:59:06Z

@kaixi-wang - docker integration tests are failing. Can you please run them locally and see if they pass?

I didn't do any docker changes, but it looks like they may be failing due to the hello response changing:

TestDockerComposeUpLegacyAuth/TestDockerComposeUp/composeDedicatedPlugins 2026-06-30T22:44:36Z retry.go:103: HTTP GET to URL http://127.0.0.1:3000/api/hello returned an error: Validation failed for URL http://127.0.0.1:3000/api/hello. Response status: 200. Response body:
{"status":"available"}. Sleeping for 3s and will try again.

So probably needs kacey/fix-flaky-api-log-tests ? I wonder why this is just happening now

kaixi-wang self-assigned this Jun 9, 2026

kaixi-wang requested a review from a team as a code owner June 9, 2026 00:21

kevin-dimichel mentioned this pull request Jun 15, 2026

docs(helm): Add recommended post-installation configuration guide #556

Merged

4 tasks

mo-getter self-requested a review June 23, 2026 16:25

mo-getter mentioned this pull request Jun 30, 2026

add env vars to DO job templates and add tests #592

Merged

4 tasks

kaixi-wang force-pushed the kacey/fix-k8-gpu-sidecar-detection branch from 01f128e to ad7a364 Compare June 30, 2026 19:30

mo-getter added the priority-high High Priority PR - review immediately label Jun 30, 2026

mo-getter requested review from kevin-dimichel and removed request for mo-getter June 30, 2026 21:04

kevin-dimichel reviewed Jun 30, 2026

View reviewed changes

kaixi-wang changed the base branch from main to release/v2.21.0 June 30, 2026 21:19

dependabot Bot and others added 4 commits June 30, 2026 14:20

fix: add env vars to k8 gpu executor sidecars like docker

9a5fa20

fix: simplify and assume accept-nvidia-visible-devices-envvar-when-un…

0f3513b

…privileged is true

fix: add env vars to DO job templates and add tests (#592)

42c44ba

kaixi-wang force-pushed the kacey/fix-k8-gpu-sidecar-detection branch from 43341ef to 42c44ba Compare June 30, 2026 21:20

kaixi-wang and others added 4 commits June 30, 2026 14:21

Apply suggestion from @kevin-dimichel

5dd008f

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Kacey <kacey@voxel51.com>

chore: create shared utils and add negative test

7283cc1

Merge remote-tracking branch 'refs/remotes/origin/kacey/fix-k8-gpu-si…

b5beed8

…decar-detection' into kacey/fix-k8-gpu-sidecar-detection

chore: add telemetry disabled test to instance tests and update to us…

eead2dc

…e helpers

kevin-dimichel approved these changes Jul 1, 2026

View reviewed changes

kaixi-wang merged commit 8fced45 into release/v2.21.0 Jul 1, 2026
14 of 21 checks passed

kaixi-wang deleted the kacey/fix-k8-gpu-sidecar-detection branch July 1, 2026 00:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: detect gpu metrics in k8 gpu delegated-executor sidecars#583

fix: detect gpu metrics in k8 gpu delegated-executor sidecars#583
kaixi-wang merged 8 commits into
release/v2.21.0from
kacey/fix-k8-gpu-sidecar-detection

kaixi-wang commented Jun 9, 2026 •

edited by mo-getter

Loading

Uh oh!

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

Review skipped

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kevin-dimichel Jun 30, 2026

Uh oh!

kevin-dimichel Jun 30, 2026

Uh oh!

Uh oh!

kevin-dimichel Jun 30, 2026

Uh oh!

kaixi-wang Jun 30, 2026

Uh oh!

kevin-dimichel Jun 30, 2026

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kaixi-wang commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kaixi-wang commented Jun 9, 2026 • edited by mo-getter Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Changes

Testing

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kevin-dimichel Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

kevin-dimichel Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevin-dimichel Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

kaixi-wang Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

kevin-dimichel Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kevin-dimichel commented Jun 30, 2026

Uh oh!

kaixi-wang commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kaixi-wang commented Jun 9, 2026 •

edited by mo-getter

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading