Skip to content

revert redis retirement - #5545

Merged
battermann merged 3 commits into
developfrom
battermann/revert-redis-retirement
Sep 18, 2026
Merged

battermann merged 3 commits into
developfrom
battermann/revert-redis-retirement

Conversation

@battermann

@battermann battermann commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

This is a straight forward git revert of:

plus a compatibility change of gundeck's configmap, to keep support for both versions.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@battermann
battermann marked this pull request as ready for review September 18, 2026 13:50
@battermann
battermann requested review from a team as code owners September 18, 2026 13:50
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 18, 2026
@battermann
battermann requested a lite review from Copilot September 18, 2026 13:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical Redis write lifecycle issues and multiple deployment/configuration defects remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Reverts Gundeck’s Redis retirement, restoring Redis-backed presence storage, migration support, deployment infrastructure, and compatibility configuration.

Changes:

  • Restores Redis presence handling and integration tests.
  • Adds Redis TLS, Helm, Docker, and ephemeral infrastructure.
  • Removes PostgreSQL presence migration artifacts.
  • Updates documentation and configuration.
File summaries
File Status
services/integration.yaml Reviewed
services/gundeck/test/unit/MockGundeck.hs Reviewed
services/gundeck/test/integration/Util.hs Changes required: clean up Redis workers
services/gundeck/test/integration/TestSetup.hs Reviewed
services/gundeck/test/integration/Main.hs Reviewed
services/gundeck/test/integration/API.hs Reviewed
services/gundeck/src/Gundeck/Util/Redis.hs Reviewed
services/gundeck/src/Gundeck/Run.hs Reviewed
services/gundeck/src/Gundeck/Redis.hs Changes required: preserve async cancellation
services/gundeck/src/Gundeck/Push/Websocket.hs Reviewed
services/gundeck/src/Gundeck/Push.hs Reviewed
services/gundeck/src/Gundeck/Presence/Data.hs Reviewed
services/gundeck/src/Gundeck/Presence.hs Reviewed
services/gundeck/src/Gundeck/Options.hs Reviewed
services/gundeck/src/Gundeck/Monad.hs Changes required: bound and await additional Redis writes
services/gundeck/src/Gundeck/Env.hs Reviewed
services/gundeck/gundeck.integration.yaml Reviewed
services/gundeck/gundeck.cabal Reviewed
services/gundeck/default.nix Reviewed
services/brig/src/Brig/Run.hs Reviewed
postgres-schema.sql Reviewed
Makefile Reviewed
libs/wire-subsystems/src/Wire/Postgres.hs Reviewed
libs/wire-subsystems/src/Wire/JobSubsystem/Migrations.hs Reviewed
libs/wire-subsystems/postgres-migrations/20260828093750-gundeck-presence.sql Reviewed
libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/Presence.hs Reviewed
libs/wire-api/src/Wire/API/Presence.hs Reviewed
hack/helmfile.yaml.gotmpl Reviewed
hack/helm_vars/wire-server/values.yaml.gotmpl Reviewed
hack/helm_vars/redis-ephemeral/values.yaml Reviewed
hack/helm_vars/certs/values.yaml.gotmpl Reviewed
hack/bin/gen-certs.sh Reviewed
docs/src/how-to/install/troubleshooting.md Reviewed
docs/src/how-to/install/infrastructure-configuration.md Reviewed
docs/src/developer/reference/config-options.md Fix configuration typos
docs/src/developer/developer/building.md Reviewed
deploy/dockerephemeral/docker/redis-node-6.conf Reviewed
deploy/dockerephemeral/docker/redis-node-6-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-6-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-node-5.conf Reviewed
deploy/dockerephemeral/docker/redis-node-5-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-5-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-node-4.conf Reviewed
deploy/dockerephemeral/docker/redis-node-4-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-4-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-node-3.conf Reviewed
deploy/dockerephemeral/docker/redis-node-3-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-3-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-node-2.conf Reviewed
deploy/dockerephemeral/docker/redis-node-2-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-2-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-node-1.conf Reviewed
deploy/dockerephemeral/docker/redis-node-1-key.pem Reviewed
deploy/dockerephemeral/docker/redis-node-1-cert.pem Reviewed
deploy/dockerephemeral/docker/redis-master-mode.conf Reviewed
deploy/dockerephemeral/docker-compose.yaml Reviewed
charts/wire-server/values.yaml Reviewed
charts/wire-server/templates/gundeck/tests/secret.yaml Reviewed
charts/wire-server/templates/gundeck/tests/gundeck-integration.yaml Reviewed
charts/wire-server/templates/gundeck/tests/configmap.yaml Reviewed
charts/wire-server/templates/gundeck/secret.yaml Reviewed
charts/wire-server/templates/gundeck/redis-ca-secret.yaml Changes required: use redisAdditionalWrite.tlsCa
charts/wire-server/templates/gundeck/deployment.yaml Changes required: restore the PostgreSQL secret mount
charts/wire-server/templates/gundeck/configmap.yaml Reviewed
charts/wire-server/templates/cannon/statefulset.yaml Reviewed
charts/wire-server/templates/_helpers.tpl Changes required: use the additional Redis endpoint fields
charts/redis-ephemeral/values.yaml Reviewed
charts/redis-ephemeral/requirements.yaml Reviewed
charts/redis-ephemeral/Chart.yaml Reviewed
charts/reaper/values.yaml Reviewed
charts/reaper/templates/rbac.yaml Reviewed
charts/reaper/templates/deployment.yaml Reviewed
charts/reaper/templates/configmap.yaml Reviewed
charts/reaper/templates/_helpers.tpl Reviewed
charts/reaper/scripts/reaper.sh Reviewed
charts/reaper/README.md Reviewed
charts/reaper/Chart.yaml Reviewed
charts/reaper/.helmignore Reviewed
charts/integration/templates/secret.yaml Reviewed
charts/integration/templates/integration-integration.yaml Reviewed
charts/databases-ephemeral/templates/NOTES.txt Reviewed
charts/databases-ephemeral/requirements.yaml Reviewed
changelog.d/5-internal/WPB-28377-gundeck-presence-postgres Add the required revert changelog entry
changelog.d/3-bug-fixes/WPB-28645 Reviewed
changelog.d/0-release-notes/WPB-28377-remove-redis Reviewed
Review details

Suppressed comments (6)

changelog.d/5-internal/WPB-28377-gundeck-presence-postgres:1

  • The PR description says a new changelog entry was added, but this change only removes the previous presence-migration entry and does not add an entry describing the Redis retirement revert/compatibility change. Please add the required changelog entry rather than leaving the revert undocumented.
    charts/wire-server/templates/gundeck/deployment.yaml:61
  • The deployment no longer mounts the gundeck Secret, but configmap.yaml still emits postgresqlPassword: /etc/wire/gundeck/secrets/pgPassword whenever the optional PostgreSQL compatibility block and secrets.pgPassword are configured. That leaves the configured password path absent and breaks the advertised support for older gundeck configurations. Keep the conditional gundeck-secrets volume and matching mount, or remove the compatibility PostgreSQL/password block consistently.
        {{- if eq (include "gundeck.configureRedisCa" .Values.gundeck.config) "true" }}
        - name: "redis-ca"
          secret:
            secretName: {{ include "gundeck.redisTlsSecretName" .Values.gundeck.config }}
        {{- end }}
        {{- if eq (include "gundeck.configureAdditionalRedisCa" .Values.gundeck.config) "true" }}
        - name: "additional-redis-ca"
          secret:
            secretName: {{ include "gundeck.additionalRedisTlsSecretName" .Values.gundeck.config }}

docs/src/developer/reference/config-options.md:2014

  • The configuration key is misspelled as redisAddtiionalWriteUsername, which does not match the actual redisAdditionalWriteUsername key shown immediately above.
    docs/src/developer/reference/config-options.md:2004
  • The new documentation spells “additional” as addtional; please correct this user-facing configuration description.
    services/gundeck/src/Gundeck/Monad.hs:152
  • This migration path deliberately starts the write to the additional Redis in an unawaited async, so the request can return before the second write has happened and failures are discarded. During a migration, a successful response followed by a failover can therefore lose presence data on the new Redis; wait for the additional write (or otherwise provide an explicit durable retry/acknowledgement) before reporting the operation complete.
    services/gundeck/test/integration/Util.hs:42
  • Each override creates one or two reconnecting Redis worker threads, but _rThreads is discarded and neither the workers nor their connections are stopped after action. The migration test invokes this helper twice, so the suite accumulates live Redis clients and reconnect/logging loops across tests; bracket the environment and cancel/disconnect it during cleanup.
  • Files reviewed: 84/85 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread services/gundeck/src/Gundeck/Monad.hs
Comment thread charts/wire-server/templates/_helpers.tpl
Comment thread charts/wire-server/templates/gundeck/redis-ca-secret.yaml
Comment thread services/gundeck/src/Gundeck/Redis.hs
@battermann
battermann merged commit 1dc34ef into develop Sep 18, 2026
11 checks passed
@battermann
battermann deleted the battermann/revert-redis-retirement branch September 18, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants