feat: Add additional configurability for breaking out caches

[danielpanzella]

Summary by CodeRabbit

• New Features

  • Per-cache Redis support: separate configs for task queue, settings cache, and metadata cache.
  • Centralized Redis configuration accessor and improved CA/certificate handling with templated volume/secret mounts.
  • Exposed per-name Redis connection strings and cert path options.

• Bug Fixes

  • Redis secret key and connection-string resolution updated (may require value adjustments).

• Chores

  • Chart version bumped to 0.37.0.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

charts/operator-wandb/templates/_redis.tpl (1)

119-144: Fix parameter resolution to use per-cache configuration.

Line 122 always uses $.Values.global.redis.params and $.Values.global.redis.parameters, which means per-cache params and parameters fields are ignored. This is inconsistent with the per-cache configuration approach.

The _caParams and _portParams helpers already use getRedisConfig, but line 122 bypasses this for the explicit params fields.

Apply this diff:

 {{- define "wandb.redis.parametersQuery" }}
+    {{- $redisConfig := fromYaml (include "wandb.redis.getRedisConfig" .) -}}
     {{- $caParams := include "_caParams" . | fromJson }}
     {{/* Both `params` and `parameters` have been used, historically */}}
-    {{- $valueParams := merge $.Values.global.redis.params $.Values.global.redis.parameters }}
+    {{- $valueParams := merge (default (dict) $redisConfig.params) (default (dict) $redisConfig.parameters) }}
     {{- $portParams := include "_portParams" . | fromJson }}
     {{- $defaultParams := include "_defaultParams" . | fromJson }}

♻️ Duplicate comments (1)

charts/operator-wandb/templates/_env.tpl (1)

40-77: Per-cache Redis environment variables look correct.

The per-cache environment variables (taskQueue, settingsCache, metadataCache) consistently use the centralized wandb.redis.* templates with the appropriate redisName parameter. This resolves the previous concern about inconsistent secret key references.

🧹 Nitpick comments (1)

charts/operator-wandb/values.yaml (1)

177-181: Consider documenting the expected structure for per-cache configurations.

The per-cache sections (settingsCache, metadataCache, taskQueue) are defined as empty dictionaries without showing their expected schema. Based on the global redis structure (lines 163-175), these likely support the same fields: host, port, password, external, parameters, params, caCert, certPath, and secret.

Adding a comment with the expected structure would help users understand how to configure these sections correctly.

Example:

 # (Optional) These (settingsCache, metadataCache, or taskQueue) almost never need to be set, do not set these unless you understand the risks.
 # Settings cache and metadata cache allow specifying a non-default redis instance for these caches.  Defaults to global.redis if not specified.
+# Each cache supports the same fields as global.redis:
+#   host, port, password, external, parameters, params, caCert, certPath, secret.secretName, secret.secretKey
 settingsCache: {}
 metadataCache: {}
 taskQueue: {}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c309e1b and a00be7a.

⛔ Files ignored due to path filters (1)

• test-configs/operator-wandb/__snapshots__/user-defined-secrets.snap is excluded by !**/*.snap

📒 Files selected for processing (4)

• charts/operator-wandb/charts/prometheus/charts/redis-exporter/templates/deployment.yaml (1 hunks)
• charts/operator-wandb/templates/_env.tpl (2 hunks)
• charts/operator-wandb/templates/_redis.tpl (4 hunks)
• charts/operator-wandb/values.yaml (5 hunks)

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: zacharyblasczyk
Repo: wandb/helm-charts PR: 479
File: charts/operator-wandb/values.yaml:776-776
Timestamp: 2025-09-04T13:54:32.322Z
Learning: The `wandb.executor.taskQueue` template in charts/operator-wandb/templates/_redis.tpl gets executed in the base-chart context, so values are accessed at `.Values.workerConcurrency` rather than `.Values.executor.workerConcurrency`.

📚 Learning: 2025-09-04T13:54:32.322Z

Learnt from: zacharyblasczyk
Repo: wandb/helm-charts PR: 479
File: charts/operator-wandb/values.yaml:776-776
Timestamp: 2025-09-04T13:54:32.322Z
Learning: The `wandb.executor.taskQueue` template in charts/operator-wandb/templates/_redis.tpl gets executed in the base-chart context, so values are accessed at `.Values.workerConcurrency` rather than `.Values.executor.workerConcurrency`.

Applied to files:

• charts/operator-wandb/templates/_env.tpl
• charts/operator-wandb/templates/_redis.tpl
• charts/operator-wandb/values.yaml

📚 Learning: 2025-04-21T16:46:09.791Z

Learnt from: danielpanzella
Repo: wandb/helm-charts PR: 390
File: charts/operator-wandb/values.yaml:1008-1013
Timestamp: 2025-04-21T16:46:09.791Z
Learning: The template "wandb.clickhouse.passwordSecret.passwordKey" exists in the Helm chart and is properly defined in ./charts/operator-wandb/templates/_clickhouse.tpl.

Applied to files:

• charts/operator-wandb/templates/_env.tpl
• charts/operator-wandb/templates/_redis.tpl
• charts/operator-wandb/charts/prometheus/charts/redis-exporter/templates/deployment.yaml
• charts/operator-wandb/values.yaml

📚 Learning: 2025-02-19T00:26:45.489Z

Learnt from: zacharyblasczyk
Repo: wandb/helm-charts PR: 0
File: :0-0
Timestamp: 2025-02-19T00:26:45.489Z
Learning: In Helm templates, when using include functions, proper template structure must be maintained. In PR #351 of helm-charts repository, a template syntax error was fixed by removing an incorrectly structured imagePullSecrets section that was using the include function without proper template formatting.

Applied to files:

• charts/operator-wandb/charts/prometheus/charts/redis-exporter/templates/deployment.yaml
• charts/operator-wandb/values.yaml

📚 Learning: 2025-02-19T00:26:45.489Z

Learnt from: zacharyblasczyk
Repo: wandb/helm-charts PR: 0
File: :0-0
Timestamp: 2025-02-19T00:26:45.489Z
Learning: In PR #351 of helm-charts repository, a template syntax error was fixed by removing an unclosed `with` block in the imagePullSecrets section of the deployment template.

Applied to files:

• charts/operator-wandb/charts/prometheus/charts/redis-exporter/templates/deployment.yaml
• charts/operator-wandb/values.yaml

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)

• GitHub Check: Test Chart (v1.30.10, user-defined-secrets)
• GitHub Check: Test Chart (v1.30.10, weave-trace-with-worker)
• GitHub Check: Test Chart (v1.31.6, default)
• GitHub Check: Test Chart (v1.30.10, default)
• GitHub Check: Test Chart (v1.30.10, oidc-enabled)
• GitHub Check: Test Chart (v1.30.10, url-encoded-password)
• GitHub Check: Test Chart (v1.30.10, weave-trace)
• GitHub Check: Test Chart (v1.30.10, separate-pods)
• GitHub Check: Test Chart (v1.30.10, runs-v2-bufstream)
• GitHub Check: Test Chart (v1.31.6, weave-trace-with-worker)
• GitHub Check: Test Chart (v1.32.2, separate-pods)
• GitHub Check: Test Chart (v1.31.6, runs-v2-bufstream)
• GitHub Check: Test Chart (v1.31.6, url-encoded-password)
• GitHub Check: Test Chart (v1.30.10, fmb)
• GitHub Check: Test Chart (v1.31.6, separate-pods)
• GitHub Check: Test Chart (v1.31.6, weave-trace)
• GitHub Check: Test Chart (v1.31.6, user-defined-secrets)
• GitHub Check: Test Chart (v1.31.6, oidc-enabled)
• GitHub Check: Test Chart (v1.31.6, fmb)
• GitHub Check: Test Chart (v1.32.2, url-encoded-password)
• GitHub Check: Test Chart (v1.32.2, user-defined-secrets)
• GitHub Check: Test Chart (v1.32.2, weave-trace-with-worker)
• GitHub Check: Test Chart (v1.32.2, weave-trace)
• GitHub Check: Test Chart (v1.32.2, runs-v2-bufstream)
• GitHub Check: Test Chart (v1.32.2, oidc-enabled)
• GitHub Check: Test Chart (v1.32.2, fmb)
• GitHub Check: Test Chart (v1.32.2, default)

🔇 Additional comments (11)

charts/operator-wandb/charts/prometheus/charts/redis-exporter/templates/deployment.yaml (1)

64-64: LGTM! Centralized password secret key reference.

The change correctly delegates to the centralized wandb.redis.passwordSecretKey template, aligning with the PR's goal of centralizing Redis configuration.

charts/operator-wandb/templates/_env.tpl (2)

26-38: LGTM! Updated Redis environment variables.

The changes correctly use the centralized templates for Redis configuration, including the new wandb.redis.passwordSecretKey template and explicit environment variables for port, host, and parameters.

---

79-99: LGTM! Connection strings properly differentiated.

The updates correctly use per-cache connection strings for GORILLA_METADATA_CACHE and GORILLA_SETTINGS_CACHE while maintaining the generic connection string for other caches that don't require per-cache configuration.

charts/operator-wandb/values.yaml (2)

1538-1550: LGTM! Consolidated volume template usage.

The migration to volumeMountsTpls and volumesTpls using centralized CA certificate templates is consistent with the pattern used across other services in the chart.

---

2243-2244: LGTM! Centralized secret key reference.

The use of the wandb.redis.passwordSecretKey template for existingSecretPasswordKey is consistent with the PR's centralization goals.

charts/operator-wandb/templates/_redis.tpl (6)

7-13: Verify precedence logic relies solely on host field.

The configuration precedence logic only checks whether the host field is set to determine which config level to use. This means if a user sets other fields (e.g., password, port) without setting host, those values will be ignored in favor of the default config.

While this enforces explicit configuration, it might be surprising for users who expect partial overrides. Consider whether this is the intended behavior or if a merge-based approach would be more appropriate.

Example scenario:

global:
  redis:
    host: "default-redis"
    port: 6379
  taskQueue:
    password: "custom-password"
    # No host specified

In this case, the taskQueue password would be ignored because host is not set, and the default redis config would be used entirely.

---

28-38: LGTM! New password secret key template.

The wandb.redis.passwordSecretKey template correctly extracts the secret key from the Redis configuration with a sensible default.

---

43-70: LGTM! Core Redis accessor templates updated.

The port, host, and password templates correctly delegate to getRedisConfig, maintaining backward compatibility with defaults.

---

149-158: LGTM! New CA certificate accessor templates.

The wandb.redis.caCert and wandb.redis.certPath templates correctly extract certificate configuration from the centralized Redis config.

---

164-189: LGTM! Connection string templates.

The main and per-cache connection string templates (connectionString, settingsCache.connectionString, metadataCache.connectionString) correctly construct Redis connection strings using environment variables and handle both authenticated and non-authenticated scenarios.

---

191-217: LGTM! Task queue templates.

The wandb.redis.taskQueue and wandb.executor.taskQueue templates correctly use the per-cache configuration for taskQueue, including proper handling of worker concurrency parameters.

Based on learnings: The wandb.executor.taskQueue template executes in the base-chart context, correctly accessing .Values.workerConcurrency.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix invalid property access.

Line 3 references .Values.AsMap, which does not exist in Helm. This should be .Values to properly access local configuration values.

Apply this diff:

 {{- define "wandb.redis.getRedisConfig" -}}
   {{- $redisName := default "redis" .redisName }}
-  {{- $localConfig := dig $redisName (dict) .Values.AsMap }}
+  {{- $localConfig := dig $redisName (dict) .Values }}
   {{- $globalConfig := dig $redisName (dict) .Values.global }}
   {{- $defaultConfig := .Values.global.redis }}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	{{- define "wandb.redis.getRedisConfig" -}}
	{{- $redisName := default "redis" .redisName }}
	{{- $localConfig := dig $redisName (dict) .Values.AsMap }}
	{{- $globalConfig := dig $redisName (dict) .Values.global }}
	{{- $defaultConfig := .Values.global.redis }}
	{{- if $localConfig.host }}
	{{- $localConfig | toYaml }}
	{{- else if $globalConfig.host }}
	{{- $globalConfig | toYaml }}
	{{- else }}
	{{- $defaultConfig | toYaml }}
	{{- end }}
	{{- end -}}
	{{- define "wandb.redis.getRedisConfig" -}}
	{{- $redisName := default "redis" .redisName }}
	{{- $localConfig := dig $redisName (dict) .Values }}
	{{- $globalConfig := dig $redisName (dict) .Values.global }}
	{{- $defaultConfig := .Values.global.redis }}
	{{- if $localConfig.host }}
	{{- $localConfig | toYaml }}
	{{- else if $globalConfig.host }}
	{{- $globalConfig | toYaml }}
	{{- else }}
	{{- $defaultConfig | toYaml }}
	{{- end }}
	{{- end -}}

🤖 Prompt for AI Agents

In charts/operator-wandb/templates/_redis.tpl around lines 1 to 14, the template
incorrectly references `.Values.AsMap` (which doesn't exist in Helm) when
digging local redis config; change that reference to `.Values` so the dig call
uses the correct values map (i.e., replace `.Values.AsMap` with `.Values`) so
local configuration is properly resolved.