Skip to content

tcp: add FilterState hash policy for TcpProxy and other network filters#18701

Merged
htuch merged 9 commits intoenvoyproxy:mainfrom
mergeconflict:hash_policy
Oct 29, 2021
Merged

tcp: add FilterState hash policy for TcpProxy and other network filters#18701
htuch merged 9 commits intoenvoyproxy:mainfrom
mergeconflict:hash_policy

Conversation

@mergeconflict
Copy link
Copy Markdown

@mergeconflict mergeconflict commented Oct 21, 2021 •
edited
Loading

Adds a new variant to type.v3.HashPolicy to generate a hash from an object in FilterState. This implementation mirrors what already exists for HTTP filters (see RouteAction.HashPolicy.FilterState), and is usable by network filters such as TcpProxy.

Risk Level: low
Testing: updated tests in tcp_proxy:config_test
Docs Changes: N/A
Release Notes: added an entry under tcp
Platform Specific Features: N/A

Signed-off-by: Dan Rosen mergeconflict@google.com

tcp: add FilterState hash policy for TcpProxy and other network filters
e5a3a19 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 21, 2021

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #18701 was opened by mergeconflict.

see: more, trace.

@mergeconflict
Copy link
Copy Markdown
Author

mergeconflict commented Oct 21, 2021

attn @rshriram

mergeconflict
mergeconflict commented Oct 21, 2021
View reviewed changes
fix format (use clang 12.0.1 instead of 13)
1b5376f 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
@mergeconflict
Copy link
Copy Markdown
Author

mergeconflict commented Oct 21, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 21, 2021

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18701 (comment) was created by @mergeconflict.

see: more, trace.

@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 22, 2021

/assign-from @envoyproxy/first-pass-reviewers

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 22, 2021

@envoyproxy/first-pass-reviewers assignee is @adisuissa

🐱

Caused by: a #18701 (comment) was created by @htuch.

see: more, trace.

htuch
htuch reviewed Oct 22, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM modulo docs nit.

adisuissa
adisuissa reviewed Oct 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.
Left a question about the design here.

fuqianggao
fuqianggao reviewed Oct 27, 2021
View reviewed changes
Dan Rosen added 3 commits October 27, 2021 17:14
Merge branch 'main' into hash_policy
4549d02 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
adisuissa and htuch feedback
e3ac806 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
fix typo
412e5c8 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 27, 2021

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #18701 was synchronize by mergeconflict.

see: more, trace.

Dan Rosen added 3 commits October 28, 2021 16:37
tests
15033c0 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
Merge branch 'main' into hash_policy
c8b4817 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
formatting
f313d17 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
@mergeconflict
Copy link
Copy Markdown
Author

mergeconflict commented Oct 28, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 28, 2021

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18701 (comment) was created by @mergeconflict.

see: more, trace.

adisuissa
adisuissa previously approved these changes Oct 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm api

Code changes LGTM.

fix deps
75e1c0d 
Signed-off-by: Dan Rosen <mergeconflict@google.com>
htuch
htuch reviewed Oct 29, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo one question.

htuch
htuch approved these changes Oct 29, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@htuch htuch merged commit 38739c3 into envoyproxy:main Oct 29, 2021
@mergeconflict mergeconflict deleted the hash_policy branch October 29, 2021 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adisuissa adisuissa Awaiting requested review from adisuissa

2 more reviewers

@fuqianggao fuqianggao fuqianggao left review comments

@htuch htuch htuch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adisuissa adisuissa

@htuch htuch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mergeconflict @htuch @adisuissa @fuqianggao