From 5768a81d8425790d622c32cb10cab28ca27f0a98 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Wed, 10 Dec 2025 15:28:45 -0800 Subject: [PATCH 01/12] Add support for multiple inferencepool backends --- internal/controller/nginx/config/generator.go | 5 + internal/controller/nginx/config/maps.go | 21 +- internal/controller/nginx/config/maps_test.go | 206 +++++++-- internal/controller/nginx/config/servers.go | 386 ++++++++++++---- .../controller/nginx/config/servers_test.go | 415 +++++++++++++++--- .../controller/nginx/config/split_clients.go | 32 +- .../nginx/config/split_clients_test.go | 2 +- internal/controller/state/graph/httproute.go | 52 ++- .../controller/state/graph/httproute_test.go | 165 +++++-- tests/Makefile | 8 +- 10 files changed, 1049 insertions(+), 243 deletions(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 6fa7602c04..28c84baed6 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -173,6 +173,11 @@ func (g GeneratorImpl) executeConfigTemplates( } } + // for fp, bytes := range fileBytes { + // fmt.Println("Generated NGINX configuration file: " + fp) + // fmt.Println(string(bytes)) + // } + var mgmtFiles []agent.File if g.plus { mgmtFiles = g.generateMgmtFiles(conf) diff --git a/internal/controller/nginx/config/maps.go b/internal/controller/nginx/config/maps.go index 2be256c776..beef173c60 100644 --- a/internal/controller/nginx/config/maps.go +++ b/internal/controller/nginx/config/maps.go @@ -185,7 +185,7 @@ func createAddHeadersMap(name string) shared.Map { // buildInferenceMaps creates maps for InferencePool Backends. func buildInferenceMaps(groups []dataplane.BackendGroup) []shared.Map { - inferenceMaps := make([]shared.Map, 0, len(groups)) + uniqueMaps := make(map[string]shared.Map) for _, group := range groups { for _, backend := range group.Backends { @@ -193,6 +193,13 @@ func buildInferenceMaps(groups []dataplane.BackendGroup) []shared.Map { continue } + backendVarName := strings.ReplaceAll(backend.UpstreamName, "-", "_") + mapKey := backendVarName // Use this as the key to detect duplicates + + // Skip if we've already processed this upstream + if _, exists := uniqueMaps[mapKey]; exists { + continue + } // Decide what the map must return when the picker didn’t set a value. var defaultResult string switch backend.EndpointPickerConfig.EndpointPickerRef.FailureMode { @@ -230,14 +237,18 @@ func buildInferenceMaps(groups []dataplane.BackendGroup) []shared.Map { Result: defaultResult, }) - backendVarName := strings.ReplaceAll(backend.UpstreamName, "-", "_") - - inferenceMaps = append(inferenceMaps, shared.Map{ + uniqueMaps[mapKey] = shared.Map{ Source: `$inference_workload_endpoint`, Variable: fmt.Sprintf("$inference_backend_%s", backendVarName), Parameters: params, - }) + } } } + + inferenceMaps := make([]shared.Map, 0, len(uniqueMaps)) + for _, inferenceMap := range uniqueMaps { + inferenceMaps = append(inferenceMaps, inferenceMap) + } + return inferenceMaps } diff --git a/internal/controller/nginx/config/maps_test.go b/internal/controller/nginx/config/maps_test.go index bac84b0067..a66af0de14 100644 --- a/internal/controller/nginx/config/maps_test.go +++ b/internal/controller/nginx/config/maps_test.go @@ -397,55 +397,183 @@ func TestCreateStreamMapsWithEmpty(t *testing.T) { func TestBuildInferenceMaps(t *testing.T) { t.Parallel() - g := NewWithT(t) - group := dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "upstream1", - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - NsName: "default", - EndpointPickerRef: &inference.EndpointPickerRef{ - FailureMode: inference.EndpointPickerFailClose, + tests := []struct { + expectedConfig map[string]struct { + failureMode inference.EndpointPickerFailureMode + defaultResult string + } + name string + backendGroups []dataplane.BackendGroup + expectedMaps int + }{ + { + name: "unique backends with different failure modes", + backendGroups: []dataplane.BackendGroup{ + { + Backends: []dataplane.Backend{ + { + UpstreamName: "upstream1", + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailClose, + }, + }, + }, + { + UpstreamName: "upstream2", + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailOpen, + }, + }, + }, + { + UpstreamName: "upstream3", + EndpointPickerConfig: nil, + }, }, }, }, - { - UpstreamName: "upstream2", - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - NsName: "default", - EndpointPickerRef: &inference.EndpointPickerRef{ - FailureMode: inference.EndpointPickerFailOpen, + expectedMaps: 2, + expectedConfig: map[string]struct { + failureMode inference.EndpointPickerFailureMode + defaultResult string + }{ + "upstream1": { + failureMode: inference.EndpointPickerFailClose, + defaultResult: "invalid-backend-ref", + }, + "upstream2": { + failureMode: inference.EndpointPickerFailOpen, + defaultResult: "upstream2", + }, + }, + }, + { + name: "duplicate upstreams should be deduplicated", + backendGroups: []dataplane.BackendGroup{ + { + Backends: []dataplane.Backend{ + { + UpstreamName: "upstream1", + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailClose, + }, + }, + }, + { + UpstreamName: "upstream1", // Duplicate + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailClose, + }, + }, + }, + }, + }, + { + Backends: []dataplane.Backend{ + { + UpstreamName: "upstream1", // Another duplicate + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailClose, + }, + }, + }, + { + UpstreamName: "upstream2", + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "default", + EndpointPickerRef: &inference.EndpointPickerRef{ + FailureMode: inference.EndpointPickerFailOpen, + }, + }, + }, }, }, }, - { - UpstreamName: "upstream3", - EndpointPickerConfig: nil, + expectedMaps: 2, // Only 2 unique upstreams + expectedConfig: map[string]struct { + failureMode inference.EndpointPickerFailureMode + defaultResult string + }{ + "upstream1": { + failureMode: inference.EndpointPickerFailClose, + defaultResult: "invalid-backend-ref", + }, + "upstream2": { + failureMode: inference.EndpointPickerFailOpen, + defaultResult: "upstream2", + }, }, }, + { + name: "no endpoint picker configs", + backendGroups: []dataplane.BackendGroup{ + { + Backends: []dataplane.Backend{ + { + UpstreamName: "upstream1", + EndpointPickerConfig: nil, + }, + { + UpstreamName: "upstream2", + EndpointPickerConfig: nil, + }, + }, + }, + }, + expectedMaps: 0, + }, } - maps := buildInferenceMaps([]dataplane.BackendGroup{group}) - g.Expect(maps).To(HaveLen(2)) - g.Expect(maps[0].Source).To(Equal("$inference_workload_endpoint")) - g.Expect(maps[0].Variable).To(Equal("$inference_backend_upstream1")) - g.Expect(maps[0].Parameters).To(HaveLen(3)) - g.Expect(maps[0].Parameters[0].Value).To(Equal("\"\"")) - g.Expect(maps[0].Parameters[0].Result).To(Equal("upstream1")) - g.Expect(maps[0].Parameters[1].Value).To(Equal("~.+")) - g.Expect(maps[0].Parameters[1].Result).To(Equal("$inference_workload_endpoint")) - g.Expect(maps[0].Parameters[2].Value).To(Equal("default")) - g.Expect(maps[0].Parameters[2].Result).To(Equal("invalid-backend-ref")) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + maps := buildInferenceMaps(tc.backendGroups) + g.Expect(maps).To(HaveLen(tc.expectedMaps)) - // Check the second map - g.Expect(maps[1].Source).To(Equal("$inference_workload_endpoint")) - g.Expect(maps[1].Variable).To(Equal("$inference_backend_upstream2")) - g.Expect(maps[1].Parameters).To(HaveLen(3)) - g.Expect(maps[1].Parameters[0].Value).To(Equal("\"\"")) - g.Expect(maps[1].Parameters[0].Result).To(Equal("upstream2")) - g.Expect(maps[1].Parameters[1].Value).To(Equal("~.+")) - g.Expect(maps[1].Parameters[1].Result).To(Equal("$inference_workload_endpoint")) - g.Expect(maps[1].Parameters[2].Value).To(Equal("default")) - g.Expect(maps[1].Parameters[2].Result).To(Equal("upstream2")) + // Verify each map has the correct structure + seenUpstreams := make(map[string]bool) + for _, m := range maps { + g.Expect(m.Source).To(Equal("$inference_workload_endpoint")) + g.Expect(m.Parameters).To(HaveLen(3)) + + // Extract upstream name from variable name + varName := strings.TrimPrefix(m.Variable, "$inference_backend_") + upstreamName := strings.ReplaceAll(varName, "_", "-") + + // Verify we haven't seen this upstream before (no duplicates) + g.Expect(seenUpstreams[upstreamName]).To(BeFalse(), "Duplicate upstream found: %s", upstreamName) + seenUpstreams[upstreamName] = true + + // Verify parameter structure + g.Expect(m.Parameters[0].Value).To(Equal("\"\"")) + g.Expect(m.Parameters[0].Result).To(Equal(upstreamName)) + g.Expect(m.Parameters[1].Value).To(Equal("~.+")) + g.Expect(m.Parameters[1].Result).To(Equal("$inference_workload_endpoint")) + g.Expect(m.Parameters[2].Value).To(Equal("default")) + + // Verify the default result matches expected failure mode + if expectedConfig, exists := tc.expectedConfig[upstreamName]; exists { + g.Expect(m.Parameters[2].Result).To(Equal(expectedConfig.defaultResult)) + } + } + + // Verify all expected upstreams are present + for expectedUpstream := range tc.expectedConfig { + g.Expect(seenUpstreams[expectedUpstream]).To(BeTrue(), "Expected upstream not found: %s", expectedUpstream) + } + }) + } } diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 717664c87c..cccec02d32 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -9,6 +9,8 @@ import ( "strings" gotemplate "text/template" + "k8s.io/apimachinery/pkg/types" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" @@ -258,6 +260,7 @@ func extractMirrorTargetsWithPercentages(pathRules []dataplane.PathRule) map[str return mirrorTargets } +//nolint:lll /* There are several different flows of location blocks, depending on the user configuration. The following describes them, with basic location examples. @@ -303,6 +306,38 @@ location /_ngf-internal-rule0-route0-inference { proxy_pass http://$inference-backend; } --------------- +Inference extension with multiple inference backends. + +location /coffee { + rewrite $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 last; + proxy_http_version 1.1; +} + +location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { + internal; + proxy_pass http://$inference-backend0; +} + +location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0 { + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference +} + +location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { + internal; + proxy_pass http://$inference-backend1; +} + +location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-1 { + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference +} + +split_clients $request_id $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 { + 70.00% /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0; + 30.00% /_ngf-internal-inference-conformance-app-backend_secondary-inference-pool-pool-svc_3000-backend-1; +} +--------------- Inference extension with HTTP matching conditions. External location calls httpmatch NJS module. The module determines the HTTP request conditions that exist @@ -318,16 +353,59 @@ it was the final location that had -inference in the name. location /coffee { js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location } -location /_ngf-internal-rule0-route0-inference { +location /_ngf-internal-test_coffee_80-backend-0 { internal; - set $epp_internal_path /_ngf-internal-rule0-route0; - js_content epp.getEndpoint; // redirects to /_ngf-internal-rule0-route0 + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } -location /_ngf-internal-rule0-route0 { +location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { internal; proxy_pass http://$inference-backend; } +--------------- +Inference extension with multiple backends with HTTP matching conditions. + +External location calls httpmatch NJS module. The module determines the HTTP request conditions that exist +and which backend to use, then redirects to an internal location which will rewrite to another internal inference +location based on a split clients variable. That internal inference location calls the inference NJS module +to get the AI endpoint to proxy to, then redirects to the internal location that proxies to the backend. + +location /coffee { + js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location +} + +location /_ngf-internal-split-clients-rule0-route0-inference { + internal; + + rewrite $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 last; + proxy_http_version 1.1; +} + +location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { + internal; + proxy_pass http://$inference-backend0; +} + +location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0 { + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference +} + +location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { + internal; + proxy_pass http://$inference-backend1; +} + +location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-1 { + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference +} + +split_clients $request_id $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 { + 70.00% /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0; + 30.00% /_ngf-internal-inference-conformance-app-backend_secondary-inference-pool-pool-svc_3000-backend-1; +} */ type httpMatchPairs map[string][]routeMatch @@ -409,6 +487,8 @@ func createLocations( locs = append(locs, createDefaultRootLocation()) } + // fmt.Println("These are all the locations", locs) + return locs, matchPairs, grpcServer } @@ -448,39 +528,90 @@ func createInternalLocationsForRule( var match routeMatch if !rule.HasInferenceBackends { intLocation, match = initializeInternalMatchLocation(pathRuleIdx, matchRuleIdx, r.Match, rule.GRPC) + intLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + intLocation = updateLocation( + r, + rule, + intLocation, + port, + keepAliveCheck, + mirrorPercentage, + ) + internalLocations = append(internalLocations, intLocation) } else { - intLocation, match = initializeInternalMatchLocationWithInference(pathRuleIdx, matchRuleIdx, r.Match) - intInfLocation := initializeInternalInferenceRedirectLocation(pathRuleIdx, matchRuleIdx) - for _, b := range r.BackendGroup.Backends { - if b.EndpointPickerConfig != nil && b.EndpointPickerConfig.EndpointPickerRef != nil { - eppRef := b.EndpointPickerConfig.EndpointPickerRef - var portNum int - if eppRef.Port != nil { - portNum = int(eppRef.Port.Number) - } - intInfLocation.EPPInternalPath = intLocation.Path - if b.EndpointPickerConfig.NsName != "" { - intInfLocation.EPPHost = string(eppRef.Name) + "." + b.EndpointPickerConfig.NsName - } else { - intInfLocation.EPPHost = string(eppRef.Name) - } - intInfLocation.EPPPort = portNum + // If there are multiple inference backends, we need 4 locations: + // 1. (external location) external match location which redirects to split clients location + // 2. (intSplitClientsLocation) internal split clients location which rewrites to internal inference + // location based on SC variable + // 3. (intEPPLocation) internal inference location which calls the EPP NJS module to get endpoint and + // redirects to final internal location + // 4. (intProxyPassLocation) final internal inference location which proxy_passes to backend + // + // The match needs to point to the internal split clients location (intSplitClientsLocation) + // + // If there is only one inference backend, we need 3 locations: + // 1. (external location) external match location which redirects to internal inference location + // 2. (intEPPLocation) internal inference location which calls the EPP NJS module to get endpoint and + // redirects to final internal location + // 3. (intProxyPassLocation) final internal inference location which proxy_passes to backend + // + // The match needs to point to the internal inference location which calls the EPP NJS module (intEPPLocation) + + if len(r.BackendGroup.Backends) > 1 { + intSplitClientsLocation := initializeInternalInferenceSplitClientsLocation(pathRuleIdx, matchRuleIdx) + intSplitClientsLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + + splitClientsVariableName := createInferenceSplitClientsVariableName( + convertStringToSafeVariableName(r.BackendGroup.Name()), + ) + intSplitClientsLocation.Rewrites = append(intSplitClientsLocation.Rewrites, + fmt.Sprintf("^ $%s last", splitClientsVariableName)) + + internalLocations = append(internalLocations, intSplitClientsLocation) + + match = createRouteMatch(r.Match, intSplitClientsLocation.Path) + } + + for backendIdx, b := range r.BackendGroup.Backends { + intProxyPassLocation := initializeInternalInferenceProxyPassLocation(pathRuleIdx, matchRuleIdx, backendIdx) + intProxyPassLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + + // Since we are creating a separate intProxyPassLocation per backend, + // we need to update the rule to only have that backend for the location. + // This ensures the correct name gets generated to correlate with the split clients generation. + // If there is only one backend, this is effectively a no-op. + tempRule := r + tempRule.BackendGroup.Backends = []dataplane.Backend{b} + intProxyPassLocation = updateLocation( + tempRule, + rule, + intProxyPassLocation, + port, + keepAliveCheck, + mirrorPercentage, + ) + internalLocations = append(internalLocations, intProxyPassLocation) + + intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx) + intEPPLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + + eppHost, portNum := extractEPPConfig(b) + intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum) + internalLocations = append(internalLocations, intEPPLocation) + + if len(r.BackendGroup.Backends) == 1 { + match = createRouteMatch(r.Match, intEPPLocation.Path) } } - internalLocations = append(internalLocations, intInfLocation) } - intLocation.Includes = createIncludesFromPolicyGenerateResult( - generator.GenerateForInternalLocation(rule.Policies), - ) - intLocation = updateLocation( - r, - rule, - intLocation, - port, - keepAliveCheck, - mirrorPercentage, - ) - internalLocations = append(internalLocations, intLocation) matches = append(matches, match) } @@ -496,45 +627,116 @@ func createInferenceLocationsForRule( keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) []http.Location { - locs := make([]http.Location, 0, len(rule.MatchRules)+len(extLocations)) - for matchRuleIdx, r := range rule.MatchRules { - intLocation := initializeInternalInferenceLocation(pathRuleIdx, matchRuleIdx) - intLocation.Includes = createIncludesFromPolicyGenerateResult( - generator.GenerateForInternalLocation(rule.Policies), - ) - intLocation = updateLocation( - r, - rule, - intLocation, - port, - keepAliveCheck, - mirrorPercentage, - ) + capacity := len(extLocations) + + for _, r := range rule.MatchRules { for _, b := range r.BackendGroup.Backends { + capacity++ // intProxyPassLocation (always created) + + // intEPPLocation (created only for multiple backends with EPP config) + if len(r.BackendGroup.Backends) > 1 && + b.EndpointPickerConfig != nil && + b.EndpointPickerConfig.EndpointPickerRef != nil { + capacity++ // intEPPLocation + } + } + } + + locs := make([]http.Location, 0, capacity) + + for matchRuleIdx, r := range rule.MatchRules { + // If there are multiple inference backends, we need 3 locations: + // 1. (external location) external location which rewrites to the EPP internal location based on a + // split clients variable + // 2. (intEPPLocation) internal inference location which calls the EPP NJS module to get endpoint + // and redirects to final internal location + // 3. (intProxyPassLocation) final internal inference location which proxy_passes to backend + // + // If there is only one inference backend, we need 2 locations: + // 1. (external location) external location which calls the EPP NJS module to get endpoint and redirects + // to internal inference location + // 2. (intProxyPassLocation) final internal inference location which proxy_passes to backend + + if len(r.BackendGroup.Backends) > 1 { + splitClientsVariableName := createInferenceSplitClientsVariableName( + convertStringToSafeVariableName(r.BackendGroup.Name()), + ) + for i := range extLocations { + extLocations[i].Rewrites = append(extLocations[i].Rewrites, fmt.Sprintf("^ $%s last", splitClientsVariableName)) + extLocations[i].Type = http.ExternalLocationType + } + } + + for backendIdx, b := range r.BackendGroup.Backends { + intProxyPassLocation := initializeInternalInferenceProxyPassLocation(pathRuleIdx, matchRuleIdx, backendIdx) + intProxyPassLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + + // Since we are creating a separate intProxyPassLocation per backend, + // we need to update the rule to only have that backend for the location. + // This ensures the correct name gets generated to correlate with the split clients generation. + // If there is only one backend, this is effectively a no-op. + tempRule := r + tempRule.BackendGroup.Backends = []dataplane.Backend{b} + intProxyPassLocation = updateLocation( + tempRule, + rule, + intProxyPassLocation, + port, + keepAliveCheck, + mirrorPercentage, + ) + locs = append(locs, intProxyPassLocation) + if b.EndpointPickerConfig != nil && b.EndpointPickerConfig.EndpointPickerRef != nil { - for i := range extLocations { - eppRef := b.EndpointPickerConfig.EndpointPickerRef - var portNum int - if eppRef.Port != nil { - portNum = int(eppRef.Port.Number) + eppHost, portNum := extractEPPConfig(b) + + if len(r.BackendGroup.Backends) > 1 { + intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx) + intEPPLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum) + locs = append(locs, intEPPLocation) + } else { + for i := range extLocations { + extLocations[i] = setLocationEPPConfig(extLocations[i], intProxyPassLocation.Path, eppHost, portNum) } - extLocations[i].EPPInternalPath = intLocation.Path - if b.EndpointPickerConfig.NsName != "" { - extLocations[i].EPPHost = string(eppRef.Name) + "." + b.EndpointPickerConfig.NsName - } else { - extLocations[i].EPPHost = string(eppRef.Name) - } - extLocations[i].EPPPort = portNum } } } - locs = append(locs, intLocation) } locs = append(locs, extLocations...) return locs } +func setLocationEPPConfig(location http.Location, eppInternalPath, eppHost string, eppPort int) http.Location { + location.EPPInternalPath = eppInternalPath + location.EPPHost = eppHost + location.EPPPort = eppPort + return location +} + +func extractEPPConfig(backend dataplane.Backend) (string, int) { + var eppHost string + var eppPort int + + eppRef := backend.EndpointPickerConfig.EndpointPickerRef + if eppRef.Port != nil { + eppPort = int(eppRef.Port.Number) + } + + if backend.EndpointPickerConfig.NsName != "" { + eppHost = string(eppRef.Name) + "." + backend.EndpointPickerConfig.NsName + } else { + eppHost = string(eppRef.Name) + } + + return eppHost, eppPort +} + func needsInternalLocationsForMatches(rule dataplane.PathRule) bool { if len(rule.MatchRules) > 1 { return true @@ -645,43 +847,55 @@ func initializeInternalMatchLocation( return createMatchLocation(path, grpc), createRouteMatch(match, path) } -// initializeInternalInferenceRedirectLocation initializes the internal inference location that is redirected to by -// an external HTTP matching location. This location then redirects to the final proxy_pass location. -func initializeInternalInferenceRedirectLocation(pathruleIdx, matchRuleIdx int) http.Location { +// initializeInternalInferenceEPPLocation initializes the internal inference EPP location. This location calls the +// inference njs module to get the correct endpoint for the request and redirects to the final internal location +// that does the proxy_pass to the backend. +func initializeInternalInferenceEPPLocation( + b dataplane.Backend, + source types.NamespacedName, + ruleIdx int, +) http.Location { return http.Location{ - Path: inferencePath(pathruleIdx, matchRuleIdx), + // This path needs to be recreated in the split_clients directive generation to match correctly. + Path: fmt.Sprintf( + "%s-%s-%s-%s-rule%d", + http.InternalRoutePathPrefix, + b.UpstreamName, + source.Namespace, + source.Name, + ruleIdx, + ), Type: http.InferenceInternalLocationType, } } -// initializeInternalMatchLocationWithInference initializes the internal location that is redirected to by -// an internal inference location, which was redirected to by the external HTTP matching location. -// This location will proxy_pass to the backend. -// The routeMatch is created with the inference internal location path, so that the HTTP match in the external -// location can redirect to the proper inference location, which then redirects to this location. -func initializeInternalMatchLocationWithInference( - pathruleIdx, - matchRuleIdx int, - match dataplane.Match, -) (http.Location, routeMatch) { - path := fmt.Sprintf("%s-rule%d-route%d", http.InternalRoutePathPrefix, pathruleIdx, matchRuleIdx) - grpc := false - - return createMatchLocation(path, grpc), createRouteMatch(match, inferencePath(pathruleIdx, matchRuleIdx)) -} - -// initializeInternalInferenceLocation initializes the internal inference location that does the final +// initializeInternalInferenceProxyPassLocation initializes the internal inference location that does the final // proxy_pass to the inference backend. -// This is used when the external location redirects directly here, without any HTTP matching. -func initializeInternalInferenceLocation(pathruleIdx, matchRuleIdx int) http.Location { +func initializeInternalInferenceProxyPassLocation(pathruleIdx, matchRuleIdx, backendIdx int) http.Location { return http.Location{ - Path: inferencePath(pathruleIdx, matchRuleIdx), + Path: fmt.Sprintf( + "%s-proxy-pass-rule%d-route%d-backend%d-inference", + http.InternalRoutePathPrefix, + pathruleIdx, + matchRuleIdx, + backendIdx, + ), Type: http.InternalLocationType, } } -func inferencePath(pathruleIdx int, matchRuleIdx int) string { - return fmt.Sprintf("%s-rule%d-route%d-inference", http.InternalRoutePathPrefix, pathruleIdx, matchRuleIdx) +// initializeInternalInferenceSplitClientsLocation initializes the internal inference location that rewrites +// to a location determined by a split_clients variable. +func initializeInternalInferenceSplitClientsLocation(pathruleIdx, matchRuleIdx int) http.Location { + return http.Location{ + Path: fmt.Sprintf( + "%s-split-clients-rule%d-route%d-inference", + http.InternalRoutePathPrefix, + pathruleIdx, + matchRuleIdx, + ), + Type: http.InternalLocationType, + } } // updateLocation updates a location with any relevant configurations, like proxy_pass, filters, tls settings, etc. diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 2e35224362..3e4899e834 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -2449,27 +2449,111 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { hrNsName := types.NamespacedName{Namespace: "test", Name: "route1"} - fooGroup := dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: 0, - Backends: []dataplane.Backend{ - { - UpstreamName: "test_foo_80", - Valid: true, - Weight: 1, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "test-epp", - Port: &inference.Port{ - Number: 80, - }, - }, - NsName: hrNsName.Namespace, - }, + // Reusable backend definitions + singleInferenceBackend := dataplane.Backend{ + UpstreamName: "test_foo_80", + Valid: true, + Weight: 1, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "test-epp", + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, + }, + } + + singleMatchInferenceBackend := dataplane.Backend{ + UpstreamName: "test_single_match_pool_80", + Valid: true, + Weight: 1, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "single-match-pool", + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, + }, + } + + multiInferencePrimaryBackend := dataplane.Backend{ + UpstreamName: "test_primary_pool_80", + Valid: true, + Weight: 70, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "primary-pool", + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, + }, + } + + multiInferenceSecondaryBackend := dataplane.Backend{ + UpstreamName: "test_secondary_pool_80", + Valid: true, + Weight: 30, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "secondary-pool", + Port: &inference.Port{Number: 80}, }, + NsName: hrNsName.Namespace, }, } + multiMatchPrimaryBackend := dataplane.Backend{ + UpstreamName: "test_multi_match_primary_80", + Valid: true, + Weight: 80, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "multi-match-primary", + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, + }, + } + + multiMatchSecondaryBackend := dataplane.Backend{ + UpstreamName: "test_multi_match_secondary_80", + Valid: true, + Weight: 20, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: "multi-match-secondary", + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, + }, + } + + // Reusable backend groups + singleInferenceGroup := dataplane.BackendGroup{ + Source: hrNsName, + RuleIdx: 0, + Backends: []dataplane.Backend{singleInferenceBackend}, + } + + singleMatchInferenceGroup := dataplane.BackendGroup{ + Source: hrNsName, + RuleIdx: 1, + Backends: []dataplane.Backend{singleMatchInferenceBackend}, + } + + multipleInferenceGroup := dataplane.BackendGroup{ + Source: hrNsName, + RuleIdx: 2, + Backends: []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, + } + + multipleMatchInferenceGroup := dataplane.BackendGroup{ + Source: hrNsName, + RuleIdx: 3, + Backends: []dataplane.Backend{multiMatchPrimaryBackend, multiMatchSecondaryBackend}, + } + + // Reusable path rules pathRuleInferenceOnly := dataplane.PathRule{ Path: "/inference", PathType: dataplane.PathTypeExact, @@ -2477,7 +2561,7 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { MatchRules: []dataplane.MatchRule{ { Match: dataplane.Match{}, - BackendGroup: fooGroup, + BackendGroup: singleInferenceGroup, }, }, } @@ -2491,11 +2575,48 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { Match: dataplane.Match{ Method: helpers.GetPointer("POST"), }, - BackendGroup: fooGroup, + BackendGroup: singleMatchInferenceGroup, }, }, } + pathRuleMultipleInferenceBackends := dataplane.PathRule{ + Path: "/weighted-inference", + PathType: dataplane.PathTypeExact, + HasInferenceBackends: true, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: multipleInferenceGroup, + }, + }, + } + + pathRuleMultipleInferenceWithMatch := dataplane.PathRule{ + Path: "/weighted-inference-match", + PathType: dataplane.PathTypeExact, + HasInferenceBackends: true, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Method: helpers.GetPointer("GET"), + }, + BackendGroup: multipleMatchInferenceGroup, + }, + }, + } + + proxySetHeaders := []http.Header{ + {Name: "Host", Value: "$gw_api_compliant_host"}, + {Name: "X-Forwarded-For", Value: "$proxy_add_x_forwarded_for"}, + {Name: "X-Real-IP", Value: "$remote_addr"}, + {Name: "X-Forwarded-Proto", Value: "$scheme"}, + {Name: "X-Forwarded-Host", Value: "$host"}, + {Name: "X-Forwarded-Port", Value: "$server_port"}, + {Name: "Upgrade", Value: "$http_upgrade"}, + {Name: "Connection", Value: "$connection_upgrade"}, + } + tests := []struct { expMatches httpMatchPairs name string @@ -2507,24 +2628,15 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { pathRules: []dataplane.PathRule{pathRuleInferenceOnly}, expLocs: []http.Location{ { - Path: "/_ngf-internal-rule0-route0-inference", - Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_foo_80$request_uri", - ProxySetHeaders: []http.Header{ - {Name: "Host", Value: "$gw_api_compliant_host"}, - {Name: "X-Forwarded-For", Value: "$proxy_add_x_forwarded_for"}, - {Name: "X-Real-IP", Value: "$remote_addr"}, - {Name: "X-Forwarded-Proto", Value: "$scheme"}, - {Name: "X-Forwarded-Host", Value: "$host"}, - {Name: "X-Forwarded-Port", Value: "$server_port"}, - {Name: "Upgrade", Value: "$http_upgrade"}, - {Name: "Connection", Value: "$connection_upgrade"}, - }, + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_foo_80$request_uri", + ProxySetHeaders: proxySetHeaders, }, { Path: "= /inference", Type: http.InferenceExternalLocationType, - EPPInternalPath: "/_ngf-internal-rule0-route0-inference", + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", EPPHost: "test-epp.test", EPPPort: 80, }, @@ -2542,32 +2654,235 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { HTTPMatchKey: "1_0", }, { - Path: "/_ngf-internal-rule0-route0-inference", + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_single_match_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1", Type: http.InferenceInternalLocationType, - EPPInternalPath: "/_ngf-internal-rule0-route0", - EPPHost: "test-epp.test", + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "single-match-pool.test", + EPPPort: 80, + }, + createDefaultRootLocation(), + }, + expMatches: httpMatchPairs{ + "1_0": { + {Method: "POST", RedirectPath: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1"}, + }, + }, + }, + { + name: "multiple weighted inference backends, no match conditions", + pathRules: []dataplane.PathRule{pathRuleMultipleInferenceBackends}, + expLocs: []http.Location{ + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_primary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_primary_pool_80-test-route1-rule2", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "primary-pool.test", EPPPort: 80, }, { - Path: "/_ngf-internal-rule0-route0", - Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_foo_80$request_uri", - ProxySetHeaders: []http.Header{ - {Name: "Host", Value: "$gw_api_compliant_host"}, - {Name: "X-Forwarded-For", Value: "$proxy_add_x_forwarded_for"}, - {Name: "X-Real-IP", Value: "$remote_addr"}, - {Name: "X-Forwarded-Proto", Value: "$scheme"}, - {Name: "X-Forwarded-Host", Value: "$host"}, - {Name: "X-Forwarded-Port", Value: "$server_port"}, - {Name: "Upgrade", Value: "$http_upgrade"}, - {Name: "Connection", Value: "$connection_upgrade"}, - }, + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_secondary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_secondary_pool_80-test-route1-rule2", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + EPPHost: "secondary-pool.test", + EPPPort: 80, + }, + { + Path: "= /weighted-inference", + Type: http.ExternalLocationType, + Rewrites: []string{"^ $inference_backend_group_test__route1_rule2 last"}, + }, + createDefaultRootLocation(), + }, + expMatches: httpMatchPairs{}, + }, + { + name: "multiple weighted inference backends with match conditions", + pathRules: []dataplane.PathRule{pathRuleMultipleInferenceWithMatch}, + expLocs: []http.Location{ + { + Path: "= /weighted-inference-match", + Type: http.RedirectLocationType, + HTTPMatchKey: "1_0", + }, + { + Path: "/_ngf-internal-split-clients-rule0-route0-inference", + Type: http.InternalLocationType, + Rewrites: []string{"^ $inference_backend_group_test__route1_rule3 last"}, + }, + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_multi_match_primary_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_multi_match_primary_80-test-route1-rule3", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "multi-match-primary.test", + EPPPort: 80, + }, + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_multi_match_secondary_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_multi_match_secondary_80-test-route1-rule3", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + EPPHost: "multi-match-secondary.test", + EPPPort: 80, }, createDefaultRootLocation(), }, expMatches: httpMatchPairs{ "1_0": { - {Method: "POST", RedirectPath: "/_ngf-internal-rule0-route0-inference"}, + {Method: "GET", RedirectPath: "/_ngf-internal-split-clients-rule0-route0-inference"}, + }, + }, + }, + { + name: "mixed multiple path rules with different inference configurations", + pathRules: []dataplane.PathRule{ + pathRuleInferenceOnly, + pathRuleInferenceWithMatch, + pathRuleMultipleInferenceBackends, + pathRuleMultipleInferenceWithMatch, + }, + expLocs: []http.Location{ + // 1. Single inference pool locations (pathRuleInferenceOnly - rule index 0) + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_foo_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "= /inference", + Type: http.InferenceExternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "test-epp.test", + EPPPort: 80, + }, + // 2. Single inference pool with match (pathRuleInferenceWithMatch - rule index 1) + { + Path: "= /inference-match", + Type: http.RedirectLocationType, + HTTPMatchKey: "1_1", + }, + { + Path: "/_ngf-internal-proxy-pass-rule1-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_single_match_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule1-route0-backend0-inference", + EPPHost: "single-match-pool.test", + EPPPort: 80, + }, + // 3. Multiple inference pools, no match (pathRuleMultipleInferenceBackends - rule index 2) + { + Path: "/_ngf-internal-proxy-pass-rule2-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_primary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_primary_pool_80-test-route1-rule2", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule2-route0-backend0-inference", + EPPHost: "primary-pool.test", + EPPPort: 80, + }, + { + Path: "/_ngf-internal-proxy-pass-rule2-route0-backend1-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_secondary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_secondary_pool_80-test-route1-rule2", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule2-route0-backend1-inference", + EPPHost: "secondary-pool.test", + EPPPort: 80, + }, + { + Path: "= /weighted-inference", + Type: http.ExternalLocationType, + Rewrites: []string{"^ $inference_backend_group_test__route1_rule2 last"}, + }, + // 4. Multiple inference pools with match (pathRuleMultipleInferenceWithMatch - rule index 3) + { + Path: "= /weighted-inference-match", + Type: http.RedirectLocationType, + HTTPMatchKey: "1_3", + }, + { + Path: "/_ngf-internal-split-clients-rule3-route0-inference", + Type: http.InternalLocationType, + Rewrites: []string{"^ $inference_backend_group_test__route1_rule3 last"}, + }, + { + Path: "/_ngf-internal-proxy-pass-rule3-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_multi_match_primary_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_multi_match_primary_80-test-route1-rule3", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule3-route0-backend0-inference", + EPPHost: "multi-match-primary.test", + EPPPort: 80, + }, + { + Path: "/_ngf-internal-proxy-pass-rule3-route0-backend1-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_multi_match_secondary_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_multi_match_secondary_80-test-route1-rule3", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule3-route0-backend1-inference", + EPPHost: "multi-match-secondary.test", + EPPPort: 80, + }, + createDefaultRootLocation(), + }, + expMatches: httpMatchPairs{ + "1_1": { + {Method: "POST", RedirectPath: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1"}, + }, + "1_3": { + { + Method: "GET", + RedirectPath: "/_ngf-internal-split-clients-rule3-route0-inference", + }, }, }, }, diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index 2c9febf6bc..d0d9361582 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -6,6 +6,8 @@ import ( "strings" gotemplate "text/template" + "k8s.io/apimachinery/pkg/types" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" @@ -105,13 +107,20 @@ func createBackendGroupSplitClients(backendGroups []dataplane.BackendGroup) []ht splitClients := make([]http.SplitClient, 0, numSplits) for _, group := range backendGroups { + variableName := convertStringToSafeVariableName(group.Name()) + distributions := createBackendGroupSplitClientDistributions(group) if distributions == nil { continue } + if group.Backends[0].EndpointPickerConfig != nil { + // This is an inferencePool backend group, need to adjust the name. + variableName = createInferenceSplitClientsVariableName(variableName) + } + splitClients = append(splitClients, http.SplitClient{ - VariableName: convertStringToSafeVariableName(group.Name()), + VariableName: variableName, Distributions: distributions, }) } @@ -119,6 +128,10 @@ func createBackendGroupSplitClients(backendGroups []dataplane.BackendGroup) []ht return splitClients } +func createInferenceSplitClientsVariableName(groupName string) string { + return "inference_backend_" + groupName +} + func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) []http.SplitClientDistribution { if !backendGroupNeedsSplit(group) { return nil @@ -155,7 +168,7 @@ func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) [] distributions = append(distributions, http.SplitClientDistribution{ Percent: fmt.Sprintf("%.2f", percentage), - Value: getSplitClientValue(b), + Value: getSplitClientValue(b, group.Source, group.RuleIdx), }) } @@ -165,14 +178,25 @@ func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) [] distributions = append(distributions, http.SplitClientDistribution{ Percent: fmt.Sprintf("%.2f", availablePercentage), - Value: getSplitClientValue(lastBackend), + Value: getSplitClientValue(lastBackend, group.Source, group.RuleIdx), }) return distributions } -func getSplitClientValue(b dataplane.Backend) string { +func getSplitClientValue(b dataplane.Backend, source types.NamespacedName, ruleIdx int) string { if b.Valid { + if b.EndpointPickerConfig != nil { + return fmt.Sprintf( + "%s-%s-%s-%s-rule%d", + http.InternalRoutePathPrefix, + b.UpstreamName, + source.Namespace, + source.Name, + ruleIdx, + ) + } + return b.UpstreamName } return invalidBackendRef diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index 29f54986f2..d9d36bb40c 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -861,7 +861,7 @@ func TestGetSplitClientValue(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := getSplitClientValue(test.backend) + result := getSplitClientValue(test.backend, types.NamespacedName{Namespace: "test", Name: "hr"}, 0) g.Expect(result).To(Equal(test.expValue)) }) } diff --git a/internal/controller/state/graph/httproute.go b/internal/controller/state/graph/httproute.go index 30b28f1800..1a4749b984 100644 --- a/internal/controller/state/graph/httproute.go +++ b/internal/controller/state/graph/httproute.go @@ -207,6 +207,21 @@ func processHTTPRouteRule( backendRefs := make([]RouteBackendRef, 0, len(specRule.BackendRefs)) + if checkForMixedBackendTypes(specRule, routeNamespace, inferencePools) { + err := field.Forbidden( + rulePath.Child("backendRefs"), + "mixing InferencePool and non-InferencePool backends in a rule is not supported", + ) + errors.invalid = append(errors.invalid, err) + + return RouteRule{ + ValidMatches: validMatches, + Matches: specRule.Matches, + Filters: routeFilters, + RouteBackendRefs: backendRefs, + }, errors + } + // rule.BackendRefs are validated separately because of their special requirements for _, b := range specRule.BackendRefs { var interfaceFilters []any @@ -225,18 +240,6 @@ func processHTTPRouteRule( // headless Service backend (that we created), so nginx config can be built properly. // Only do this if the InferencePool actually exists. if ok, key := inferencePoolBackend(b, routeNamespace, inferencePools); ok { - // We don't support traffic splitting at the Route level for - // InferencePool backends, so if there's more than one backendRef, and one of them - // is an InferencePool, we mark the rule as invalid. - if len(specRule.BackendRefs) > 1 { - err := field.Forbidden( - rulePath.Child("backendRefs"), - "cannot use InferencePool backend when multiple backendRefs are specified in a single rule", - ) - errors.invalid = append(errors.invalid, err) - break - } - svcName := controller.CreateInferencePoolServiceName(string(b.Name)) rbr = RouteBackendRef{ IsInferencePool: true, @@ -643,3 +646,28 @@ func checkForUnsupportedHTTPFields(rule v1.HTTPRouteRule, rulePath *field.Path) return ruleErrors } + +// checkForMixedBackendTypes returns true if the rule contains a mix of +// InferencePool and non-InferencePool backends. +func checkForMixedBackendTypes( + specRule v1.HTTPRouteRule, + routeNamespace string, + inferencePools map[types.NamespacedName]*inference.InferencePool, +) bool { + var hasInferencePool, hasNonInferencePool bool + + for _, backendRef := range specRule.BackendRefs { + if ok, _ := inferencePoolBackend(backendRef, routeNamespace, inferencePools); ok { + hasInferencePool = true + } else { + hasNonInferencePool = true + } + + // Early exit if we find both types + if hasInferencePool && hasNonInferencePool { + return true + } + } + + return false +} diff --git a/internal/controller/state/graph/httproute_test.go b/internal/controller/state/graph/httproute_test.go index 828d17d749..ff59e84eba 100644 --- a/internal/controller/state/graph/httproute_test.go +++ b/internal/controller/state/graph/httproute_test.go @@ -1256,63 +1256,144 @@ func TestBuildHTTPRouteWithMirrorRoutes(t *testing.T) { func TestProcessHTTPRouteRule_InferencePoolWithMultipleBackendRefs(t *testing.T) { t.Parallel() - g := NewWithT(t) validator := &validationfakes.FakeHTTPFieldsValidator{} - inferencePoolName := "ipool" + inferencePoolName1 := "primary-pool" + inferencePoolName2 := "secondary-pool" routeNamespace := "test" inferencePools := map[types.NamespacedName]*inference.InferencePool{ - {Namespace: routeNamespace, Name: inferencePoolName}: {}, + {Namespace: routeNamespace, Name: inferencePoolName1}: {}, + {Namespace: routeNamespace, Name: inferencePoolName2}: {}, } - // BackendRef 1: InferencePool - backendRef1 := gatewayv1.HTTPBackendRef{ - BackendRef: gatewayv1.BackendRef{ - BackendObjectReference: gatewayv1.BackendObjectReference{ - Group: helpers.GetPointer[gatewayv1.Group](inferenceAPIGroup), - Kind: helpers.GetPointer[gatewayv1.Kind](kinds.InferencePool), - Name: gatewayv1.ObjectName(inferencePoolName), - Namespace: helpers.GetPointer(gatewayv1.Namespace(routeNamespace)), - }, - }, - } - // BackendRef 2: Service - backendRef2 := gatewayv1.HTTPBackendRef{ - BackendRef: gatewayv1.BackendRef{ - BackendObjectReference: gatewayv1.BackendObjectReference{ - Kind: helpers.GetPointer[gatewayv1.Kind](kinds.Service), - Name: "backend", + tests := []struct { + specRule gatewayv1.HTTPRouteRule + name string + expectErrorMsg string + expectedBackend int + expectValid bool + }{ + { + name: "multiple weighted InferencePool backends (valid)", + specRule: gatewayv1.HTTPRouteRule{ + Matches: []gatewayv1.HTTPRouteMatch{ + { + Path: &gatewayv1.HTTPPathMatch{ + Type: helpers.GetPointer(gatewayv1.PathMatchPathPrefix), + Value: helpers.GetPointer("/inference"), + }, + }, + }, + BackendRefs: []gatewayv1.HTTPBackendRef{ + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Group: helpers.GetPointer[gatewayv1.Group](inferenceAPIGroup), + Kind: helpers.GetPointer[gatewayv1.Kind](kinds.InferencePool), + Name: gatewayv1.ObjectName(inferencePoolName1), + Namespace: helpers.GetPointer(gatewayv1.Namespace(routeNamespace)), + }, + Weight: helpers.GetPointer(int32(70)), + }, + }, + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Group: helpers.GetPointer[gatewayv1.Group](inferenceAPIGroup), + Kind: helpers.GetPointer[gatewayv1.Kind](kinds.InferencePool), + Name: gatewayv1.ObjectName(inferencePoolName2), + Namespace: helpers.GetPointer(gatewayv1.Namespace(routeNamespace)), + }, + Weight: helpers.GetPointer(int32(30)), + }, + }, + }, }, + expectValid: true, + expectedBackend: 2, }, - } - - specRule := gatewayv1.HTTPRouteRule{ - Matches: []gatewayv1.HTTPRouteMatch{ - { - Path: &gatewayv1.HTTPPathMatch{ - Type: helpers.GetPointer(gatewayv1.PathMatchPathPrefix), - Value: helpers.GetPointer("/"), + { + name: "InferencePool mixed with Service backend (invalid)", + specRule: gatewayv1.HTTPRouteRule{ + Matches: []gatewayv1.HTTPRouteMatch{ + { + Path: &gatewayv1.HTTPPathMatch{ + Type: helpers.GetPointer(gatewayv1.PathMatchPathPrefix), + Value: helpers.GetPointer("/mixed"), + }, + }, + }, + BackendRefs: []gatewayv1.HTTPBackendRef{ + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Group: helpers.GetPointer[gatewayv1.Group](inferenceAPIGroup), + Kind: helpers.GetPointer[gatewayv1.Kind](kinds.InferencePool), + Name: gatewayv1.ObjectName(inferencePoolName1), + Namespace: helpers.GetPointer(gatewayv1.Namespace(routeNamespace)), + }, + }, + }, + { + BackendRef: gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Kind: helpers.GetPointer[gatewayv1.Kind](kinds.Service), + Name: "service-backend", + }, + }, + }, }, }, + expectValid: false, + expectErrorMsg: "mixing InferencePool and non-InferencePool backends in a rule is not supported", + expectedBackend: 0, }, - BackendRefs: []gatewayv1.HTTPBackendRef{backendRef1, backendRef2}, } - rulePath := field.NewPath("spec").Child("rules").Index(0) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) - routeRule, errs := processHTTPRouteRule( - specRule, - rulePath, - validator, - nil, - inferencePools, - routeNamespace, - ) + rulePath := field.NewPath("spec").Child("rules").Index(0) - g.Expect(routeRule.RouteBackendRefs).To(BeEmpty()) - g.Expect(errs.invalid).To(HaveLen(1)) - errMsg := "cannot use InferencePool backend when multiple backendRefs are specified in a single rule" - g.Expect(errs.invalid[0].Error()).To(ContainSubstring(errMsg)) + routeRule, errs := processHTTPRouteRule( + tc.specRule, + rulePath, + validator, + nil, + inferencePools, + routeNamespace, + ) + + if tc.expectValid { + g.Expect(errs.invalid).To(BeEmpty()) + g.Expect(routeRule.RouteBackendRefs).To(HaveLen(tc.expectedBackend)) + + if tc.expectedBackend == 2 { + // Verify both backends are converted to services with weights + g.Expect(routeRule.RouteBackendRefs[0].IsInferencePool).To(BeTrue()) + g.Expect(routeRule.RouteBackendRefs[1].IsInferencePool).To(BeTrue()) + + // Verify service name conversion (primary-pool -> primary-pool-pool-svc) + g.Expect(string(routeRule.RouteBackendRefs[0].BackendRef.Name)).To(Equal("primary-pool-pool-svc")) + g.Expect(string(routeRule.RouteBackendRefs[1].BackendRef.Name)).To(Equal("secondary-pool-pool-svc")) + + // Verify weights are preserved + g.Expect(routeRule.RouteBackendRefs[0].BackendRef.Weight).To(Equal(helpers.GetPointer(int32(70)))) + g.Expect(routeRule.RouteBackendRefs[1].BackendRef.Weight).To(Equal(helpers.GetPointer(int32(30)))) + + // Verify kind is converted to Service + g.Expect(*routeRule.RouteBackendRefs[0].BackendRef.Kind).To(Equal(gatewayv1.Kind(kinds.Service))) + g.Expect(*routeRule.RouteBackendRefs[1].BackendRef.Kind).To(Equal(gatewayv1.Kind(kinds.Service))) + } + } else { + g.Expect(errs.invalid).To(HaveLen(1)) + g.Expect(errs.invalid[0].Error()).To(ContainSubstring(tc.expectErrorMsg)) + g.Expect(routeRule.RouteBackendRefs).To(HaveLen(tc.expectedBackend)) + } + }) + } } func TestValidateMatch(t *testing.T) { diff --git a/tests/Makefile b/tests/Makefile index d8be4d2be8..2b8dd4bb45 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -19,8 +19,8 @@ CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the SKIP_TESTS_OPENSHIFT = HTTPRouteServiceTypes # Doesn't work on OpenShift due to security restrictions SKIP_TESTS = CEL_TEST_TARGET = -INFERENCE_SUPPORTED_FEATURES = GatewayFollowingEPPRouting,EppUnAvailableFailOpen,HTTPRouteInvalidInferencePoolRef,InferencePoolAccepted,HTTPRouteMultipleGatewaysDifferentPools,HTTPRouteMultipleRulesDifferentPools,InferencePoolHTTPRoutePortValidation,InferencePoolInvalidEPPService,InferencePoolResolvedRefsCondition -INFERENCE_SKIP_TESTS = GatewayWeightedAcrossTwoInferencePools +INFERENCE_SUPPORTED_FEATURES = GatewayFollowingEPPRouting,EppUnAvailableFailOpen,HTTPRouteInvalidInferencePoolRef,InferencePoolAccepted,HTTPRouteMultipleGatewaysDifferentPools,HTTPRouteMultipleRulesDifferentPools,InferencePoolHTTPRoutePortValidation,InferencePoolInvalidEPPService,InferencePoolResolvedRefsCondition,GatewayWeightedAcrossTwoInferencePools +INFERENCE_SKIP_TESTS = # Check if ENABLE_EXPERIMENTAL is true ifeq ($(ENABLE_EXPERIMENTAL),true) @@ -115,9 +115,9 @@ run-inference-conformance-tests: ## Run inference conformance tests .PHONY: cleanup-conformance-tests cleanup-conformance-tests: ## Clean up conformance tests fixtures - kubectl delete pod conformance + kubectl delete pod conformance --ignore-not-found kubectl delete pod conformance-inference --ignore-not-found - kubectl delete -f conformance/conformance-rbac.yaml + kubectl delete -f conformance/conformance-rbac.yaml --ignore-not-found .PHONY: reset-go-modules reset-go-modules: ## Reset the go modules changes From 19135ec5f7b2353f714cea3ab321b145251ee6fa Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Wed, 10 Dec 2025 16:27:37 -0800 Subject: [PATCH 02/12] Add another split clients test case --- .../nginx/config/split_clients_test.go | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index d9d36bb40c..ac83b59dfc 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -834,10 +834,14 @@ func TestCreateBackendGroupSplitClientDistributions(t *testing.T) { func TestGetSplitClientValue(t *testing.T) { t.Parallel() + hrNsName := types.NamespacedName{Namespace: "test", Name: "hr"} + tests := []struct { + source types.NamespacedName msg string expValue string backend dataplane.Backend + ruleIdx int }{ { msg: "valid backend", @@ -845,6 +849,8 @@ func TestGetSplitClientValue(t *testing.T) { UpstreamName: "valid", Valid: true, }, + source: hrNsName, + ruleIdx: 0, expValue: "valid", }, { @@ -853,6 +859,34 @@ func TestGetSplitClientValue(t *testing.T) { UpstreamName: "invalid", Valid: false, }, + source: hrNsName, + ruleIdx: 0, + expValue: invalidBackendRef, + }, + { + msg: "valid backend with endpoint picker config", + backend: dataplane.Backend{ + UpstreamName: "inference-backend", + Valid: true, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "test-namespace", + }, + }, + source: hrNsName, + ruleIdx: 2, + expValue: "/_ngf-internal-inference-backend-test-hr-rule2", + }, + { + msg: "invalid backend with endpoint picker config", + backend: dataplane.Backend{ + UpstreamName: "invalid-inference-backend", + Valid: false, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + NsName: "test-namespace", + }, + }, + source: hrNsName, + ruleIdx: 1, expValue: invalidBackendRef, }, } @@ -861,7 +895,7 @@ func TestGetSplitClientValue(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := getSplitClientValue(test.backend, types.NamespacedName{Namespace: "test", Name: "hr"}, 0) + result := getSplitClientValue(test.backend, test.source, test.ruleIdx) g.Expect(result).To(Equal(test.expValue)) }) } From b9699b4c76592b4c21830cd6bd4b780ac004feaa Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Wed, 10 Dec 2025 17:04:30 -0800 Subject: [PATCH 03/12] Update comment with correct naming --- internal/controller/nginx/config/servers.go | 32 ++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index cccec02d32..e3b5c60151 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -298,10 +298,10 @@ External location calls inference NJS module. The module gets the AI endpoint to then redirects to the internal inference location that proxies to the backend. location /coffee { - set $epp_internal_path /_ngf-internal-rule0-route0-inference; - js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-rule0-route0-inference + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } -location /_ngf-internal-rule0-route0-inference { +location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { internal; proxy_pass http://$inference-backend; } @@ -309,7 +309,7 @@ location /_ngf-internal-rule0-route0-inference { Inference extension with multiple inference backends. location /coffee { - rewrite $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 last; + rewrite $inference_backend_group_routeNS__routeName_rule0 last; proxy_http_version 1.1; } @@ -318,7 +318,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { proxy_pass http://$inference-backend0; } -location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0 { +location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } @@ -328,14 +328,14 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { proxy_pass http://$inference-backend1; } -location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-1 { +location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } -split_clients $request_id $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 { - 70.00% /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0; - 30.00% /_ngf-internal-inference-conformance-app-backend_secondary-inference-pool-pool-svc_3000-backend-1; +split_clients $request_id $inference_backend_group_routeNS__routeName_rule0 { + 70.00% /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0; + 30.00% /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0; } --------------- Inference extension with HTTP matching conditions. @@ -353,7 +353,7 @@ it was the final location that had -inference in the name. location /coffee { js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location } -location /_ngf-internal-test_coffee_80-backend-0 { +location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { internal; set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; @@ -378,7 +378,7 @@ location /coffee { location /_ngf-internal-split-clients-rule0-route0-inference { internal; - rewrite $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 last; + rewrite $inference_backend_group_routeNS__routeName_rule0 last; proxy_http_version 1.1; } @@ -387,7 +387,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { proxy_pass http://$inference-backend0; } -location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0 { +location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } @@ -397,14 +397,14 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { proxy_pass http://$inference-backend1; } -location /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-1 { +location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } -split_clients $request_id $inference_backend_group_inference_conformance_app_backend__httproute_weighted_two_pools_rule0 { - 70.00% /_ngf-internal-inference-conformance-app-backend_primary-inference-pool-pool-svc_3000-backend-0; - 30.00% /_ngf-internal-inference-conformance-app-backend_secondary-inference-pool-pool-svc_3000-backend-1; +split_clients $request_id $inference_backend_group_routeNS__routeName_rule0 { + 70.00% /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0; + 30.00% /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0; } */ From 36eb26bfb0f71d4c9909c1c8d8d22844a40a3670 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Thu, 11 Dec 2025 10:24:15 -0800 Subject: [PATCH 04/12] Remove some comments and update array capacity calculation --- internal/controller/nginx/config/generator.go | 5 ----- internal/controller/nginx/config/servers.go | 19 ++++++++++++++++--- .../controller/nginx/config/servers_test.go | 3 --- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 28c84baed6..6fa7602c04 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -173,11 +173,6 @@ func (g GeneratorImpl) executeConfigTemplates( } } - // for fp, bytes := range fileBytes { - // fmt.Println("Generated NGINX configuration file: " + fp) - // fmt.Println(string(bytes)) - // } - var mgmtFiles []agent.File if g.plus { mgmtFiles = g.generateMgmtFiles(conf) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index e3b5c60151..3d7039a19b 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -487,8 +487,6 @@ func createLocations( locs = append(locs, createDefaultRootLocation()) } - // fmt.Println("These are all the locations", locs) - return locs, matchPairs, grpcServer } @@ -521,7 +519,22 @@ func createInternalLocationsForRule( keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) ([]http.Location, []routeMatch) { - internalLocations := make([]http.Location, 0, len(rule.MatchRules)) + // Calculate the exact capacity needed + capacity := 0 + for _, r := range rule.MatchRules { + if !rule.HasInferenceBackends { + capacity++ // intLocation (always created for non-inference) + } else { + // For inference backends with matches + if len(r.BackendGroup.Backends) > 1 { + capacity++ // intSplitClientsLocation (created for multiple backends) + } + + capacity += len(r.BackendGroup.Backends) * 2 // intEPPLocation and intProxyPassLocation per backend + } + } + + internalLocations := make([]http.Location, 0, capacity) matches := make([]routeMatch, 0, len(rule.MatchRules)) for matchRuleIdx, r := range rule.MatchRules { var intLocation http.Location diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 3e4899e834..99dd418704 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -2449,7 +2449,6 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { hrNsName := types.NamespacedName{Namespace: "test", Name: "route1"} - // Reusable backend definitions singleInferenceBackend := dataplane.Backend{ UpstreamName: "test_foo_80", Valid: true, @@ -2528,7 +2527,6 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { }, } - // Reusable backend groups singleInferenceGroup := dataplane.BackendGroup{ Source: hrNsName, RuleIdx: 0, @@ -2553,7 +2551,6 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { Backends: []dataplane.Backend{multiMatchPrimaryBackend, multiMatchSecondaryBackend}, } - // Reusable path rules pathRuleInferenceOnly := dataplane.PathRule{ Path: "/inference", PathType: dataplane.PathTypeExact, From cfb46463c9e1d89bee7e80f8d62ce6fcb7f49c15 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Thu, 11 Dec 2025 10:35:33 -0800 Subject: [PATCH 05/12] Update max location calculation --- internal/controller/nginx/config/servers.go | 78 +++++++++++++++++---- 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 3d7039a19b..0ede35b2c5 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -762,27 +762,79 @@ func needsInternalLocationsForMatches(rule dataplane.PathRule) bool { // for example, {/foo: {exact: {}, prefix: {}}}. type pathAndTypeMap map[string]map[dataplane.PathType]struct{} -// To calculate the maximum number of locations, we need to take into account the following: -// 1. Each match rule for a path rule will have one location. -// 2. Each path rule may have an additional location if it contains non-path-only matches. -// 3. Each prefix path rule may have an additional location if it doesn't contain trailing slash. -// 4. There may be an additional location for the default root path. -// 5. There may be an additional location per parent location for the inference extension. -// We also return a map of all paths and their types. func getMaxLocationCountAndPathMap(pathRules []dataplane.PathRule) (int, pathAndTypeMap) { - maxLocs := 1 + // To calculate the maximum number of locations, we need to take into account the following: + // 1. Each path rule will have at least one external location. + // 2. Each path rule may have an additional external location if it's a non-slashed prefix path. + // 3. There may be an additional location for the default root path. + // 4. For inference backends: + // - Single backend without matches: 2 locations (external EPP + internal proxy pass) + // - Single backend with matches: 3 locations (external redirect + internal EPP + internal proxy pass) + // - Multiple backends without matches: 1 external + (2 * numBackends) internal locations + // - Multiple backends with matches: 1 external + 1 split clients + (2 * numBackends) internal locations + // 5. For non-inference backends with matches: + // - Each match rule gets an internal location + // We also return a map of all paths and their types. + + maxLocs := 0 pathsAndTypes := make(pathAndTypeMap) + for _, rule := range pathRules { - maxLocs += (len(rule.MatchRules) * 2) + 2 + // External locations calculation + maxLocs++ // Base external location for the path + + // Add the path to the map if pathsAndTypes[rule.Path] == nil { - pathsAndTypes[rule.Path] = map[dataplane.PathType]struct{}{ - rule.PathType: {}, + pathsAndTypes[rule.Path] = make(map[dataplane.PathType]struct{}) + } + pathsAndTypes[rule.Path][rule.PathType] = struct{}{} + + // Check if we need an additional external location for non-slashed prefix paths + if isNonSlashedPrefixPath(rule.PathType, rule.Path) { + maxLocs++ // Additional external location for exact match + } + + // Determine if we need internal locations for matches + needsInternalMatches := needsInternalLocationsForMatches(rule) + + // Internal locations calculation + for _, matchRule := range rule.MatchRules { + if !rule.HasInferenceBackends { + // Non-inference backends with matches need internal locations + if needsInternalMatches { + maxLocs++ // Internal match location per match rule + } + } else { + // Inference backends calculation + numBackends := len(matchRule.BackendGroup.Backends) + + if needsInternalMatches { + // Has HTTP matching conditions + if numBackends > 1 { + // Multiple backends with matches: split clients + 2 locations per backend + maxLocs++ // Internal split clients location + maxLocs += numBackends * 2 // EPP + proxy pass per backend + } else { + // Single backend with matches: EPP + proxy pass + maxLocs += 2 + } + } else { + // No HTTP matching conditions + if numBackends > 1 { + // Multiple backends without matches: 2 locations per backend (no split clients for external) + maxLocs += numBackends * 2 // EPP + proxy pass per backend + } else { + // Single backend without matches: proxy pass only (external becomes EPP) + maxLocs++ // Just the internal proxy pass location + } + } } - } else { - pathsAndTypes[rule.Path][rule.PathType] = struct{}{} } } + // Add 1 for potential default root location + maxLocs++ + return maxLocs, pathsAndTypes } From 6c7c4fe7c2b881cc6910b037abc8bd4eca8aee90 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Thu, 11 Dec 2025 10:57:56 -0800 Subject: [PATCH 06/12] Add ordering to inferenceMaps --- internal/controller/nginx/config/maps.go | 13 +++++++++++-- internal/controller/nginx/config/maps_test.go | 10 +++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/controller/nginx/config/maps.go b/internal/controller/nginx/config/maps.go index beef173c60..ac903f1433 100644 --- a/internal/controller/nginx/config/maps.go +++ b/internal/controller/nginx/config/maps.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "sort" "strings" gotemplate "text/template" @@ -245,9 +246,17 @@ func buildInferenceMaps(groups []dataplane.BackendGroup) []shared.Map { } } + // Sort the map keys to ensure deterministic ordering + mapKeys := make([]string, 0, len(uniqueMaps)) + for key := range uniqueMaps { + mapKeys = append(mapKeys, key) + } + sort.Strings(mapKeys) + + // Build the result slice in sorted order inferenceMaps := make([]shared.Map, 0, len(uniqueMaps)) - for _, inferenceMap := range uniqueMaps { - inferenceMaps = append(inferenceMaps, inferenceMap) + for _, key := range mapKeys { + inferenceMaps = append(inferenceMaps, uniqueMaps[key]) } return inferenceMaps diff --git a/internal/controller/nginx/config/maps_test.go b/internal/controller/nginx/config/maps_test.go index a66af0de14..46fa1fca5a 100644 --- a/internal/controller/nginx/config/maps_test.go +++ b/internal/controller/nginx/config/maps_test.go @@ -408,25 +408,25 @@ func TestBuildInferenceMaps(t *testing.T) { expectedMaps int }{ { - name: "unique backends with different failure modes", + name: "unique backends with different failure modes, result is ordered by upstream name", backendGroups: []dataplane.BackendGroup{ { Backends: []dataplane.Backend{ { - UpstreamName: "upstream1", + UpstreamName: "upstream2", EndpointPickerConfig: &dataplane.EndpointPickerConfig{ NsName: "default", EndpointPickerRef: &inference.EndpointPickerRef{ - FailureMode: inference.EndpointPickerFailClose, + FailureMode: inference.EndpointPickerFailOpen, }, }, }, { - UpstreamName: "upstream2", + UpstreamName: "upstream1", EndpointPickerConfig: &dataplane.EndpointPickerConfig{ NsName: "default", EndpointPickerRef: &inference.EndpointPickerRef{ - FailureMode: inference.EndpointPickerFailOpen, + FailureMode: inference.EndpointPickerFailClose, }, }, }, From f49dc00cb9e2f572898e1eaf0d5d753fcbf54304 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Thu, 11 Dec 2025 23:21:29 -0800 Subject: [PATCH 07/12] Add pathRuleIdx to epp location --- internal/controller/nginx/config/servers.go | 97 +++-- .../controller/nginx/config/servers_test.go | 412 ++++++++++-------- .../controller/nginx/config/split_clients.go | 9 +- .../nginx/config/split_clients_test.go | 30 +- .../state/dataplane/configuration.go | 16 +- .../state/dataplane/configuration_test.go | 88 ++-- internal/controller/state/dataplane/types.go | 4 +- 7 files changed, 393 insertions(+), 263 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 0ede35b2c5..bf32f2bb5b 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -319,6 +319,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { } location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { + internal; set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } @@ -329,6 +330,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { } location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { + internal; set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } @@ -388,6 +390,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { } location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { + internal; set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } @@ -398,6 +401,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { } location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { + internal; set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } @@ -536,9 +540,16 @@ func createInternalLocationsForRule( internalLocations := make([]http.Location, 0, capacity) matches := make([]routeMatch, 0, len(rule.MatchRules)) + + // If there are multiple matches on a single route rule, they will share the same intEPPLocation and + // intProxyPassLocation. To avoid creating duplicates, we track the unique names here. + uniqueEPPNameMap := make(map[string]struct{}) + for matchRuleIdx, r := range rule.MatchRules { var intLocation http.Location var match routeMatch + skipMatch := false + if !rule.HasInferenceBackends { intLocation, match = initializeInternalMatchLocation(pathRuleIdx, matchRuleIdx, r.Match, rule.GRPC) intLocation.Includes = createIncludesFromPolicyGenerateResult( @@ -572,23 +583,7 @@ func createInternalLocationsForRule( // // The match needs to point to the internal inference location which calls the EPP NJS module (intEPPLocation) - if len(r.BackendGroup.Backends) > 1 { - intSplitClientsLocation := initializeInternalInferenceSplitClientsLocation(pathRuleIdx, matchRuleIdx) - intSplitClientsLocation.Includes = createIncludesFromPolicyGenerateResult( - generator.GenerateForInternalLocation(rule.Policies), - ) - - splitClientsVariableName := createInferenceSplitClientsVariableName( - convertStringToSafeVariableName(r.BackendGroup.Name()), - ) - intSplitClientsLocation.Rewrites = append(intSplitClientsLocation.Rewrites, - fmt.Sprintf("^ $%s last", splitClientsVariableName)) - - internalLocations = append(internalLocations, intSplitClientsLocation) - - match = createRouteMatch(r.Match, intSplitClientsLocation.Path) - } - + var intEPPLocation http.Location for backendIdx, b := range r.BackendGroup.Backends { intProxyPassLocation := initializeInternalInferenceProxyPassLocation(pathRuleIdx, matchRuleIdx, backendIdx) intProxyPassLocation.Includes = createIncludesFromPolicyGenerateResult( @@ -609,22 +604,61 @@ func createInternalLocationsForRule( keepAliveCheck, mirrorPercentage, ) - internalLocations = append(internalLocations, intProxyPassLocation) - intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx) + intEPPLocation = initializeInternalInferenceEPPLocation( + b, + r.BackendGroup.Source, + r.BackendGroup.RuleIdx, + r.BackendGroup.PathRuleIdx, + ) intEPPLocation.Includes = createIncludesFromPolicyGenerateResult( generator.GenerateForInternalLocation(rule.Policies), ) - eppHost, portNum := extractEPPConfig(b) - intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum) - internalLocations = append(internalLocations, intEPPLocation) + mapKey := intEPPLocation.Path // Use this as the key to detect duplicates - if len(r.BackendGroup.Backends) == 1 { - match = createRouteMatch(r.Match, intEPPLocation.Path) + // The only time this happens is on a single route rule with multiple matches with the same path. + // In this case, we only need to create one set of internal locations, and can skip the duplicate matches. + if _, exists := uniqueEPPNameMap[mapKey]; exists { + skipMatch = true + break + } + uniqueEPPNameMap[mapKey] = struct{}{} + // we only append intEPPLocation and intProxyPassLocation once per unique intEPPLocation name + internalLocations = append(internalLocations, intProxyPassLocation) + + if b.EndpointPickerConfig != nil && b.EndpointPickerConfig.EndpointPickerRef != nil { + eppHost, portNum := extractEPPConfig(b) + intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum) + internalLocations = append(internalLocations, intEPPLocation) } } + + // skip adding match and creating split clients location if its a duplicate intEPPLocation.Path + if skipMatch { + continue + } + + if len(r.BackendGroup.Backends) > 1 { + intSplitClientsLocation := initializeInternalInferenceSplitClientsLocation(pathRuleIdx, matchRuleIdx) + intSplitClientsLocation.Includes = createIncludesFromPolicyGenerateResult( + generator.GenerateForInternalLocation(rule.Policies), + ) + + splitClientsVariableName := createInferenceSplitClientsVariableName( + convertStringToSafeVariableName(r.BackendGroup.Name()), + ) + intSplitClientsLocation.Rewrites = append(intSplitClientsLocation.Rewrites, + fmt.Sprintf("^ $%s last", splitClientsVariableName)) + + internalLocations = append(internalLocations, intSplitClientsLocation) + + match = createRouteMatch(r.Match, intSplitClientsLocation.Path) + } else { + match = createRouteMatch(r.Match, intEPPLocation.Path) + } } + matches = append(matches, match) } @@ -657,6 +691,8 @@ func createInferenceLocationsForRule( locs := make([]http.Location, 0, capacity) + // There will only be one rule.MatchRules, since if there are multiple, createInternalLocationsForRule + // would have been called instead. for matchRuleIdx, r := range rule.MatchRules { // If there are multiple inference backends, we need 3 locations: // 1. (external location) external location which rewrites to the EPP internal location based on a @@ -706,7 +742,12 @@ func createInferenceLocationsForRule( eppHost, portNum := extractEPPConfig(b) if len(r.BackendGroup.Backends) > 1 { - intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx) + intEPPLocation := initializeInternalInferenceEPPLocation( + b, + r.BackendGroup.Source, + r.BackendGroup.RuleIdx, + r.BackendGroup.PathRuleIdx, + ) intEPPLocation.Includes = createIncludesFromPolicyGenerateResult( generator.GenerateForInternalLocation(rule.Policies), ) @@ -918,17 +959,19 @@ func initializeInternalMatchLocation( func initializeInternalInferenceEPPLocation( b dataplane.Backend, source types.NamespacedName, - ruleIdx int, + ruleIdx, + pathruleIdx int, ) http.Location { return http.Location{ // This path needs to be recreated in the split_clients directive generation to match correctly. Path: fmt.Sprintf( - "%s-%s-%s-%s-rule%d", + "%s-%s-%s-%s-routeRule%d-pathRule%d", http.InternalRoutePathPrefix, b.UpstreamName, source.Namespace, source.Name, ruleIdx, + pathruleIdx, ), Type: http.InferenceInternalLocationType, } diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 99dd418704..85f5063f07 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -1433,7 +1433,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/_ngf-internal-rule1-route0", - ProxyPass: "http://$group_test__route1_rule1$request_uri", + ProxyPass: "http://$group_test__route1_rule1_pathRule0$request_uri", ProxySetHeaders: httpBaseHeaders, Type: http.InternalLocationType, Includes: internalIncludes, @@ -2447,160 +2447,149 @@ func TestCreateLocations_Includes(t *testing.T) { func TestCreateLocations_InferenceBackends(t *testing.T) { t.Parallel() - hrNsName := types.NamespacedName{Namespace: "test", Name: "route1"} + hrNsName := types.NamespacedName{Namespace: "testNS", Name: "routeName"} - singleInferenceBackend := dataplane.Backend{ - UpstreamName: "test_foo_80", - Valid: true, - Weight: 1, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "test-epp", - Port: &inference.Port{Number: 80}, + createInferenceBackend := func(upstreamName string, weight int32, eppName string) dataplane.Backend { + return dataplane.Backend{ + UpstreamName: upstreamName, + Valid: true, + Weight: weight, + EndpointPickerConfig: &dataplane.EndpointPickerConfig{ + EndpointPickerRef: &inference.EndpointPickerRef{ + Name: inference.ObjectName(eppName), + Port: &inference.Port{Number: 80}, + }, + NsName: hrNsName.Namespace, }, - NsName: hrNsName.Namespace, - }, + } } - singleMatchInferenceBackend := dataplane.Backend{ - UpstreamName: "test_single_match_pool_80", - Valid: true, - Weight: 1, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "single-match-pool", - Port: &inference.Port{Number: 80}, - }, - NsName: hrNsName.Namespace, - }, - } + singleInferenceBackend := createInferenceBackend("test_foo_80", 1, "test-epp") - multiInferencePrimaryBackend := dataplane.Backend{ - UpstreamName: "test_primary_pool_80", - Valid: true, - Weight: 70, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "primary-pool", - Port: &inference.Port{Number: 80}, - }, - NsName: hrNsName.Namespace, - }, + multiInferencePrimaryBackend := createInferenceBackend("test_primary_pool_80", 70, "primary-pool") + multiInferenceSecondaryBackend := createInferenceBackend("test_secondary_pool_80", 30, "secondary-pool") + + createBackendGroup := func(backends []dataplane.Backend, ruleIdx int, pathRuleIdx int) dataplane.BackendGroup { + return dataplane.BackendGroup{ + Source: hrNsName, + RuleIdx: ruleIdx, + PathRuleIdx: pathRuleIdx, + Backends: backends, + } } - multiInferenceSecondaryBackend := dataplane.Backend{ - UpstreamName: "test_secondary_pool_80", - Valid: true, - Weight: 30, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "secondary-pool", - Port: &inference.Port{Number: 80}, - }, - NsName: hrNsName.Namespace, - }, + singleInferenceGroup := createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 0, 0) + multipleInferenceGroup := createBackendGroup( + []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, + 0, + 0, + ) + + // Helper function to create path rules + createPathRule := func(path string, matchRules []dataplane.MatchRule) dataplane.PathRule { + return dataplane.PathRule{ + Path: path, + PathType: dataplane.PathTypeExact, + HasInferenceBackends: true, + MatchRules: matchRules, + } } - multiMatchPrimaryBackend := dataplane.Backend{ - UpstreamName: "test_multi_match_primary_80", - Valid: true, - Weight: 80, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "multi-match-primary", - Port: &inference.Port{Number: 80}, - }, - NsName: hrNsName.Namespace, + pathRuleInferenceOnly := createPathRule("/inference", []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: singleInferenceGroup, }, - } + }) - multiMatchSecondaryBackend := dataplane.Backend{ - UpstreamName: "test_multi_match_secondary_80", - Valid: true, - Weight: 20, - EndpointPickerConfig: &dataplane.EndpointPickerConfig{ - EndpointPickerRef: &inference.EndpointPickerRef{ - Name: "multi-match-secondary", - Port: &inference.Port{Number: 80}, + pathRuleInferenceWithMatch := createPathRule("/inference-match", []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Method: helpers.GetPointer("POST"), }, - NsName: hrNsName.Namespace, + BackendGroup: singleInferenceGroup, }, - } - - singleInferenceGroup := dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: 0, - Backends: []dataplane.Backend{singleInferenceBackend}, - } - - singleMatchInferenceGroup := dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: 1, - Backends: []dataplane.Backend{singleMatchInferenceBackend}, - } + }) - multipleInferenceGroup := dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: 2, - Backends: []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, - } + pathRuleMultipleInferenceBackends := createPathRule("/weighted-inference", []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: multipleInferenceGroup, + }, + }) - multipleMatchInferenceGroup := dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: 3, - Backends: []dataplane.Backend{multiMatchPrimaryBackend, multiMatchSecondaryBackend}, - } + pathRuleMultipleInferenceWithMatch := createPathRule("/weighted-inference-match", []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Method: helpers.GetPointer("GET"), + }, + BackendGroup: multipleInferenceGroup, + }, + }) - pathRuleInferenceOnly := dataplane.PathRule{ - Path: "/inference", - PathType: dataplane.PathTypeExact, - HasInferenceBackends: true, - MatchRules: []dataplane.MatchRule{ + singleRouteMultipleMatchesSingleBackend := createPathRule( + "/inference-multiple-matches-single-backend", + []dataplane.MatchRule{ { Match: dataplane.Match{}, BackendGroup: singleInferenceGroup, }, - }, - } - - pathRuleInferenceWithMatch := dataplane.PathRule{ - Path: "/inference-match", - PathType: dataplane.PathTypeExact, - HasInferenceBackends: true, - MatchRules: []dataplane.MatchRule{ { - Match: dataplane.Match{ - Method: helpers.GetPointer("POST"), - }, - BackendGroup: singleMatchInferenceGroup, + Match: dataplane.Match{}, + BackendGroup: singleInferenceGroup, }, - }, - } + }) - pathRuleMultipleInferenceBackends := dataplane.PathRule{ - Path: "/weighted-inference", - PathType: dataplane.PathTypeExact, - HasInferenceBackends: true, - MatchRules: []dataplane.MatchRule{ + singleRouteMultipleMatchesMultipleBackends := createPathRule( + "/inference-multiple-matches-multiple-backends", + []dataplane.MatchRule{ { Match: dataplane.Match{}, BackendGroup: multipleInferenceGroup, }, - }, - } + { + Match: dataplane.Match{}, + BackendGroup: multipleInferenceGroup, + }, + }) - pathRuleMultipleInferenceWithMatch := dataplane.PathRule{ - Path: "/weighted-inference-match", - PathType: dataplane.PathTypeExact, - HasInferenceBackends: true, - MatchRules: []dataplane.MatchRule{ + combinedPathRules := []dataplane.PathRule{ + createPathRule("/inference", []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 0, 0), + }, + }), + createPathRule("/inference-match", []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Method: helpers.GetPointer("POST"), + }, + BackendGroup: createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 1, 1), + }, + }), + createPathRule("/weighted-inference", []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: createBackendGroup( + []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, + 2, + 2, + ), + }, + }), + createPathRule("/weighted-inference-match", []dataplane.MatchRule{ { Match: dataplane.Match{ Method: helpers.GetPointer("GET"), }, - BackendGroup: multipleMatchInferenceGroup, + BackendGroup: createBackendGroup( + []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, + 3, + 3, + ), }, - }, + }), } proxySetHeaders := []http.Header{ @@ -2634,7 +2623,7 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { Path: "= /inference", Type: http.InferenceExternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", - EPPHost: "test-epp.test", + EPPHost: "test-epp.testNS", EPPPort: 80, }, createDefaultRootLocation(), @@ -2653,21 +2642,21 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { { Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_single_match_pool_80$request_uri", + ProxyPass: "http://$inference_backend_test_foo_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1", + Path: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule0-pathRule0", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", - EPPHost: "single-match-pool.test", + EPPHost: "test-epp.testNS", EPPPort: 80, }, createDefaultRootLocation(), }, expMatches: httpMatchPairs{ "1_0": { - {Method: "POST", RedirectPath: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1"}, + {Method: "POST", RedirectPath: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule0-pathRule0"}, }, }, }, @@ -2682,10 +2671,10 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_primary_pool_80-test-route1-rule2", + Path: "/_ngf-internal-test_primary_pool_80-testNS-routeName-routeRule0-pathRule0", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", - EPPHost: "primary-pool.test", + EPPHost: "primary-pool.testNS", EPPPort: 80, }, { @@ -2695,61 +2684,139 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_secondary_pool_80-test-route1-rule2", + Path: "/_ngf-internal-test_secondary_pool_80-testNS-routeName-routeRule0-pathRule0", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", - EPPHost: "secondary-pool.test", + EPPHost: "secondary-pool.testNS", EPPPort: 80, }, { Path: "= /weighted-inference", Type: http.ExternalLocationType, - Rewrites: []string{"^ $inference_backend_group_test__route1_rule2 last"}, + Rewrites: []string{"^ $inference_backend_group_testNS__routeName_rule0_pathRule0 last"}, }, createDefaultRootLocation(), }, expMatches: httpMatchPairs{}, }, { - name: "multiple weighted inference backends with match conditions", - pathRules: []dataplane.PathRule{pathRuleMultipleInferenceWithMatch}, + name: "single route multiple matches with single backend", + pathRules: []dataplane.PathRule{singleRouteMultipleMatchesSingleBackend}, expLocs: []http.Location{ { - Path: "= /weighted-inference-match", + Path: "= /inference-multiple-matches-single-backend", Type: http.RedirectLocationType, HTTPMatchKey: "1_0", }, + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_foo_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule0-pathRule0", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "test-epp.testNS", + EPPPort: 80, + }, + createDefaultRootLocation(), + }, + expMatches: httpMatchPairs{ + "1_0": { + {Any: true, RedirectPath: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule0-pathRule0"}, + }, + }, + }, + { + name: "single route multiple matches with multiple backends", + pathRules: []dataplane.PathRule{singleRouteMultipleMatchesMultipleBackends}, + expLocs: []http.Location{ + { + Path: "= /inference-multiple-matches-multiple-backends", + Type: http.RedirectLocationType, + HTTPMatchKey: "1_0", + }, + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_primary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_primary_pool_80-testNS-routeName-routeRule0-pathRule0", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", + EPPHost: "primary-pool.testNS", + EPPPort: 80, + }, + { + Path: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + Type: http.InternalLocationType, + ProxyPass: "http://$inference_backend_test_secondary_pool_80$request_uri", + ProxySetHeaders: proxySetHeaders, + }, + { + Path: "/_ngf-internal-test_secondary_pool_80-testNS-routeName-routeRule0-pathRule0", + Type: http.InferenceInternalLocationType, + EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", + EPPHost: "secondary-pool.testNS", + EPPPort: 80, + }, { Path: "/_ngf-internal-split-clients-rule0-route0-inference", Type: http.InternalLocationType, - Rewrites: []string{"^ $inference_backend_group_test__route1_rule3 last"}, + Rewrites: []string{"^ $inference_backend_group_testNS__routeName_rule0_pathRule0 last"}, + }, + createDefaultRootLocation(), + }, + expMatches: httpMatchPairs{ + "1_0": { + {Any: true, RedirectPath: "/_ngf-internal-split-clients-rule0-route0-inference"}, + }, + }, + }, + { + name: "multiple weighted inference backends with match conditions", + pathRules: []dataplane.PathRule{pathRuleMultipleInferenceWithMatch}, + expLocs: []http.Location{ + { + Path: "= /weighted-inference-match", + Type: http.RedirectLocationType, + HTTPMatchKey: "1_0", }, { Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_multi_match_primary_80$request_uri", + ProxyPass: "http://$inference_backend_test_primary_pool_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_multi_match_primary_80-test-route1-rule3", + Path: "/_ngf-internal-test_primary_pool_80-testNS-routeName-routeRule0-pathRule0", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", - EPPHost: "multi-match-primary.test", + EPPHost: "primary-pool.testNS", EPPPort: 80, }, { Path: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_multi_match_secondary_80$request_uri", + ProxyPass: "http://$inference_backend_test_secondary_pool_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_multi_match_secondary_80-test-route1-rule3", + Path: "/_ngf-internal-test_secondary_pool_80-testNS-routeName-routeRule0-pathRule0", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend1-inference", - EPPHost: "multi-match-secondary.test", + EPPHost: "secondary-pool.testNS", EPPPort: 80, }, + { + Path: "/_ngf-internal-split-clients-rule0-route0-inference", + Type: http.InternalLocationType, + Rewrites: []string{"^ $inference_backend_group_testNS__routeName_rule0_pathRule0 last"}, + }, createDefaultRootLocation(), }, expMatches: httpMatchPairs{ @@ -2759,15 +2826,10 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { }, }, { - name: "mixed multiple path rules with different inference configurations", - pathRules: []dataplane.PathRule{ - pathRuleInferenceOnly, - pathRuleInferenceWithMatch, - pathRuleMultipleInferenceBackends, - pathRuleMultipleInferenceWithMatch, - }, + name: "mixed multiple path rules with different inference configurations", + pathRules: combinedPathRules, expLocs: []http.Location{ - // 1. Single inference pool locations (pathRuleInferenceOnly - rule index 0) + // 1. Single inference pool locations (rule index 0) { Path: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", Type: http.InternalLocationType, @@ -2778,10 +2840,10 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { Path: "= /inference", Type: http.InferenceExternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule0-route0-backend0-inference", - EPPHost: "test-epp.test", + EPPHost: "test-epp.testNS", EPPPort: 80, }, - // 2. Single inference pool with match (pathRuleInferenceWithMatch - rule index 1) + // 2. Single inference pool with match (rule index 1) { Path: "= /inference-match", Type: http.RedirectLocationType, @@ -2790,17 +2852,17 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { { Path: "/_ngf-internal-proxy-pass-rule1-route0-backend0-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_single_match_pool_80$request_uri", + ProxyPass: "http://$inference_backend_test_foo_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1", + Path: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule1-pathRule1", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule1-route0-backend0-inference", - EPPHost: "single-match-pool.test", + EPPHost: "test-epp.testNS", EPPPort: 80, }, - // 3. Multiple inference pools, no match (pathRuleMultipleInferenceBackends - rule index 2) + // 3. Multiple inference pools, no match (rule index 2) { Path: "/_ngf-internal-proxy-pass-rule2-route0-backend0-inference", Type: http.InternalLocationType, @@ -2808,10 +2870,10 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_primary_pool_80-test-route1-rule2", + Path: "/_ngf-internal-test_primary_pool_80-testNS-routeName-routeRule2-pathRule2", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule2-route0-backend0-inference", - EPPHost: "primary-pool.test", + EPPHost: "primary-pool.testNS", EPPPort: 80, }, { @@ -2821,59 +2883,59 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_secondary_pool_80-test-route1-rule2", + Path: "/_ngf-internal-test_secondary_pool_80-testNS-routeName-routeRule2-pathRule2", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule2-route0-backend1-inference", - EPPHost: "secondary-pool.test", + EPPHost: "secondary-pool.testNS", EPPPort: 80, }, { Path: "= /weighted-inference", Type: http.ExternalLocationType, - Rewrites: []string{"^ $inference_backend_group_test__route1_rule2 last"}, + Rewrites: []string{"^ $inference_backend_group_testNS__routeName_rule2_pathRule2 last"}, }, - // 4. Multiple inference pools with match (pathRuleMultipleInferenceWithMatch - rule index 3) + // 4. Multiple inference pools with match (rule index 3) { Path: "= /weighted-inference-match", Type: http.RedirectLocationType, HTTPMatchKey: "1_3", }, - { - Path: "/_ngf-internal-split-clients-rule3-route0-inference", - Type: http.InternalLocationType, - Rewrites: []string{"^ $inference_backend_group_test__route1_rule3 last"}, - }, { Path: "/_ngf-internal-proxy-pass-rule3-route0-backend0-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_multi_match_primary_80$request_uri", + ProxyPass: "http://$inference_backend_test_primary_pool_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_multi_match_primary_80-test-route1-rule3", + Path: "/_ngf-internal-test_primary_pool_80-testNS-routeName-routeRule3-pathRule3", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule3-route0-backend0-inference", - EPPHost: "multi-match-primary.test", + EPPHost: "primary-pool.testNS", EPPPort: 80, }, { Path: "/_ngf-internal-proxy-pass-rule3-route0-backend1-inference", Type: http.InternalLocationType, - ProxyPass: "http://$inference_backend_test_multi_match_secondary_80$request_uri", + ProxyPass: "http://$inference_backend_test_secondary_pool_80$request_uri", ProxySetHeaders: proxySetHeaders, }, { - Path: "/_ngf-internal-test_multi_match_secondary_80-test-route1-rule3", + Path: "/_ngf-internal-test_secondary_pool_80-testNS-routeName-routeRule3-pathRule3", Type: http.InferenceInternalLocationType, EPPInternalPath: "/_ngf-internal-proxy-pass-rule3-route0-backend1-inference", - EPPHost: "multi-match-secondary.test", + EPPHost: "secondary-pool.testNS", EPPPort: 80, }, + { + Path: "/_ngf-internal-split-clients-rule3-route0-inference", + Type: http.InternalLocationType, + Rewrites: []string{"^ $inference_backend_group_testNS__routeName_rule3_pathRule3 last"}, + }, createDefaultRootLocation(), }, expMatches: httpMatchPairs{ "1_1": { - {Method: "POST", RedirectPath: "/_ngf-internal-test_single_match_pool_80-test-route1-rule1"}, + {Method: "POST", RedirectPath: "/_ngf-internal-test_foo_80-testNS-routeName-routeRule1-pathRule1"}, }, "1_3": { { @@ -4183,7 +4245,7 @@ func TestCreateProxyPass(t *testing.T) { inferenceBackend: true, }, { - expected: "http://$group_ns1__bg_rule0$request_uri", + expected: "http://$group_ns1__bg_rule0_pathRule0$request_uri", grp: dataplane.BackendGroup{ Source: types.NamespacedName{Namespace: "ns1", Name: "bg"}, Backends: []dataplane.Backend{ diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index d0d9361582..4cda9d54ae 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -168,7 +168,7 @@ func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) [] distributions = append(distributions, http.SplitClientDistribution{ Percent: fmt.Sprintf("%.2f", percentage), - Value: getSplitClientValue(b, group.Source, group.RuleIdx), + Value: getSplitClientValue(b, group.Source, group.RuleIdx, group.PathRuleIdx), }) } @@ -178,22 +178,23 @@ func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) [] distributions = append(distributions, http.SplitClientDistribution{ Percent: fmt.Sprintf("%.2f", availablePercentage), - Value: getSplitClientValue(lastBackend, group.Source, group.RuleIdx), + Value: getSplitClientValue(lastBackend, group.Source, group.RuleIdx, group.PathRuleIdx), }) return distributions } -func getSplitClientValue(b dataplane.Backend, source types.NamespacedName, ruleIdx int) string { +func getSplitClientValue(b dataplane.Backend, source types.NamespacedName, ruleIdx, pathRuleIdx int) string { if b.Valid { if b.EndpointPickerConfig != nil { return fmt.Sprintf( - "%s-%s-%s-%s-rule%d", + "%s-%s-%s-%s-routeRule%d-pathRule%d", http.InternalRoutePathPrefix, b.UpstreamName, source.Namespace, source.Name, ruleIdx, + pathRuleIdx, ) } diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index ac83b59dfc..30c2a7cbfb 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -619,7 +619,7 @@ func TestBackendGroupCreateSplitClients(t *testing.T) { }, expSplitClients: []http.SplitClient{ { - VariableName: "group_test__hr_one_split_rule0", + VariableName: "group_test__hr_one_split_rule0_pathRule0", Distributions: []http.SplitClientDistribution{ { Percent: "50.00", @@ -632,7 +632,7 @@ func TestBackendGroupCreateSplitClients(t *testing.T) { }, }, { - VariableName: "group_test__hr_two_splits_rule0", + VariableName: "group_test__hr_two_splits_rule0_pathRule0", Distributions: []http.SplitClientDistribution{ { Percent: "50.00", @@ -645,7 +645,7 @@ func TestBackendGroupCreateSplitClients(t *testing.T) { }, }, { - VariableName: "group_test__hr_two_splits_rule1", + VariableName: "group_test__hr_two_splits_rule1_pathRule0", Distributions: []http.SplitClientDistribution{ { Percent: "33.33", @@ -837,11 +837,12 @@ func TestGetSplitClientValue(t *testing.T) { hrNsName := types.NamespacedName{Namespace: "test", Name: "hr"} tests := []struct { - source types.NamespacedName - msg string - expValue string - backend dataplane.Backend - ruleIdx int + source types.NamespacedName + msg string + expValue string + backend dataplane.Backend + ruleIdx int + pathRuleIdx int }{ { msg: "valid backend", @@ -872,9 +873,10 @@ func TestGetSplitClientValue(t *testing.T) { NsName: "test-namespace", }, }, - source: hrNsName, - ruleIdx: 2, - expValue: "/_ngf-internal-inference-backend-test-hr-rule2", + source: hrNsName, + ruleIdx: 2, + pathRuleIdx: 1, + expValue: "/_ngf-internal-inference-backend-test-hr-routeRule2-pathRule1", }, { msg: "invalid backend with endpoint picker config", @@ -895,7 +897,7 @@ func TestGetSplitClientValue(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := getSplitClientValue(test.backend, test.source, test.ruleIdx) + result := getSplitClientValue(test.backend, test.source, test.ruleIdx, test.pathRuleIdx) g.Expect(result).To(Equal(test.expValue)) }) } @@ -1126,7 +1128,7 @@ func TestBackendGroupName(t *testing.T) { Weight: 1, }, }, - expName: "group_test__hr_rule0", + expName: "group_test__hr_rule0_pathRule0", }, { msg: "multiple invalid backends", @@ -1142,7 +1144,7 @@ func TestBackendGroupName(t *testing.T) { Weight: 1, }, }, - expName: "group_test__hr_rule0", + expName: "group_test__hr_rule0_pathRule0", }, } diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 26bc3c58ee..def60605b6 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -347,8 +347,9 @@ func buildCertBundles( func buildBackendGroups(servers []VirtualServer) []BackendGroup { type key struct { - nsname types.NamespacedName - ruleIdx int + nsname types.NamespacedName + ruleIdx int + pathRuleIdx int } // There can be duplicate backend groups if a route is attached to multiple listeners. @@ -361,8 +362,9 @@ func buildBackendGroups(servers []VirtualServer) []BackendGroup { group := mr.BackendGroup k := key{ - nsname: group.Source, - ruleIdx: group.RuleIdx, + nsname: group.Source, + ruleIdx: group.RuleIdx, + pathRuleIdx: group.PathRuleIdx, } uniqueGroups[k] = group @@ -694,6 +696,12 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { return s.PathRules[i].PathType < s.PathRules[j].PathType }) + for pathRuleIdx := range s.PathRules { + for matchRuleIdx := range s.PathRules[pathRuleIdx].MatchRules { + s.PathRules[pathRuleIdx].MatchRules[matchRuleIdx].BackendGroup.PathRuleIdx = pathRuleIdx + } + } + servers = append(servers, s) } diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 330a0cef79..b9c3bc27e8 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -401,6 +401,19 @@ func assertBuildConfiguration(g *WithT, result, expected Configuration) { } func TestBuildConfiguration(t *testing.T) { + // setPathRuleIdx creates a new BackendGroup with the specified PathRuleIdx + // This is needed because pathRuleIdx cannot be determined in createTestResources when + // the BackendGroup is created, but can only be determined when the VirtualServer's PathRules are + // being defined in the Configuration. + setPathRuleIdx := func(bg BackendGroup, pathRuleIdx int) BackendGroup { + return BackendGroup{ + Source: bg.Source, + RuleIdx: bg.RuleIdx, + PathRuleIdx: pathRuleIdx, + Backends: bg.Backends, + } + } + t.Parallel() fakeResolver := &resolverfakes.FakeServiceResolver{} @@ -669,7 +682,6 @@ func TestBuildConfiguration(t *testing.T) { "listener-8443", pathAndType{path: "/", pathType: prefix}, pathAndType{path: "/third", pathType: prefix}, ) - httpsHR8, expHTTPSHR8Groups, httpsRouteHR8 := createTestResources( "https-hr-8", "foo.example.com", @@ -1257,11 +1269,11 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR3Groups[0], + BackendGroup: setPathRuleIdx(expHR3Groups[0], 0), Source: &hr3.ObjectMeta, }, { - BackendGroup: expHR4Groups[1], + BackendGroup: setPathRuleIdx(expHR4Groups[1], 0), Source: &hr4.ObjectMeta, }, }, @@ -1271,7 +1283,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR4Groups[0], + BackendGroup: setPathRuleIdx(expHR4Groups[0], 1), Source: &hr4.ObjectMeta, }, }, @@ -1281,7 +1293,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR3Groups[1], + BackendGroup: setPathRuleIdx(expHR3Groups[1], 2), Source: &hr3.ObjectMeta, }, }, @@ -1300,11 +1312,11 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR3Groups[0], + BackendGroup: setPathRuleIdx(expHTTPSHR3Groups[0], 0), Source: &httpsHR3.ObjectMeta, }, { - BackendGroup: expHTTPSHR4Groups[1], + BackendGroup: setPathRuleIdx(expHTTPSHR4Groups[1], 0), Source: &httpsHR4.ObjectMeta, }, }, @@ -1314,7 +1326,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR4Groups[0], + BackendGroup: setPathRuleIdx(expHTTPSHR4Groups[0], 1), Source: &httpsHR4.ObjectMeta, }, }, @@ -1324,7 +1336,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR3Groups[1], + BackendGroup: setPathRuleIdx(expHTTPSHR3Groups[1], 2), Source: &httpsHR3.ObjectMeta, }, }, @@ -1340,14 +1352,14 @@ func TestBuildConfiguration(t *testing.T) { }...) conf.Upstreams = append(conf.Upstreams, fooUpstream) conf.BackendGroups = []BackendGroup{ - expHR3Groups[0], - expHR3Groups[1], - expHR4Groups[0], - expHR4Groups[1], - expHTTPSHR3Groups[0], - expHTTPSHR3Groups[1], - expHTTPSHR4Groups[0], - expHTTPSHR4Groups[1], + setPathRuleIdx(expHR3Groups[0], 0), + setPathRuleIdx(expHR3Groups[1], 2), + setPathRuleIdx(expHR4Groups[0], 1), + setPathRuleIdx(expHR4Groups[1], 0), + setPathRuleIdx(expHTTPSHR3Groups[0], 0), + setPathRuleIdx(expHTTPSHR3Groups[1], 2), + setPathRuleIdx(expHTTPSHR4Groups[0], 1), + setPathRuleIdx(expHTTPSHR4Groups[1], 0), } return conf }), @@ -1417,7 +1429,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR3Groups[0], + BackendGroup: setPathRuleIdx(expHR3Groups[0], 0), Source: &hr3.ObjectMeta, }, }, @@ -1427,7 +1439,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR3Groups[1], + BackendGroup: setPathRuleIdx(expHR3Groups[1], 1), Source: &hr3.ObjectMeta, }, }, @@ -1447,7 +1459,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR8Groups[0], + BackendGroup: setPathRuleIdx(expHR8Groups[0], 0), Source: &hr8.ObjectMeta, }, }, @@ -1457,7 +1469,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR8Groups[1], + BackendGroup: setPathRuleIdx(expHR8Groups[1], 1), Source: &hr8.ObjectMeta, }, }, @@ -1476,7 +1488,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR3Groups[0], + BackendGroup: setPathRuleIdx(expHTTPSHR3Groups[0], 0), Source: &httpsHR3.ObjectMeta, }, }, @@ -1486,7 +1498,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR3Groups[1], + BackendGroup: setPathRuleIdx(expHTTPSHR3Groups[1], 1), Source: &httpsHR3.ObjectMeta, }, }, @@ -1512,7 +1524,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR7Groups[0], + BackendGroup: setPathRuleIdx(expHTTPSHR7Groups[0], 0), Source: &httpsHR7.ObjectMeta, }, }, @@ -1522,7 +1534,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHR7Groups[1], + BackendGroup: setPathRuleIdx(expHTTPSHR7Groups[1], 1), Source: &httpsHR7.ObjectMeta, }, }, @@ -1538,14 +1550,14 @@ func TestBuildConfiguration(t *testing.T) { }...) conf.Upstreams = append(conf.Upstreams, fooUpstream) conf.BackendGroups = []BackendGroup{ - expHR3Groups[0], - expHR3Groups[1], - expHR8Groups[0], - expHR8Groups[1], - expHTTPSHR3Groups[0], - expHTTPSHR3Groups[1], - expHTTPSHR7Groups[0], - expHTTPSHR7Groups[1], + setPathRuleIdx(expHR3Groups[0], 0), + setPathRuleIdx(expHR3Groups[1], 1), + setPathRuleIdx(expHR8Groups[0], 0), + setPathRuleIdx(expHR8Groups[1], 1), + setPathRuleIdx(expHTTPSHR3Groups[0], 0), + setPathRuleIdx(expHTTPSHR3Groups[1], 1), + setPathRuleIdx(expHTTPSHR7Groups[0], 0), + setPathRuleIdx(expHTTPSHR7Groups[1], 1), } return conf }), @@ -1609,7 +1621,7 @@ func TestBuildConfiguration(t *testing.T) { MatchRules: []MatchRule{ { Source: &hr5.ObjectMeta, - BackendGroup: expHR5Groups[1], + BackendGroup: setPathRuleIdx(expHR5Groups[1], 1), Filters: HTTPFilters{ InvalidFilter: &InvalidHTTPFilter{}, }, @@ -1622,7 +1634,7 @@ func TestBuildConfiguration(t *testing.T) { }...) conf.SSLServers = []VirtualServer{} conf.Upstreams = []Upstream{fooUpstream} - conf.BackendGroups = []BackendGroup{expHR5Groups[0], expHR5Groups[1]} + conf.BackendGroups = []BackendGroup{expHR5Groups[0], setPathRuleIdx(expHR5Groups[1], 1)} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} return conf }), @@ -1815,7 +1827,7 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHR7Groups[0], + BackendGroup: setPathRuleIdx(expHR7Groups[0], 1), Source: &hr7.ObjectMeta, }, }, @@ -1826,7 +1838,7 @@ func TestBuildConfiguration(t *testing.T) { }...) conf.SSLServers = []VirtualServer{} conf.Upstreams = []Upstream{fooUpstream} - conf.BackendGroups = []BackendGroup{expHR7Groups[0], expHR7Groups[1]} + conf.BackendGroups = []BackendGroup{setPathRuleIdx(expHR7Groups[0], 1), expHR7Groups[1]} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} return conf }), @@ -3323,7 +3335,7 @@ func TestBackendGroupName(t *testing.T) { t.Parallel() backendGroup := createBackendGroup("route1", 2, "foo", "bar") - expectedGroupName := "group_test__route1_rule2" + expectedGroupName := "group_test__route1_rule2_pathRule0" g := NewWithT(t) g.Expect(backendGroup.Name()).To(Equal(expectedGroupName)) diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 1b0ccbde6a..94b0293ff5 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -311,6 +311,8 @@ type BackendGroup struct { Backends []Backend // RuleIdx is the index of the corresponding rule in the HTTPRoute. RuleIdx int + // PathRuleIdx is the index of the corresponding path rule in the HTTPRoute. + PathRuleIdx int } // Name returns the name of the backend group. @@ -321,7 +323,7 @@ type BackendGroup struct { // The RuleIdx may change for a given rule if an update is made to the HTTPRoute, but it will always match the index // of the rule in the stored HTTPRoute. func (bg *BackendGroup) Name() string { - return fmt.Sprintf("group_%s__%s_rule%d", bg.Source.Namespace, bg.Source.Name, bg.RuleIdx) + return fmt.Sprintf("group_%s__%s_rule%d_pathRule%d", bg.Source.Namespace, bg.Source.Name, bg.RuleIdx, bg.PathRuleIdx) } // Backend represents a Backend for a routing rule. From 2114de8d09bb9bc3ca7cffe07a6adc21df07ae6c Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Thu, 11 Dec 2025 23:46:36 -0800 Subject: [PATCH 08/12] Refactor server test --- .../controller/nginx/config/servers_test.go | 85 +++++++------------ 1 file changed, 33 insertions(+), 52 deletions(-) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 85f5063f07..59f4303778 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -2469,21 +2469,36 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { multiInferencePrimaryBackend := createInferenceBackend("test_primary_pool_80", 70, "primary-pool") multiInferenceSecondaryBackend := createInferenceBackend("test_secondary_pool_80", 30, "secondary-pool") - createBackendGroup := func(backends []dataplane.Backend, ruleIdx int, pathRuleIdx int) dataplane.BackendGroup { + createBackendGroup := func(backends []dataplane.Backend) dataplane.BackendGroup { return dataplane.BackendGroup{ - Source: hrNsName, - RuleIdx: ruleIdx, - PathRuleIdx: pathRuleIdx, - Backends: backends, + Source: hrNsName, + Backends: backends, } } - singleInferenceGroup := createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 0, 0) - multipleInferenceGroup := createBackendGroup( - []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, - 0, - 0, - ) + singleInferenceGroup := createBackendGroup([]dataplane.Backend{singleInferenceBackend}) + multipleInferenceGroup := createBackendGroup([]dataplane.Backend{ + multiInferencePrimaryBackend, + multiInferenceSecondaryBackend, + }) + + // setBackendGroupIndices returns a new PathRule with updated RuleIdx and PathRuleIdx in all BackendGroups + setBackendGroupIndices := func(pathRule dataplane.PathRule, ruleIdx int, pathRuleIdx int) dataplane.PathRule { + // Create a copy of the PathRule + newPathRule := pathRule + + // Create a copy of the MatchRules slice + newPathRule.MatchRules = make([]dataplane.MatchRule, len(pathRule.MatchRules)) + copy(newPathRule.MatchRules, pathRule.MatchRules) + + // Update the BackendGroup indices in the copied MatchRules + for matchRuleIdx := range newPathRule.MatchRules { + newPathRule.MatchRules[matchRuleIdx].BackendGroup.RuleIdx = ruleIdx + newPathRule.MatchRules[matchRuleIdx].BackendGroup.PathRuleIdx = pathRuleIdx + } + + return newPathRule + } // Helper function to create path rules createPathRule := func(path string, matchRules []dataplane.MatchRule) dataplane.PathRule { @@ -2553,45 +2568,6 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { }, }) - combinedPathRules := []dataplane.PathRule{ - createPathRule("/inference", []dataplane.MatchRule{ - { - Match: dataplane.Match{}, - BackendGroup: createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 0, 0), - }, - }), - createPathRule("/inference-match", []dataplane.MatchRule{ - { - Match: dataplane.Match{ - Method: helpers.GetPointer("POST"), - }, - BackendGroup: createBackendGroup([]dataplane.Backend{singleInferenceBackend}, 1, 1), - }, - }), - createPathRule("/weighted-inference", []dataplane.MatchRule{ - { - Match: dataplane.Match{}, - BackendGroup: createBackendGroup( - []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, - 2, - 2, - ), - }, - }), - createPathRule("/weighted-inference-match", []dataplane.MatchRule{ - { - Match: dataplane.Match{ - Method: helpers.GetPointer("GET"), - }, - BackendGroup: createBackendGroup( - []dataplane.Backend{multiInferencePrimaryBackend, multiInferenceSecondaryBackend}, - 3, - 3, - ), - }, - }), - } - proxySetHeaders := []http.Header{ {Name: "Host", Value: "$gw_api_compliant_host"}, {Name: "X-Forwarded-For", Value: "$proxy_add_x_forwarded_for"}, @@ -2826,8 +2802,13 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { }, }, { - name: "mixed multiple path rules with different inference configurations", - pathRules: combinedPathRules, + name: "mixed multiple path rules with different inference configurations", + pathRules: []dataplane.PathRule{ + setBackendGroupIndices(pathRuleInferenceOnly, 0, 0), + setBackendGroupIndices(pathRuleInferenceWithMatch, 1, 1), + setBackendGroupIndices(pathRuleMultipleInferenceBackends, 2, 2), + setBackendGroupIndices(pathRuleMultipleInferenceWithMatch, 3, 3), + }, expLocs: []http.Location{ // 1. Single inference pool locations (rule index 0) { From 45283d9584b534a2fe2cfea84dade182c42d7e69 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Fri, 12 Dec 2025 00:02:40 -0800 Subject: [PATCH 09/12] Refactor servers tempRule for deep copy --- internal/controller/nginx/config/servers.go | 26 +++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index bf32f2bb5b..e405924dd4 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -594,8 +594,17 @@ func createInternalLocationsForRule( // we need to update the rule to only have that backend for the location. // This ensures the correct name gets generated to correlate with the split clients generation. // If there is only one backend, this is effectively a no-op. - tempRule := r - tempRule.BackendGroup.Backends = []dataplane.Backend{b} + tempRule := dataplane.MatchRule{ + Source: r.Source, + Match: r.Match, + Filters: r.Filters, + BackendGroup: dataplane.BackendGroup{ + Source: r.BackendGroup.Source, + RuleIdx: r.BackendGroup.RuleIdx, + PathRuleIdx: r.BackendGroup.PathRuleIdx, + Backends: []dataplane.Backend{b}, // Only include the current backend + }, + } intProxyPassLocation = updateLocation( tempRule, rule, @@ -726,8 +735,17 @@ func createInferenceLocationsForRule( // we need to update the rule to only have that backend for the location. // This ensures the correct name gets generated to correlate with the split clients generation. // If there is only one backend, this is effectively a no-op. - tempRule := r - tempRule.BackendGroup.Backends = []dataplane.Backend{b} + tempRule := dataplane.MatchRule{ + Source: r.Source, + Match: r.Match, + Filters: r.Filters, + BackendGroup: dataplane.BackendGroup{ + Source: r.BackendGroup.Source, + RuleIdx: r.BackendGroup.RuleIdx, + PathRuleIdx: r.BackendGroup.PathRuleIdx, + Backends: []dataplane.Backend{b}, // Only include the current backend + }, + } intProxyPassLocation = updateLocation( tempRule, rule, From 3e49d1153e3b9f12f1ad2ab94b15a01fc2a67aba Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Fri, 12 Dec 2025 00:09:48 -0800 Subject: [PATCH 10/12] Update locations comment --- internal/controller/nginx/config/servers.go | 103 +++++++++----------- 1 file changed, 47 insertions(+), 56 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index e405924dd4..28d092decd 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -284,12 +284,12 @@ location /coffee { js_content httpmatches.match; // chooses backend1 or backend2, and redirects to appropriate internal location } location /_ngf-internal-rule0-route0 { - internal; - proxy_pass http://backend1; + internal; + proxy_pass http://backend1; } location /_ngf-internal-rule1-route0 { - internal; - proxy_pass http://backend2; + internal; + proxy_pass http://backend2; } --------------- Inference extension, no HTTP matching conditions. @@ -302,42 +302,41 @@ location /coffee { js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { - internal; - proxy_pass http://$inference-backend; + internal; + proxy_pass http://$inference_backend_test_foo_80; } --------------- Inference extension with multiple inference backends. location /coffee { - rewrite $inference_backend_group_routeNS__routeName_rule0 last; - proxy_http_version 1.1; + rewrite ^ $inference_backend_group_routeNS__routeName_rule0_pathRule0 last; } location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { - internal; - proxy_pass http://$inference-backend0; + internal; + proxy_pass http://$inference_backend_test_primary_pool_80; } -location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { - internal; - set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; - js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference +location /_ngf-internal-test_primary_pool_80-routeNS-routeName-routeRule0-pathRule0 { + internal; + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { - internal; - proxy_pass http://$inference-backend1; + internal; + proxy_pass http://$inference_backend_test_secondary_pool_80; } -location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { - internal; - set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; - js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference +location /_ngf-internal-test_secondary_pool_80-routeNS-routeName-routeRule0-pathRule0 { + internal; + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } -split_clients $request_id $inference_backend_group_routeNS__routeName_rule0 { - 70.00% /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0; - 30.00% /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0; +split_clients $request_id $inference_backend_group_routeNS__routeName_rule0_pathRule0 { + 70.00% /_ngf-internal-test_primary_pool_80-routeNS-routeName-routeRule0-pathRule0; + 30.00% /_ngf-internal-test_secondary_pool_80-routeNS-routeName-routeRule0-pathRule0; } --------------- Inference extension with HTTP matching conditions. @@ -347,23 +346,17 @@ and which backend to use, then redirects to the internal inference location. The location calls the inference NJS module to get the AI endpoint to proxy to, then redirects to the internal location that proxies to the backend. -Note that the location path naming here is a little different than the previous example. -The final location that proxy_passes has the non-inference name to avoid too much refactoring -in the code, and the intermediate location has -inference in the name, whereas in the previous example -it was the final location that had -inference in the name. - location /coffee { - js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location + js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location } -location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { - internal; - - set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; - js_content epp.getEndpoint; // redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference +location /_ngf-internal-test_foo_80-routeNS-routeName-routeRule0-pathRule0 { + internal; + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { - internal; - proxy_pass http://$inference-backend; + internal; + proxy_pass http://$inference_backend_test_foo_80; } --------------- Inference extension with multiple backends with HTTP matching conditions. @@ -374,41 +367,39 @@ location based on a split clients variable. That internal inference location cal to get the AI endpoint to proxy to, then redirects to the internal location that proxies to the backend. location /coffee { - js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location + js_content httpmatches.match; // chooses backend and redirects to appropriate internal inference location } location /_ngf-internal-split-clients-rule0-route0-inference { - internal; - - rewrite $inference_backend_group_routeNS__routeName_rule0 last; - proxy_http_version 1.1; + internal; + rewrite ^ $inference_backend_group_routeNS__routeName_rule0_pathRule0 last; } location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference { - internal; - proxy_pass http://$inference-backend0; + internal; + proxy_pass http://$inference_backend_test_primary_pool_80; } -location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 { - internal; - set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; - js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference +location /_ngf-internal-test_primary_pool_80-routeNS-routeName-routeRule0-pathRule0 { + internal; + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference } location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference { - internal; - proxy_pass http://$inference-backend1; + internal; + proxy_pass http://$inference_backend_test_secondary_pool_80; } -location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 { - internal; - set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; - js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference +location /_ngf-internal-test_secondary_pool_80-routeNS-routeName-routeRule0-pathRule0 { + internal; + set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference; + js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference } -split_clients $request_id $inference_backend_group_routeNS__routeName_rule0 { - 70.00% /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0; - 30.00% /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0; +split_clients $request_id $inference_backend_group_routeNS__routeName_rule0_pathRule0 { + 70.00% /_ngf-internal-test_primary_pool_80-routeNS-routeName-routeRule0-pathRule0; + 30.00% /_ngf-internal-test_secondary_pool_80-routeNS-routeName-routeRule0-pathRule0; } */ From 35fc68ac074f24d17d344ee6646675955ce0d289 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Fri, 12 Dec 2025 00:20:17 -0800 Subject: [PATCH 11/12] Fix includes spacing in template --- internal/controller/nginx/config/servers_template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/servers_template.go b/internal/controller/nginx/config/servers_template.go index 9575b77480..68a53d6a6b 100644 --- a/internal/controller/nginx/config/servers_template.go +++ b/internal/controller/nginx/config/servers_template.go @@ -104,7 +104,7 @@ server { {{- range $i := $l.Includes }} include {{ $i.Name }}; - {{- end -}} + {{- end }} {{ range $r := $l.Rewrites }} rewrite {{ $r }}; From ebae18d52866e94115210b6bb86bbe3917fc3ae5 Mon Sep 17 00:00:00 2001 From: Ben Jee Date: Fri, 12 Dec 2025 10:38:44 -0800 Subject: [PATCH 12/12] Address feedback --- internal/controller/nginx/config/servers.go | 26 ++++++++++++++----- .../controller/nginx/config/split_clients.go | 7 ++--- internal/controller/state/dataplane/types.go | 3 ++- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 28d092decd..578cf5e2b2 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -634,7 +634,7 @@ func createInternalLocationsForRule( } } - // skip adding match and creating split clients location if its a duplicate intEPPLocation.Path + // skip adding match and creating split clients location if it's a duplicate intEPPLocation.Path if skipMatch { continue } @@ -973,12 +973,9 @@ func initializeInternalInferenceEPPLocation( ) http.Location { return http.Location{ // This path needs to be recreated in the split_clients directive generation to match correctly. - Path: fmt.Sprintf( - "%s-%s-%s-%s-routeRule%d-pathRule%d", - http.InternalRoutePathPrefix, + Path: generateInternalInferenceEPPLocationPath( b.UpstreamName, - source.Namespace, - source.Name, + source, ruleIdx, pathruleIdx, ), @@ -986,6 +983,23 @@ func initializeInternalInferenceEPPLocation( } } +func generateInternalInferenceEPPLocationPath( + upstreamName string, + source types.NamespacedName, + ruleIdx int, + pathRuleIdx int, +) string { + return fmt.Sprintf( + "%s-%s-%s-%s-routeRule%d-pathRule%d", + http.InternalRoutePathPrefix, + upstreamName, + source.Namespace, + source.Name, + ruleIdx, + pathRuleIdx, + ) +} + // initializeInternalInferenceProxyPassLocation initializes the internal inference location that does the final // proxy_pass to the inference backend. func initializeInternalInferenceProxyPassLocation(pathruleIdx, matchRuleIdx, backendIdx int) http.Location { diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index 4cda9d54ae..35e7d4c2ab 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -187,12 +187,9 @@ func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) [] func getSplitClientValue(b dataplane.Backend, source types.NamespacedName, ruleIdx, pathRuleIdx int) string { if b.Valid { if b.EndpointPickerConfig != nil { - return fmt.Sprintf( - "%s-%s-%s-%s-routeRule%d-pathRule%d", - http.InternalRoutePathPrefix, + return generateInternalInferenceEPPLocationPath( b.UpstreamName, - source.Namespace, - source.Name, + source, ruleIdx, pathRuleIdx, ) diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 94b0293ff5..9e1beeaa0f 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -311,7 +311,8 @@ type BackendGroup struct { Backends []Backend // RuleIdx is the index of the corresponding rule in the HTTPRoute. RuleIdx int - // PathRuleIdx is the index of the corresponding path rule in the HTTPRoute. + // PathRuleIdx is the index of the corresponding path rule when attached to a VirtualServer. + // BackendGroups attached to a MatchRule that have the same Path match will have the same PathRuleIdx. PathRuleIdx int }