diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index dedfd04349..14af2c8ca7 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 + LoadBalancingMethod string + KeepAlive UpstreamKeepAlive + 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..d3b235a3ba 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 } diff --git a/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go b/internal/controller/nginx/config/policies/upstreamsettings/validator_test.go index df4c4e0770..1bae51e5bb 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{}, } @@ -176,6 +177,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 +248,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..bf56ec052d 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. + defaultLBMethod = "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 := defaultLBMethod + 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..c87761b35f 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: defaultLBMethod, }, { Name: "up2", @@ -211,6 +223,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "11.0.0.0:80", }, }, + LoadBalancingMethod: defaultLBMethod, }, { Name: "up3", @@ -229,6 +242,7 @@ func TestCreateUpstreams(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: defaultLBMethod, }, { 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: defaultLBMethod, }, msg: "multiple endpoints", }, @@ -354,6 +370,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: defaultLBMethod, }, 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: defaultLBMethod, }, msg: "empty upstreamSettingsPolicies", }, @@ -524,9 +546,43 @@ func TestCreateUpstream(t *testing.T) { Time: "5s", Timeout: "10s", }, + LoadBalancingMethod: defaultLBMethod, }, 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: defaultLBMethod, }, msg: "ExternalName service with DNS name", }, @@ -585,6 +642,7 @@ func TestCreateUpstream(t *testing.T) { Address: "[fd00:10:244:1::7]:80", }, }, + LoadBalancingMethod: defaultLBMethod, }, msg: "mixed IP addresses and DNS names", }, @@ -605,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", @@ -629,6 +687,7 @@ func TestCreateUpstreamPlus(t *testing.T) { Address: "10.0.0.1:80", }, }, + LoadBalancingMethod: defaultLBMethod, }, }, {