refactor(flowcontrol): remove the queue plugin registry#2036
Merged
shmuelk merged 3 commits intoJul 19, 2026
Conversation
The string-keyed queue factory was plugin-registration scaffolding left over from before ordering/fairness policies moved to the EPP framework registry. Queues were never promoted to plugins, and with a single queue type the indirection selects nothing. - Delete the factory (RegisteredQueueName, QueueConstructor, RegisteredQueues, MustRegisterQueue, NewQueueFromName) and the init()-time self-registration; add a direct queue.New(policy). - Remove the per-band Queue config field, the WithQueue option, and the defaultQueue default. Queues are an internal detail, not user-selectable. - buildFlowComponents constructs the queue directly and guards against a nil ordering policy (defense in depth: a corrupted config would otherwise panic when the queue first compares items). - Drive the queue conformance suite and benchmarks directly instead of ranging over the registry. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
Follow-on cleanups after removing the queue plugin registry: - Remove the now-vestigial Name() from the queue contract. Its only consumer was a "queueType" log field that is a constant given the single queue implementation. Drop it from the interface, the implementation, the accessor, the mocks, and the two log sites. - Consolidate the queue tests into priorityqueue_test.go and delete functional_test.go: construct via queue.New directly instead of the registry-era constructor indirection, table-drive the ordering cases, and split the suite into focused TestPriorityQueue_* functions. - Drop the nil-OrderingPolicy guard in buildFlowComponents. It was unreachable (config validation guarantees a non-nil policy) and only existed to give the provisioning-failure tests a trigger. Those tests now use ErrPriorityBandNotFound, a real failure mode that exercises the same lease-rollback and error-scoping paths. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
github-actions
Bot
added
kind/cleanup
size/XXL
Contributor
|
Nice straight forward cleanup. Two nits:
I am happy for these nits to be dealt with in a follow up PR. @LukeAVanDrie your choice. |
Contributor
Author
Thanks for the review! Regarding the first point, I agree that moving the file makes sense, but I will defer that to a follow-up PR to keep this specific cleanup focused as you suggested. On the second point about naming, I am honestly a bit torn. I definitely understand your point about the potential confusion since flow control already uses the term I am not completely sure which tradeoff is better. Maybe we can ask in the Slack to get a few more opinions on whether to keep it as |
Signed-off-by: Luke Van Drie <lukevandrie@google.com>
shmuelk
approved these changes
Jul 19, 2026
ahg-g
pushed a commit
that referenced
this pull request
Jul 23, 2026
#2146) With the queue plugin registry removed (#2036), the priority queue is a plain internal component of flow control, not a plugin. Move it from pkg/epp/flowcontrol/framework/plugins/queue to pkg/epp/flowcontrol/queue, deleting the now-empty pkg/epp/flowcontrol/framework tree so the only "framework" package left is the plugin SDK at pkg/epp/framework. Update the flow-control README pillars that still described the deleted sub-package and the long-removed QueueCapability system. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
With flow control unified on a single priority queue (#1831), the queue-selection machinery no longer selects anything. This removes it:
MustRegisterQueue,NewQueueFromName,RegisteredQueueName, theinit()self-registration) and construct the priority queue directly viaqueue.New.Queueconfig field, theWithQueueoption, and thedefaultQueuedefault. Queues are now a pure internal detail, not user-selectable.Name()from the queue contract; its only consumer was aqueueTypelog field that is constant given one implementation.priorityqueue_test.go(table-driven ordering, focusedTestPriorityQueue_*functions); deletefunctional_test.go.OrderingPolicyguard inbuildFlowComponents; the provisioning-failure tests now trigger the realErrPriorityBandNotFound, which exercises the same lease-rollback and error-scoping paths.Which issue(s) this PR fixes:
Fixes #2021
Release note: