Skip to content

[cherrypick/v1.33] feature: inbound only graceful draining (#37873)#38331

Closed
keithmattix wants to merge 1 commit intoenvoyproxy:release/v1.33from
keithmattix:release-1.33-inbound-only-graceful-draining
Closed

[cherrypick/v1.33] feature: inbound only graceful draining (#37873)#38331
keithmattix wants to merge 1 commit intoenvoyproxy:release/v1.33from
keithmattix:release-1.33-inbound-only-graceful-draining

Conversation

@keithmattix
Copy link
Copy Markdown
Contributor

Cherry-pick of #37873 for release 1.33. I'm proposing a cherry-pick because this solves a significant problem for envoy sidecar users, particularly native sidecar users.

Commit Message:

Fixes #35020. The inbound_only query param for the drain_listeners admin endpoint doesn't work with the graceful query parameter. As a result, outbound listeners will send connection: close headers to upstreams which is undesired. This PR adds the ability for drain_manager to drain in a single direction.

Prior to this change, the inbound_only query
param on the drain_listeners admin endpoint
only modified which listeners were stopped. If graceful is set, listeners of all directions are drained, regardless if inbound_only is set. If skip_exit is set, inbound_only has zero effect. This PR adds the ability to drain only inbound listeners, allowing outbound listeners to continue functioning as normal. This is useful in the Kubernetes sidecar use-case where outbound traffic should not set connection: close headers to the upstream.

Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Fixes envoyproxy#35020. The inbound_only query param for the drain_listeners admin
endpoint doesn't work with the graceful query parameter. As a result,
outbound listeners will send connection: close headers to upstreams
which is undesired. This PR adds the ability for drain_manager to drain
in a single direction.

Prior to this change, the `inbound_only` query
param on the [`drain_listeners` admin endpoint
](https://www.envoyproxy.io/docs/envoy/latest/operations/admin#operations-admin-interface-drain)
only modified which listeners were stopped. If `graceful` is set,
listeners of all directions are drained, regardless if `inbound_only` is
set. If `skip_exit` is set, `inbound_only` has zero effect. This PR adds
the ability to drain only inbound listeners, allowing outbound listeners
to continue functioning as normal. This is useful in the Kubernetes
sidecar use-case where outbound traffic should not set connection: close
headers to the upstream.

Risk Level: Medium
fixes envoyproxy#35020

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@phlax
Copy link
Copy Markdown
Member

phlax commented Feb 6, 2025

something has broken main still unclear what it is - but this is a possible candidate so testing it

also we generally dont backport features

/wait

@keithmattix keithmattix marked this pull request as draft February 6, 2025 15:25
// This prevents us from double incrementing if listeners are stopped twice.
// This can happen if the admin endpoint is triggered for inbound_only and then
// all.
if (stopped_listener_tags_.find(listener_tag) == stopped_listener_tags_.end()) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check to see if this function can be called by two threads at the same time; the first tag may not be added to the map yet

@keithmattix
Copy link
Copy Markdown
Contributor Author

We missed the Istio 1.25 release, so not going to try and cherry-pick this

@keithmattix keithmattix closed this Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants