Listener: the API change for multiple addresses support#21391
Listener: the API change for multiple addresses support#21391mattklein123 merged 9 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
This the API change separated from #19367 |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
mattklein123
left a comment
There was a problem hiding this comment.
Thanks a few comments.
/wait
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
seems CI issue also. not waste resources to retest again. |
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
|
emm...why this keep failure, is that my problem? is it just a timeout https://dev.azure.com/cncf/envoy/_build/results?buildId=109367&view=logs&j=9847b632-4506-56d9-5b24-dbc653a96189&t=77343030-d1d4-546e-4c1c-b92bbd14ed15&l=76 |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
The CI passed after merge with main |
mattklein123
left a comment
There was a problem hiding this comment.
LGTM with small comments, thanks.
/wait
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
mattklein123
left a comment
There was a problem hiding this comment.
Thanks LGTM with a small comment.
/wait
There was a problem hiding this comment.
Before finalizing this, I just want to confirm that we do NOT think we need any other data with each address. Some examples of things that might be useful, but possibly none of these are needed:
- different socket options
- different stat prefix (if no stat-prefix is specified, we currently use the address as the stat-prefix; if a stat-prefix is specified then there's no way to get different stats for the different addresses)
- anything related to internal listeners (cc @lambdai)
- different value of
freebind
Adding a message to repeat here gives us the ability to add more settings in the future if we need to, so it seems more future-proof.
Another way of thinking about it: the result of this is that additional listening sockets are setup. Will we ever need to set configuration to change any of the settings that are on a socket, to be different between the different addresses on the same envoy listener?
Yeah I like this idea. Good point. Let's do it. |
At least ensure that we already consider the pre-address stat prefix, but it is too complex for now. Thanks for the good point! let me update the PR |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/retest |
|
Retrying Azure Pipelines: |
mattklein123
left a comment
There was a problem hiding this comment.
LGTM, thanks. Will defer to @ggreenway for approve/merge.
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
@mattklein123 sorry, I lost your approval since I updated the PR :) |
…1391) Signed-off-by: He Jie Xu <hejie.xu@intel.com> Signed-off-by: Tianyu Xia <tyxia@google.com>
…1391) Signed-off-by: He Jie Xu <hejie.xu@intel.com> Signed-off-by: Amila Senadheera <amila.15@cse.mrt.ac.lk>
Commit Message: Listener: the API change for multiple addresses support
Additional Description:
Both listener and admin API are changed to support multiple addresses.
Risk Level: low
Testing: n/a
Docs Changes: API doc
Release Notes: n/a
Platform Specific Features: n/a
Part of Fixes #11184