fix(datastore): retry dropped endpoint registrations instead of silently untracking pods

[LukeAVanDrie]

What type of PR is this?

/kind bug

What this PR does / why we need it:

An endpoint upsert that overlaps an in-flight delete observes Runtime.NewEndpoint returning nil (the previous collector is still registered) while the pods-map entry is already gone. The datastore skipped storing the pod and the reconciler reported success, so the pod stayed untracked until the next pod event; for a stable Ready pod that event may never come. An empty datastore pods map makes the flow control saturation detectors fail closed, pinning flow_control_pool_saturation at 1.0 and halting dispatch (#2060).

sequenceDiagram
    participant Del as Delete path (PodDelete / resync)
    participant DS as Datastore
    participant R as Runtime (collector registry)
    participant Rec as PodReconciler (upsert)

    Del->>DS: PodDelete(pod)
    DS->>DS: pods.Delete(key)
    DS->>R: ReleaseEndpoint(ep)
    Note over R: dispatches EventDelete to extractors<br/>(collector still registered)
    Rec->>DS: PodUpdateOrAddIfNotExist(pod)
    DS->>DS: pods.Load(key): miss
    DS->>R: NewEndpoint(meta)
    R-->>DS: nil (collector still registered)
    DS->>DS: pods.Load(key): still miss
    DS-->>Rec: errRegistrationDropped
    Note over Rec: before this PR: nil swallowed, reconcile "succeeds",<br/>pod untracked until the next pod event.<br/>Now: error returned, controller-runtime requeues with backoff.
    R->>R: collectors.Remove(key), Stop()
    Rec->>DS: retry: PodUpdateOrAddIfNotExist(pod)
    DS->>R: NewEndpoint(meta)
    R-->>DS: endpoint
    DS->>DS: pods.Store(key, ep)
    DS-->>Rec: nil (tracked)

Loading

This PR makes the drop detectable and retryable:

• upsertEndpoint distinguishes the benign duplicate-start race (a concurrent upsert stored the entry; this call's metadata is applied through the update path) from a dropped registration, which is returned as an error wrapping a sentinel.
• PodReconciler propagates the error so controller-runtime requeues with backoff.
• podResyncAll propagates the error through PoolSet, and a needsResync flag makes the retried PoolSet re-run the resync (the pool is stored before resyncing, so the pool comparison alone would skip the retry).
• Discovery-source upserts log the drop; no requeue mechanism exists on that path.
• Tests: dropped-registration unit and reconciler tests, a concurrent add/remove completeness test, and a resync-retry regression test (fails without the needsResync flag).

Follow-up, deliberately not in this PR: have Runtime.NewEndpoint return a typed error distinguishing "collector already registered" from "collector failed to start", and evaluate closing the window in ReleaseEndpoint itself. The current release ordering (dispatch the delete event, then deregister the collector) is critical for extractor state, so a naive reorder is not safe.

Which issue(s) this PR fixes:

Related to #2060. Closure is gated on a multi-replica re-test against main; see the issue thread.

Release note:

Fixed a race where an endpoint added while its previous registration was still being deleted could be silently dropped from the EPP datastore, leaving the endpoint unroutable and pinning flow_control_pool_saturation at 1.0 until the next pod event. Dropped registrations are now retried.

[ahg-g]

Why not address the race condition itself? I am surprised that we don't do that for all things related to pod and endpoint updates to the datastore.

[LukeAVanDrie]

Why not address the race condition itself? I am surprised that we don't do that for all things related to pod and endpoint updates to the datastore.

Mostly because the nil from NewEndpoint isn't only the race. collector.Start failing produces the same thing with no concurrency at all, so the caller needs a retry path either way.

Fixing the race at the source is also less simple than it looks. ReleaseEndpoint intentionally dispatches EventDelete before deregistering the collector. If we flip that, a racing NewEndpoint can register a fresh collector whose state the stale EventDelete then tears down. Locking in the datastore instead means holding a lock across plugin dispatch and a blocking collector Stop.

We probably need per-key serialization in the collector registry plus a typed error from NewEndpoint. This is a datalayer change I'd rather do separately (it's the follow-up listed in the description; happy to file the issue).

With the retry, losing the race costs one requeue instead of a permanently missing endpoint.

If you have any other ideas here that are less complex, please let me know. To my best knowledge this race window is very small and unlikely to be hit in practice.

[ahg-g]

/approve