Skip to content

Clean up disabled connection pooler objects after operator restart - #3211

Open
sarankumar1325 wants to merge 1 commit into
zalando:masterfrom
sarankumar1325:fix-pooler-cleanup-after-restart
Open

sarankumar1325 wants to merge 1 commit into
zalando:masterfrom
sarankumar1325:fix-pooler-cleanup-after-restart

Conversation

@sarankumar1325

Copy link
Copy Markdown

Problem description

Fixes #3208.

When the connection pooler is disabled, cleanup only works on the objects the operator remembers in memory (c.ConnectionPooler[role].Deployment/Service/AuthSecret). Two things went wrong:

  1. deleteConnectionPooler returned as soon as one deletion failed. If deleting the Service failed, the auth Secret was never attempted.
  2. After an operator restart the new Cluster object starts with empty references. syncConnectionPooler then skips cleanup for a disabled pooler, because nothing is remembered. Leftover objects (the Service and auth Secret from the issue, or the whole pooler if it was disabled while the operator was down) stayed forever, and sync reported success.

Changes:

  • When syncConnectionPooler first initialises the pooler objects of a role (first sync of the cluster, e.g. after a restart) and the pooler is not needed, the new discoverConnectionPoolerObjects looks up the Deployment, Service and <name>-u auth Secret by name. It only remembers objects that carry the operator's connection-pooler: <name> label, so unrelated objects with the same name are left alone. This is a one-time lookup per cluster object, not on every sync.
  • deleteConnectionPooler now tries all three objects, forgets only the ones that are really deleted (or already gone), and returns the combined error. A failed object stays remembered and is retried on the next sync.
  • The disabled-pooler branch also runs cleanup when only the auth Secret is left.

Linked issues

Fixes #3208

Checklist

New unit tests in pkg/cluster/connection_pooler_test.go:

  • TestConnectionPoolerCleanupAfterRestart: creates the pooler, then syncs a fresh Cluster with the pooler disabled (same old and new spec, as on startup) and checks that the Deployment, Service and auth Secret are gone for both roles. It fails on master.
  • TestConnectionPoolerDeleteContinuesOnError: a fake client reactor fails the Service delete once. The auth Secret is still deleted, the Service stays remembered, and a second call removes it. It fails on master.
  • TestConnectionPoolerCleanupKeepsForeignObjects: a Service with the pooler name but no pooler label is not deleted.

go test ./... (after go generate ./... for mocks), go vet ./pkg/cluster and staticcheck ./pkg/cluster all pass. I didn't run the e2e suite.

I wrote this change with the help of an AI coding assistant, and I reviewed and tested it.

When the connection pooler is disabled, cleanup only looked at the
objects remembered in memory. After an operator restart those
references are gone, so a Deployment, Service or auth Secret left over
from a failed or interrupted cleanup was never removed.

- On the first sync of a cluster with a disabled pooler, look up the
  pooler Deployment, Service and auth Secret by name (only objects
  carrying the operator's connection-pooler label) so they get deleted.
- deleteConnectionPooler now tries all three objects even if one
  deletion fails, forgets only the ones that are gone, and returns the
  combined error, so the failed one is retried on the next sync.
- The disabled-pooler sync path also cleans up when only the auth
  Secret is left.

Fixes zalando#3208

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pooler resources are not cleaned up after operator restart

1 participant