diff --git a/pkg/ingress/kube/gateway/istio/conversion_test.go b/pkg/ingress/kube/gateway/istio/conversion_test.go index e77e9d446..dcec88a94 100644 --- a/pkg/ingress/kube/gateway/istio/conversion_test.go +++ b/pkg/ingress/kube/gateway/istio/conversion_test.go @@ -130,9 +130,9 @@ var services = []*model.Service{ Attributes: model.ServiceAttributes{ Namespace: "default", Labels: map[string]string{ - "higress.io/inferencepool-extension-service": "ext-proc-svc", - "higress.io/inferencepool-extension-port": "9002", - "higress.io/inferencepool-extension-failure-mode": "FailClose", + InferencePoolExtensionRefSvc: "ext-proc-svc", + InferencePoolExtensionRefPort: "9002", + InferencePoolExtensionRefFailureMode: "FailClose", }, }, Ports: ports, @@ -145,9 +145,9 @@ var services = []*model.Service{ Attributes: model.ServiceAttributes{ Namespace: "default", Labels: map[string]string{ - "higress.io/inferencepool-extension-service": "ext-proc-svc-2", - "higress.io/inferencepool-extension-port": "9002", - "higress.io/inferencepool-extension-failure-mode": "FailClose", + InferencePoolExtensionRefSvc: "ext-proc-svc-2", + InferencePoolExtensionRefPort: "9002", + InferencePoolExtensionRefFailureMode: "FailClose", }, }, Ports: ports, @@ -156,6 +156,36 @@ var services = []*model.Service{ return name }())), }, + { + Attributes: model.ServiceAttributes{ + Namespace: "default", + Labels: map[string]string{ + InferencePoolExtensionRefSvc: "model1-epp", + InferencePoolExtensionRefPort: "9002", + InferencePoolExtensionRefFailureMode: "FailClose", + }, + }, + Ports: ports, + Hostname: host.Name(fmt.Sprintf("%s.default.svc.domain.suffix", func() string { + name, _ := InferencePoolServiceName("infpool-model1") + return name + }())), + }, + { + Attributes: model.ServiceAttributes{ + Namespace: "default", + Labels: map[string]string{ + InferencePoolExtensionRefSvc: "model2-epp", + InferencePoolExtensionRefPort: "9002", + InferencePoolExtensionRefFailureMode: "FailClose", + }, + }, + Ports: ports, + Hostname: host.Name(fmt.Sprintf("%s.default.svc.domain.suffix", func() string { + name, _ := InferencePoolServiceName("infpool-model2") + return name + }())), + }, { Attributes: model.ServiceAttributes{ diff --git a/pkg/ingress/kube/gateway/istio/route_collections.go b/pkg/ingress/kube/gateway/istio/route_collections.go index 957e877af..16e40e1a9 100644 --- a/pkg/ingress/kube/gateway/istio/route_collections.go +++ b/pkg/ingress/kube/gateway/istio/route_collections.go @@ -766,12 +766,63 @@ func mergeHTTPRoutes(baseVirtualServices krt.Collection[RouteWithKey], opts ...k sortRoutesByCreationTime(configs) base := configs[0].DeepCopy() baseVS := base.Spec.(*istio.VirtualService) - for _, config := range configs[1:] { + // Deep copy the InferencePool configs map to avoid race conditions + // The default DeepCopy() only does shallow copy of Extra field + if base.Extra != nil { + if ipConfigs, ok := base.Extra[constants.ConfigExtraPerRouteRuleInferencePoolConfigs].(map[string]kube.InferencePoolRouteRuleConfig); ok { + // Create a new map to avoid modifying the shared underlying map + newIPConfigs := make(map[string]kube.InferencePoolRouteRuleConfig, len(ipConfigs)) + for k, v := range ipConfigs { + newIPConfigs[k] = v + } + base.Extra[constants.ConfigExtraPerRouteRuleInferencePoolConfigs] = newIPConfigs + } + } + for i, config := range configs[1:] { thisVS := config.Spec.(*istio.VirtualService) baseVS.Http = append(baseVS.Http, thisVS.Http...) // append parents base.Annotations[constants.InternalParentNames] = fmt.Sprintf("%s,%s", base.Annotations[constants.InternalParentNames], config.Annotations[constants.InternalParentNames]) + // Merge Extra field (especially for InferencePool configs) + if base.Extra == nil && config.Extra != nil { + base.Extra = make(map[string]any) + } + if config.Extra != nil { + for k, v := range config.Extra { + // For non-InferencePool configs, keep the first value for stability + if k != constants.ConfigExtraPerRouteRuleInferencePoolConfigs { + if _, exists := base.Extra[k]; !exists { + base.Extra[k] = v + } + continue + } + // For InferencePool configs, merge the maps + baseMap, baseOk := base.Extra[k].(map[string]kube.InferencePoolRouteRuleConfig) + configMap, configOk := v.(map[string]kube.InferencePoolRouteRuleConfig) + if baseOk && configOk { + log.Debugf("Merging InferencePool configs: adding %d route configs from VirtualService %d to base (namespace=%s)", + len(configMap), i+1, config.Namespace) + // Route names are composed of the HTTPRoute/VirtualService namespaced name so they can't possibly conflict + for routeName, routeConfig := range configMap { + baseMap[routeName] = routeConfig + } + } else if configOk { + if _, exists := base.Extra[k]; !exists { + log.Debugf("Creating new InferencePool config map from VirtualService %d (namespace=%s)", i+1, config.Namespace) + base.Extra[k] = v + } + } else if !configOk { + log.Debugf("Skipping InferencePool config from VirtualService %d due to unexpected type (namespace=%s)", i+1, config.Namespace) + } + } + } + } + // Log final merged InferencePool configs + if base.Extra != nil { + if ipConfigs, ok := base.Extra[constants.ConfigExtraPerRouteRuleInferencePoolConfigs].(map[string]kube.InferencePoolRouteRuleConfig); ok { + log.Debugf("Final merged VirtualService for key %s has %d InferencePool route configs", object.Key, len(ipConfigs)) + } } sortHTTPRoutes(baseVS.Http) base.Name = strings.ReplaceAll(object.Key, "/", "~") diff --git a/pkg/ingress/kube/gateway/istio/route_collections_test.go b/pkg/ingress/kube/gateway/istio/route_collections_test.go new file mode 100644 index 000000000..506cc4ebd --- /dev/null +++ b/pkg/ingress/kube/gateway/istio/route_collections_test.go @@ -0,0 +1,140 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package istio + +import ( + "strings" + "testing" + "time" + + istio "istio.io/api/networking/v1alpha3" + "istio.io/istio/pkg/config" + "istio.io/istio/pkg/config/constants" + "istio.io/istio/pkg/config/gateway/kube" + "istio.io/istio/pkg/kube/krt" + "istio.io/istio/pkg/test" +) + +func TestMergeHTTPRoutesMergesInferencePoolExtra(t *testing.T) { + stop := test.NewStop(t) + routeKey := "default/gateway/example.com" + baseRouteName := "default/local-ai-chat" + otherRouteName := "default/local-ai-chat-360m" + baseInferenceConfigs := map[string]kube.InferencePoolRouteRuleConfig{ + baseRouteName: { + FQDN: "local-ai-chat-pool-epp.default.svc.cluster.local", + Port: "9002", + FailureModeAllow: true, + }, + } + otherInferenceConfigs := map[string]kube.InferencePoolRouteRuleConfig{ + otherRouteName: { + FQDN: "local-ai-chat-360m-pool-epp.default.svc.cluster.local", + Port: "9002", + }, + } + baseCfg := &config.Config{ + Meta: config.Meta{ + Name: "local-ai-chat", + Namespace: "default", + CreationTimestamp: time.Unix(1, 0), + Annotations: map[string]string{ + constants.InternalParentNames: "parent-a", + }, + }, + Spec: &istio.VirtualService{ + Hosts: []string{"example.com"}, + Gateways: []string{"default/gateway"}, + Http: []*istio.HTTPRoute{{ + Name: baseRouteName, + }}, + }, + Extra: map[string]any{ + constants.ConfigExtraPerRouteRuleInferencePoolConfigs: baseInferenceConfigs, + "non-inference-extra": "kept-from-base", + }, + } + otherCfg := &config.Config{ + Meta: config.Meta{ + Name: "local-ai-chat-360m", + Namespace: "default", + CreationTimestamp: time.Unix(2, 0), + Annotations: map[string]string{ + constants.InternalParentNames: "parent-b", + }, + }, + Spec: &istio.VirtualService{ + Hosts: []string{"example.com"}, + Gateways: []string{"default/gateway"}, + Http: []*istio.HTTPRoute{{ + Name: otherRouteName, + }}, + }, + Extra: map[string]any{ + constants.ConfigExtraPerRouteRuleInferencePoolConfigs: otherInferenceConfigs, + "non-inference-extra": "ignored-from-later-route", + "other-extra": "added-from-later-route", + }, + } + baseVirtualServices := krt.NewStaticCollection[RouteWithKey](nil, []RouteWithKey{ + { + Config: baseCfg, + Key: routeKey, + }, + { + Config: otherCfg, + Key: routeKey, + }, + }, krt.WithStop(stop), krt.WithName("base")) + + merged := mergeHTTPRoutes(baseVirtualServices, krt.WithStop(stop), krt.WithName("merged")) + merged.WaitUntilSynced(stop) + gotList := merged.List() + if len(gotList) != 1 { + t.Fatalf("expected one merged VirtualService, got %d", len(gotList)) + } + + got := gotList[0] + if got.Name != strings.ReplaceAll(routeKey, "/", "~") { + t.Fatalf("expected merged VirtualService name %q, got %q", strings.ReplaceAll(routeKey, "/", "~"), got.Name) + } + gotVS := got.Spec.(*istio.VirtualService) + if len(gotVS.Http) != 2 { + t.Fatalf("expected merged VirtualService to contain 2 HTTP routes, got %d", len(gotVS.Http)) + } + + gotInferenceConfigs, ok := got.Extra[constants.ConfigExtraPerRouteRuleInferencePoolConfigs].(map[string]kube.InferencePoolRouteRuleConfig) + if !ok { + t.Fatalf("expected merged InferencePool configs, got %T", got.Extra[constants.ConfigExtraPerRouteRuleInferencePoolConfigs]) + } + if len(gotInferenceConfigs) != 2 { + t.Fatalf("expected 2 merged InferencePool configs, got %d: %v", len(gotInferenceConfigs), gotInferenceConfigs) + } + if gotInferenceConfigs[baseRouteName].FQDN != baseInferenceConfigs[baseRouteName].FQDN { + t.Fatalf("expected base route InferencePool config to be preserved, got %v", gotInferenceConfigs[baseRouteName]) + } + if gotInferenceConfigs[otherRouteName].FQDN != otherInferenceConfigs[otherRouteName].FQDN { + t.Fatalf("expected later route InferencePool config to be merged, got %v", gotInferenceConfigs[otherRouteName]) + } + if got.Extra["non-inference-extra"] != "kept-from-base" { + t.Fatalf("expected non-InferencePool Extra to keep base value, got %v", got.Extra["non-inference-extra"]) + } + if got.Extra["other-extra"] != "added-from-later-route" { + t.Fatalf("expected missing non-InferencePool Extra to be added, got %v", got.Extra["other-extra"]) + } + if _, found := baseInferenceConfigs[otherRouteName]; found { + t.Fatalf("expected base InferencePool config map not to be mutated by merge") + } +} diff --git a/pkg/ingress/kube/gateway/istio/testdata/http.status.yaml.golden b/pkg/ingress/kube/gateway/istio/testdata/http.status.yaml.golden index c64cb156d..7a5d4b3cb 100644 --- a/pkg/ingress/kube/gateway/istio/testdata/http.status.yaml.golden +++ b/pkg/ingress/kube/gateway/istio/testdata/http.status.yaml.golden @@ -14,6 +14,22 @@ metadata: spec: null status: {} --- +apiVersion: inference.networking.k8s.io/v1 +kind: InferencePool +metadata: + name: infpool-model1 + namespace: default +spec: null +status: {} +--- +apiVersion: inference.networking.k8s.io/v1 +kind: InferencePool +metadata: + name: infpool-model2 + namespace: default +spec: null +status: {} +--- apiVersion: gateway.networking.k8s.io/v1beta1 kind: GatewayClass metadata: @@ -49,7 +65,7 @@ status: status: "True" type: Programmed listeners: - - attachedRoutes: 11 + - attachedRoutes: 13 conditions: - lastTransitionTime: fake message: No errors found @@ -272,6 +288,54 @@ status: --- apiVersion: gateway.networking.k8s.io/v1beta1 kind: HTTPRoute +metadata: + name: multi-route-infpool-1 + namespace: default +spec: null +status: + parents: + - conditions: + - lastTransitionTime: fake + message: Route was valid + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: fake + message: All references resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: higress.io/gateway-controller + parentRef: + name: gateway + namespace: higress-system +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: multi-route-infpool-2 + namespace: default +spec: null +status: + parents: + - conditions: + - lastTransitionTime: fake + message: Route was valid + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: fake + message: All references resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: higress.io/gateway-controller + parentRef: + name: gateway + namespace: higress-system +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute metadata: name: multiple-inferencepool-backend-refs namespace: default diff --git a/pkg/ingress/kube/gateway/istio/testdata/http.yaml b/pkg/ingress/kube/gateway/istio/testdata/http.yaml index 45dda70d6..f75936a2e 100644 --- a/pkg/ingress/kube/gateway/istio/testdata/http.yaml +++ b/pkg/ingress/kube/gateway/istio/testdata/http.yaml @@ -421,3 +421,80 @@ spec: name: vllm-llama3-8b-instruct-epp port: number: 9002 +--- +# Test case for multiple HTTPRoutes with InferencePools on same gateway. +# This verifies that InferencePool configs in Config.Extra are preserved when +# the routes are merged into a single VirtualService. +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: multi-route-infpool-1 + namespace: default +spec: + parentRefs: + - name: gateway + namespace: higress-system + hostnames: ["multi-infpool.domain.example"] + rules: + - matches: + - path: + type: PathPrefix + value: /model1 + backendRefs: + - name: infpool-model1 + group: inference.networking.k8s.io + kind: InferencePool + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: multi-route-infpool-2 + namespace: default +spec: + parentRefs: + - name: gateway + namespace: higress-system + hostnames: ["multi-infpool.domain.example"] + rules: + - matches: + - path: + type: PathPrefix + value: /model2 + backendRefs: + - name: infpool-model2 + group: inference.networking.k8s.io + kind: InferencePool + port: 80 +--- +apiVersion: inference.networking.k8s.io/v1 +kind: InferencePool +metadata: + name: infpool-model1 + namespace: default +spec: + targetPorts: + - number: 8000 + selector: + matchLabels: + app: model1-server + endpointPickerRef: + name: model1-epp + port: + number: 9002 +--- +apiVersion: inference.networking.k8s.io/v1 +kind: InferencePool +metadata: + name: infpool-model2 + namespace: default +spec: + targetPorts: + - number: 8000 + selector: + matchLabels: + app: model2-server + endpointPickerRef: + name: model2-epp + port: + number: 9002 diff --git a/pkg/ingress/kube/gateway/istio/testdata/http.yaml.golden b/pkg/ingress/kube/gateway/istio/testdata/http.yaml.golden index 9b2f30641..5f4a82332 100644 --- a/pkg/ingress/kube/gateway/istio/testdata/http.yaml.golden +++ b/pkg/ingress/kube/gateway/istio/testdata/http.yaml.golden @@ -242,6 +242,35 @@ spec: --- apiVersion: networking.istio.io/v1alpha3 kind: VirtualService +metadata: + annotations: + internal.istio.io/parents: HTTPRoute/multi-route-infpool-1.default,HTTPRoute/multi-route-infpool-2.default + internal.istio.io/route-semantics: gateway + name: higress-system~gateway-istio-autogenerated-k8s-gateway-default~multi-infpool.domain.example + namespace: default +spec: + gateways: + - higress-system/gateway-istio-autogenerated-k8s-gateway-default + hosts: + - multi-infpool.domain.example + http: + - match: + - uri: + prefix: /model1 + name: default/multi-route-infpool-1 + route: + - destination: + host: infpool-model1-ip-aaaaf2d6.default.svc.domain.suffix + - match: + - uri: + prefix: /model2 + name: default/multi-route-infpool-2 + route: + - destination: + host: infpool-model2-ip-f857bff9.default.svc.domain.suffix +--- +apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService metadata: annotations: internal.istio.io/parents: HTTPRoute/redirect-prefix-replace.default