Skip to content
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

fix: Switch to an immediate drain strategy #4230

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

arkodg
Copy link
Contributor

@arkodg arkodg commented Sep 12, 2024

  • Ensures clients immediately receive a connection: close / GOAWAY instead of a probabilistic approach of receiving one b/w drain start and drain end (defaults to 600s). This should speed up shutdown with clients reconnecting to newer upgraded proxies.

Fixes: #4205

* Ensures clients immediately receive a `connection: close` / `GOAWAY`
instead of a probabilistic approach of receiving one b/w drain start and
drain end (defaults to 600s). This should speed up shutdown with clients
reconnecting to newer upgraded proxies.

Fixes: envoyproxy#4205

Signed-off-by: Arko Dasgupta <[email protected]>
@arkodg arkodg requested a review from a team as a code owner September 12, 2024 23:06
@arkodg arkodg changed the title fix: Switch to a immediate drain strategy fix: Switch to an immediate drain strategy Sep 12, 2024
Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 66.48%. Comparing base (6b8e37a) to head (8247e85).
Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
internal/cmd/envoy/shutdown_manager.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4230      +/-   ##
==========================================
+ Coverage   66.45%   66.48%   +0.03%     
==========================================
  Files         195      195              
  Lines       23734    23732       -2     
==========================================
+ Hits        15773    15779       +6     
+ Misses       6838     6833       -5     
+ Partials     1123     1120       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arkodg arkodg requested a review from a team September 12, 2024 23:17
@zirain
Copy link
Member

zirain commented Sep 13, 2024

/retest

@arkodg arkodg merged commit 14f687f into envoyproxy:main Sep 13, 2024
23 of 24 checks passed
@arkodg arkodg deleted the immediate-drain branch September 13, 2024 00:33
guydc pushed a commit to guydc/gateway that referenced this pull request Sep 23, 2024
Switch to a immediate drain strategy

* Ensures clients immediately receive a `connection: close` / `GOAWAY`
instead of a probabilistic approach of receiving one b/w drain start and
drain end (defaults to 600s). This should speed up shutdown with clients
reconnecting to newer upgraded proxies.

Fixes: envoyproxy#4205

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 14f687f)
Signed-off-by: Guy Daich <[email protected]>
guydc pushed a commit to guydc/gateway that referenced this pull request Sep 23, 2024
Switch to a immediate drain strategy

* Ensures clients immediately receive a `connection: close` / `GOAWAY`
instead of a probabilistic approach of receiving one b/w drain start and
drain end (defaults to 600s). This should speed up shutdown with clients
reconnecting to newer upgraded proxies.

Fixes: envoyproxy#4205

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 14f687f)
Signed-off-by: Guy Daich <[email protected]>
arkodg added a commit that referenced this pull request Sep 24, 2024
* fix: Reconcile on HTTPRoute labels change (#4279)

* added label change predicate

Signed-off-by: Luv <[email protected]>

* added labels predicate for xroute and gw

Signed-off-by: Luv <[email protected]>

* changed predicate to use .Or

Signed-off-by: Luv <[email protected]>

---------

Signed-off-by: Luv <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit 0d1ccae)
Signed-off-by: Guy Daich <[email protected]>

* fix: handle invalid sectionName in BackendTLSPolicy for Backend (#4296)

(cherry picked from commit 73c223e)
Signed-off-by: Guy Daich <[email protected]>

* fix: Switch to an immediate drain strategy (#4230)

Switch to a immediate drain strategy

* Ensures clients immediately receive a `connection: close` / `GOAWAY`
instead of a probabilistic approach of receiving one b/w drain start and
drain end (defaults to 600s). This should speed up shutdown with clients
reconnecting to newer upgraded proxies.

Fixes: #4205

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 14f687f)
Signed-off-by: Guy Daich <[email protected]>

---------

Signed-off-by: Luv <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Co-authored-by: Luv <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
@arkodg arkodg mentioned this pull request Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminating pods don't respond with "Connection: close" (H1) or GOAWAY(H2) on shutdown
3 participants