johannes-engler-mw commented on PR #2834: URL: https://github.com/apache/apisix-ingress-controller/pull/2834#issuecomment-5178742314
A review pass raised whether the early return should also clear this controller's own `status.parents` entry, per [RouteParentStatus](https://gateway-api.sigs.k8s.io/reference/api-spec/1.4/spec/#gateway.networking.k8s.io/v1.RouteParentStatus). Worth recording what I found, since the answer is not obvious from the diff. **In the migration this PR targets, the stale entry does not survive.** The status write is a wholesale replacement rather than a merge: ```go // TODO: diff the old and new status hr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0, len(gateways)) ... hCopy.Status = hr.Status ``` So when the *new* controller reconciles the moved route, it replaces the whole `parents` list and the previous controller's entry goes with it. Confirmed on a live two-controller cluster after exactly this migration — all four moved routes carry a single entry, the new owner's: ```console $ kubectl get httproute -A -o json | jq -r '.items[] | "\(.metadata.name) parents=\(.status.parents|length) [\(.status.parents[]?.controllerName)]"' gdpr-service parents=1 [apisix.apache.org/apisix-ingress-controller-internal] pii-data-service parents=1 [apisix.apache.org/apisix-ingress-controller-internal] rbac parents=1 [apisix.apache.org/apisix-ingress-controller-internal] structure-data-service parents=1 [apisix.apache.org/apisix-ingress-controller-internal] ``` **A stale entry does persist in one narrower case:** moving a route to a class no live controller serves — which is what the e2e test does with `apisix.apache.org/not-exist`, since that is the cheapest way to take a route out of scope in a single-controller test environment. Nobody rewrites the status there, so the old entry remains. That behaviour is identical on `master`; this PR does not introduce or worsen it, and the data-plane cleanup is independent of it. **The underlying issue is the wholesale replacement itself**, which the `TODO` above already flags. It is arguably a larger spec deviation than a leftover entry: controller A overwrites controller B's `RouteParentStatus`, so a route legitimately parented to Gateways owned by two different controllers cannot hold a stable status at all. Fixing that means giving the status updater per-controller merge semantics across all five route kinds — a change to shared status handling that I would rather not fold into a data-plane cleanup. Happy to open a separate issue for the merge semantics if that is useful, or to fold it in here if maintainers prefer them together. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
