feat(ingress gateway: support configuring limits in ingress-gateway c… #14749

huikang · 2022-09-26T14:20:02Z

Description

As a user of ingress gateway, I want to increase the max_connections of the service in the ingress-gateway config entry (link to the detailed description). However, in current implementation, max_coonections can’t be changed and is set to the default value of 1024 by envoy.

This PR adds a new field named Defaults to the ingress-gateway config entry and max_xyz to individual listening services. The new fields allow user to configure the max_xyz of the upstream cluster of a specified service behind the ingress gateway.

updated unit test and integration test
updated external-facing doc

Fixes #13374

Testing & Reproduction steps

A full example is verified in the integration test (test/integration/connect/envoy/case-ingress-gateway-multiple-services/config_entries.hcl).

Kind = "ingress-gateway"
Name = "my-ingress-gateway"

Defaults {
    MaxConnections = 2048          // <----- increase the max_connections of all upstream service instances
}
Listeners = [
 {
   Port = 8082
   Protocol = "http"
   Services = [
     {
        Name = "counting"
        Hosts = ["*"]
        MaxConnections = 5120          // <----- increase the max_connections, overwrite the value in Defaults
     },
     {
       Name = "counting-v2"
       Hosts = ["v2"]
     }
   ]
 }
]

envoy cluster

counting.default.dc2.internal.ca9229ae-1b1d-af9d-47fa-43885f09557a.consul::default_priority::max_connections::5120
counting.default.dc2.internal.ca9229ae-1b1d-af9d-47fa-43885f09557a.consul::default_priority::max_pending_requests::1024
counting.default.dc2.internal.ca9229ae-1b1d-af9d-47fa-43885f09557a.consul::default_priority::max_requests::1024
counting.default.dc2.internal.ca9229ae-1b1d-af9d-47fa-43885f09557a.consul::default_priority::max_retries::3

Links

Address #13374
Supersede #14708

Please be mindful not to leak any customer or confidential information. HashiCorp employees may want to use our internal URL shortener to obfuscate links.

PR Checklist

updated test coverage
external facing docs updated
not a security concern

DanStough

I had some editorial comments we could discuss but overall looks great!

agent/structs/config_entry_gateways.go

website/content/docs/connect/config-entries/ingress-gateway.mdx

DanStough · 2022-09-27T14:57:45Z

website/content/docs/connect/config-entries/ingress-gateway.mdx

+        {
+          name: 'MaxConnections',
+          type: 'int: 0',
+          description: `The maximum number of connections a service instance
+              will be allowed to establish against the given upstream. Use this to limit
+              HTTP/1.1 traffic, since HTTP/1.1 has a request per connection.`,
+        },
+        {
+          name: 'MaxPendingRequests',
+          type: 'int: 0',
+          description: `The maximum number of requests that will be queued
+              while waiting for a connection to be established. For this configuration to
+              be respected, a L7 protocol must be defined in the \`protocol\` field.`,
+        },
+        {
+          name: 'MaxConcurrentRequests',
+          type: 'int: 0',
+          description: `The maximum number of concurrent requests that
+              will be allowed at a single point in time. Use this to limit HTTP/2 traffic,
+              since HTTP/2 has many requests per connection. For this configuration to be
+              respected, a L7 protocol must be defined in the \`protocol\` field.`,
+        },


Should we mention the default values?

Added in the following commit

DanStough · 2022-09-27T14:58:50Z

website/content/docs/connect/config-entries/ingress-gateway.mdx

+            {
+              name: 'MaxConnections',
+              type: 'int: 0',
+              description: overrides for `Defaults` configuration,


I would provide a link to these configuration items.

Added in the following commit

api/config_entry_gateways.go

DanStough · 2022-09-27T15:13:30Z

agent/structs/config_entry_gateways.go

+	MaxConnections        int32
+	MaxPendingRequests    int32
+	MaxConcurrentRequests int32


uint32 would be more accurate per envoy, correct?

api/config_entry_gateways_test.go

…onfig entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]>

Co-authored-by: Dan Stough <[email protected]>

DanStough

Awesome! 🚀

david-yu · 2022-09-28T18:16:19Z

@huikang This will also need a PR on consul-k8s to apply this new config via CRDs

#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]>

…ss-gateway c… into release/1.13.x (#14791) * no-op commit due to failed cherry-picking * feat(ingress gateway: support configuring limits in ingress-gateway c… (#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]>

#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]>

…ss-gateway c… into release/1.12.x (#14790) * feat(ingress gateway: support configuring limits in ingress-gateway c… (#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]> * fix flaky integration test (#14843) * remove pbconfigentry Co-authored-by: temp <[email protected]> Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]> Co-authored-by: cskh <[email protected]>

…ss-gateway c… into release/1.12.x (#14790) * feat(ingress gateway: support configuring limits in ingress-gateway c… (#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]>

…ss-gateway c… into release/1.11.x (#14789) * Backport of feat(ingress gateway: support configuring limits in ingress-gateway c… into release/1.12.x (#14790) * feat(ingress gateway: support configuring limits in ingress-gateway c… (#14749) * feat(ingress gateway: support configuring limits in ingress-gateway config entry - a new Defaults field with max_connections, max_pending_connections, max_requests is added to ingress gateway config entry - new field max_connections, max_pending_connections, max_requests in individual services to overwrite the value in Default - added unit test and integration test - updated doc Co-authored-by: Chris S. Kim <[email protected]> Co-authored-by: Jeff Boruszak <[email protected]> Co-authored-by: Dan Stough <[email protected]>

huikang requested a review from a team as a code owner September 26, 2022 14:20

huikang mentioned this pull request Sep 26, 2022

feat(ingress gateway): configure max_connections to upstream service #14708

Closed

3 tasks

github-actions bot added theme/api Relating to the HTTP API interface theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified labels Sep 26, 2022

huikang requested review from freddygv, kyhavlov and DanStough September 26, 2022 14:25

DanStough reviewed Sep 27, 2022

View reviewed changes

vercel bot had a problem deploying to Preview – consul September 27, 2022 18:40 Failure

huikang and others added 5 commits September 27, 2022 14:46

rename chaneglog

a61b169

fix proto

42ab367

Apply suggestions from code review

2a2c275

Co-authored-by: Dan Stough <[email protected]>

api: change to *uint32

d896381

huikang force-pushed the gh-add-ingress-gateway-configentry-limits branch from 7a9962b to d896381 Compare September 27, 2022 19:17

change to uint32 in structs

accdb00

huikang added backport/1.13 labels Sep 27, 2022

docs: add defaults and link to Defaults

6134f3f

vercel bot had a problem deploying to Preview – consul September 27, 2022 20:04 Failure

fix doc

52c2dd6

huikang force-pushed the gh-add-ingress-gateway-configentry-limits branch from 13b47ca to 52c2dd6 Compare September 27, 2022 20:10

vercel bot deployed to Preview – consul September 27, 2022 20:15 View deployment

DanStough approved these changes Sep 28, 2022

View reviewed changes

huikang merged commit 69f40df into main Sep 28, 2022

huikang deleted the gh-add-ingress-gateway-configentry-limits branch September 28, 2022 18:56

hc-github-team-consul-core mentioned this pull request Sep 28, 2022

Backport of feat(ingress gateway: support configuring limits in ingress-gateway c… into release/1.11.x #14789

Merged

3 tasks

This was referenced Sep 28, 2022

Backport of feat(ingress gateway: support configuring limits in ingress-gateway c… into release/1.12.x #14790

Merged

Backport of feat(ingress gateway: support configuring limits in ingress-gateway c… into release/1.13.x #14791

Merged

david-yu mentioned this pull request Oct 4, 2022

Configurable max connections for ingress-gateway #13374

Closed

analogue mentioned this pull request Nov 10, 2022

Add support for maxConnections, maxConcurrentRequests, and maxPendingRequests to IngressGateway CRD hashicorp/consul-k8s#1691

Merged

2 tasks

hc-github-team-consul-ecosystem mentioned this pull request Nov 10, 2022

Backport of Add support for maxConnections, maxConcurrentRequests, and maxPendingRequests to IngressGateway CRD into release/0.49.x hashicorp/consul-k8s#1700

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ingress gateway: support configuring limits in ingress-gateway c… #14749

feat(ingress gateway: support configuring limits in ingress-gateway c… #14749

huikang commented Sep 26, 2022 •

edited by nathancoleman

Loading

DanStough left a comment

DanStough Sep 27, 2022

huikang Sep 27, 2022

DanStough Sep 27, 2022

huikang Sep 27, 2022

DanStough Sep 27, 2022

DanStough left a comment

david-yu commented Sep 28, 2022

feat(ingress gateway: support configuring limits in ingress-gateway c… #14749

feat(ingress gateway: support configuring limits in ingress-gateway c… #14749

Conversation

huikang commented Sep 26, 2022 • edited by nathancoleman Loading

Description

Testing & Reproduction steps

Links

PR Checklist

DanStough left a comment

Choose a reason for hiding this comment

DanStough Sep 27, 2022

Choose a reason for hiding this comment

huikang Sep 27, 2022

Choose a reason for hiding this comment

DanStough Sep 27, 2022

Choose a reason for hiding this comment

huikang Sep 27, 2022

Choose a reason for hiding this comment

DanStough Sep 27, 2022

Choose a reason for hiding this comment

DanStough left a comment

Choose a reason for hiding this comment

david-yu commented Sep 28, 2022

huikang commented Sep 26, 2022 •

edited by nathancoleman

Loading