Skip to content

Commit e6850fb

Browse files
mo-getterclaude
andcommitted
feat(helm): support pre-provisioned PVC for telemetry Redis
Adds telemetry.redis.persistence.existingClaim so operators on clusters without a dynamic PV provisioner (or wanting to reuse a volume across releases) can hand the chart a pre-created PVC instead of relying on chart-managed provisioning. When set, the chart skips its PVC render and the Deployment mounts the named claim; persistence.enabled=false still wins and yields an emptyDir. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2d17007 commit e6850fb

5 files changed

Lines changed: 87 additions & 2 deletions

File tree

helm/fiftyone-teams-app/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,7 @@ If pods show unhealthy states (e.g., `0/1`, `CrashLoopBackOff`, `Pending`):
10921092
| telemetry.redis.maxmemory | string | `"400mb"` | `--maxmemory` flag passed to `redis-server`. |
10931093
| telemetry.redis.maxmemoryPolicy | string | `"allkeys-lru"` | `--maxmemory-policy` flag passed to `redis-server`. |
10941094
| telemetry.redis.persistence.enabled | bool | `true` | Controls whether a `PersistentVolumeClaim` is created for the bundled telemetry Redis. When `false`, Redis runs with an `emptyDir` volume and state is lost on pod restart — fine for dev clusters without a dynamic PV provisioner, not for production. |
1095+
| telemetry.redis.persistence.existingClaim | string | `""` | Name of an existing `PersistentVolumeClaim` (in `namespace.name`) to bind the telemetry Redis to. When set, the chart will NOT create a `PersistentVolumeClaim` and the `size` / `storageClass` fields above are ignored — you are responsible for provisioning the claim out of band. Use this on clusters without a dynamic PV provisioner, or to reuse an existing volume across releases. Has no effect when `persistence.enabled` is `false`. |
10951096
| telemetry.redis.persistence.size | string | `"1Gi"` | Storage size for the telemetry Redis `PersistentVolumeClaim`. |
10961097
| telemetry.redis.persistence.storageClass | string | `""` | `StorageClass` name for the telemetry Redis `PersistentVolumeClaim`. Leave unset to use the cluster's default `StorageClass`. |
10971098
| telemetry.redis.podSecurityContext | object | `{"fsGroup":999,"runAsGroup":999,"runAsNonRoot":true,"runAsUser":999}` | Pod-level security attributes for the telemetry Redis. Defaults to a non-root UID/GID matching the `redis` user in the upstream `redis:7-alpine` image (999). `fsGroup` ensures the mounted `/data` volume is group-writable. [Reference][security-context]. |

helm/fiftyone-teams-app/templates/telemetry-redis.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{{- if and .Values.telemetry.enabled (not .Values.telemetry.redis.external.url) }}
2-
{{- if .Values.telemetry.redis.persistence.enabled }}
2+
{{- if and .Values.telemetry.redis.persistence.enabled (not .Values.telemetry.redis.persistence.existingClaim) }}
33
apiVersion: v1
44
kind: PersistentVolumeClaim
55
metadata:
@@ -72,7 +72,7 @@ spec:
7272
- name: redis-data
7373
{{- if .Values.telemetry.redis.persistence.enabled }}
7474
persistentVolumeClaim:
75-
claimName: {{ include "telemetry.redis.name" . }}-data
75+
claimName: {{ .Values.telemetry.redis.persistence.existingClaim | default (printf "%s-data" (include "telemetry.redis.name" .)) }}
7676
{{- else }}
7777
emptyDir: {}
7878
{{- end }}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4017,6 +4017,12 @@
40174017
"title": "enabled",
40184018
"type": "boolean"
40194019
},
4020+
"existingClaim": {
4021+
"default": "",
4022+
"description": "Name of an existing `PersistentVolumeClaim` (in `namespace.name`) to\nbind the telemetry Redis to. When set, the chart will NOT create a\n`PersistentVolumeClaim` and the `size` / `storageClass` fields above\nare ignored — you are responsible for provisioning the claim out of\nband. Use this on clusters without a dynamic PV provisioner, or to\nreuse an existing volume across releases. Has no effect when\n`persistence.enabled` is `false`.",
4023+
"title": "existingClaim",
4024+
"type": "string"
4025+
},
40204026
"size": {
40214027
"default": "1Gi",
40224028
"description": "Storage size for the telemetry Redis `PersistentVolumeClaim`.",

helm/fiftyone-teams-app/values.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,14 @@ telemetry:
16321632
# -- `StorageClass` name for the telemetry Redis `PersistentVolumeClaim`.
16331633
# Leave unset to use the cluster's default `StorageClass`.
16341634
storageClass: ""
1635+
# -- Name of an existing `PersistentVolumeClaim` (in `namespace.name`) to
1636+
# bind the telemetry Redis to. When set, the chart will NOT create a
1637+
# `PersistentVolumeClaim` and the `size` / `storageClass` fields above
1638+
# are ignored — you are responsible for provisioning the claim out of
1639+
# band. Use this on clusters without a dynamic PV provisioner, or to
1640+
# reuse an existing volume across releases. Has no effect when
1641+
# `persistence.enabled` is `false`.
1642+
existingClaim: ""
16351643
# -- `--maxmemory` flag passed to `redis-server`.
16361644
maxmemory: 400mb
16371645
# -- `--maxmemory-policy` flag passed to `redis-server`.

tests/unit/helm/telemetry-redis_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,73 @@ func (s *telemetryRedisTemplateTest) TestPersistenceDisabledSkipsPVCAndUsesEmpty
344344
s.Nil(volumes[0].PersistentVolumeClaim,
345345
"redis-data volume should NOT reference a PVC when persistence is disabled")
346346
}
347+
348+
// TestExistingClaimSkipsPVCAndMountsNamedClaim ensures that
349+
// persistence.existingClaim points the Deployment at the named PVC and
350+
// suppresses chart-managed PVC creation — the path for clusters without
351+
// a dynamic PV provisioner where the operator pre-creates the claim.
352+
func (s *telemetryRedisTemplateTest) TestExistingClaimSkipsPVCAndMountsNamedClaim() {
353+
options := &helm.Options{SetValues: map[string]string{
354+
"telemetry.redis.persistence.existingClaim": "my-prebuilt-redis-pvc",
355+
}}
356+
357+
output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName, s.templates)
358+
359+
s.NotContains(output, "kind: PersistentVolumeClaim",
360+
"PVC should not render when persistence.existingClaim is set")
361+
362+
deployment := s.extractDeployment(output)
363+
volumes := deployment.Spec.Template.Spec.Volumes
364+
s.Require().Len(volumes, 1, "Deployment should have exactly one volume")
365+
s.Equal("redis-data", volumes[0].Name)
366+
s.Require().NotNil(volumes[0].PersistentVolumeClaim,
367+
"redis-data volume should reference a PVC when existingClaim is set")
368+
s.Equal("my-prebuilt-redis-pvc", volumes[0].PersistentVolumeClaim.ClaimName,
369+
"claimName should match the user-supplied existingClaim, not the chart-generated name")
370+
s.Nil(volumes[0].EmptyDir,
371+
"redis-data volume should NOT be emptyDir when existingClaim is set")
372+
}
373+
374+
// TestExistingClaimIgnoresStorageClassAndSize ensures that size/storageClass
375+
// have no effect once the user has taken over claim provisioning via
376+
// existingClaim — the chart has no PVC to apply them to.
377+
func (s *telemetryRedisTemplateTest) TestExistingClaimIgnoresStorageClassAndSize() {
378+
options := &helm.Options{SetValues: map[string]string{
379+
"telemetry.redis.persistence.existingClaim": "my-prebuilt-redis-pvc",
380+
"telemetry.redis.persistence.size": "100Gi",
381+
"telemetry.redis.persistence.storageClass": "gp3",
382+
}}
383+
384+
output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName, s.templates)
385+
386+
s.NotContains(output, "kind: PersistentVolumeClaim",
387+
"PVC should not render when existingClaim is set, even with size/storageClass")
388+
s.NotContains(output, "100Gi",
389+
"size should not leak into the rendered output when existingClaim is set")
390+
s.NotContains(output, "storageClassName",
391+
"storageClassName should not appear in any rendered object when existingClaim is set")
392+
}
393+
394+
// TestPersistenceDisabledBeatsExistingClaim ensures persistence.enabled=false
395+
// remains the kill-switch — even with existingClaim set, the user opting out
396+
// of persistence should yield an emptyDir volume rather than a stale claim
397+
// mount.
398+
func (s *telemetryRedisTemplateTest) TestPersistenceDisabledBeatsExistingClaim() {
399+
options := &helm.Options{SetValues: map[string]string{
400+
"telemetry.redis.persistence.enabled": "false",
401+
"telemetry.redis.persistence.existingClaim": "my-prebuilt-redis-pvc",
402+
}}
403+
404+
output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName, s.templates)
405+
406+
s.NotContains(output, "kind: PersistentVolumeClaim",
407+
"PVC should not render when persistence.enabled=false")
408+
409+
deployment := s.extractDeployment(output)
410+
volumes := deployment.Spec.Template.Spec.Volumes
411+
s.Require().Len(volumes, 1)
412+
s.NotNil(volumes[0].EmptyDir,
413+
"persistence.enabled=false should take precedence and yield an emptyDir volume")
414+
s.Nil(volumes[0].PersistentVolumeClaim,
415+
"redis-data volume should NOT reference the existingClaim when persistence is disabled")
416+
}

0 commit comments

Comments
 (0)