From 7863f8101900b74c94e7dadc31f0a0fc0b5e442d Mon Sep 17 00:00:00 2001 From: Harry Turton Date: Fri, 10 May 2024 04:53:18 +1000 Subject: [PATCH] feat(translator): Implement header hash policy for consistent hash load balancers (#3357) * Map consistent hash configuration Signed-off-by: Harry Turton * Implement mapping and add tests Signed-off-by: Harry Turton * Small refactors, add yaml keys Signed-off-by: Harry Turton * Add documentation and lints Signed-off-by: Harry Turton * Add additional tests Signed-off-by: Harry Turton --------- Signed-off-by: Harry Turton --- api/v1alpha1/loadbalancer_types.go | 3 +- ....envoyproxy.io_backendtrafficpolicies.yaml | 3 +- internal/gatewayapi/backendtrafficpolicy.go | 23 +++-- internal/ir/xds.go | 8 +- internal/ir/xds_test.go | 14 +++- internal/ir/zz_generated.deepcopy.go | 5 ++ internal/xds/translator/listener_test.go | 2 +- internal/xds/translator/route.go | 19 ++++- internal/xds/translator/route_test.go | 84 +++++++++++++++++++ .../testdata/in/xds-ir/load-balancer.yaml | 12 +++ .../out/xds-ir/load-balancer.clusters.yaml | 17 ++++ .../out/xds-ir/load-balancer.endpoints.yaml | 12 +++ .../out/xds-ir/load-balancer.routes.yaml | 10 +++ site/content/en/latest/api/extension_types.md | 3 +- 14 files changed, 199 insertions(+), 16 deletions(-) create mode 100644 internal/xds/translator/route_test.go diff --git a/api/v1alpha1/loadbalancer_types.go b/api/v1alpha1/loadbalancer_types.go index 2a0fb21e515..795390bb525 100644 --- a/api/v1alpha1/loadbalancer_types.go +++ b/api/v1alpha1/loadbalancer_types.go @@ -57,7 +57,7 @@ const ( // // +kubebuilder:validation:XValidation:rule="self.type == 'Header' ? has(self.header) : !has(self.header)",message="If consistent hash type is header, the header field must be set." type ConsistentHash struct { - // Valid Type values are "SourceIP". + // ConsistentHashType defines the type of input to hash on. Valid Type values are "SourceIP" or "Header". // // +unionDiscriminator Type ConsistentHashType `json:"type"` @@ -65,7 +65,6 @@ type ConsistentHash struct { // Header configures the header hash policy when the consistent hash type is set to Header. // // +optional - // +notImplementedHide Header *Header `json:"header,omitempty"` } diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index ba195c65408..6f5b369ce73 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -423,7 +423,8 @@ spec: - name type: object type: - description: Valid Type values are "SourceIP". + description: ConsistentHashType defines the type of input + to hash on. Valid Type values are "SourceIP" or "Header". enum: - SourceIP - Header diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 37d61a2a41d..852cc201d28 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -764,11 +764,7 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir. switch policy.Spec.LoadBalancer.Type { case egv1a1.ConsistentHashLoadBalancerType: lb = &ir.LoadBalancer{ - ConsistentHash: &ir.ConsistentHash{}, - } - if policy.Spec.LoadBalancer.ConsistentHash != nil && - policy.Spec.LoadBalancer.ConsistentHash.Type == egv1a1.SourceIPConsistentHashType { - lb.ConsistentHash.SourceIP = ptr.To(true) + ConsistentHash: t.buildConsistentHashLoadBalancer(policy), } case egv1a1.LeastRequestLoadBalancerType: lb = &ir.LoadBalancer{} @@ -805,6 +801,23 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir. return lb } +func (t *Translator) buildConsistentHashLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir.ConsistentHash { + switch policy.Spec.LoadBalancer.ConsistentHash.Type { + case egv1a1.SourceIPConsistentHashType: + return &ir.ConsistentHash{ + SourceIP: ptr.To(true), + } + case egv1a1.HeaderConsistentHashType: + return &ir.ConsistentHash{ + Header: &ir.Header{ + Name: policy.Spec.LoadBalancer.ConsistentHash.Header.Name, + }, + } + default: + return &ir.ConsistentHash{} + } +} + func (t *Translator) buildProxyProtocol(policy *egv1a1.BackendTrafficPolicy) *ir.ProxyProtocol { var pp *ir.ProxyProtocol switch policy.Spec.ProxyProtocol.Version { diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 02f3f448aee..cc6adecbebb 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -1575,7 +1575,13 @@ type Random struct{} // +k8s:deepcopy-gen=true type ConsistentHash struct { // Hash based on the Source IP Address - SourceIP *bool `json:"sourceIP,omitempty" yaml:"sourceIP,omitempty"` + SourceIP *bool `json:"sourceIP,omitempty" yaml:"sourceIP,omitempty"` + Header *Header `json:"header,omitempty" yaml:"header,omitempty"` +} + +// Header consistent hash type settings +type Header struct { + Name string `json:"name" yaml:"name"` } type ProxyProtocolVersion string diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go index ab42d763711..7251c7cb0f1 100644 --- a/internal/ir/xds_test.go +++ b/internal/ir/xds_test.go @@ -1243,7 +1243,7 @@ func TestValidateLoadBalancer(t *testing.T) { want: nil, }, { - name: "consistent hash", + name: "consistent hash with source IP hash policy", input: LoadBalancer{ ConsistentHash: &ConsistentHash{ SourceIP: ptr.To(true), @@ -1251,7 +1251,17 @@ func TestValidateLoadBalancer(t *testing.T) { }, want: nil, }, - + { + name: "consistent hash with header hash policy", + input: LoadBalancer{ + ConsistentHash: &ConsistentHash{ + Header: &Header{ + Name: "name", + }, + }, + }, + want: nil, + }, { name: "least request and random set", input: LoadBalancer{ diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index f400de7019c..584a4a588b1 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -358,6 +358,11 @@ func (in *ConsistentHash) DeepCopyInto(out *ConsistentHash) { *out = new(bool) **out = **in } + if in.Header != nil { + in, out := &in.Header, &out.Header + *out = new(Header) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConsistentHash. diff --git a/internal/xds/translator/listener_test.go b/internal/xds/translator/listener_test.go index 97512fae5ae..28572bb06be 100644 --- a/internal/xds/translator/listener_test.go +++ b/internal/xds/translator/listener_test.go @@ -64,7 +64,7 @@ func Test_buildTCPProxyHashPolicy(t *testing.T) { want: nil, }, { - name: "ConsistentHash without SourceIP", + name: "ConsistentHash without hash policy", lb: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{}}, want: nil, }, diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index fb894d66105..a52371103f0 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -440,7 +440,20 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy return nil } - if httpRoute.LoadBalancer.ConsistentHash.SourceIP != nil && *httpRoute.LoadBalancer.ConsistentHash.SourceIP { + switch { + case httpRoute.LoadBalancer.ConsistentHash.Header != nil: + hashPolicy := &routev3.RouteAction_HashPolicy{ + PolicySpecifier: &routev3.RouteAction_HashPolicy_Header_{ + Header: &routev3.RouteAction_HashPolicy_Header{ + HeaderName: httpRoute.LoadBalancer.ConsistentHash.Header.Name, + }, + }, + } + return []*routev3.RouteAction_HashPolicy{hashPolicy} + case httpRoute.LoadBalancer.ConsistentHash.SourceIP != nil: + if !*httpRoute.LoadBalancer.ConsistentHash.SourceIP { + return nil + } hashPolicy := &routev3.RouteAction_HashPolicy{ PolicySpecifier: &routev3.RouteAction_HashPolicy_ConnectionProperties_{ ConnectionProperties: &routev3.RouteAction_HashPolicy_ConnectionProperties{ @@ -449,9 +462,9 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy }, } return []*routev3.RouteAction_HashPolicy{hashPolicy} + default: + return nil } - - return nil } func buildRetryPolicy(route *ir.HTTPRoute) (*routev3.RetryPolicy, error) { diff --git a/internal/xds/translator/route_test.go b/internal/xds/translator/route_test.go new file mode 100644 index 00000000000..ed9198b6c01 --- /dev/null +++ b/internal/xds/translator/route_test.go @@ -0,0 +1,84 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package translator + +import ( + "reflect" + "testing" + + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + "k8s.io/utils/ptr" + + "github.com/envoyproxy/gateway/internal/ir" +) + +func Test_buildHashPolicy(t *testing.T) { + tests := []struct { + name string + httpRoute *ir.HTTPRoute + want []*routev3.RouteAction_HashPolicy + }{ + { + name: "Nil HttpRoute", + httpRoute: nil, + want: nil, + }, + { + name: "Nil LoadBalancer in HttpRoute", + httpRoute: &ir.HTTPRoute{}, + want: nil, + }, + { + name: "Nil ConsistentHash in LoadBalancer", + httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{}}, + want: nil, + }, + { + name: "ConsistentHash with nil SourceIP and Header", + httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{}}}, + want: nil, + }, + { + name: "ConsistentHash with SourceIP set to false", + httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{SourceIP: ptr.To(false)}}}, + want: nil, + }, + { + name: "ConsistentHash with SourceIP set to true", + httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{SourceIP: ptr.To(true)}}}, + want: []*routev3.RouteAction_HashPolicy{ + { + PolicySpecifier: &routev3.RouteAction_HashPolicy_ConnectionProperties_{ + ConnectionProperties: &routev3.RouteAction_HashPolicy_ConnectionProperties{ + SourceIp: true, + }, + }, + }, + }, + }, + { + name: "ConsistentHash with Header", + httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{Header: &ir.Header{Name: "name"}}}}, + want: []*routev3.RouteAction_HashPolicy{ + { + PolicySpecifier: &routev3.RouteAction_HashPolicy_Header_{ + Header: &routev3.RouteAction_HashPolicy_Header{ + HeaderName: "name", + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildHashPolicy(tt.httpRoute) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("buildHashPolicy() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml b/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml index fbf13ae8865..9c76ca46bff 100644 --- a/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml @@ -73,3 +73,15 @@ http: - endpoints: - host: "1.2.3.4" port: 50000 + - name: "seventh-route" + hostname: "*" + loadBalancer: + consistentHash: + header: + name: name + destination: + name: "seventh-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml index fc755fed368..d9fedf2c8d3 100644 --- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml @@ -104,3 +104,20 @@ slowStartConfig: slowStartWindow: 300s type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: seventh-route-dest + lbPolicy: MAGLEV + name: seventh-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml index ee22b49d2b2..0860c1852a5 100644 --- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml @@ -70,3 +70,15 @@ loadBalancingWeight: 1 locality: region: sixth-route-dest/backend/0 +- clusterName: seventh-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: seventh-route-dest/backend/0 diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml index 49db596538b..080b9aaff0a 100644 --- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml @@ -50,3 +50,13 @@ cluster: sixth-route-dest upgradeConfigs: - upgradeType: websocket + - match: + prefix: / + name: seventh-route + route: + cluster: seventh-route-dest + hashPolicy: + - header: + headerName: name + upgradeConfigs: + - upgradeType: websocket diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index f99e636a2d8..bc85633137c 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -562,7 +562,8 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `type` | _[ConsistentHashType](#consistenthashtype)_ | true | Valid Type values are "SourceIP". | +| `type` | _[ConsistentHashType](#consistenthashtype)_ | true | ConsistentHashType defines the type of input to hash on. Valid Type values are "SourceIP" or "Header". | +| `header` | _[Header](#header)_ | false | Header configures the header hash policy when the consistent hash type is set to Header. | #### ConsistentHashType