AlinsRan commented on issue #2795:
URL: 
https://github.com/apache/apisix-ingress-controller/issues/2795#issuecomment-5502297588

   The problem is real, but I don't think this design works as described. Four 
concerns:
   
   **1. It would silently break the data path.**
   There is no separate "push on event" path — every change ultimately goes 
through `client.Sync()`. Excluding a type there (say `consumer`) means new or 
updated CRDs of that type never reach the data plane, while their status still 
reports success. That isn't "skipping GC", it breaks synchronization for the 
resource entirely.
   
   **2. No validation, and a bad value takes everything down.**
   The example uses capitalized names (`Consumer`), but ADC expects lowercase 
snake_case (`consumer`). Its schema validation is strict, so an invalid enum 
value fails the whole `/sync` request — every resource type stops syncing, not 
just the one that was configured. This needs a fast-fail check at controller 
startup.
   
   **3. The scope is too coarse.**
   The option sits at the `provider` level, so it applies globally. If one 
GatewayProxy needs its consumers managed externally, every other GatewayProxy 
has to give up consumer reconciliation as well.
   
   **4. It doesn't apply to every provider.**
   In `apisix-standalone` mode there is only whole-configuration GET/PUT, so 
out-of-band consumer creation isn't possible in the first place. If the option 
is added, it should be restricted to `provider.type: apisix`.
   
   ---
   
   **Suggested direction: scope the sweep by the `manager-by` label.**
   
   The root cause is that `Client.Sync()` builds its Task without `Labels`, so 
ADC compares the controller's local config against the entire data plane and 
deletes anything created out of band.
   
   Everything the controller translates already carries a `manager-by` label, 
and ADC's `labelSelector` is asymmetric — it stamps the labels onto what you 
push, and filters what it reads back from the data plane. Passing it in the 
sweep changes the rule from "delete everything I don't have" to "delete what I 
created and no longer have". No new configuration, and no resource type left 
unmanaged.
   
   Worth fixing alongside it: deleting a Gateway or IngressClass runs a sync 
with no labels and no resources, which wipes the whole data plane.
   


-- 
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]

Reply via email to