health check: add support health checking EDS endpoints with different port #3007
Conversation
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
|
Sorry for the noise. Had trouble with ASAN but couldn't catch it in my local (I have a very slow 2009 MacBook 😆) |
Signed-off-by: Dhi Aurrahman <[email protected]>
include/envoy/upstream/upstream.h
Outdated
| /** | ||
| * Create a health check connection for this host. | ||
| * @param dispatcher supplies the owning dispatcher. | ||
| * @param options supplies the socket options that will be set on the new connection. |
There was a problem hiding this comment.
I think socket options can be dropped here for now, as it's not used. We can add it back later as needed.
There was a problem hiding this comment.
OK got it. I have updated it.
| @@ -346,8 +372,12 @@ class HttpHealthCheckerImplTest : public testing::Test { | |||
| NiceMock<Runtime::MockRandomGenerator> random_; | |||
| std::list<uint32_t> connection_index_{}; | |||
| std::list<uint32_t> codec_index_{}; | |||
| const static HostWithHealthCheckMap health_checker_map_; | |||
There was a problem hiding this comment.
I wanted to provide a default value for this arg here: https://github.com/dio/envoy/blob/f63e400ad7e9d04df504fdfc182d7e644d86cc58/test/common/upstream/health_checker_impl_test.cc#L285. Tried to use literal {}, but caught by ASAN in this build. The relevant line: https://gist.github.com/dio/1d2b925862f439cd7216595660bd6b8c#file-asan-log-L50.
There was a problem hiding this comment.
I think it would be preferable to do method overloading here than using a default arg; we're a bit allergic to the use of static non-PoD in the Envoy code base due to static initializaiton fiasco; even though it's probably perfectly safe here, it would be cleaner to just do method overloading in this use case.
There was a problem hiding this comment.
Got it. Hopefully, f2b5a32 is fine.
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
|
LGTM, needs master merge and we can ship. |
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
|
@htuch OK. |
health check: add support health checking EDS endpoints with different port
This patch adds a way to override endpoint health check port.
Only EDS cluster is affected for now.
Risk Level:
Testing:
Docs Changes:
To be updated (unhiding)
Release Notes:
To be updated
Partially remedies #439
API Changes:
To be updated (unhiding)
Signed-off-by: Dhi Aurrahman [email protected]