fix(connection): use MergeFrom patch for finalizer writes to avoid 409 conflicts

[freeznet]

Problem

When a Pulsar sub-resource (e.g. a PulsarSource) is deleted, the operator deletes the backing Pulsar resource and then removes the Kubernetes finalizer. The finalizer-removal write could fail with a 409 Conflict ("the object has been modified"), leaving the object stuck Terminating and the owning PulsarConnection requeuing in a hot loop. It only cleared once the informer cache happened to catch up. This can also block GitOps convergence and hold up teardown of the connection, since a stuck child blocks PulsarConnection deletion.

Root cause

Sub-resource reconcilers add/remove the finalizer with a full-object Update:

controllerutil.RemoveFinalizer(obj, resourcev1alpha1.FinalizerName)
if err := r.conn.client.Update(ctx, obj); err != nil { ... }

The object is loaded once from the controller-runtime informer cache (the cached List in Observe), so the Update carries the cached resourceVersion as an optimistic-lock precondition. When the server copy is newer — informer-cache lag, or a concurrent writer such as a GitOps controller re-syncing the same object — the API server rejects the write with 409 Conflict, and the finalizer is never removed. An unrelated error on a sibling resource of the same connection makes the connection requeue faster than the cache settles, so the conflict repeats instead of self-correcting.

Fix

Replace each finalizer Update with a client.MergeFrom patch, guarded by the boolean return of Add/RemoveFinalizer:

patch := client.MergeFrom(obj.DeepCopy())
if controllerutil.RemoveFinalizer(obj, resourcev1alpha1.FinalizerName) {
    if err := r.conn.client.Patch(ctx, obj, patch); err != nil { ... }
}

A MergeFrom patch sends only the finalizer diff and carries no resourceVersion precondition, so it always applies to the live object regardless of cache lag or concurrent writers. The boolean guard avoids an API call when the finalizer set is unchanged.

Scope

The same Add/RemoveFinalizer + Update pattern existed in every PulsarConnection sub-reconciler, so the change is applied consistently across all of them and the PulsarConnection reconciler itself:

reconcile_tenant.go, reconcile_namespace.go, reconcile_topic.go, reconcile_source.go, reconcile_sink.go, reconcile_function.go, reconcile_package.go, reconcile_permission.go, reconcile_geo_replication.go, reconcile_nsisolationpolicy.go, reconciler.go.

Status().Update calls and the single non-finalizer object Update (the permission state annotation) are intentionally left unchanged.

Testing

• Existing pkg/connection unit tests pass.
• New regression test (reconcile_source_test.go) reconciles a deleting PulsarSource from a stale-resourceVersion copy (after an out-of-band server write) and asserts: no conflict error, the finalizer is persisted via Patch rather than a full-object Update, and the object is garbage-collected promptly.
• A companion test confirms the pre-fix path (a full Update from a stale copy) returns a 409 Conflict, so the regression test is non-vacuous — it fails against the old Update-based code and passes with the patch-based fix.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

@freeznet:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)