Skip to content

Commit bfb1138

Browse files
committed
fix: ext_proc-driven multi-provider routing, weight=0, user-agent metering (opendatahub-io#399)
- model-provider-resolver: set x-ipp-selected-provider header using provider CR name (providerName field) after selectByWeight - selectByWeight: skip refs with weight <= 0 (disabled providers) - CRD: allow weight=0 (Minimum changed from 1 to 0) - HTTPRoute reconciler: per-provider route rules with header match on x-ipp-selected-provider, fallback to refs[0] - external-metering: capture User-Agent from request headers for client attribution in usage and error events
1 parent 68b2a2f commit bfb1138

9 files changed

Lines changed: 200 additions & 132 deletions

File tree

api/inference/v1alpha1/externalmodel_types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,10 @@ type ExternalProviderRef struct {
100100

101101
// Weight determines the relative traffic proportion for this provider binding.
102102
// Higher weight means more traffic. Used for weighted random selection across
103-
// multiple provider refs. Defaults to 1 if not set.
103+
// multiple provider refs. A weight of 0 disables the ref (no traffic routed
104+
// to it). Defaults to 1 if not set.
104105
// +optional
105-
// +kubebuilder:validation:Minimum=1
106+
// +kubebuilder:validation:Minimum=0
106107
// +kubebuilder:validation:Maximum=100
107108
// +kubebuilder:default=1
108109
Weight *int `json:"weight,omitempty"`

config/crd/bases/inference.opendatahub.io_externalmodels.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,10 @@ spec:
139139
description: |-
140140
Weight determines the relative traffic proportion for this provider binding.
141141
Higher weight means more traffic. Used for weighted random selection across
142-
multiple provider refs. Defaults to 1 if not set.
142+
multiple provider refs. A weight of 0 disables the ref (no traffic routed
143+
to it). Defaults to 1 if not set.
143144
maximum: 100
144-
minimum: 1
145+
minimum: 0
145146
type: integer
146147
required:
147148
- apiFormat

pkg/controller/externalmodel/reconciler.go

Lines changed: 126 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package externalmodel
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223

2324
"github.com/go-logr/logr"
2425
"k8s.io/apimachinery/pkg/api/equality"
@@ -111,33 +112,55 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
111112
return ctrl.Result{}, nil
112113
}
113114

115+
// resolvedRef holds the ExternalProvider info resolved for one ExternalProviderRef.
116+
type resolvedRef struct {
117+
providerName string
118+
providerEndpoint string
119+
targetModel string
120+
}
121+
114122
func (r *Reconciler) reconcileHTTPRoute(ctx context.Context, logger logr.Logger, model *inferencev1alpha1.ExternalModel) error {
115123
if len(model.Spec.ExternalProviderRefs) == 0 {
116124
return fmt.Errorf("ExternalModel %q has no externalProviderRefs", model.Name)
117125
}
118-
ref := model.Spec.ExternalProviderRefs[0]
119126

120-
provider := &inferencev1alpha1.ExternalProvider{}
121-
providerKey := types.NamespacedName{Name: ref.Ref.Name, Namespace: model.Namespace}
122-
if err := r.Get(ctx, providerKey, provider); err != nil {
123-
if apierrors.IsNotFound(err) {
124-
return fmt.Errorf("ExternalProvider %q not found in namespace %q", ref.Ref.Name, model.Namespace)
127+
var resolved []resolvedRef
128+
var skipReasons []string
129+
for _, ref := range model.Spec.ExternalProviderRefs {
130+
provider := &inferencev1alpha1.ExternalProvider{}
131+
providerKey := types.NamespacedName{Name: ref.Ref.Name, Namespace: model.Namespace}
132+
if err := r.Get(ctx, providerKey, provider); err != nil {
133+
if apierrors.IsNotFound(err) {
134+
logger.Error(err, "ExternalProvider not found, skipping", "provider", ref.Ref.Name)
135+
skipReasons = append(skipReasons, fmt.Sprintf("ExternalProvider %q not found in namespace %q", ref.Ref.Name, model.Namespace))
136+
continue
137+
}
138+
return fmt.Errorf("failed to get ExternalProvider %q: %w", ref.Ref.Name, err)
125139
}
126-
return fmt.Errorf("failed to get ExternalProvider %q: %w", ref.Ref.Name, err)
127-
}
128-
129-
if provider.Status.Phase != "Ready" {
130-
return fmt.Errorf("ExternalProvider %q is not ready (phase: %s)", ref.Ref.Name, provider.Status.Phase)
140+
if provider.Status.Phase != "Ready" {
141+
logger.Info("ExternalProvider not ready, skipping", "provider", ref.Ref.Name, "phase", provider.Status.Phase)
142+
skipReasons = append(skipReasons, fmt.Sprintf("ExternalProvider %q is not ready (phase: %s)", ref.Ref.Name, provider.Status.Phase))
143+
continue
144+
}
145+
if _, err := ctrlcommon.ResolvePath(ref.Path, mergeConfig(provider.Spec.Config, ref.Config), ref.TargetModel); err != nil {
146+
logger.Error(err, "path resolution failed, skipping", "provider", ref.Ref.Name, "path", ref.Path)
147+
skipReasons = append(skipReasons, fmt.Sprintf("path %q: %v", ref.Path, err))
148+
continue
149+
}
150+
resolved = append(resolved, resolvedRef{
151+
providerName: provider.Name,
152+
providerEndpoint: provider.Spec.Endpoint,
153+
targetModel: ref.TargetModel,
154+
})
131155
}
132156

133-
if _, err := ctrlcommon.ResolvePath(ref.Path, mergeConfig(provider.Spec.Config, ref.Config), ref.TargetModel); err != nil {
134-
return fmt.Errorf("path %q: %w", ref.Path, err)
157+
if len(resolved) == 0 {
158+
return fmt.Errorf("ExternalModel %q: no provider refs resolved successfully: %s", model.Name, strings.Join(skipReasons, "; "))
135159
}
136160

137161
labels := commonLabels(model.Name)
138162
hr := buildHTTPRoute(
139-
provider.Spec.Endpoint,
140-
provider.Name,
163+
resolved,
141164
model.Name,
142165
model.Namespace,
143166
ctrlcommon.DefaultTLSPort,
@@ -155,10 +178,14 @@ func (r *Reconciler) reconcileHTTPRoute(ctx context.Context, logger logr.Logger,
155178
return fmt.Errorf("failed to apply HTTPRoute: %w", err)
156179
}
157180

181+
providerNames := make([]string, len(resolved))
182+
for i, rr := range resolved {
183+
providerNames[i] = rr.providerName
184+
}
158185
logger.Info("ExternalModel HTTPRoute reconciled",
159186
"httpRoute", model.Name,
160-
"provider", provider.Name,
161-
"targetModel", ref.TargetModel,
187+
"providers", providerNames,
188+
"targetModel", resolved[0].targetModel,
162189
)
163190
return nil
164191
}
@@ -242,39 +269,102 @@ func commonLabels(modelName string) map[string]string {
242269
}
243270
}
244271

245-
func buildHTTPRoute(providerEndpoint, providerName, modelName, namespace string, port int32, gatewayName, gatewayNamespace, routeTimeout string, labels map[string]string) *gatewayapiv1.HTTPRoute {
272+
// selectedProviderHeader is the header the ext_proc sets to drive routing.
273+
const selectedProviderHeader = "x-ipp-selected-provider"
274+
275+
func buildHTTPRoute(refs []resolvedRef, modelName, namespace string, port int32, gatewayName, gatewayNamespace, routeTimeout string, labels map[string]string) *gatewayapiv1.HTTPRoute {
246276
gwNamespace := gatewayapiv1.Namespace(gatewayNamespace)
247277
pathType := gatewayapiv1.PathMatchPathPrefix
248278
pathPrefix := "/" + namespace + "/" + modelName
249279
headerType := gatewayapiv1.HeaderMatchExact
250280
gwPort := gatewayapiv1.PortNumber(port)
251281
timeout := gatewayapiv1.Duration(routeTimeout)
252282

253-
backendRefs := []gatewayapiv1.HTTPBackendRef{
254-
{
283+
var rules []gatewayapiv1.HTTPRouteRule
284+
285+
for _, ref := range refs {
286+
backendRefs := []gatewayapiv1.HTTPBackendRef{{
255287
BackendRef: gatewayapiv1.BackendRef{
256288
BackendObjectReference: gatewayapiv1.BackendObjectReference{
257-
Name: gatewayapiv1.ObjectName(providerName),
289+
Name: gatewayapiv1.ObjectName(ref.providerName),
258290
Port: &gwPort,
259291
},
260292
},
261-
},
262-
}
263-
264-
filters := []gatewayapiv1.HTTPRouteFilter{
265-
{
293+
}}
294+
filters := []gatewayapiv1.HTTPRouteFilter{{
266295
Type: gatewayapiv1.HTTPRouteFilterRequestHeaderModifier,
267296
RequestHeaderModifier: &gatewayapiv1.HTTPHeaderFilter{
268297
Set: []gatewayapiv1.HTTPHeader{
269-
{
270-
Name: "Host",
271-
Value: providerEndpoint,
272-
},
298+
{Name: "Host", Value: ref.providerEndpoint},
273299
},
274300
},
275-
},
301+
}}
302+
303+
rules = append(rules, gatewayapiv1.HTTPRouteRule{
304+
Matches: []gatewayapiv1.HTTPRouteMatch{{
305+
Path: &gatewayapiv1.HTTPPathMatch{Type: &pathType, Value: &pathPrefix},
306+
Headers: []gatewayapiv1.HTTPHeaderMatch{{
307+
Name: selectedProviderHeader, Type: &headerType, Value: ref.providerName,
308+
}},
309+
}},
310+
BackendRefs: backendRefs,
311+
Filters: filters,
312+
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
313+
})
314+
315+
rules = append(rules, gatewayapiv1.HTTPRouteRule{
316+
Matches: []gatewayapiv1.HTTPRouteMatch{{
317+
Path: &gatewayapiv1.HTTPPathMatch{Type: &pathType, Value: func() *string { s := "/"; return &s }()},
318+
Headers: []gatewayapiv1.HTTPHeaderMatch{
319+
{Name: "X-Gateway-Model-Name", Type: &headerType, Value: ref.targetModel},
320+
{Name: selectedProviderHeader, Type: &headerType, Value: ref.providerName},
321+
},
322+
}},
323+
BackendRefs: backendRefs,
324+
Filters: filters,
325+
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
326+
})
276327
}
277328

329+
// Fallback rules (no x-ipp-selected-provider): route to refs[0]
330+
fallbackBackendRefs := []gatewayapiv1.HTTPBackendRef{{
331+
BackendRef: gatewayapiv1.BackendRef{
332+
BackendObjectReference: gatewayapiv1.BackendObjectReference{
333+
Name: gatewayapiv1.ObjectName(refs[0].providerName),
334+
Port: &gwPort,
335+
},
336+
},
337+
}}
338+
fallbackFilters := []gatewayapiv1.HTTPRouteFilter{{
339+
Type: gatewayapiv1.HTTPRouteFilterRequestHeaderModifier,
340+
RequestHeaderModifier: &gatewayapiv1.HTTPHeaderFilter{
341+
Set: []gatewayapiv1.HTTPHeader{
342+
{Name: "Host", Value: refs[0].providerEndpoint},
343+
},
344+
},
345+
}}
346+
347+
rules = append(rules,
348+
gatewayapiv1.HTTPRouteRule{
349+
Matches: []gatewayapiv1.HTTPRouteMatch{{
350+
Path: &gatewayapiv1.HTTPPathMatch{Type: &pathType, Value: &pathPrefix},
351+
}},
352+
BackendRefs: fallbackBackendRefs,
353+
Filters: fallbackFilters,
354+
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
355+
},
356+
gatewayapiv1.HTTPRouteRule{
357+
Matches: []gatewayapiv1.HTTPRouteMatch{{
358+
Headers: []gatewayapiv1.HTTPHeaderMatch{
359+
{Name: "X-Gateway-Model-Name", Type: &headerType, Value: refs[0].targetModel},
360+
},
361+
}},
362+
BackendRefs: fallbackBackendRefs,
363+
Filters: fallbackFilters,
364+
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
365+
},
366+
)
367+
278368
return &gatewayapiv1.HTTPRoute{
279369
ObjectMeta: metav1.ObjectMeta{
280370
Name: modelName,
@@ -283,45 +373,12 @@ func buildHTTPRoute(providerEndpoint, providerName, modelName, namespace string,
283373
},
284374
Spec: gatewayapiv1.HTTPRouteSpec{
285375
CommonRouteSpec: gatewayapiv1.CommonRouteSpec{
286-
ParentRefs: []gatewayapiv1.ParentReference{
287-
{
288-
Name: gatewayapiv1.ObjectName(gatewayName),
289-
Namespace: &gwNamespace,
290-
},
291-
},
292-
},
293-
Rules: []gatewayapiv1.HTTPRouteRule{
294-
// TODO: remove path prefix rule when unified entrypoint (RHAISTRAT-1540) is wired.
295-
{
296-
Matches: []gatewayapiv1.HTTPRouteMatch{
297-
{
298-
Path: &gatewayapiv1.HTTPPathMatch{
299-
Type: &pathType,
300-
Value: &pathPrefix,
301-
},
302-
},
303-
},
304-
BackendRefs: backendRefs,
305-
Filters: filters,
306-
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
307-
},
308-
{
309-
Matches: []gatewayapiv1.HTTPRouteMatch{
310-
{
311-
Headers: []gatewayapiv1.HTTPHeaderMatch{
312-
{
313-
Name: "X-Gateway-Model-Name",
314-
Type: &headerType,
315-
Value: modelName,
316-
},
317-
},
318-
},
319-
},
320-
BackendRefs: backendRefs,
321-
Filters: filters,
322-
Timeouts: &gatewayapiv1.HTTPRouteTimeouts{Request: &timeout},
323-
},
376+
ParentRefs: []gatewayapiv1.ParentReference{{
377+
Name: gatewayapiv1.ObjectName(gatewayName),
378+
Namespace: &gwNamespace,
379+
}},
324380
},
381+
Rules: rules,
325382
},
326383
}
327384
}

pkg/controller/externalmodel/reconciler_test.go

Lines changed: 36 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -360,72 +360,53 @@ func TestCommonLabels(t *testing.T) {
360360
assert.Len(t, labels, 2)
361361
}
362362

363-
func TestBuildHTTPRoute(t *testing.T) {
364-
hr := buildHTTPRoute(
365-
"api.openai.com", "my-openai",
366-
"gpt4",
367-
"models", 443,
368-
"default-gateway", "openshift-ingress", "300s",
369-
commonLabels("gpt4"),
370-
)
363+
func TestBuildHTTPRoute_SingleProvider(t *testing.T) {
364+
refs := []resolvedRef{{
365+
providerName: "my-openai", providerEndpoint: "api.openai.com", targetModel: "gpt-4o",
366+
}}
367+
hr := buildHTTPRoute(refs, "gpt4", "models", 443,
368+
"default-gateway", "openshift-ingress", "300s", commonLabels("gpt4"))
371369

372370
assert.Equal(t, "gpt4", hr.Name)
373371
assert.Equal(t, "models", hr.Namespace)
374-
assert.Equal(t, managedByValue, hr.Labels[ctrlcommon.LabelManagedBy])
375-
376-
// Parent gateway ref
377-
require.Len(t, hr.Spec.ParentRefs, 1)
378-
assert.Equal(t, "default-gateway", string(hr.Spec.ParentRefs[0].Name))
379-
assert.Equal(t, "openshift-ingress", string(*hr.Spec.ParentRefs[0].Namespace))
380-
381-
// Must have 2 rules: path-based and header-based
382-
require.Len(t, hr.Spec.Rules, 2)
383372

384-
// Rule 1: path-based match with namespace prefix
385-
rule1 := hr.Spec.Rules[0]
386-
assert.Equal(t, "/models/gpt4", *rule1.Matches[0].Path.Value)
373+
// 4 rules: 2 per-provider + 2 fallback
374+
require.Len(t, hr.Spec.Rules, 4)
387375

388-
// Rule 2: header-based match uses modelName (what /v1/models returns)
389-
rule2 := hr.Spec.Rules[1]
390-
assert.Equal(t, "X-Gateway-Model-Name", string(rule2.Matches[0].Headers[0].Name))
391-
assert.Equal(t, "gpt4", rule2.Matches[0].Headers[0].Value)
376+
// Rule 0: path + x-ipp-selected-provider
377+
assert.Equal(t, "/models/gpt4", *hr.Spec.Rules[0].Matches[0].Path.Value)
378+
require.Len(t, hr.Spec.Rules[0].Matches[0].Headers, 1)
379+
assert.Equal(t, "x-ipp-selected-provider", string(hr.Spec.Rules[0].Matches[0].Headers[0].Name))
380+
assert.Equal(t, "my-openai", hr.Spec.Rules[0].Matches[0].Headers[0].Value)
381+
assert.Equal(t, "my-openai", string(hr.Spec.Rules[0].BackendRefs[0].Name))
392382

393-
// Backend ref points to the PROVIDER's Service, not the model
394-
for i, rule := range hr.Spec.Rules {
395-
require.Len(t, rule.BackendRefs, 1, "rule %d", i)
396-
assert.Equal(t, "my-openai", string(rule.BackendRefs[0].Name),
397-
"rule %d: backend should be the provider's Service", i)
398-
}
383+
// Rule 2: fallback path (no provider header)
384+
assert.Empty(t, hr.Spec.Rules[2].Matches[0].Headers)
399385

400-
// Host header filter for TLS SNI uses provider endpoint
401-
for i, rule := range hr.Spec.Rules {
402-
require.Len(t, rule.Filters, 1, "rule %d", i)
403-
assert.Equal(t, gatewayapiv1.HTTPRouteFilterRequestHeaderModifier, rule.Filters[0].Type)
404-
assert.Equal(t, "Host", string(rule.Filters[0].RequestHeaderModifier.Set[0].Name))
405-
assert.Equal(t, "api.openai.com", rule.Filters[0].RequestHeaderModifier.Set[0].Value)
406-
}
386+
// Host filter
387+
assert.Equal(t, "api.openai.com", hr.Spec.Rules[0].Filters[0].RequestHeaderModifier.Set[0].Value)
407388
}
408389

409-
func TestBuildHTTPRoute_TargetModelDiffersFromName(t *testing.T) {
410-
hr := buildHTTPRoute(
411-
"bedrock.us-east-1.amazonaws.com", "my-bedrock",
412-
"claude",
413-
"models", 443,
414-
"my-gateway", "gateway-ns", "300s",
415-
commonLabels("claude"),
416-
)
390+
func TestBuildHTTPRoute_MultiProvider(t *testing.T) {
391+
refs := []resolvedRef{
392+
{providerName: "anthropic", providerEndpoint: "api.anthropic.com", targetModel: "claude-opus-4-8"},
393+
{providerName: "sim-vertex", providerEndpoint: "sim.example.com", targetModel: "claude-opus-4-8"},
394+
}
395+
hr := buildHTTPRoute(refs, "claude", "models", 443,
396+
"my-gateway", "gateway-ns", "300s", commonLabels("claude"))
417397

418-
// Name and path use ExternalModel name
419-
assert.Equal(t, "claude", hr.Name)
420-
assert.Equal(t, "/models/claude", *hr.Spec.Rules[0].Matches[0].Path.Value)
398+
// 6 rules: 2 per-provider x 2 + 2 fallback
399+
require.Len(t, hr.Spec.Rules, 6)
421400

422-
// Header match uses modelName (what /v1/models returns to clients)
423-
assert.Equal(t, "claude", hr.Spec.Rules[1].Matches[0].Headers[0].Value)
401+
// Anthropic rules (0,1)
402+
assert.Equal(t, "anthropic", hr.Spec.Rules[0].Matches[0].Headers[0].Value)
403+
assert.Equal(t, "anthropic", string(hr.Spec.Rules[0].BackendRefs[0].Name))
424404

425-
// Backend points to provider Service
426-
assert.Equal(t, "my-bedrock", string(hr.Spec.Rules[0].BackendRefs[0].Name))
405+
// Sim-vertex rules (2,3)
406+
assert.Equal(t, "sim-vertex", hr.Spec.Rules[2].Matches[0].Headers[0].Value)
407+
assert.Equal(t, "sim-vertex", string(hr.Spec.Rules[2].BackendRefs[0].Name))
427408

428-
// Host header uses provider endpoint
429-
assert.Equal(t, "bedrock.us-east-1.amazonaws.com",
430-
hr.Spec.Rules[0].Filters[0].RequestHeaderModifier.Set[0].Value)
409+
// Fallback rules (4,5) point to refs[0] (anthropic)
410+
assert.Empty(t, hr.Spec.Rules[4].Matches[0].Headers)
411+
assert.Equal(t, "anthropic", string(hr.Spec.Rules[4].BackendRefs[0].Name))
431412
}

0 commit comments

Comments
 (0)