-
Notifications
You must be signed in to change notification settings - Fork 689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal/dag: Add support for per-host max connections #6016
internal/dag: Add support for per-host max connections #6016
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6016 +/- ##
=======================================
Coverage 78.68% 78.69%
=======================================
Files 138 138
Lines 19675 19681 +6
=======================================
+ Hits 15481 15487 +6
Misses 3890 3890
Partials 304 304
|
Support per-host max connections circuit breaker threshold enabling Envoy to enforce a maximum number of connections for each individual Kubernetes service endpoint using a new service-level annotation: `projectcontour.io/per-host-max-connections`. Resolves projectcontour#6015 Signed-off-by: Aurel Canciu <[email protected]>
d13985a
to
cf85ee0
Compare
Envoy docs are a bit confusing to me, does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a release note for this.
#6013 is also conflicting. I think we need to be able to set a global default for this.
otherwise the PR lgtm, thanks for the feature, we might even put it into use actually |
That is what I understand from here.
That could make sense if one wants to configure a safe default max-connections limit covering all ingress configurations. Sorry, it seems I forgot to include the release note. I'll add it in a few minutes. Thank you! |
Signed-off-by: Aurel Canciu <[email protected]>
49ee675
to
6d517e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if you merge before #6013 gets in then you have to do the defaults otherwise I can do it
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
@projectcontour/maintainers can you PTAL so we can merge it. It has some conflicts with #6013 so whoever merges it first need to handle it |
@davinci26 thanks for reviewing, I'll take a look this week |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @relu, LGTM
@sunjayBhatia will give you a chance to take a look if you want |
Support per-host max connections circuit breaker threshold enabling Envoy to enforce a maximum number of connections for each individual Kubernetes service endpoint using a new service-level annotation:
projectcontour.io/per-host-max-connections
.Resolves #6015