Skip to content

Commit 43341ef

Browse files
mo-gettermo-getter
authored
fix: add env vars to DO job templates and add tests (#592)
1 parent ad7a364 commit 43341ef

3 files changed

Lines changed: 223 additions & 1 deletion

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:

‎tests/unit/helm/delegated-operator-instance-deployment_test.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5783,3 +5783,108 @@ func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySocketInj
57835783
})
57845784
}
57855785
}
5786+
5787+
// TestTelemetrySidecarGpuEnv verifies that when a delegated-operator executor
5788+
// requests a GPU (via resources.limits or resources.requests), its
5789+
// telemetry-sidecar is given the NVIDIA_* env vars needed to read GPU metrics,
5790+
// without the sidecar requesting its own nvidia.com/gpu allocation. When no GPU
5791+
// is requested, the sidecar receives no NVIDIA_* env vars.
5792+
func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySidecarGpuEnv() {
5793+
const gpuResource = "nvidia.com/gpu"
5794+
5795+
findContainer := func(containers []corev1.Container, name string) *corev1.Container {
5796+
for i, c := range containers {
5797+
if c.Name == name {
5798+
return &containers[i]
5799+
}
5800+
}
5801+
return nil
5802+
}
5803+
envValue := func(env []corev1.EnvVar, name string) (string, bool) {
5804+
for _, e := range env {
5805+
if e.Name == name {
5806+
return e.Value, true
5807+
}
5808+
}
5809+
return "", false
5810+
}
5811+
5812+
// gpuKey escapes the dot in nvidia.com/gpu so helm --set treats the whole
5813+
// string as a single map key rather than a nested path.
5814+
gpuKey := func(section string) string {
5815+
return "delegatedOperatorDeployments.deployments.teamsDoCpuDefault.resources." +
5816+
section + ".nvidia\\.com/gpu"
5817+
}
5818+
5819+
testCases := []struct {
5820+
name string
5821+
values map[string]string
5822+
expectGpu bool
5823+
}{
5824+
{
5825+
name: "gpuInLimitsExposesEnvToSidecar",
5826+
values: map[string]string{
5827+
"telemetry.enabled": "true",
5828+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5829+
gpuKey("limits"): "1",
5830+
},
5831+
expectGpu: true,
5832+
},
5833+
{
5834+
name: "gpuInRequestsExposesEnvToSidecar",
5835+
values: map[string]string{
5836+
"telemetry.enabled": "true",
5837+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5838+
gpuKey("requests"): "1",
5839+
},
5840+
expectGpu: true,
5841+
},
5842+
{
5843+
name: "noGpuOmitsEnvFromSidecar",
5844+
values: map[string]string{
5845+
"telemetry.enabled": "true",
5846+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5847+
},
5848+
expectGpu: false,
5849+
},
5850+
}
5851+
5852+
for _, tc := range testCases {
5853+
tc := tc
5854+
s.Run(tc.name, func() {
5855+
subT := s.T()
5856+
subT.Parallel()
5857+
5858+
options := &helm.Options{SetValues: tc.values}
5859+
output := helm.RenderTemplate(subT, options, s.chartPath, s.releaseName, s.templates)
5860+
5861+
docs := strings.Split(output, "---")
5862+
s.Require().GreaterOrEqual(len(docs), 2, "expected at least one rendered deployment")
5863+
5864+
var deployment appsv1.Deployment
5865+
helm.UnmarshalK8SYaml(subT, docs[1], &deployment)
5866+
5867+
sidecar := findContainer(deployment.Spec.Template.Spec.Containers, "telemetry-sidecar")
5868+
s.Require().NotNil(sidecar, "telemetry-sidecar container not found")
5869+
5870+
visibleDevices, hasVisibleDevices := envValue(sidecar.Env, "NVIDIA_VISIBLE_DEVICES")
5871+
driverCaps, hasDriverCaps := envValue(sidecar.Env, "NVIDIA_DRIVER_CAPABILITIES")
5872+
5873+
if tc.expectGpu {
5874+
s.True(hasVisibleDevices, "sidecar should have NVIDIA_VISIBLE_DEVICES")
5875+
s.Equal("all", visibleDevices, "NVIDIA_VISIBLE_DEVICES value mismatch")
5876+
s.True(hasDriverCaps, "sidecar should have NVIDIA_DRIVER_CAPABILITIES")
5877+
s.Equal("compute,utility", driverCaps, "NVIDIA_DRIVER_CAPABILITIES value mismatch")
5878+
5879+
// The sidecar reads the executor's GPU; it must not request its own.
5880+
_, limitsHasGpu := sidecar.Resources.Limits[corev1.ResourceName(gpuResource)]
5881+
_, requestsHasGpu := sidecar.Resources.Requests[corev1.ResourceName(gpuResource)]
5882+
s.False(limitsHasGpu, "sidecar must not request nvidia.com/gpu in limits")
5883+
s.False(requestsHasGpu, "sidecar must not request nvidia.com/gpu in requests")
5884+
} else {
5885+
s.False(hasVisibleDevices, "sidecar should not have NVIDIA_VISIBLE_DEVICES when no GPU requested")
5886+
s.False(hasDriverCaps, "sidecar should not have NVIDIA_DRIVER_CAPABILITIES when no GPU requested")
5887+
}
5888+
})
5889+
}
5890+
}

‎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)