From ae537912871181c5e1f6a55a3f27a0ef9c4755a2 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 11 Nov 2025 18:34:53 -0700 Subject: [PATCH 1/4] add support for ip_hash lb method --- .../controller/nginx/config/http/config.go | 11 +- .../policies/upstreamsettings/processor.go | 6 ++ .../upstreamsettings/processor_test.go | 39 +++++++ .../policies/upstreamsettings/validator.go | 33 ++++++ .../upstreamsettings/validator_test.go | 23 ++++ internal/controller/nginx/config/upstreams.go | 18 +++- .../nginx/config/upstreams_template.go | 4 +- .../controller/nginx/config/upstreams_test.go | 101 ++++++++++++++---- 8 files changed, 203 insertions(+), 32 deletions(-) diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index dedfd04349..2311840a2c 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -119,11 +119,12 @@ const ( // Upstream holds all configuration for an HTTP upstream. type Upstream struct { - Name string - ZoneSize string // format: 512k, 1m - StateFile string - KeepAlive UpstreamKeepAlive - Servers []UpstreamServer + Name string + ZoneSize string // format: 512k, 1m + StateFile string + KeepAlive UpstreamKeepAlive + LoadBalancingMethod string + Servers []UpstreamServer } // UpstreamKeepAlive holds the keepalive configuration for an HTTP upstream. diff --git a/internal/controller/nginx/config/policies/upstreamsettings/processor.go b/internal/controller/nginx/config/policies/upstreamsettings/processor.go index 1a646a45e1..7c29f807c4 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/processor.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/processor.go @@ -13,6 +13,8 @@ type Processor struct{} type UpstreamSettings struct { // ZoneSize is the zone size setting. ZoneSize string + // LoadBalancingMethod is the load balancing method setting. + LoadBalancingMethod string // KeepAlive contains the keepalive settings. KeepAlive http.UpstreamKeepAlive } @@ -61,6 +63,10 @@ func processPolicies(pols []policies.Policy) UpstreamSettings { upstreamSettings.KeepAlive.Timeout = string(*usp.Spec.KeepAlive.Timeout) } } + + if usp.Spec.LoadBalancingMethod != nil { + upstreamSettings.LoadBalancingMethod = string(*usp.Spec.LoadBalancingMethod) + } } return upstreamSettings diff --git a/internal/controller/nginx/config/policies/upstreamsettings/processor_test.go b/internal/controller/nginx/config/policies/upstreamsettings/processor_test.go index a67c1df186..4156781663 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/processor_test.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/processor_test.go @@ -37,6 +37,7 @@ func TestProcess(t *testing.T) { Time: helpers.GetPointer[ngfAPIv1alpha1.Duration]("5s"), Timeout: helpers.GetPointer[ngfAPIv1alpha1.Duration]("10s"), }), + LoadBalancingMethod: helpers.GetPointer(ngfAPIv1alpha1.LoadBalancingTypeIPHash), }, }, }, @@ -48,6 +49,24 @@ func TestProcess(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPIv1alpha1.LoadBalancingTypeIPHash), + }, + }, + { + name: "load balancing method set", + policies: []policies.Policy{ + &ngfAPIv1alpha1.UpstreamSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "usp", + Namespace: "test", + }, + Spec: ngfAPIv1alpha1.UpstreamSettingsPolicySpec{ + LoadBalancingMethod: helpers.GetPointer(ngfAPIv1alpha1.LoadBalancingTypeRandomTwoLeastConnection), + }, + }, + }, + expUpstreamSettings: UpstreamSettings{ + LoadBalancingMethod: string(ngfAPIv1alpha1.LoadBalancingTypeRandomTwoLeastConnection), }, }, { @@ -220,6 +239,15 @@ func TestProcess(t *testing.T) { }), }, }, + &ngfAPIv1alpha1.UpstreamSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "usp-loadBalancingMethod", + Namespace: "test", + }, + Spec: ngfAPIv1alpha1.UpstreamSettingsPolicySpec{ + LoadBalancingMethod: helpers.GetPointer(ngfAPIv1alpha1.LoadBalancingTypeIPHash), + }, + }, }, expUpstreamSettings: UpstreamSettings{ ZoneSize: "2m", @@ -229,6 +257,7 @@ func TestProcess(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPIv1alpha1.LoadBalancingTypeIPHash), }, }, { @@ -310,6 +339,15 @@ func TestProcess(t *testing.T) { }, }, }, + &ngfAPIv1alpha1.UpstreamSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "usp-lb-method", + Namespace: "test", + }, + Spec: ngfAPIv1alpha1.UpstreamSettingsPolicySpec{ + LoadBalancingMethod: helpers.GetPointer(ngfAPIv1alpha1.LoadBalancingTypeIPHash), + }, + }, }, expUpstreamSettings: UpstreamSettings{ ZoneSize: "2m", @@ -319,6 +357,7 @@ func TestProcess(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPIv1alpha1.LoadBalancingTypeIPHash), }, }, } diff --git a/internal/controller/nginx/config/policies/upstreamsettings/validator.go b/internal/controller/nginx/config/policies/upstreamsettings/validator.go index 9b54fb48e2..398fa7f202 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/validator.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/validator.go @@ -83,6 +83,10 @@ func conflicts(a, b ngfAPI.UpstreamSettingsPolicySpec) bool { } } + if a.LoadBalancingMethod != nil && b.LoadBalancingMethod != nil { + return true + } + return false } @@ -103,6 +107,13 @@ func (v Validator) validateSettings(spec ngfAPI.UpstreamSettingsPolicySpec) erro allErrs = append(allErrs, v.validateUpstreamKeepAlive(*spec.KeepAlive, fieldPath.Child("keepAlive"))...) } + if spec.LoadBalancingMethod != nil { + allErrs = append( + allErrs, + v.validateLoadBalancingMethod(*spec.LoadBalancingMethod, fieldPath.Child("loadBalancingMethod"))..., + ) + } + return allErrs.ToAggregate() } @@ -130,3 +141,25 @@ func (v Validator) validateUpstreamKeepAlive( return allErrs } + +func (v Validator) validateLoadBalancingMethod( + method ngfAPI.LoadBalancingType, + fieldPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + switch method { + case ngfAPI.LoadBalancingTypeIPHash, ngfAPI.LoadBalancingTypeRandomTwoLeastConnection: + default: + allErrs = append(allErrs, field.NotSupported( + fieldPath, + method, + []string{ + string(ngfAPI.LoadBalancingTypeIPHash), + string(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), + }, + )) + } + + return allErrs +} diff --git a/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go b/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go index df4c4e0770..54db7ddb46 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go @@ -38,6 +38,7 @@ func createValidPolicy() *ngfAPI.UpstreamSettingsPolicy { Timeout: helpers.GetPointer[ngfAPI.Duration]("30s"), Connections: helpers.GetPointer[int32](100), }, + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), }, Status: v1.PolicyStatus{}, } @@ -117,6 +118,17 @@ func TestValidator_Validate(t *testing.T) { "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h'')]"), }, }, + { + name: "invalid load balancing method", + policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy { + p.Spec.LoadBalancingMethod = helpers.GetPointer[ngfAPI.LoadBalancingType]("invalid-lb-method") + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.loadBalancingMethod: Unsupported value: \"invalid-lb-method\": " + + "supported values: \"ip_hash\", \"random two least_conn\""), + }, + }, { name: "valid", policy: createValidPolicy(), @@ -176,6 +188,7 @@ func TestValidator_Conflicts(t *testing.T) { Requests: helpers.GetPointer[int32](900), Time: helpers.GetPointer[ngfAPI.Duration]("50s"), }, + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), }, }, polB: &ngfAPI.UpstreamSettingsPolicy{ @@ -246,6 +259,16 @@ func TestValidator_Conflicts(t *testing.T) { }, conflicts: true, }, + { + name: "load balancing method conflicts", + polA: createValidPolicy(), + polB: &ngfAPI.UpstreamSettingsPolicy{ + Spec: ngfAPI.UpstreamSettingsPolicySpec{ + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeIPHash), + }, + }, + conflicts: true, + }, } v := upstreamsettings.NewValidator(nil) diff --git a/internal/controller/nginx/config/upstreams.go b/internal/controller/nginx/config/upstreams.go index 4b7e6bd16f..6397036484 100644 --- a/internal/controller/nginx/config/upstreams.go +++ b/internal/controller/nginx/config/upstreams.go @@ -32,6 +32,8 @@ const ( plusZoneSizeStream = "1m" // stateDir is the directory for storing state files. stateDir = "/var/lib/nginx/state" + // default load balancing method. + randomTwoLeastConnLB = "random two least_conn" ) // keepAliveChecker takes an upstream name and returns if it has keep alive settings enabled. @@ -185,12 +187,18 @@ func (g GeneratorImpl) createUpstream( } } + chosenLBMethod := randomTwoLeastConnLB + if upstreamPolicySettings.LoadBalancingMethod != "" { + chosenLBMethod = upstreamPolicySettings.LoadBalancingMethod + } + return http.Upstream{ - Name: up.Name, - ZoneSize: zoneSize, - StateFile: stateFile, - Servers: upstreamServers, - KeepAlive: upstreamPolicySettings.KeepAlive, + Name: up.Name, + ZoneSize: zoneSize, + StateFile: stateFile, + Servers: upstreamServers, + KeepAlive: upstreamPolicySettings.KeepAlive, + LoadBalancingMethod: chosenLBMethod, } } diff --git a/internal/controller/nginx/config/upstreams_template.go b/internal/controller/nginx/config/upstreams_template.go index 15e9b0c1fc..b3af0cad2e 100644 --- a/internal/controller/nginx/config/upstreams_template.go +++ b/internal/controller/nginx/config/upstreams_template.go @@ -10,7 +10,9 @@ package config const upstreamsTemplateText = ` {{ range $u := . }} upstream {{ $u.Name }} { - random two least_conn; + {{ if $u.LoadBalancingMethod -}} + {{ $u.LoadBalancingMethod }}; + {{- end }} {{ if $u.ZoneSize -}} zone {{ $u.Name }} {{ $u.ZoneSize }}; {{ end -}} diff --git a/internal/controller/nginx/config/upstreams_test.go b/internal/controller/nginx/config/upstreams_test.go index c13904d25d..8ded9991b9 100644 --- a/internal/controller/nginx/config/upstreams_test.go +++ b/internal/controller/nginx/config/upstreams_test.go @@ -1,6 +1,8 @@ package config import ( + "fmt" + "strings" "testing" . "github.com/onsi/gomega" @@ -75,31 +77,35 @@ func TestExecuteUpstreams(t *testing.T) { Time: helpers.GetPointer[ngfAPI.Duration]("5s"), Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"), }), + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeIPHash), }, }, }, }, } - expectedSubStrings := []string{ - "upstream up1", - "upstream up2", - "upstream up3", - "upstream up4-ipv6", - "upstream up5-usp", - "upstream invalid-backend-ref", + expectedSubStrings := map[string]int{ + "upstream up1": 1, + "upstream up2": 1, + "upstream up3": 1, + "upstream up4-ipv6": 1, + "upstream up5-usp": 1, + "upstream invalid-backend-ref": 1, - "server 10.0.0.0:80;", - "server 11.0.0.0:80;", - "server [2001:db8::1]:80", - "server 12.0.0.0:80;", - "server unix:/var/run/nginx/nginx-503-server.sock;", + "server 10.0.0.0:80;": 1, + "server 11.0.0.0:80;": 1, + "server [2001:db8::1]:80": 1, + "server 12.0.0.0:80;": 1, + "server unix:/var/run/nginx/nginx-503-server.sock;": 1, - "keepalive 1;", - "keepalive_requests 1;", - "keepalive_time 5s;", - "keepalive_timeout 10s;", - "zone up5-usp 2m;", + "keepalive 1;": 1, + "keepalive_requests 1;": 1, + "keepalive_time 5s;": 1, + "keepalive_timeout 10s;": 1, + "zone up5-usp 2m;": 1, + "ip_hash;": 1, + + "random two least_conn;": 3, } upstreams := gen.createUpstreams(stateUpstreams, upstreamsettings.NewProcessor()) @@ -107,11 +113,15 @@ func TestExecuteUpstreams(t *testing.T) { upstreamResults := executeUpstreams(upstreams) g := NewWithT(t) g.Expect(upstreamResults).To(HaveLen(1)) - nginxUpstreams := string(upstreamResults[0].data) - g.Expect(upstreamResults[0].dest).To(Equal(httpConfigFile)) - for _, expSubString := range expectedSubStrings { - g.Expect(nginxUpstreams).To(ContainSubstring(expSubString)) + + nginxUpstreams := string(upstreamResults[0].data) + for expSubString, expectedCount := range expectedSubStrings { + actualCount := strings.Count(nginxUpstreams, expSubString) + g.Expect(actualCount).To( + Equal(expectedCount), + fmt.Sprintf("substring %q expected %d occurrence(s), got %d", expSubString, expectedCount, actualCount), + ) } } @@ -181,6 +191,7 @@ func TestCreateUpstreams(t *testing.T) { Time: helpers.GetPointer[ngfAPI.Duration]("5s"), Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"), }), + LoadBalancingMethod: helpers.GetPointer((ngfAPI.LoadBalancingTypeIPHash)), }, }, }, @@ -202,6 +213,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "10.0.0.2:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, { Name: "up2", @@ -211,6 +223,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "11.0.0.0:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, { Name: "up3", @@ -229,6 +242,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, { Name: "up5-usp", @@ -244,6 +258,7 @@ func TestCreateUpstreams(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPI.LoadBalancingTypeIPHash), }, { Name: invalidBackendRef, @@ -332,6 +347,7 @@ func TestCreateUpstream(t *testing.T) { Address: "10.0.0.3:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "multiple endpoints", }, @@ -354,6 +370,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "endpoint ipv6", }, @@ -380,6 +397,7 @@ func TestCreateUpstream(t *testing.T) { Time: helpers.GetPointer[ngfAPI.Duration]("5s"), Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"), }), + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeIPHash), }, }, }, @@ -398,6 +416,7 @@ func TestCreateUpstream(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPI.LoadBalancingTypeIPHash), }, msg: "single upstreamSettingsPolicy", }, @@ -422,6 +441,7 @@ func TestCreateUpstream(t *testing.T) { Time: helpers.GetPointer[ngfAPI.Duration]("5s"), Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"), }), + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), }, }, &ngfAPI.UpstreamSettingsPolicy{ @@ -452,6 +472,7 @@ func TestCreateUpstream(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: string(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), }, msg: "multiple upstreamSettingsPolicies", }, @@ -481,6 +502,7 @@ func TestCreateUpstream(t *testing.T) { Address: "10.0.0.1:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "empty upstreamSettingsPolicies", }, @@ -524,9 +546,43 @@ func TestCreateUpstream(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "upstreamSettingsPolicy with only keep alive settings", }, + { + stateUpstream: dataplane.Upstream{ + Name: "upstreamSettingsPolicy with only load balancing settings", + Endpoints: []resolver.Endpoint{ + { + Address: "11.0.20.9", + Port: 80, + }, + }, + Policies: []policies.Policy{ + &ngfAPI.UpstreamSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "usp1", + Namespace: "test", + }, + Spec: ngfAPI.UpstreamSettingsPolicySpec{ + LoadBalancingMethod: helpers.GetPointer(ngfAPI.LoadBalancingTypeIPHash), + }, + }, + }, + }, + expectedUpstream: http.Upstream{ + Name: "upstreamSettingsPolicy with only load balancing settings", + ZoneSize: ossZoneSize, + Servers: []http.UpstreamServer{ + { + Address: "11.0.20.9:80", + }, + }, + LoadBalancingMethod: string(ngfAPI.LoadBalancingTypeIPHash), + }, + msg: "upstreamSettingsPolicy with only load balancing settings", + }, { stateUpstream: dataplane.Upstream{ Name: "external-name-service", @@ -547,6 +603,7 @@ func TestCreateUpstream(t *testing.T) { Resolve: true, }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "ExternalName service with DNS name", }, @@ -585,6 +642,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, msg: "mixed IP addresses and DNS names", }, @@ -629,6 +687,7 @@ func TestCreateUpstreamPlus(t *testing.T) { Address: "10.0.0.1:80", }, }, + LoadBalancingMethod: randomTwoLeastConnLB, }, }, { From 963115d66a1481ae84875d28d26ebf6eacef3abd Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Wed, 12 Nov 2025 11:43:04 -0700 Subject: [PATCH 2/4] update variable name and improve fieldalignment in struct --- .../controller/nginx/config/http/config.go | 4 ++-- internal/controller/nginx/config/upstreams.go | 4 ++-- .../controller/nginx/config/upstreams_test.go | 22 +++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index 2311840a2c..1bcaf4f773 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -120,10 +120,10 @@ const ( // Upstream holds all configuration for an HTTP upstream. type Upstream struct { Name string - ZoneSize string // format: 512k, 1m + ZoneSize string StateFile string - KeepAlive UpstreamKeepAlive LoadBalancingMethod string + KeepAlive UpstreamKeepAlive Servers []UpstreamServer } diff --git a/internal/controller/nginx/config/upstreams.go b/internal/controller/nginx/config/upstreams.go index 6397036484..bf56ec052d 100644 --- a/internal/controller/nginx/config/upstreams.go +++ b/internal/controller/nginx/config/upstreams.go @@ -33,7 +33,7 @@ const ( // stateDir is the directory for storing state files. stateDir = "/var/lib/nginx/state" // default load balancing method. - randomTwoLeastConnLB = "random two least_conn" + defaultLBMethod = "random two least_conn" ) // keepAliveChecker takes an upstream name and returns if it has keep alive settings enabled. @@ -187,7 +187,7 @@ func (g GeneratorImpl) createUpstream( } } - chosenLBMethod := randomTwoLeastConnLB + chosenLBMethod := defaultLBMethod if upstreamPolicySettings.LoadBalancingMethod != "" { chosenLBMethod = upstreamPolicySettings.LoadBalancingMethod } diff --git a/internal/controller/nginx/config/upstreams_test.go b/internal/controller/nginx/config/upstreams_test.go index 8ded9991b9..c87761b35f 100644 --- a/internal/controller/nginx/config/upstreams_test.go +++ b/internal/controller/nginx/config/upstreams_test.go @@ -213,7 +213,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "10.0.0.2:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, { Name: "up2", @@ -223,7 +223,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "11.0.0.0:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, { Name: "up3", @@ -242,7 +242,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, { Name: "up5-usp", @@ -347,7 +347,7 @@ func TestCreateUpstream(t *testing.T) { Address: "10.0.0.3:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "multiple endpoints", }, @@ -370,7 +370,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "endpoint ipv6", }, @@ -502,7 +502,7 @@ func TestCreateUpstream(t *testing.T) { Address: "10.0.0.1:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "empty upstreamSettingsPolicies", }, @@ -546,7 +546,7 @@ func TestCreateUpstream(t *testing.T) { Time: "5s", Timeout: "10s", }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "upstreamSettingsPolicy with only keep alive settings", }, @@ -603,7 +603,7 @@ func TestCreateUpstream(t *testing.T) { Resolve: true, }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "ExternalName service with DNS name", }, @@ -642,7 +642,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, msg: "mixed IP addresses and DNS names", }, @@ -663,9 +663,9 @@ func TestCreateUpstreamPlus(t *testing.T) { gen := GeneratorImpl{plus: true} tests := []struct { + expectedUpstream http.Upstream msg string stateUpstream dataplane.Upstream - expectedUpstream http.Upstream }{ { msg: "with endpoints", @@ -687,7 +687,7 @@ func TestCreateUpstreamPlus(t *testing.T) { Address: "10.0.0.1:80", }, }, - LoadBalancingMethod: randomTwoLeastConnLB, + LoadBalancingMethod: defaultLBMethod, }, }, { From d113907ea79cfb36f20d2cdbe82bf1b1b6218836 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Wed, 12 Nov 2025 13:37:26 -0700 Subject: [PATCH 3/4] add struct field comment --- internal/controller/nginx/config/http/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index 1bcaf4f773..14af2c8ca7 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -120,7 +120,7 @@ const ( // Upstream holds all configuration for an HTTP upstream. type Upstream struct { Name string - ZoneSize string + ZoneSize string // format: 512k, 1m StateFile string LoadBalancingMethod string KeepAlive UpstreamKeepAlive From 03c61d2df155ecfe77ec382483ce00bfbfdabe84 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Wed, 12 Nov 2025 17:40:53 -0700 Subject: [PATCH 4/4] remove validation checks for spec changes --- .../policies/upstreamsettings/validator.go | 29 ------------------- .../upstreamsettings/validator_test.go | 11 ------- 2 files changed, 40 deletions(-) diff --git a/internal/controller/nginx/config/policies/upstreamsettings/validator.go b/internal/controller/nginx/config/policies/upstreamsettings/validator.go index 398fa7f202..d3b235a3ba 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/validator.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/validator.go @@ -107,13 +107,6 @@ func (v Validator) validateSettings(spec ngfAPI.UpstreamSettingsPolicySpec) erro allErrs = append(allErrs, v.validateUpstreamKeepAlive(*spec.KeepAlive, fieldPath.Child("keepAlive"))...) } - if spec.LoadBalancingMethod != nil { - allErrs = append( - allErrs, - v.validateLoadBalancingMethod(*spec.LoadBalancingMethod, fieldPath.Child("loadBalancingMethod"))..., - ) - } - return allErrs.ToAggregate() } @@ -141,25 +134,3 @@ func (v Validator) validateUpstreamKeepAlive( return allErrs } - -func (v Validator) validateLoadBalancingMethod( - method ngfAPI.LoadBalancingType, - fieldPath *field.Path, -) field.ErrorList { - var allErrs field.ErrorList - - switch method { - case ngfAPI.LoadBalancingTypeIPHash, ngfAPI.LoadBalancingTypeRandomTwoLeastConnection: - default: - allErrs = append(allErrs, field.NotSupported( - fieldPath, - method, - []string{ - string(ngfAPI.LoadBalancingTypeIPHash), - string(ngfAPI.LoadBalancingTypeRandomTwoLeastConnection), - }, - )) - } - - return allErrs -} diff --git a/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go b/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go index 54db7ddb46..1bae51e5bb 100644 --- a/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go +++ b/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go @@ -118,17 +118,6 @@ func TestValidator_Validate(t *testing.T) { "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h'')]"), }, }, - { - name: "invalid load balancing method", - policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy { - p.Spec.LoadBalancingMethod = helpers.GetPointer[ngfAPI.LoadBalancingType]("invalid-lb-method") - return p - }), - expConditions: []conditions.Condition{ - conditions.NewPolicyInvalid("spec.loadBalancingMethod: Unsupported value: \"invalid-lb-method\": " + - "supported values: \"ip_hash\", \"random two least_conn\""), - }, - }, { name: "valid", policy: createValidPolicy(),