feat: implement EPP plugin stability lifecycle and --allow-experimental-plugins flag (#1912) - #2073
feat: implement EPP plugin stability lifecycle and --allow-experimental-plugins flag (#1912)#2073liu-cong wants to merge 15 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces plugin stability levels (Alpha/Beta/Stable) into the EPP plugin registry, adds an experimentalPlugins feature gate to block Alpha plugin initialization by default, and adds stability dependency validation between Consumer/Producer plugins in the data layer.
Changes:
- Extend plugin registration to include stability metadata and expose stability lookup via
GetPluginStability. - Add
experimentalPluginsfeature-gate enforcement during plugin instantiation (Alpha blocked unless explicitly enabled). - Add data-layer validation to prevent higher-stability consumers from depending on lower-stability producers.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/epp/framework/plugins/scheduling/profilehandler/disagg/disagg_headers_handler_test.go | Updates test-time plugin registration to pass stability level. |
| pkg/epp/framework/plugins/README.md | Documents stability levels and the experimentalPlugins feature gate. |
| pkg/epp/framework/plugins/flowcontrol/eviction/ordering/priority_time.go | Updates plugin self-registration to include stability. |
| pkg/epp/framework/plugins/flowcontrol/eviction/filtering/sheddable.go | Updates plugin self-registration to include stability. |
| pkg/epp/framework/interface/plugin/stability.go | Adds StabilityLevel constants, feature gate name, and plugin metadata struct. |
| pkg/epp/framework/interface/plugin/registry.go | Extends registration APIs to accept stability and records metadata per plugin type. |
| pkg/epp/framework/interface/plugin/registry_test.go | Updates registry tests for new stability-aware registration and metadata snapshotting. |
| pkg/epp/datalayer/data_graph.go | Adds stability dependency validation after auto-creating missing data producers. |
| pkg/epp/datalayer/data_graph_test.go | Adds unit test coverage for stability dependency validation. |
| pkg/epp/config/loader/configloader.go | Enforces Alpha gating during instantiation and emits deprecation warnings via metadata. |
| pkg/epp/config/loader/configloader_test.go | Updates feature-gate defaults and adds coverage for Alpha gating behavior. |
| cmd/epp/runner/test_runner.go | Updates integration-test runner helper registration to include stability. |
| cmd/epp/runner/runner.go | Centralizes in-tree plugin stability assignments during registration and registers the new feature gate. |
Comments suppressed due to low confidence (1)
cmd/epp/runner/test_runner.go:46
fwkplugin.Registernow also populatesfwkplugin.RegistryMetadata, but the integration-test helper only deletes fromfwkplugin.Registry. This leaves stale metadata in-process and can affect later tests that inspect plugin metadata.
fwkplugin.Register(mockType, fwkplugin.StabilityStable, func(name string, _ *json.Decoder, _ fwkplugin.Handle) (fwkplugin.Plugin, error) {
return mockDataSource, nil
})
defer delete(fwkplugin.Registry, mockType)
}
|
@liu-cong Overall the PR seems quite good. One question. You dealt with Consumers depending on Producers of a lower level of stability. This is good. You didn't deal with other cases of dependencies betwen plugins. Such as the latest Disaggregated plugin which is dependent on decider plugins. Shouldn't this PR deal with that as well? |
LukeAVanDrie
left a comment
There was a problem hiding this comment.
One follow-up... the Alpha gate is only enforced in instantiatePlugins, so plugins auto-created via registerDefaultPlugin or CreateMissingDataProducers bypass it. No Alpha default producers exist today, so probably just a TODO.
…s feature gate (llm-d#1912) Signed-off-by: Cong Liu <conliu@google.com>
3677a53 to
5fe4fad
Compare
|
@LukeAVanDrie @shmuelk I removed the dependency validation on lower stability plugins, and use the feature gate as the single gating mechanism. Let me know what you think. |
| const SheddableFilterType = "sheddable-eviction-filter" | ||
|
|
||
| func init() { | ||
| plugin.Register(SheddableFilterType, SheddableFilterFactory) |
| // breaking ties by newest dispatch time (least KV-cache investment). | ||
| const PriorityThenTimeOrderingType = "priority-then-time-eviction-order-policy" | ||
|
|
||
| func init() { |
… under major refactoring Signed-off-by: Cong Liu <conliu@google.com>
|
@liu-cong The hermetic integration tests are failing. Partly to the use of an Alpha level plugin without your new feature gate. |
Signed-off-by: Cong Liu <conliu@google.com>
Signed-off-by: Cong Liu <conliu@google.com>
…le experimentalPlugins in test Signed-off-by: Cong Liu <conliu@google.com>
Signed-off-by: Cong Liu <conliu@google.com>
Signed-off-by: Cong Liu <conliu@google.com>
…lpha plugins Signed-off-by: Cong Liu <conliu@google.com>
…tal-plugins CLI flag Signed-off-by: Cong Liu <conliu@google.com>
…ts and data producers Signed-off-by: Cong Liu <conliu@google.com>
…tor configs_test.go Signed-off-by: Cong Liu <conliu@google.com>
Signed-off-by: Cong Liu <conliu@google.com>
…ed by utilization filter in upstream llm-d#2218) Signed-off-by: Cong Liu <conliu@google.com>
…on parsing to run after all phases complete Signed-off-by: Cong Liu <conliu@google.com>
…nPhaseTwo Signed-off-by: Cong Liu <conliu@google.com>
elevran
left a comment
There was a problem hiding this comment.
Solid implementation of the plugin stability lifecycle from #1912 - the Alpha/Beta gating logic, CLI flag wiring, and validation placement (after all plugin init phases, covering configured, default, and auto-created producer plugins) are all correct and well tested at the unit level. A few small things to clean up before merge.
Two docs outside this diff are now stale and worth a follow-up:
- docs/create_new_filter.md:172 -
plugin.Register(bylabel.LabelSelectorFilterType, bylabel.SelectorFactory)uses the old 2-arg signature. Following this guide to add a new plugin will fail to compile after this PR. Worth updating toplugin.Register(type, stability, factory)plus a note on choosing Alpha/Beta/Stable. - docs/plugins-vs-builtins.md:20 - same stale 2-arg
plugin.Register(type, factory)reference.
| fwkplugin.Register(random.RandomPickerType, fwkplugin.StabilityBeta, random.RandomPickerFactory) | ||
| fwkplugin.Register(weightedrandom.WeightedRandomPickerType, fwkplugin.StabilityBeta, weightedrandom.WeightedRandomPickerFactory) | ||
| fwkplugin.Register(single.SingleProfileHandlerType, fwkplugin.StabilityBeta, single.SingleProfileHandlerFactory) | ||
| fwkplugin.Register(headerprofile.HeaderProfileHandlerType, fwkplugin.StabilityAlpha, headerprofile.HeaderProfileHandlerFactory) |
There was a problem hiding this comment.
headerprofile.HeaderProfileHandlerType is registered here under the "Beta" heading but tagged StabilityAlpha, and registered again below under "Alpha" (~L615). While the second call silently overwrites the first , suggest deleting the stray line here and keeping only the one under the Alpha section.
| |---|---|---| | ||
| | **Alpha** | Experimental features under active development. No backwards-compatibility guarantees (parameters or behavior may change anytime). | Requires `--allow-experimental-plugins` CLI flag. | | ||
| | **Beta** | Feature-complete and enabled by default. Backwards-compatible within current version; subject to a +2 minor version deprecation policy before removal. | Allowed by default (no CLI flag required). | | ||
| | **Stable** | Production-grade and fully backwards-compatible across minor releases. Breaking changes only on major version bumps. | Allowed by default (no CLI flag required). | |
There was a problem hiding this comment.
nit: this table documents Stable as a real tier with its own guarantees, but no in-tree plugin in runner.go is currently registered as StabilityStable. I think we can promote a few long-lived plugins (e.g. single, maxscore) or note that Stable isn't populated yet.
| scheduler := scheduling.NewSchedulerWithConfig(r.schedulerConfig) | ||
|
|
||
| if err := fwkplugin.ValidatePluginStability(r.PluginHandle, opts.AllowExperimentalPlugins); err != nil { | ||
| setupLog.Error(err, "Plugin stability validation failed") |
There was a problem hiding this comment.
No test drives the real startup path (setup/runWithFileDiscovery) with an Alpha plugin configured and --allow-experimental-plugins left at its default false, asserting startup fails. Existing tests call ValidatePluginStability directly in isolation. A test through the actual CLI-flag-to-runner path would catch a future regression where the flag stops reaching the check.
This PR implements the proposal for EPP plugin stability lifecycle and CLI flag gating discussed in #1912.
FIXES #1912
--allow-experimental-pluginsis set on the EPP runner.sheddableandpriority_timeplugins are marked Alpha despite pre-dating 0.9. These are never really used and still under active iteration.Release note: