Skip to content

Commit a87582a

Browse files
committed
fix: add NVIDIA env vars to on-demand orc jobs as well
1 parent 0b80cb4 commit a87582a

3 files changed

Lines changed: 118 additions & 3 deletions

File tree

helm/fiftyone-teams-app/templates/delegated-operator-job-configmap.yaml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ data:
4444
{{- $mergedVolumeMounts := $jobConfig.volumeMounts | default $baseTpl.volumeMounts | default list }}
4545
{{- $mergedVolumes := $jobConfig.volumes | default $baseTpl.volumes | default list }}
4646

47+
{{- /* When the executor requests a GPU, expose that same GPU to its telemetry
48+
sidecar for read-only metrics. This sets env vars only — the sidecar never
49+
requests its own nvidia.com/gpu, so it consumes no GPU quota and the
50+
executor keeps the allocated GPU. Honored where the NVIDIA runtime accepts
51+
NVIDIA_VISIBLE_DEVICES (most setups; GKE blocks env-based exposure). */}}
52+
{{- $gpuRequested := or (not (empty (dig "limits" "nvidia.com/gpu" "" $mergedResources))) (not (empty (dig "requests" "nvidia.com/gpu" "" $mergedResources))) }}
53+
{{- $sidecarEnv := dict }}
54+
{{- if $gpuRequested }}
55+
{{- $sidecarEnv = dict "NVIDIA_VISIBLE_DEVICES" "all" "NVIDIA_DRIVER_CAPABILITIES" "compute,utility" }}
56+
{{- end }}
57+
4758
{{- if $.Values.telemetry.enabled }}
4859
{{- $hasSocketVolume := false }}
4960
{{- range $vol := $mergedVolumes }}
@@ -143,7 +154,7 @@ data:
143154
{{- end }}
144155
{{- if $.Values.telemetry.enabled }}
145156
initContainers:
146-
{{- include "telemetry.native-sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true "targetContainer" "executor") | nindent 12 }}
157+
{{- include "telemetry.native-sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true "targetContainer" "executor" "sidecarEnv" $sidecarEnv) | nindent 12 }}
147158
{{- end }}
148159
{{- with $mergedNodeSelector }}
149160
nodeSelector:

helm/fiftyone-teams-app/values.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -898,8 +898,6 @@ delegatedOperatorDeployments:
898898
# cpu: 8
899899
# ephemeral-storage: 1Gi
900900
# memory: 16Gi
901-
# # GPU metrics appear automatically — the telemetry sidecar reads this
902-
# # operator's GPU without requesting its own allocation.
903901
# volumes:
904902
# - name: shm-vol
905903
# emptyDir:

tests/unit/helm/delegated-operator-job-configmap_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,112 @@ func (s *doK8sConfigMapTemplateTest) TestTelemetrySocketInjection() {
754754
}
755755
}
756756

757+
// TestTelemetrySidecarGpuEnv verifies that when a delegated-operator job's
758+
// executor requests a GPU (via resources.limits or resources.requests), its
759+
// telemetry native-sidecar is given the NVIDIA_* env vars needed to read GPU
760+
// metrics, without the sidecar requesting its own nvidia.com/gpu allocation.
761+
// When no GPU is requested, the sidecar receives no NVIDIA_* env vars.
762+
func (s *doK8sConfigMapTemplateTest) TestTelemetrySidecarGpuEnv() {
763+
const gpuResource = "nvidia.com/gpu"
764+
jobKey := "cpuDefault.yaml"
765+
766+
findContainer := func(containers []corev1.Container, name string) *corev1.Container {
767+
for i, c := range containers {
768+
if c.Name == name {
769+
return &containers[i]
770+
}
771+
}
772+
return nil
773+
}
774+
envValue := func(env []corev1.EnvVar, name string) (string, bool) {
775+
for _, e := range env {
776+
if e.Name == name {
777+
return e.Value, true
778+
}
779+
}
780+
return "", false
781+
}
782+
783+
// gpuKey escapes the dot in nvidia.com/gpu so helm --set treats the whole
784+
// string as a single map key rather than a nested path.
785+
gpuKey := func(section string) string {
786+
return "delegatedOperatorJobTemplates.jobs.cpuDefault.resources." +
787+
section + ".nvidia\\.com/gpu"
788+
}
789+
790+
testCases := []struct {
791+
name string
792+
values map[string]string
793+
expectGpu bool
794+
}{
795+
{
796+
name: "gpuInLimitsExposesEnvToSidecar",
797+
values: map[string]string{
798+
"telemetry.enabled": "true",
799+
"delegatedOperatorJobTemplates.jobs.cpuDefault.unused": "nil",
800+
gpuKey("limits"): "1",
801+
},
802+
expectGpu: true,
803+
},
804+
{
805+
name: "gpuInRequestsExposesEnvToSidecar",
806+
values: map[string]string{
807+
"telemetry.enabled": "true",
808+
"delegatedOperatorJobTemplates.jobs.cpuDefault.unused": "nil",
809+
gpuKey("requests"): "1",
810+
},
811+
expectGpu: true,
812+
},
813+
{
814+
name: "noGpuOmitsEnvFromSidecar",
815+
values: map[string]string{
816+
"telemetry.enabled": "true",
817+
"delegatedOperatorJobTemplates.jobs.cpuDefault.unused": "nil",
818+
},
819+
expectGpu: false,
820+
},
821+
}
822+
823+
for _, testCase := range testCases {
824+
testCase := testCase
825+
s.Run(testCase.name, func() {
826+
subT := s.T()
827+
subT.Parallel()
828+
829+
options := &helm.Options{SetValues: testCase.values}
830+
output := helm.RenderTemplate(subT, options, s.chartPath, s.releaseName, s.templates)
831+
832+
var configMap corev1.ConfigMap
833+
helm.UnmarshalK8SYaml(subT, output, &configMap)
834+
835+
job := s.renderJob(configMap.Data, jobKey)
836+
837+
// The native-sidecar runs as an initContainer (restartPolicy: Always).
838+
sidecar := findContainer(job.Spec.Template.Spec.InitContainers, "telemetry-sidecar")
839+
s.Require().NotNil(sidecar, "telemetry-sidecar initContainer not found")
840+
841+
visibleDevices, hasVisibleDevices := envValue(sidecar.Env, "NVIDIA_VISIBLE_DEVICES")
842+
driverCaps, hasDriverCaps := envValue(sidecar.Env, "NVIDIA_DRIVER_CAPABILITIES")
843+
844+
if testCase.expectGpu {
845+
s.True(hasVisibleDevices, "sidecar should have NVIDIA_VISIBLE_DEVICES")
846+
s.Equal("all", visibleDevices, "NVIDIA_VISIBLE_DEVICES value mismatch")
847+
s.True(hasDriverCaps, "sidecar should have NVIDIA_DRIVER_CAPABILITIES")
848+
s.Equal("compute,utility", driverCaps, "NVIDIA_DRIVER_CAPABILITIES value mismatch")
849+
850+
// The sidecar reads the executor's GPU; it must not request its own.
851+
_, limitsHasGpu := sidecar.Resources.Limits[corev1.ResourceName(gpuResource)]
852+
_, requestsHasGpu := sidecar.Resources.Requests[corev1.ResourceName(gpuResource)]
853+
s.False(limitsHasGpu, "sidecar must not request nvidia.com/gpu in limits")
854+
s.False(requestsHasGpu, "sidecar must not request nvidia.com/gpu in requests")
855+
} else {
856+
s.False(hasVisibleDevices, "sidecar should not have NVIDIA_VISIBLE_DEVICES when no GPU requested")
857+
s.False(hasDriverCaps, "sidecar should not have NVIDIA_DRIVER_CAPABILITIES when no GPU requested")
858+
}
859+
})
860+
}
861+
}
862+
757863
func (s *doK8sConfigMapTemplateTest) loadTestFile(filename, chartVersion string) string {
758864
content, err := os.ReadFile(filename)
759865
s.NoError(err, "Failed to read test file: %s", filename)

0 commit comments

Comments
 (0)