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

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

Draft
wants to merge 1 commit into
base: release/v1.33
Choose a base branch
from

Conversation

keithmattix
Copy link
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 <[email protected]>
@phlax
Copy link
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
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

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