Skip to content

Commit 592da02

Browse files
authored
feat: Enable the disabling of feature gates that are by default on/active (#1728)
* Support enabling and disabling feature flags Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Removed deprecated dataLayer feature flag Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Updated feature flag registration with default value Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Updated tests WRT new feature flag features Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> --------- Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
1 parent 9da2097 commit 592da02

7 files changed

Lines changed: 104 additions & 46 deletions

File tree

cmd/epp/runner/runner.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -602,8 +602,7 @@ func (r *Runner) parseConfigurationPhaseOne(ctx context.Context, opts *runserver
602602
}
603603
}
604604

605-
loader.RegisterFeatureGate(datalayer.ExperimentalDatalayerFeatureGate)
606-
loader.RegisterFeatureGate(flowcontrol.FeatureGate)
605+
loader.RegisterFeatureGate(flowcontrol.FeatureGate, false)
607606

608607
r.registerInTreePlugins()
609608

@@ -614,11 +613,6 @@ func (r *Runner) parseConfigurationPhaseOne(ctx context.Context, opts *runserver
614613

615614
r.featureGates = featureGates
616615

617-
if r.featureGates[datalayer.ExperimentalDatalayerFeatureGate] {
618-
setupLog.Info("The data layer is now enabled by default. " +
619-
"Please remove the 'dataLayer' feature gate from your config.")
620-
}
621-
622616
setupLog.Info("Data layer: ENABLED")
623617

624618
r.rawConfig = rawConfig

pkg/epp/config/loader/configloader.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package loader
1919
import (
2020
"errors"
2121
"fmt"
22+
"strconv"
23+
"strings"
2224
"sync"
2325

2426
"github.com/go-logr/logr"
@@ -46,7 +48,7 @@ import (
4648
var (
4749
scheme = runtime.NewScheme()
4850
registeredFeatureGatesMu sync.RWMutex
49-
registeredFeatureGates = sets.New[string]()
51+
registeredFeatureGates = make(map[string]bool)
5052
deprecatedSchemeGroupVersion = schema.GroupVersion{Group: "inference.networking.x-k8s.io", Version: "v1alpha1"} // TODO: deprecated should be clean up
5153
)
5254

@@ -68,10 +70,10 @@ func init() {
6870
}
6971

7072
// RegisterFeatureGate registers a feature gate name for validation purposes.
71-
func RegisterFeatureGate(gate string) {
73+
func RegisterFeatureGate(gate string, isEnabledByDefault bool) {
7274
registeredFeatureGatesMu.Lock()
7375
defer registeredFeatureGatesMu.Unlock()
74-
registeredFeatureGates.Insert(gate)
76+
registeredFeatureGates[gate] = isEnabledByDefault
7577
}
7678

7779
// LoadRawConfig parses the raw configuration bytes, applies initial defaults, and extracts feature gates.
@@ -132,7 +134,11 @@ func LoadRawConfig(configBytes []byte, logger logr.Logger) (*configapi.EndpointP
132134
return nil, nil, fmt.Errorf("feature gate validation failed: %w", err)
133135
}
134136

135-
featureConfig := loadFeatureConfig(rawConfig.FeatureGates)
137+
featureConfig, err := loadFeatureConfig(rawConfig.FeatureGates)
138+
if err != nil {
139+
return nil, nil, fmt.Errorf("failed to load feature gates: %w", err)
140+
}
141+
136142
return rawConfig, featureConfig, nil
137143
}
138144

@@ -162,7 +168,11 @@ func InstantiateAndConfigure(
162168
return nil, fmt.Errorf("scheduler config build failed: %w", err)
163169
}
164170

165-
featureGates := loadFeatureConfig(rawConfig.FeatureGates)
171+
featureGates, err := loadFeatureConfig(rawConfig.FeatureGates)
172+
if err != nil {
173+
return nil, fmt.Errorf("failed to load feature gates: %w", err)
174+
}
175+
166176
dataConfig, err := buildDataLayerConfig(rawConfig.DataLayer, handle)
167177
if err != nil {
168178
return nil, fmt.Errorf("data layer config build failed: %w", err)
@@ -294,17 +304,26 @@ func buildSchedulerConfig(
294304
return scheduling.NewSchedulerConfig(profileHandler, profiles), nil
295305
}
296306

297-
func loadFeatureConfig(gates configapi.FeatureGates) map[string]bool {
307+
func loadFeatureConfig(gates configapi.FeatureGates) (map[string]bool, error) {
298308
registeredFeatureGatesMu.RLock()
299309
defer registeredFeatureGatesMu.RUnlock()
300310
config := make(map[string]bool, len(registeredFeatureGates))
301-
for gate := range registeredFeatureGates {
302-
config[gate] = false
311+
for gate, defaultValue := range registeredFeatureGates {
312+
config[gate] = defaultValue
303313
}
304314
for _, gate := range gates {
305-
config[gate] = true
315+
value := true
316+
parts := strings.Split(gate, "=")
317+
if len(parts) > 1 {
318+
var err error
319+
value, err = strconv.ParseBool(strings.TrimSpace(strings.ToLower(parts[1])))
320+
if err != nil {
321+
return nil, err
322+
}
323+
}
324+
config[parts[0]] = value
306325
}
307-
return config
326+
return config, nil
308327
}
309328

310329
func buildParserRegistry(rawParserConfigs []configapi.ParserConfig, handle fwkplugin.Handle, logger logr.Logger) (*handlers.ParserRegistry, error) {

pkg/epp/config/loader/configloader_test.go

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
configapi "github.com/llm-d/llm-d-router/apix/config/v1alpha1"
3333
"github.com/llm-d/llm-d-router/pkg/common/observability/logging"
3434
"github.com/llm-d/llm-d-router/pkg/epp/config"
35-
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
3635
"github.com/llm-d/llm-d-router/pkg/epp/flowcontrol"
3736
"github.com/llm-d/llm-d-router/pkg/epp/flowcontrol/registry"
3837
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
@@ -67,6 +66,8 @@ const (
6766
testProfileHandler = "test-profile-handler"
6867
testSourceType = "test-source"
6968
testExtractorType = "test-extractor"
69+
70+
testFeatureGate = "test-feature-gate"
7071
)
7172

7273
// --- Test: Phase 1 (Raw Loading & Static Defaults) ---
@@ -75,19 +76,20 @@ func TestLoadRawConfiguration(t *testing.T) {
7576
t.Parallel()
7677

7778
// Register known feature gates for validation.
78-
RegisterFeatureGate(datalayer.ExperimentalDatalayerFeatureGate)
79-
RegisterFeatureGate(flowcontrol.FeatureGate)
79+
RegisterFeatureGate(testFeatureGate, true)
80+
RegisterFeatureGate(flowcontrol.FeatureGate, false)
8081

8182
queueScorerWeight := 2.0
8283
kvCacheUtilizationScorerWeight := 2.0
8384
prefixCacheScorerWeight := 3.0
8485

8586
tests := []struct {
86-
name string
87-
configText string
88-
want *configapi.EndpointPickerConfig
89-
wantErr bool
90-
deprecated bool
87+
name string
88+
configText string
89+
want *configapi.EndpointPickerConfig
90+
wantFeatures map[string]bool
91+
wantErr bool
92+
deprecated bool
9193
}{
9294
{
9395
name: "Success - Full Configuration",
@@ -114,7 +116,7 @@ func TestLoadRawConfiguration(t *testing.T) {
114116
},
115117
},
116118
FeatureGates: configapi.FeatureGates{
117-
datalayer.ExperimentalDatalayerFeatureGate,
119+
testFeatureGate,
118120
flowcontrol.FeatureGate,
119121
},
120122
FlowControl: &configapi.FlowControlConfig{
@@ -123,6 +125,10 @@ func TestLoadRawConfiguration(t *testing.T) {
123125
},
124126
},
125127
},
128+
wantFeatures: map[string]bool{
129+
testFeatureGate: true,
130+
flowcontrol.FeatureGate: true,
131+
},
126132
wantErr: false,
127133
deprecated: false,
128134
},
@@ -151,7 +157,7 @@ func TestLoadRawConfiguration(t *testing.T) {
151157
},
152158
},
153159
FeatureGates: configapi.FeatureGates{
154-
datalayer.ExperimentalDatalayerFeatureGate,
160+
testFeatureGate,
155161
flowcontrol.FeatureGate,
156162
},
157163
FlowControl: &configapi.FlowControlConfig{
@@ -174,7 +180,13 @@ func TestLoadRawConfiguration(t *testing.T) {
174180
Plugins: []configapi.PluginSpec{
175181
{Name: "test1", Type: testPluginType, Parameters: json.RawMessage(`{"threshold":10}`)},
176182
},
177-
FeatureGates: configapi.FeatureGates{},
183+
FeatureGates: configapi.FeatureGates{
184+
testFeatureGate + "=false",
185+
},
186+
},
187+
wantFeatures: map[string]bool{
188+
testFeatureGate: false,
189+
flowcontrol.FeatureGate: false,
178190
},
179191
wantErr: false,
180192
deprecated: false,
@@ -240,6 +252,10 @@ func TestLoadRawConfiguration(t *testing.T) {
240252
},
241253
},
242254
},
255+
wantFeatures: map[string]bool{
256+
testFeatureGate: true,
257+
flowcontrol.FeatureGate: false,
258+
},
243259
wantErr: false,
244260
deprecated: false,
245261
},
@@ -322,6 +338,12 @@ func TestLoadRawConfiguration(t *testing.T) {
322338
wantErr: true,
323339
deprecated: false,
324340
},
341+
{
342+
name: "Error - Bad Feature Gate",
343+
configText: errorBadFeatureGateText,
344+
wantErr: true,
345+
deprecated: false,
346+
},
325347
}
326348

327349
for _, tc := range tests {
@@ -330,7 +352,7 @@ func TestLoadRawConfiguration(t *testing.T) {
330352
writer := &strings.Builder{}
331353
logger := logging.NewTestLoggerWithWriter(writer)
332354

333-
got, _, err := LoadRawConfig([]byte(tc.configText), logger)
355+
got, featureGates, err := LoadRawConfig([]byte(tc.configText), logger)
334356

335357
if tc.wantErr {
336358
require.Error(t, err, "Expected LoadRawConfig to fail")
@@ -340,6 +362,11 @@ func TestLoadRawConfiguration(t *testing.T) {
340362
diff := cmp.Diff(tc.want, got)
341363
require.Empty(t, diff, "Config mismatch (-want +got):\n%s", diff)
342364

365+
if tc.wantFeatures != nil {
366+
diff = cmp.Diff(tc.wantFeatures, featureGates)
367+
require.Empty(t, diff, "Config feature gates mismatch (-want +got):\n%s", diff)
368+
}
369+
343370
if strings.Contains(writer.String(), "deprecated") {
344371
require.True(t, tc.deprecated, "Deprecated configuration wasn't marked as deprecated")
345372
} else {
@@ -355,8 +382,8 @@ func TestInstantiateAndConfigure(t *testing.T) {
355382
// Not parallel because it modifies global plugin registry.
356383
registerTestPlugins(t)
357384

358-
RegisterFeatureGate(datalayer.ExperimentalDatalayerFeatureGate)
359-
RegisterFeatureGate(flowcontrol.FeatureGate)
385+
RegisterFeatureGate(testFeatureGate, true)
386+
RegisterFeatureGate(flowcontrol.FeatureGate, false)
360387

361388
tests := []struct {
362389
name string

pkg/epp/config/loader/testdata_test.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ schedulingProfiles:
4444
weight: 50
4545
- pluginRef: testPicker
4646
featureGates:
47-
- dataLayer
47+
- test-feature-gate
4848
- flowControl
4949
flowControl:
5050
saturationDetector:
@@ -76,7 +76,7 @@ schedulingProfiles:
7676
weight: 50
7777
- pluginRef: testPicker
7878
featureGates:
79-
- dataLayer
79+
- test-feature-gate
8080
- flowControl
8181
flowControl:
8282
saturationDetector:
@@ -93,6 +93,8 @@ plugins:
9393
type: test-plugin
9494
parameters:
9595
threshold: 10
96+
featureGates:
97+
- test-feature-gate=false
9698
`
9799

98100
// successSchedulerConfigText represents a complex scheduler setup.
@@ -124,7 +126,7 @@ dataLayer:
124126
extractors:
125127
- pluginRef: testExtractor
126128
featureGates:
127-
- dataLayer
129+
- test-feature-gate
128130
- flowControl
129131
`
130132

@@ -360,6 +362,19 @@ featureGates:
360362
- unknown-gate
361363
`
362364

365+
// errorBadFeatureGateText includes a feature gate with an invalid value
366+
const errorBadFeatureGateText = `
367+
apiVersion: llm-d.ai/v1alpha1
368+
kind: EndpointPickerConfig
369+
plugins:
370+
- name: test1
371+
type: test-plugin
372+
parameters:
373+
threshold: 10
374+
featureGates:
375+
- flowControl=qwerty
376+
`
377+
363378
// --- Invalid Configurations (Logical/Architectural) ---
364379

365380
// errorNoProfileNameText is missing the required profile name.
@@ -585,7 +600,7 @@ dataLayer:
585600
extractors:
586601
- pluginRef: testExtractor
587602
featureGates:
588-
- dataLayer
603+
- test-feature-gate
589604
`
590605

591606
// errorBadSourceReferenceText has a bad DataSource plugin reference
@@ -605,7 +620,7 @@ dataLayer:
605620
sources:
606621
- pluginRef: test-one
607622
featureGates:
608-
- dataLayer
623+
- test-feature-gate
609624
- flowControl
610625
`
611626

@@ -629,7 +644,7 @@ dataLayer:
629644
extractors:
630645
- test-one
631646
featureGates:
632-
- dataLayer
647+
- test-feature-gate
633648
- flowControl
634649
`
635650

pkg/epp/config/loader/validation.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package loader
1919
import (
2020
"errors"
2121
"fmt"
22+
"strconv"
23+
"strings"
2224

2325
"k8s.io/apimachinery/pkg/util/sets"
2426

@@ -120,8 +122,15 @@ func validateFeatureGates(gates configapi.FeatureGates) error {
120122
registeredFeatureGatesMu.RLock()
121123
defer registeredFeatureGatesMu.RUnlock()
122124
for _, gate := range gates {
123-
if !registeredFeatureGates.Has(gate) {
124-
return fmt.Errorf("feature gate '%s' is unknown or unregistered", gate)
125+
parts := strings.Split(gate, "=")
126+
if _, ok := registeredFeatureGates[parts[0]]; !ok {
127+
return fmt.Errorf("feature gate '%s' is unknown or unregistered", parts[0])
128+
}
129+
if len(parts) > 1 {
130+
_, err := strconv.ParseBool(strings.ToLower(strings.TrimSpace(parts[1])))
131+
if err != nil {
132+
return fmt.Errorf("%s is not a valid value for the feature gate %s (error: %w)", parts[1], parts[0], err)
133+
}
125134
}
126135
}
127136

pkg/epp/datalayer/factory.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@ import (
2222
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
2323
)
2424

25-
const (
26-
// ExperimentalDatalayerFeatureGate is deprecated. The data layer is now enabled by default.
27-
// This gate is a no-op and will be removed in a future version.
28-
ExperimentalDatalayerFeatureGate = "dataLayer"
29-
)
30-
3125
// PoolInfo represents the DataStore information needed for endpoints.
3226
type PoolInfo interface {
3327
PoolGet() (*EndpointPool, error)

test/testdata/configloader_1_test.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ dataLayer:
2727
extractors:
2828
- pluginRef: test-extractor
2929
featureGates:
30-
- dataLayer
30+
- test-feature-gate
3131
flowControl:
3232
saturationDetector:
3333
metricsStalenessThreshold: 150ms

0 commit comments

Comments
 (0)