Repository navigation
Operator skips cluster update when changing controller ID #3151
Description
- Which image of the operator are you using? ghcr.io/zalando/postgres-operator:v2.0.1
- Where do you run it? Azure Kubernetes Service
- Are you running Postgres Operator in production? yes
- Type of issue? Bug report
Hello,
I'm currently experimenting with running two instances of Postgres Operator. After changing a cluster's controller ID, I've noticed that the Postgres Operator does not update the cluster immediately.
Description
When changing the acid.zalan.do/controller annotation on a Postgres cluster manifest, the corresponding Postgres Operator should update the cluster accordingly. However, the update is skipped and the cluster is updated during the next resync.
Reason
The postgresqlUpdate method checks both the previous and the current manifests using postgresqlCheck.
postgres-operator/pkg/controller/postgresql.go
Lines 526 to 529 in a7aaad0
As the previous manifest's annotation doesn't match the operator's controller ID, the check fails and the update is skipped.
postgres-operator/pkg/controller/postgresql.go
Lines 559 to 567 in a7aaad0
Suggested fix
I think the postgresqlCheck method should ignore ownership when checking the previous manifest.
I think postgresqlUpdate should enqueue an EventSync when the previous manifest's check fails (we cannot use EventUpdate here because the K8s objects may or may have been created by the other Postgres Operator, which may or may not exist).
Note: I realize we may run into a race condition because we don't know for sure that the other Postgres Operator is done with the cluster (waiting for the resync does not prevent the race condition, though it is less likely). So maybe the appropriate response should be "don't change theacid.zalan.do/controller annotation"?
Activity
omkar619-dev commentedon Oct 2, 2026
@baarde I'd like to pick this up if nobody else is on it. I think your analysis holds on current master (
80b7a074, v2.0.3) —postgresqlCheckreturns nil whenhasOwnershipfails, andpostgresqlUpdate'sif pgOld != nil && pgNew != nilhas no else, so adopting a cluster queues nothing at all and it waits for the resync.Your
EventSyncsuggestion looks like the right call too, and I think it's more idiomatic than your hedging suggests:queueClusterEventalready takes a nil old spec — its branch for that is commented// add, sync—Clone()is nil-safe, andpostgresqlAddalready callsqueueClusterEvent(nil, pg, EventAdd). SoqueueClusterEvent(nil, pgNew, EventSync)on the adoption path fits what's there rather than working around it.On the race you flagged: I don't think it can be fully closed though from this side, since we can't know whether the other operator has finished. But sync seems like the right primitive precisely because of that — it reconciles to the desired state rather than diffing against an old spec we have no reason to trust. Resync has the same exposure today, so this makes the window smaller rather than introducing a new one.
One thing not in the report: the mirror case is dropped too. When a cluster moves away (
pgOld != nil,pgNew == nil) the same missing else means the operator never sees the handover and keeps whatever it had until resync. I'd leave that out of this change — stopping management is a different decision from starting it — but flagging it so it's a choice rather than an oversight.For the test,
clusterEventStores/clusterEventQueuesare built ininitControllerrather thanNewController, so I'd add a small harness topostgresql_test.gothat sets up a single worker queue, then assert the adoption path queuesEventSyncwith a nilOldSpec. Happy to go another way if you or a maintainer prefer.