Fix: Merge Extra field during VirtualService merge for multiple Infer… (#58393) (#650)

aslakknutsen · jazzsir · web-flow · commit c8758e8adaea · 2026-03-11T17:43:41.000Z
* Fix: Merge Extra field during VirtualService merge for multiple InferencePools

When multiple HTTPRoutes referencing different InferencePools are attached
to the same Gateway, only the first InferencePool's ext_proc configuration
was applied. This fix ensures all InferencePool configurations are preserved
during VirtualService merge.

Root cause: The mergeHTTPRoutes() function merged HTTP routes and annotations
but ignored the Extra field where InferencePool ext_proc configurations are
stored. As a result, only the first HTTPRoute's InferencePool configuration
was retained.

Changes:
1. route_collections.go: Add Extra field merge logic in mergeHTTPRoutes()
   - Merge InferencePool config maps instead of overwriting
   - Add detailed logging for debugging and verification
   - Log final merged result with InferencePool count

2. conversion.go: Improve shadow service lookup for InferencePools
   - Use KRT collection (ctx.Services) for direct K8s service lookup
   - Add fallback to K8s service labels when PushContext not yet updated
   - Add comprehensive logging for troubleshooting

Impact:
- Fixes multi-InferencePool deployments on same Gateway
- No performance impact (configuration-time only, not runtime)
- Enables scalable inference routing with multiple models per Gateway

Testing:
- Verified with 2 InferencePools on 1 Gateway
- Both routes now have ext_proc configuration in Envoy
- Confirmed via config_dump and istiod logs

Fixes #58392

* Revert conversion.go changes - focus only on VirtualService merge fix

Reverted conversion.go changes as they are unnecessary. The InferencePool
dependency registered(krt.FetchOne on InferencePools collection)
is sufficient to trigger HTTPRoute recomputation when shadow services are
created during InferencePool reconciliation.

Related to #58392

* Address code review feedback for InferencePool merge

- Refactor route_collections.go for better readability
  * Pull base.Extra nil check out one level
  * Use early return for non-InferencePool configs
  * Extract type assertions to variables
  * Change log.Infof → log.Debugf
  * Add comment on route name conflict prevention

- Fix test infrastructure in conversion_test.go
  * Add mock services for infpool-model1/model2
  * Enables proper multi-route InferencePool testing

- Update golden files with correct expected behavior
  * BackendNotFound → All references resolved
  * Empty destinations → Properly populated hosts

All reviewer feedback addressed. Tests passing.

* Add test case for multiple HTTPRoutes with InferencePools on same gateway

* Add release notes for PR #58393

* Fix race condition in VirtualService merge by deep copying InferencePool configs

The Config.DeepCopy() method only performs a shallow copy of the Extra field.
When merging multiple VirtualServices with InferencePool configs, this caused
race conditions as multiple goroutines could modify the same underlying map.

This fix adds an explicit deep copy of the InferencePool configs map before
merging to ensure thread safety when running with race detector enabled.

Fixes unit-tests-arm64_istio test failures in CI.

* fix: preserve first Extra field value and add type safety check

* chore: remove unnecessary comments

* fix: preserve first Extra field value, add type safety, and log unexpected types

Co-authored-by: jazzsir &lt;jazzsir@gmail.com&gt;
diff --git a/pilot/pkg/config/kube/gateway/conversion_test.go b/pilot/pkg/config/kube/gateway/conversion_test.go
@@ -158,6 +158,30 @@ var services = []*model.Service{
 		Ports:    inferencePoolPorts,
 		Hostname: host.Name(fmt.Sprintf("%s.default.svc.domain.suffix", firstValue(InferencePoolServiceName("infpool-gen2")))),
 	},
+	{
+		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", firstValue(InferencePoolServiceName("infpool-model1")))),
+	},
+	{
+		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", firstValue(InferencePoolServiceName("infpool-model2")))),
+	},
 
 	{
 		Attributes: model.ServiceAttributes{
diff --git a/pilot/pkg/config/kube/gateway/route_collections.go b/pilot/pkg/config/kube/gateway/route_collections.go
@@ -682,12 +682,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/pilot/pkg/config/kube/gateway/testdata/http.status.yaml.golden b/pilot/pkg/config/kube/gateway/testdata/http.status.yaml.golden
@@ -16,6 +16,24 @@ metadata:
 spec: null
 status: {}
 ---
+apiVersion: inference.networking.k8s.io/v1
+kind: InferencePool
+metadata:
+  creationTimestamp: null
+  name: infpool-model1
+  namespace: default
+spec: null
+status: {}
+---
+apiVersion: inference.networking.k8s.io/v1
+kind: InferencePool
+metadata:
+  creationTimestamp: null
+  name: infpool-model2
+  namespace: default
+spec: null
+status: {}
+---
 apiVersion: gateway.networking.k8s.io/v1beta1
 kind: GatewayClass
 metadata:
@@ -53,7 +71,7 @@ status:
     status: "True"
     type: Programmed
   listeners:
-  - attachedRoutes: 11
+  - attachedRoutes: 13
     conditions:
     - lastTransitionTime: fake
       message: No errors found
@@ -284,6 +302,56 @@ status:
 ---
 apiVersion: gateway.networking.k8s.io/v1beta1
 kind: HTTPRoute
+metadata:
+  creationTimestamp: null
+  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: istio.io/gateway-controller
+    parentRef:
+      name: gateway
+      namespace: istio-system
+---
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: HTTPRoute
+metadata:
+  creationTimestamp: null
+  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: istio.io/gateway-controller
+    parentRef:
+      name: gateway
+      namespace: istio-system
+---
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: HTTPRoute
 metadata:
   creationTimestamp: null
   name: multiple-inferencepool-backend-refs
diff --git a/pilot/pkg/config/kube/gateway/testdata/http.yaml b/pilot/pkg/config/kube/gateway/testdata/http.yaml
@@ -423,3 +423,79 @@ spec:
     name: vllm-llama3-8b-instruct-epp
     port:
       number: 9002
+---
+# Test case for multiple HTTPRoutes with InferencePools on same gateway
+# This tests that Extra field (InferencePool configs) are properly merged
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: HTTPRoute
+metadata:
+  name: multi-route-infpool-1
+  namespace: default
+spec:
+  parentRefs:
+    - name: gateway
+      namespace: istio-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: istio-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/pilot/pkg/config/kube/gateway/testdata/http.yaml.golden b/pilot/pkg/config/kube/gateway/testdata/http.yaml.golden
@@ -252,6 +252,40 @@ spec:
 ---
 apiVersion: networking.istio.io/v1
 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
+  creationTimestamp: null
+  name: istio-system~gateway-istio-autogenerated-k8s-gateway-default~multi-infpool.domain.example
+  namespace: default
+spec:
+  gateways:
+  - istio-system/gateway-istio-autogenerated-k8s-gateway-default
+  hosts:
+  - multi-infpool.domain.example
+  http:
+  - match:
+    - uri:
+        prefix: /model1
+    name: default.multi-route-infpool-1.0
+    route:
+    - destination:
+        host: infpool-model1-ip-aaaaf2d6.default.svc.domain.suffix
+        port:
+          number: 80
+  - match:
+    - uri:
+        prefix: /model2
+    name: default.multi-route-infpool-2.0
+    route:
+    - destination:
+        host: infpool-model2-ip-f857bff9.default.svc.domain.suffix
+        port:
+          number: 80
+---
+apiVersion: networking.istio.io/v1
+kind: VirtualService
 metadata:
   annotations:
     internal.istio.io/parents: HTTPRoute/redirect-prefix-replace.default
diff --git a/releasenotes/notes/58393.yaml b/releasenotes/notes/58393.yaml
@@ -0,0 +1,9 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: traffic-management
+issue:
+- 58392
+
+releaseNotes:
+- |
+  **Fixed** an issue where InferencePool configurations were lost during VirtualService merging when multiple HTTPRoutes referencing different InferencePools were attached to the same Gateway.
