Operator skips cluster update when changing controller ID

[baarde]

• 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

	func (c *Controller) postgresqlUpdate(prev, cur interface{}) {
	pgOld := c.postgresqlCheck(prev)
	pgNew := c.postgresqlCheck(cur)
	if pgOld != nil && pgNew != nil {

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

	func (c *Controller) postgresqlCheck(obj interface{}) *acidv1.Postgresql {
	pg, ok := obj.(*acidv1.Postgresql)
	if !ok {
	c.logger.Errorf("could not cast to postgresql spec")
	return nil
	}
	if !c.hasOwnership(pg) {
	return nil
	}

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"?