Repository navigation
feat(openhands): PLTF-1247 migrate embedded Redis to Valkey - #973
Closed
aivong-openhands wants to merge 8 commits into
Closed
aivong-openhands wants to merge 8 commits into
aivong-openhands wants to merge 8 commits into
Conversation
Replace the frozen Bitnami redis 20.3.0 dependency with the official valkey chart 0.11.0 from https://valkey.io/valkey-helm/. Retain the `redis` / `redis-password` secret via auth.usersExistingSecret and aclUsers.default.passwordKey so existing installs keep their credential across the upgrade and rollback needs no re-derivation. REDIS_* env var names stay because application code reads them. Valkey renders a Deployment, so the support bundle analyzer moves from statefulsetStatus to deploymentStatus. The redis collector type is kept because Valkey speaks RESP and `default` is the ACL user. CI gains `helm repo add valkey` in the two workflows that run `helm dependency build`; the valkey repo is HTTP, not OCI.
aivong-openhands
force-pushed
the
pltf-1247-migrate-redis-to-valkey
branch
from
July 27, 2026 18:36
9da9063 to
47bd75d
Compare
The exec probe's `valkey-cli ping` exits 0 even when the server replies NOAUTH, so it gates on port reachability only and cannot flap under ACL enforcement. Verified on a KinD cluster: 0 restarts, no Unhealthy events.
… tests Migration notes in the chart README: the `redis` Secret is unchanged and needs no action, while a values file with a `redis:` block must be translated because a stale block is ignored without warning and the cache silently falls back to chart defaults. Replace the "Bring Your Own Redis" section, which documented `externalRedis` keys that exist in no template. The working mechanism is `valkey.enabled: false` plus `REDIS_*` env overrides, and it applies to Helm installs only since Embedded Cluster always uses the bundled cache. Unit tests cover the cache env wiring, the retained secret reference, the disabled path, the support-bundle Deployment analyzer, and the absence of any reference to the removed Bitnami workload.
aivong-openhands
marked this pull request as ready for review
July 28, 2026 16:30
aivong-openhands
requested review from
dylan-openhands,
jlav and
mamoodi
as code owners
July 28, 2026 16:30
…rt README The chart README covers Helm installs; the Admin Console path belongs in the Replicated install docs.
Comments and docs in the shipped files now state the current configuration and the action a consumer needs to take. The migration history belongs in the pull request, not in the chart. The `redis` values key and secret name still appear where they are load-bearing: the translation table a consumer follows, and the retained secret reference.
External cache support on Embedded Cluster is expected to change, so documenting today's limitation would go stale without anything flagging it.
dylan-openhands
approved these changes
Jul 28, 2026
aivong-openhands
marked this pull request as draft
August 3, 2026 16:05
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Bitnami's license change froze the free Redis chart and images, so the pinned
redis 20.3.0andbitnamilegacy/redisget no CVE patches and the index is not guaranteed to stay reachable. This swaps the embedded cache to the official Valkey chart (0.11.0, BSD-3-Clause), keeping the existingredis/redis-passwordsecret and theREDIS_*variable names so operators create nothing new and the app needs no code change. Cutover discards the cache, which invalidates in-flight OAuth and Slack flows and briefly cold-starts the identity caches.Consumers that pin cache settings must translate them in the same change. The values key becomes
valkey:, and a leftoverredis:block is silently ignored rather than rejected. All four internal SaaS environments pinredis.master.resourcesand none definevalkey:, so a chart bump alone would quietly drop production's cache from 500m/1Gi to the chart defaults of 100m/128Mi. The chart README now carries the key-by-key translation, a copy-paste block, and the rollback command.This is a narrower second attempt at #521, whose Valkey values shape, service rename and support-bundle changes carry over here. That one spanned 33 files because it bundled a CloudNativePG Postgres migration alongside the cache swap, and it renamed the secret to
valkey/valkey-passwordwith matching changes toopenhands-secrets. Keeping the existing secret name is the deliberate reversal:openhands-secretsstays untouched, and no install has to create a secret before upgrading or retain the old one in order to revert.Validation
redissecret's uid and value hash were identical after every upgrade and rollback on both surfaces, so no install needs a new secret and no revert needs the old password recovered.statefulsetStatusonopenhands-redis-mastertodeploymentStatusonopenhands-valkeyand back on revert, no StatefulSet, service or PVC survives the swap, and no subchart carries cache configuration of its own.redis:block is silently ignored — the upgrade succeeds and serves conversations, but the cache runs on chart defaults instead of the pinned values (100m in place of 50m). Nothing warns, because the values schema constrains neither key. This is the basis for the consumer note above.statefulsetStatusbreaks both the analyzer test and the guard against any lingeringopenhands-redis-masterreference.This PR was drafted by an AI agent on behalf of the user.