Merge pull request #588 from voxel51/kacey/telemetry-extravolumes

feat(sidecar): support extraVolumes

Author: kaixi-wang

helm/fiftyone-teams-app/README.md

@@ -1097,6 +1097,8 @@ If pods show unhealthy states (e.g., `0/1`, `CrashLoopBackOff`, `Pending`):
 | telemetry.redis.podSecurityContext | object | `{"fsGroup":999,"runAsGroup":999,"runAsNonRoot":true,"runAsUser":999}` | Pod-level security attributes for the telemetry Redis. UID/GID 999 matches the `redis` user in the `redis:7-alpine` image; `fsGroup` keeps the mounted `/data` volume group-writable. [Reference][security-context]. |
 | telemetry.redis.resources | object | `{"limits":{"cpu":"250m","memory":"512Mi"},"requests":{"cpu":"250m","memory":"512Mi"}}` | Resource requests/limits for the telemetry Redis container. [Reference][resources]. |
 | telemetry.serviceAccounts | list | `[]` | ServiceAccount names (in `namespace.name`) bound to the telemetry pod-logs Role. When empty, the RoleBinding binds the chart's main app service account. The teams-api sidecar uses the teams-api RBAC service account, which already grants `pods/log` GET via `api-role.yaml`. |
+| telemetry.sidecar.extraVolumeMounts | list | `[]` | Additional volume mounts for the `telemetry-sidecar` container, e.g. to mount the same CA certs the app containers use. [Reference][volumes]. |
+| telemetry.sidecar.extraVolumes | list | `[]` | Additional pod volumes available to the `telemetry-sidecar`. Omit any volume already defined on the workload (`*.volumes`) to avoid name collisions. [Reference][volumes]. |
 | telemetry.sidecar.image.pullPolicy | string | `"Always"` | Instruct when the kubelet should pull (download) the specified image. One of `IfNotPresent`, `Always` or `Never`. [Reference][image-pull-policy]. |
 | telemetry.sidecar.image.repository | string | `"voxel51/telemetry-sidecar"` | Container image for `telemetry-sidecar`. |
 | telemetry.sidecar.image.tag | string | `""` | Image tag for `telemetry-sidecar`. Defaults to `Chart.AppVersion`. |

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

@@ -150,10 +150,16 @@ Inputs: same dict as telemetry.sidecar-env.
       {{- if .executor }}
       add: ["SYS_PTRACE"]
       {{- end }}
+  {{- $mounts := list }}
   {{- if .executor }}
+  {{- $mounts = append $mounts (dict "name" "telemetry-socket" "mountPath" "/tmp/telemetry") }}
+  {{- end }}
+  {{- with .ctx.Values.telemetry.sidecar.extraVolumeMounts }}
+  {{- $mounts = concat $mounts . }}
+  {{- end }}
+  {{- with $mounts }}
   volumeMounts:
-    - name: telemetry-socket
-      mountPath: /tmp/telemetry
+    {{- toYaml . | nindent 4 }}
   {{- end }}
 {{- end }}
 
@@ -188,9 +194,12 @@ would block Job completion.
       {{- if .executor }}
       add: ["SYS_PTRACE"]
       {{- end }}
+  {{- $mounts := list (dict "name" "telemetry-socket" "mountPath" "/tmp/telemetry") }}
+  {{- with .ctx.Values.telemetry.sidecar.extraVolumeMounts }}
+  {{- $mounts = concat $mounts . }}
+  {{- end }}
   volumeMounts:
-    - name: telemetry-socket
-      mountPath: /tmp/telemetry
+    {{- toYaml $mounts | nindent 4 }}
 {{- end }}
 
 {{/*

helm/fiftyone-teams-app/templates/api-deployment.yaml

@@ -133,3 +133,8 @@ spec:
         {{- with .Values.apiSettings.volumes }}
         {{- toYaml . | nindent 8 }}
         {{- end }}
+        {{- if .Values.telemetry.enabled }}
+        {{- with .Values.telemetry.sidecar.extraVolumes }}
+        {{- toYaml . | nindent 8 }}
+        {{- end }}
+        {{- end }}

helm/fiftyone-teams-app/templates/app-deployment.yaml

@@ -108,7 +108,11 @@ spec:
       topologySpreadConstraints:
         {{- include "fiftyone-teams-app.commonTopologySpreadConstraints" (dict "constraints" .Values.appSettings.topologySpreadConstraints "selectorLabels" "teams-app.selectorLabels" "context" $) | nindent 8 }}
       {{- end }}
-      {{- with .Values.appSettings.volumes }}
+      {{- $volumes := .Values.appSettings.volumes | default list }}
+      {{- if .Values.telemetry.enabled }}
+      {{- $volumes = concat $volumes (.Values.telemetry.sidecar.extraVolumes | default list) }}
+      {{- end }}
+      {{- with $volumes }}
       volumes:
         {{- toYaml . | nindent 8 }}
       {{- end }}

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

@@ -33,6 +33,9 @@
   {{- if not $hasSocketMount }}
     {{- $mergedVolumeMounts = append $mergedVolumeMounts (dict "name" "telemetry-socket" "mountPath" "/tmp/telemetry") }}
   {{- end }}
+  {{- with $.Values.telemetry.sidecar.extraVolumes }}
+    {{- $mergedVolumes = concat $mergedVolumes . }}
+  {{- end }}
 {{- end }}
 apiVersion: apps/v1
 kind: Deployment

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

@@ -59,6 +59,9 @@ data:
     {{- if not $hasSocketMount }}
       {{- $mergedVolumeMounts = append $mergedVolumeMounts (dict "name" "telemetry-socket" "mountPath" "/tmp/telemetry") }}
     {{- end }}
+    {{- with $.Values.telemetry.sidecar.extraVolumes }}
+      {{- $mergedVolumes = concat $mergedVolumes . }}
+    {{- end }}
   {{- end }}
 
   {{- /* Trunc at 48 for the random ID to be generated via API */ -}}

helm/fiftyone-teams-app/templates/plugins-deployment.yaml

@@ -109,7 +109,11 @@ spec:
       topologySpreadConstraints:
         {{- include "fiftyone-teams-app.commonTopologySpreadConstraints" (dict "constraints" .Values.pluginsSettings.topologySpreadConstraints "selectorLabels" "teams-plugins.selectorLabels" "context" $) | nindent 8 }}
       {{- end }}
-      {{- with .Values.pluginsSettings.volumes }}
+      {{- $volumes := .Values.pluginsSettings.volumes | default list }}
+      {{- if .Values.telemetry.enabled }}
+      {{- $volumes = concat $volumes (.Values.telemetry.sidecar.extraVolumes | default list) }}
+      {{- end }}
+      {{- with $volumes }}
       volumes:
         {{- toYaml . | nindent 8 }}
       {{- end }}

helm/fiftyone-teams-app/values.schema.json

@@ -4233,6 +4233,22 @@
         },
         "sidecar": {
           "properties": {
+            "extraVolumeMounts": {
+              "description": "Additional volume mounts for the `telemetry-sidecar` container, e.g. to\nmount the same CA certs the app containers use. [Reference][volumes].",
+              "items": {
+                "required": []
+              },
+              "title": "extraVolumeMounts",
+              "type": "array"
+            },
+            "extraVolumes": {
+              "description": "Additional pod volumes available to the `telemetry-sidecar`. Omit any\nvolume already defined on the workload (`*.volumes`) to avoid name\ncollisions. [Reference][volumes].",
+              "items": {
+                "required": []
+              },
+              "title": "extraVolumes",
+              "type": "array"
+            },
             "image": {
               "properties": {
                 "pullPolicy": {

helm/fiftyone-teams-app/values.yaml

@@ -1684,6 +1684,13 @@ telemetry:
       # (e.g. `redis://my-redis.example.com:6379`).
       url: ""
   sidecar:
+    # -- Additional volume mounts for the `telemetry-sidecar` container, e.g. to
+    # mount the same CA certs the app containers use. [Reference][volumes].
+    extraVolumeMounts: []
+    # -- Additional pod volumes available to the `telemetry-sidecar`. Omit any
+    # volume already defined on the workload (`*.volumes`) to avoid name
+    # collisions. [Reference][volumes].
+    extraVolumes: []
     image:
       # @schema
       # oneOf:

tests/unit/helm/telemetry-sidecar_test.go

@@ -48,6 +48,103 @@ func findSidecar(containers []corev1.Container) *corev1.Container {
 	return nil
 }
 
+func findVolumeMount(mounts []corev1.VolumeMount, name string) *corev1.VolumeMount {
+	for i := range mounts {
+		if mounts[i].Name == name {
+			return &mounts[i]
+		}
+	}
+	return nil
+}
+
+func findVolume(volumes []corev1.Volume, name string) *corev1.Volume {
+	for i := range volumes {
+		if volumes[i].Name == name {
+			return &volumes[i]
+		}
+	}
+	return nil
+}
+
+// telemetrySidecarWorkload is the per-template fixture shared by the
+// extra-volume tests: the deployment template, the values needed to render
+// it, and whether its sidecar runs in executor (DO) mode — executor sidecars
+// also carry the telemetry-socket mount, which must survive alongside any
+// customer-supplied extra mounts.
+var telemetrySidecarWorkloads = []struct {
+	template string
+	values   map[string]string
+	executor bool
+}{
+	{"templates/api-deployment.yaml", nil, false},
+	{"templates/app-deployment.yaml", nil, false},
+	{
+		"templates/plugins-deployment.yaml",
+		map[string]string{"pluginsSettings.enabled": "true"},
+		false,
+	},
+	{
+		"templates/delegated-operator-instance-deployment.yaml",
+		map[string]string{
+			"delegatedOperatorDeployments.deployments.teamsDoCpuDefault.enabled": "true",
+		},
+		true,
+	},
+}
+
+// TestSidecarExtraVolumes asserts that telemetry.sidecar.extraVolumeMounts land
+// on the sidecar container and telemetry.sidecar.extraVolumes land on the pod
+// spec across every workload, so customers can hand the sidecar the same certs
+// the app containers already use. On the executor (DO) sidecar, the
+// telemetry-socket mount must survive alongside the customer mount.
+func (s *telemetrySidecarTemplateTest) TestSidecarExtraVolumes() {
+	extra := map[string]string{
+		"telemetry.sidecar.extraVolumeMounts[0].name":         "ca-certs",
+		"telemetry.sidecar.extraVolumeMounts[0].mountPath":    "/etc/ssl/custom",
+		"telemetry.sidecar.extraVolumeMounts[0].readOnly":     "true",
+		"telemetry.sidecar.extraVolumes[0].name":              "ca-certs",
+		"telemetry.sidecar.extraVolumes[0].secret.secretName": "my-ca",
+	}
+
+	for _, tc := range telemetrySidecarWorkloads {
+		tc := tc
+		s.Run(tc.template, func() {
+			values := map[string]string{}
+			for k, v := range tc.values {
+				values[k] = v
+			}
+			for k, v := range extra {
+				values[k] = v
+			}
+
+			options := &helm.Options{SetValues: values}
+			output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName,
+				[]string{tc.template})
+
+			var deployment appsv1.Deployment
+			helm.UnmarshalK8SYaml(s.T(), output, &deployment)
+
+			sidecar := findSidecar(deployment.Spec.Template.Spec.Containers)
+			s.Require().NotNil(sidecar, "telemetry-sidecar container should be injected into %s", tc.template)
+
+			mount := findVolumeMount(sidecar.VolumeMounts, "ca-certs")
+			s.Require().NotNil(mount, "sidecar should mount the customer extra volume on %s", tc.template)
+			s.Equal("/etc/ssl/custom", mount.MountPath, "extra mount path should pass through on %s", tc.template)
+			s.True(mount.ReadOnly, "extra mount readOnly should pass through on %s", tc.template)
+
+			vol := findVolume(deployment.Spec.Template.Spec.Volumes, "ca-certs")
+			s.Require().NotNil(vol, "pod spec should include the telemetry extra volume on %s", tc.template)
+			s.Require().NotNil(vol.Secret, "extra volume source should pass through on %s", tc.template)
+			s.Equal("my-ca", vol.Secret.SecretName, "extra volume secretName should pass through on %s", tc.template)
+
+			if tc.executor {
+				s.NotNil(findVolumeMount(sidecar.VolumeMounts, "telemetry-socket"),
+					"executor sidecar must keep the telemetry-socket mount on %s", tc.template)
+			}
+		})
+	}
+}
+
 // TestShareProcessNamespaceEnabledByDefault asserts that the api, app,
 // plugins, and delegated-operator deployments all opt into PID-namespace
 // sharing when telemetry is enabled (the default). The sidecar relies on