Skip to content

Commit 8fced45

Browse files
kaixi-wangkaixi-wang
authored
Merge pull request #583 from voxel51/kacey/fix-k8-gpu-sidecar-detection
fix: detect gpu metrics in k8 gpu delegated-executor sidecars
2 parents 893d151 + eead2dc commit 8fced45

7 files changed

Lines changed: 318 additions & 43 deletions

‎helm/fiftyone-teams-app/templates/_telemetry.tpl‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ Inputs (dict):
9090
podName — value for POD_NAME env var (defaults to fieldRef metadata.name)
9191
executor — bool, when true emit EXECUTOR_SIDECAR=true and TELEMETRY_SOCKET env
9292
targetContainer — when set, emit TARGET_CONTAINER env var (used by job sidecars)
93+
sidecarEnv — optional map of extra env vars to append (e.g. NVIDIA_* so a
94+
sidecar on a GPU node can read NVML/GPU metrics)
9395
*/}}
9496
{{- define "telemetry.sidecar-env" -}}
9597
{{- $secretName := .ctx.Values.secret.name -}}
@@ -127,6 +129,10 @@ Inputs (dict):
127129
secretKeyRef:
128130
name: {{ $secretName }}
129131
key: fiftyoneDatabaseName
132+
{{- range $name, $value := .sidecarEnv }}
133+
- name: {{ $name }}
134+
value: {{ $value | quote }}
135+
{{- end }}
130136
{{- end }}
131137

132138
{{/*

‎helm/fiftyone-teams-app/templates/delegated-operator-instance-deployment.yaml‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@
1616
}}
1717
{{- $defaultDescription := printf "Long running operations delegated to %s" $name }}
1818
{{- $updateStrategy := merge (dict) ($v.updateStrategy|default dict) ($baseTpl.updateStrategy) }}
19+
{{- $mergedResources := merge (dict) ($v.resources | default dict) ($baseTpl.resources | default dict) }}
20+
{{- /* When the executor requests a GPU, expose that same GPU to its telemetry
21+
sidecar for read-only metrics. This sets env vars only — the sidecar never
22+
requests its own nvidia.com/gpu, so it consumes no GPU quota and the
23+
executor keeps the allocated GPU. Honored where the NVIDIA runtime accepts
24+
NVIDIA_VISIBLE_DEVICES (most setups; GKE blocks env-based exposure). */}}
25+
{{- $gpuRequested := or (not (empty (dig "limits" "nvidia.com/gpu" "" $mergedResources))) (not (empty (dig "requests" "nvidia.com/gpu" "" $mergedResources))) }}
26+
{{- $sidecarEnv := dict }}
27+
{{- if $gpuRequested }}
28+
{{- $sidecarEnv = dict "NVIDIA_VISIBLE_DEVICES" "all" "NVIDIA_DRIVER_CAPABILITIES" "compute,utility" }}
29+
{{- end }}
1930
{{- $mergedVolumeMounts := $v.volumeMounts | default $baseTpl.volumeMounts | default list }}
2031
{{- $mergedVolumes := $v.volumes | default $baseTpl.volumes | default list }}
2132
{{- if $.Values.telemetry.enabled }}
@@ -99,7 +110,7 @@ spec:
99110
env:
100111
{{- include "delegated-operator-deployments.env-vars-list" $envContext | indent 12 }}
101112
resources:
102-
{{- toYaml (merge (dict) ($v.resources | default dict) ($baseTpl.resources)) | nindent 12 }}
113+
{{- toYaml $mergedResources | nindent 12 }}
103114
{{- with $mergedVolumeMounts }}
104115
volumeMounts:
105116
{{- toYaml . | nindent 12 }}
@@ -132,7 +143,7 @@ spec:
132143
periodSeconds: {{ ($v.startup).periodSeconds | default $baseTpl.startup.periodSeconds }}
133144
timeoutSeconds: {{ ($v.startup).timeoutSeconds | default $baseTpl.startup.timeoutSeconds }}
134145
{{- if $.Values.telemetry.enabled }}
135-
{{- include "telemetry.sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true) | nindent 8 }}
146+
{{- include "telemetry.sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true "sidecarEnv" $sidecarEnv) | nindent 8 }}
136147
{{- end }}
137148
{{- with (merge (dict) ($v.nodeSelector | default dict) ($baseTpl.nodeSelector)) }}
138149
nodeSelector:

‎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 }}
@@ -146,7 +157,7 @@ data:
146157
{{- end }}
147158
{{- if $.Values.telemetry.enabled }}
148159
initContainers:
149-
{{- include "telemetry.native-sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true "targetContainer" "executor") | nindent 12 }}
160+
{{- include "telemetry.native-sidecar" (dict "ctx" $ "serviceType" "delegated-operator" "targetName" "fiftyone delegated" "executor" true "targetContainer" "executor" "sidecarEnv" $sidecarEnv) | nindent 12 }}
150161
{{- end }}
151162
{{- with $mergedNodeSelector }}
152163
nodeSelector:

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

Lines changed: 120 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5711,16 +5711,6 @@ func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySocketInj
57115711
}
57125712
return n
57135713
}
5714-
// findContainer returns the named container (the main DO container,
5715-
// not the telemetry-sidecar) from a pod spec.
5716-
findContainer := func(containers []corev1.Container, name string) *corev1.Container {
5717-
for i, c := range containers {
5718-
if c.Name == name {
5719-
return &containers[i]
5720-
}
5721-
}
5722-
return nil
5723-
}
57245714

57255715
testCases := []struct {
57265716
name string
@@ -5783,3 +5773,123 @@ func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySocketInj
57835773
})
57845774
}
57855775
}
5776+
5777+
// TestTelemetryDisabledOmitsSidecar verifies that when telemetry is disabled
5778+
// the rendered DO deployment carries neither the telemetry-sidecar container nor
5779+
// the telemetry-socket volume/mount. The disabled shape is also covered
5780+
// indirectly by TestContainerCount (container count is 1, not 2) and the
5781+
// disableTelemetry-based volume/mount tests; this is the focused negative
5782+
// assertion living next to the positive cases above.
5783+
func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetryDisabledOmitsSidecar() {
5784+
const socketName = "telemetry-socket"
5785+
5786+
options := &helm.Options{SetValues: map[string]string{
5787+
"telemetry.enabled": "false",
5788+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5789+
}}
5790+
output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName, s.templates)
5791+
5792+
docs := strings.Split(output, "---")
5793+
s.Require().GreaterOrEqual(len(docs), 2, "expected at least one rendered deployment")
5794+
5795+
var deployment appsv1.Deployment
5796+
helm.UnmarshalK8SYaml(s.T(), docs[1], &deployment)
5797+
5798+
s.Nil(findContainer(deployment.Spec.Template.Spec.Containers, "telemetry-sidecar"),
5799+
"telemetry-sidecar container should be absent when telemetry is disabled")
5800+
s.Nil(findVolume(deployment.Spec.Template.Spec.Volumes, socketName),
5801+
"telemetry-socket volume should be absent when telemetry is disabled")
5802+
5803+
main := findContainer(deployment.Spec.Template.Spec.Containers, "teams-do-cpu-default")
5804+
s.Require().NotNil(main, "main DO container not found")
5805+
s.Nil(findVolumeMount(main.VolumeMounts, socketName),
5806+
"telemetry-socket volumeMount should be absent when telemetry is disabled")
5807+
}
5808+
5809+
// TestTelemetrySidecarGpuEnv verifies that when a delegated-operator executor
5810+
// requests a GPU (via resources.limits or resources.requests), its
5811+
// telemetry-sidecar is given the NVIDIA_* env vars needed to read GPU metrics,
5812+
// without the sidecar requesting its own nvidia.com/gpu allocation. When no GPU
5813+
// is requested, the sidecar receives no NVIDIA_* env vars.
5814+
func (s *deploymentDelegatedOperatorInstanceTemplateTest) TestTelemetrySidecarGpuEnv() {
5815+
const gpuResource = "nvidia.com/gpu"
5816+
5817+
// gpuKey escapes the dot in nvidia.com/gpu so helm --set treats the whole
5818+
// string as a single map key rather than a nested path.
5819+
gpuKey := func(section string) string {
5820+
return "delegatedOperatorDeployments.deployments.teamsDoCpuDefault.resources." +
5821+
section + ".nvidia\\.com/gpu"
5822+
}
5823+
5824+
testCases := []struct {
5825+
name string
5826+
values map[string]string
5827+
expectGpu bool
5828+
}{
5829+
{
5830+
name: "gpuInLimitsExposesEnvToSidecar",
5831+
values: map[string]string{
5832+
"telemetry.enabled": "true",
5833+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5834+
gpuKey("limits"): "1",
5835+
},
5836+
expectGpu: true,
5837+
},
5838+
{
5839+
name: "gpuInRequestsExposesEnvToSidecar",
5840+
values: map[string]string{
5841+
"telemetry.enabled": "true",
5842+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5843+
gpuKey("requests"): "1",
5844+
},
5845+
expectGpu: true,
5846+
},
5847+
{
5848+
name: "noGpuOmitsEnvFromSidecar",
5849+
values: map[string]string{
5850+
"telemetry.enabled": "true",
5851+
"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
5852+
},
5853+
expectGpu: false,
5854+
},
5855+
}
5856+
5857+
for _, tc := range testCases {
5858+
tc := tc
5859+
s.Run(tc.name, func() {
5860+
subT := s.T()
5861+
subT.Parallel()
5862+
5863+
options := &helm.Options{SetValues: tc.values}
5864+
output := helm.RenderTemplate(subT, options, s.chartPath, s.releaseName, s.templates)
5865+
5866+
docs := strings.Split(output, "---")
5867+
s.Require().GreaterOrEqual(len(docs), 2, "expected at least one rendered deployment")
5868+
5869+
var deployment appsv1.Deployment
5870+
helm.UnmarshalK8SYaml(subT, docs[1], &deployment)
5871+
5872+
sidecar := findContainer(deployment.Spec.Template.Spec.Containers, "telemetry-sidecar")
5873+
s.Require().NotNil(sidecar, "telemetry-sidecar container not found")
5874+
5875+
visibleDevices, hasVisibleDevices := envValue(sidecar.Env, "NVIDIA_VISIBLE_DEVICES")
5876+
driverCaps, hasDriverCaps := envValue(sidecar.Env, "NVIDIA_DRIVER_CAPABILITIES")
5877+
5878+
if tc.expectGpu {
5879+
s.True(hasVisibleDevices, "sidecar should have NVIDIA_VISIBLE_DEVICES")
5880+
s.Equal("all", visibleDevices, "NVIDIA_VISIBLE_DEVICES value mismatch")
5881+
s.True(hasDriverCaps, "sidecar should have NVIDIA_DRIVER_CAPABILITIES")
5882+
s.Equal("compute,utility", driverCaps, "NVIDIA_DRIVER_CAPABILITIES value mismatch")
5883+
5884+
// The sidecar reads the executor's GPU; it must not request its own.
5885+
_, limitsHasGpu := sidecar.Resources.Limits[corev1.ResourceName(gpuResource)]
5886+
_, requestsHasGpu := sidecar.Resources.Requests[corev1.ResourceName(gpuResource)]
5887+
s.False(limitsHasGpu, "sidecar must not request nvidia.com/gpu in limits")
5888+
s.False(requestsHasGpu, "sidecar must not request nvidia.com/gpu in requests")
5889+
} else {
5890+
s.False(hasVisibleDevices, "sidecar should not have NVIDIA_VISIBLE_DEVICES when no GPU requested")
5891+
s.False(hasDriverCaps, "sidecar should not have NVIDIA_DRIVER_CAPABILITIES when no GPU requested")
5892+
}
5893+
})
5894+
}
5895+
}

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

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

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

‎tests/unit/helm/k8s_helpers.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package unit
2+
3+
import corev1 "k8s.io/api/core/v1"
4+
5+
// findContainer returns a pointer to the container with the given name, or nil
6+
// if no container matches. Shared by tests that introspect rendered pod specs.
7+
func findContainer(containers []corev1.Container, name string) *corev1.Container {
8+
for i := range containers {
9+
if containers[i].Name == name {
10+
return &containers[i]
11+
}
12+
}
13+
return nil
14+
}
15+
16+
// findVolume returns a pointer to the pod volume with the given name, or nil.
17+
func findVolume(volumes []corev1.Volume, name string) *corev1.Volume {
18+
for i := range volumes {
19+
if volumes[i].Name == name {
20+
return &volumes[i]
21+
}
22+
}
23+
return nil
24+
}
25+
26+
// findVolumeMount returns a pointer to the volume mount with the given name, or nil.
27+
func findVolumeMount(mounts []corev1.VolumeMount, name string) *corev1.VolumeMount {
28+
for i := range mounts {
29+
if mounts[i].Name == name {
30+
return &mounts[i]
31+
}
32+
}
33+
return nil
34+
}
35+
36+
// envValue returns the value of the env var with the given name and whether it
37+
// was found.
38+
func envValue(env []corev1.EnvVar, name string) (string, bool) {
39+
for _, e := range env {
40+
if e.Name == name {
41+
return e.Value, true
42+
}
43+
}
44+
return "", false
45+
}

0 commit comments

Comments
 (0)