Merged
Conversation
added 4 commits
April 12, 2021 13:37
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Member
Author
|
@snowp do you want to do the review since you were involved in the issue? Otherwise I can bring in someone else |
Member
Author
|
cc @mattklein123 who added the warning on Tap |
added 2 commits
April 12, 2021 17:17
snowp
suggested changes
Apr 13, 2021
Contributor
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks, just a few questions
added 2 commits
April 13, 2021 21:25
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Member
Author
|
/retest |
|
Retrying Azure Pipelines: |
Contributor
|
/retest |
|
Retrying Azure Pipelines: |
added 2 commits
April 15, 2021 09:11
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Member
Author
|
Attempting to pick up main again. Honestly I do not see how this change might have broken the example verification. |
Member
Author
|
/retest |
|
Retrying Azure Pipelines: |
Member
Author
|
@snowp it took me some time fighting the CI but I expect it to pass now |
snowp
suggested changes
Apr 19, 2021
Contributor
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks this looks code overall, just some small comments
Comment on lines
+16
to
+18
| * active http health checks: properly handles HTTP/2 GOAWAY frames from the upstream. Previously a GOAWAY frame due to a graceful listener drain could cause improper failed health checks due to streams being refused by the upstream on a connection that is going away. To revert to old GOAWAY handling behavior, set the runtime feature `envoy.reloadable_features.health_check.graceful_goaway_handling` to false. | ||
| * adaptive concurrency: fixed a bug where concurrent requests on different worker threads could update minRTT back-to-back. | ||
| * buffer: tighten network connection read and write buffer high watermarks in preparation to more careful enforcement of read limits. Buffer high-watermark is now set to the exact configured value; previously it was set to value + 1. |
Contributor
There was a problem hiding this comment.
Seems like bad merge, can you sort this out?
added 4 commits
April 19, 2021 15:41
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
snowp
previously approved these changes
Apr 20, 2021
Contributor
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Unfortunately I think you need another main merge
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
snowp
approved these changes
Apr 20, 2021
mum4k
pushed a commit
to envoyproxy/nighthawk
that referenced
this pull request
Apr 27, 2021
- Sync .bazelrc from Envoy's repository. - other necessary files were syncd but unchanged in envoy. - The constructor of Envoy::Server::ValidationAdmin now requires an instance of Envoy::Network::Address::InstanceConstSharedPtr, but a nullptr shared_ptr is valid (envoyproxy/envoy#15932) Also updated some formatting/language in MAINTAINERS.md, which wasn't rendering as intended in markdown. Signed-off-by: Nathan Perry <nbperry@google.com>
gokulnair
pushed a commit
to gokulnair/envoy
that referenced
this pull request
May 6, 2021
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com> Signed-off-by: Gokul Nair <gnair@twitter.com>
wjuan-AFK
pushed a commit
to wjuan-AFK/nighthawk
that referenced
this pull request
May 11, 2021
- Sync .bazelrc from Envoy's repository. - other necessary files were syncd but unchanged in envoy. - The constructor of Envoy::Server::ValidationAdmin now requires an instance of Envoy::Network::Address::InstanceConstSharedPtr, but a nullptr shared_ptr is valid (envoyproxy/envoy#15932) Also updated some formatting/language in MAINTAINERS.md, which wasn't rendering as intended in markdown. Signed-off-by: Nathan Perry <nbperry@google.com> Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
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:
Signed-off-by: Sotiris Nanopoulos sonanopo@microsoft.com
Fixes #15902
Issue
The regression is coming from e7a0f7c
The root of the issue is that in order to generate the warning regarding using Admin tapping with UDS sockets we request the socket from the admin interface. This throws for the
config_validationadmin instance.Resolution
Add a private function to validation admin interface that initializes a dummy socket and this code passes.
Risk Level: Low (validation mode only)
Testing: Added config tests
Docs Changes: N/A
Release Notes: Fix an issue that causes TAP sockets to panic during config validation mode.