Skip to content

Fix consistent hash based on source IP for TCP proxy #38438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

howardjohn
Copy link
Member

@howardjohn howardjohn commented Apr 18, 2022

Fixes test flakes like
https://prow.istio.io/view/gs/istio-prow/logs/integ-ipv6_istio_postsubmit/1514637916112949248

We recently added TCP sourceIP consistent hash. Even more recently, this
test started to fail often. I believe this is due to a change to apply
YAML in parallel, exposing this bug.

The root cause is that we do not push LDS for DR changes, but the config
impacts LDS.

Please provide a description of this PR:

@howardjohn howardjohn added the release-notes-none Indicates a PR that does not require release notes. label Apr 18, 2022
@howardjohn howardjohn requested a review from a team as a code owner April 18, 2022 22:28
@istio-policy-bot
Copy link

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 18, 2022
@hzxuzhonghu
Copy link
Member

This is a bug fix, should be backported

Fixes test flakes like
https://prow.istio.io/view/gs/istio-prow/logs/integ-ipv6_istio_postsubmit/1514637916112949248

We recently added TCP sourceIP consistent hash. Even more recently, this
test started to fail often. I believe this is due to a change to apply
YAML in parallel, exposing this bug.

The root cause is that we do not push LDS for DR changes, but the config
impacts LDS.
@howardjohn howardjohn force-pushed the pilot/fix-tcp-source-ip branch from f592788 to e129074 Compare April 19, 2022 15:05
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 19, 2022
@ericvn
Copy link
Contributor

ericvn commented Apr 19, 2022

/test gencheck_istio

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.11":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
CONFLICT (content): Merge conflict in tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38458

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.11":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
CONFLICT (content): Merge conflict in tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38459

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.12":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38460

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.11":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
CONFLICT (content): Merge conflict in tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38461

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.12":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38462

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.13":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38463

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.12":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38464

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.13":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38465

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #38438 failed to apply on top of branch "release-1.13":

Applying: Fix consistent hash based on source IP for TCP proxy
Using index info to reconstruct a base tree...
M	pilot/pkg/xds/lds.go
M	tests/integration/pilot/common/routing.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/pilot/common/routing.go
Auto-merging pilot/pkg/xds/lds.go
CONFLICT (content): Merge conflict in pilot/pkg/xds/lds.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix consistent hash based on source IP for TCP proxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #38466

hemendrateli added a commit to hemendrateli/istio that referenced this pull request Sep 9, 2022
…evisions

We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :-
	a. gRPC fault
           injection(istio#39295)
	b. Ignore port number in domain
           matching(istio#40475)
	c. Tunneling outbound traffic :- new tunnel field got
           introduced(istio#37968)
	d. Fix consistent hash based on source IP for TCP
           proxy(istio#38438)
	e. Traffic policy load balancer API
           changes(istio#39742)
istio-testing pushed a commit that referenced this pull request Sep 12, 2022
…evisions (#40892)

We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :-
	a. gRPC fault
           injection(#39295)
	b. Ignore port number in domain
           matching(#40475)
	c. Tunneling outbound traffic :- new tunnel field got
           introduced(#37968)
	d. Fix consistent hash based on source IP for TCP
           proxy(#38438)
	e. Traffic policy load balancer API
           changes(#39742)
istio-testing pushed a commit to istio-testing/istio that referenced this pull request Sep 13, 2022
…evisions

We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :-
	a. gRPC fault
           injection(istio#39295)
	b. Ignore port number in domain
           matching(istio#40475)
	c. Tunneling outbound traffic :- new tunnel field got
           introduced(istio#37968)
	d. Fix consistent hash based on source IP for TCP
           proxy(istio#38438)
	e. Traffic policy load balancer API
           changes(istio#39742)
istio-testing added a commit that referenced this pull request Sep 13, 2022
…evisions (#40957)

We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :-
	a. gRPC fault
           injection(#39295)
	b. Ignore port number in domain
           matching(#40475)
	c. Tunneling outbound traffic :- new tunnel field got
           introduced(#37968)
	d. Fix consistent hash based on source IP for TCP
           proxy(#38438)
	e. Traffic policy load balancer API
           changes(#39742)

Co-authored-by: Hemendra Teli <hemendrat@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes-none Indicates a PR that does not require release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants