Skip to content
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

[bp/1.25] test: fix flaky test DownstreamProtocolIntegrationTest.HandleDownstreamSocketFail (#27546) #28013

Closed
wants to merge 1 commit into from

Conversation

phlax
Copy link
Member

@phlax phlax commented Jun 16, 2023

The test manually introduces EBADF error on the connection between Envoy and the test client. However, the IoHandleMatcher doesn't distinguish UDP socket from TCP. If the upstream HTTP/3 connection gets the same port number as downstream TCP connection, the EBADF error will be applied to the wrong connection.

Enforce IoHandleMatcher to match socket type.

Risk Level: low, test only
Testing: existing tests
Docs Changes: N/A
Release Notes: N/A
Partially Fix #27490

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

…amSocketFail (envoyproxy#27546)

The test manually introduces EBADF error on the connection between Envoy and the test client. However, the IoHandleMatcher doesn't distinguish UDP socket from TCP. If the upstream HTTP/3 connection gets the same port number as downstream TCP connection, the EBADF error will be applied to the wrong connection.

Enforce IoHandleMatcher to match socket type.

Risk Level: low, test only
Testing: existing tests
Docs Changes: N/A
Release Notes: N/A
Partially Fix envoyproxy#27490

Signed-off-by: Dan Zhang <danzh@google.com>

Signed-off-by: danzh <danzh2010@users.noreply.github.com>

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Jun 16, 2023

this is failing with

test/integration/shadow_policy_integration_test.cc:18:9: error: member initializer 'SocketInterfaceSwap' does not name a non-static data member or base class
        SocketInterfaceSwap(Network::Socket::Type::Stream) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

i guess if we want to pick this to this branch we will need to pick other change/s also

@danzh2010
Copy link
Contributor

this is failing with

test/integration/shadow_policy_integration_test.cc:18:9: error: member initializer 'SocketInterfaceSwap' does not name a non-static data member or base class
        SocketInterfaceSwap(Network::Socket::Type::Stream) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

i guess if we want to pick this to this branch we will need to pick other change/s also

What this branch is? The PR just fixed a test utility. Why is it need to be picked into a specific branch?

@phlax
Copy link
Member Author

phlax commented Jun 16, 2023

thanks @danzh2010 for clarification offline - seems this should not be necessary

@phlax phlax closed this Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants