fix: sync deprecated flow control env var into feature gate map - #2076
Conversation
|
🚨 Unsigned commits detected! Please sign your commits. For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation. |
a662816 to
25d648d
Compare
|
The environment variable ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYER was deprecated in GIE 1.5.0 in favor of the mentioned featureGate. GIE 1.50. was merged into the llm-d-router in release 0.8.0. I would think that with the upcoming 0.10.0 release the deprecated environment variable ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYER can be simply deleted. |
|
@shmuelk removed ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYER. please review it . |
663fe1b to
ae32bca
Compare
ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYER was deprecated when GIE 1.5.0 introduced the featureGates: [flowControl] config stanza, which router 0.8.0 picked up. It was also broken: initAdmissionControl reads r.featureGates, populated once during parseConfigurationPhaseOne, while the env var was only ever applied afterward in parseConfigurationPhaseTwo against rawConfig.FeatureGates. Setting it logged the deprecation warning but silently left the EPP on the legacy admission controller. Ahead of the 0.10.0 release, remove the env var entirely instead of fixing it, since the config-based replacement has been available since 0.8.0. Use featureGates: [flowControl] in the EndpointPickerConfig. Fixes llm-d#2065 Signed-off-by: elinacse <elina.gcek94@gmail.com>
ae32bca to
85291f9
Compare
LukeAVanDrie
left a comment
There was a problem hiding this comment.
Thanks @elinacse!
Heads up: this conflicts with #2180 (flip the flowControl gate on by default). #2180 rewrites the deprecation log message inside applyDeprecatedEnvFeatureGate, which this PR deletes, so git merge-tree reports a content conflict in cmd/epp/runner/runner.go. feature_gate_test.go merges cleanly.
No coordination needed beyond that. Whichever lands first, the other rebases, and if this one goes first I will just drop my change to that function since it goes away with the variable.
|
@LukeAVanDrie Thanks for the review . can you help in merging this PR? |
I cannot, you will need an approval from someone in llm-d/router-maintainers. I only have perms for review |
initAdmissionControl reads r.featureGates, which is populated once during parseConfigurationPhaseOne and never revisited. The deprecated
ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYERenv var was only applied later, inparseConfigurationPhaseTwo, and only patchedrawConfig.FeatureGates— neverr.featureGates. So setting the env var logged the deprecation warning but left the EPP silently running the legacy admission controller instead of Flow Control.As flagged in review,
ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYERwas deprecated when GIE 1.5.0 introducedfeatureGates: [flowControl], and that replacement has been in since router 0.8.0. Rather than fix the env var to work again, this PR removes it entirely ahead of the 0.10.0 release. UsefeatureGates: [flowControl]in the EndpointPickerConfig instead.Fixes #2065