Revert "drain_manager: Unclean Revert of "HTTP2 Proactive GOAWAY on Drain - Preamble (#17026)" (#37465)"#38333
Merged
yanavlasov merged 2 commits intoenvoyproxy:mainfrom Feb 11, 2025
Merged
Conversation
…rain - Preamble (envoyproxy#17026)" (envoyproxy#37465)" This reverts commit 072831e. Signed-off-by: Yan Avlasov <[email protected]>
yanavlasov
requested review from
RyanTheOptimist,
abeyad,
alyssawilk,
fredyw and
mattklein123
as code owners
Contributor
|
Suggest adding some comments that while the feature was never fully developed it is in use. |
Signed-off-by: Yan Avlasov <[email protected]>
Contributor
Author
I've added comment to the header. It is not obvious how to slice this change to ensure boundary proxy will work. I will need to set-up their build and then remove code in PR. I will do it as a followup. For now I need to get partners back to buildable state. |
mattklein123
approved these changes
Feb 10, 2025
yanavlasov
pushed a commit
that referenced
this pull request
Mar 4, 2025
Trying #37873 again to get to the bottom of the flakes that caused us to need to revert. This got a lot more complex after #38333, so I may need @yanavlasov to help me out with the proper semantics 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. ``` Additional Description: Risk Level: Medium 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](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Keith Mattix II <[email protected]>
alam0rt
pushed a commit
to alam0rt/envoy
that referenced
this pull request
Mar 14, 2025
Trying envoyproxy#37873 again to get to the bottom of the flakes that caused us to need to revert. This got a lot more complex after envoyproxy#38333, so I may need @yanavlasov to help me out with the proper semantics Commit Message: ``` 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. ``` Additional Description: Risk Level: Medium 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](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Keith Mattix II <[email protected]>
jewertow
pushed a commit
to jewertow/envoy
that referenced
this pull request
Apr 2, 2025
Trying envoyproxy#37873 again to get to the bottom of the flakes that caused us to need to revert. This got a lot more complex after envoyproxy#38333, so I may need @yanavlasov to help me out with the proper semantics Commit Message: ``` 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. ``` Additional Description: Risk Level: Medium 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](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Keith Mattix II <[email protected]>
agrawroh
pushed a commit
to agrawroh/envoy
that referenced
this pull request
Apr 9, 2025
Trying envoyproxy#37873 again to get to the bottom of the flakes that caused us to need to revert. This got a lot more complex after envoyproxy#38333, so I may need @yanavlasov to help me out with the proper semantics Commit Message: ``` 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. ``` Additional Description: Risk Level: Medium 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](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Keith Mattix II <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message:
This reverts commit 072831e.
Revert in PR-37465 created problems for boundary proxy for Google partner cloud, as it depends on the reverted internal API
addOnDrainCloseCb. Boundary proxy is open source and Google cloud partners will be unable to build OSS version, since it depends on OSS Envoy.Putting back the reverted code for now, as it is unclear what exactly can be removed from Envoy at this point.
Risk Level: Low
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A