mysql_filter: fix integration test flakes by venilnoronha · Pull Request #6272 · envoyproxy/envoy

venilnoronha · 2019-03-12T19:43:59Z

Description: This fixes flakes in the MySQL integration tests by introducing an exact
match for server responses. A separate issue was found when running under IPv6 due to the presence of IPv4 addresses in the test configuration. This fixes that issue as well.
Risk Level: Low
Testing: Ran mysql_integration_test x5000
Docs Changes: N/A
Release Notes: N/A
Fixes #6162
Fixes #6131

jmarantz · 2019-03-12T20:04:15Z

After patching this PR, I got that I think is the same failure:

test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc:63: Failure
Value of: fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)
  Actual: false (Timed out waiting for raw connection)
Expected: true
[2019-03-12 20:02:50.503][14][critical][assert] [source/common/network/connection_impl.cc:74] assert failure: !ioHandle().isOpen() && delayed_close_timer_ == nullptr. Details: ConnectionImpl was unexpectedly torn down without being closed.
[2019-03-12 20:02:50.503][14][critical][backtrace] [bazel-out/k8-fastbuild/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:81] Caught Aborted, suspect faulting address 0x110900000000e

qq: is there a requirement that we have mysql installed locally as a service to run Envoy tests?

venilnoronha · 2019-03-12T20:05:01Z

No, mysql isn't required to run tests.

venilnoronha · 2019-03-12T20:08:00Z

@jmarantz what's OS are you running these tests on?

This fixes flakes in the MySQL integration tests by introducing an exact match for server responses. Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha · 2019-03-13T00:53:49Z

@jmarantz I believe the flakes are fixed now.

jmarantz · 2019-03-13T01:42:36Z

Still fails for me; patching latest commit. Will poke around in the debugger for a few minutes.

jmarantz · 2019-03-13T02:34:49Z

I didn't get anywhere debugging yet, and I'm not going to be able to get back to it tonight. My OS is 4.19.20-1rodete1-amd64 #1 SMP Debian 4.19.20-1rodete1 (2019-02-12 > 2018) x86_64 GNU/Linux

jmarantz · 2019-03-13T02:46:21Z

It's worth noting that it failed for me in fastbuild, debug (g++), under tsan, and opt. The opt build was compiled without ASSERT of course, so the message was different:

[ RUN      ] IpVersions/MySQLIntegrationTest.MySQLStatsNewSessionTest/1
test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc:61: Failure
Value of: fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)
  Actual: false (Timed out waiting for raw connection)
Expected: true
[  FAILED  ] IpVersions/MySQLIntegrationTest.MySQLStatsNewSessionTest/1, where GetParam() = 4-byte object <01-00 00-00> (10027 ms)
[ RUN      ] IpVersions/MySQLIntegrationTest.MySQLLoginTest/0
[       OK ] IpVersions/MySQLIntegrationTest.MySQLLoginTest/0 (27 ms)
[ RUN      ] IpVersions/MySQLIntegrationTest.MySQLLoginTest/1
test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc:84: Failure
Value of: fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)
  Actual: false (Timed out waiting for raw connection)
Expected: true
[  FAILED  ] IpVersions/MySQLIntegrationTest.MySQLLoginTest/1, where GetParam() = 4-byte object <01-00 00-00> (10026 ms)
[ RUN      ] IpVersions/MySQLIntegrationTest.MySQLUnitTestMultiClientsLoop/0
[       OK ] IpVersions/MySQLIntegrationTest.MySQLUnitTestMultiClientsLoop/0 (32 ms)
[ RUN      ] IpVersions/MySQLIntegrationTest.MySQLUnitTestMultiClientsLoop/1
test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc:131: Failure
Value of: fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)
  Actual: false (Timed out waiting for raw connection)
Expected: true
[  FAILED  ] IpVersions/MySQLIntegrationTest.MySQLUnitTestMultiClientsLoop/1, where GetParam() = 4-byte object <01-00 00-00> (10026 ms)

This fixes the flakes in the MySQL integration test when running with IPv6 by switching IPv4 addresses with addresses determined at runtime. Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha · 2019-03-13T06:04:12Z

@jmarantz I figured it out. The tests were failing specifically for IPv6 because of static IPv4 addresses in Envoy configuration. I've fixed that in 9ed7dd0. Could you test it once more on your local system?

jmarantz · 2019-03-13T12:36:52Z

The latest patch works for me! The question that brings to mind is who was able to get these tests to pass without this? But that's a question for another day.

Thanks for the fix!

test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 · 2019-03-13T16:59:40Z

test/extensions/filters/network/mysql_proxy/mysql_test_config.yaml

@@ -2,7 +2,7 @@ admin:
  access_log_path: /dev/null


Question: Why are we using this config vs. just using the default config and config modifiers? Would that have made it easier to prevent this issue?

I think we could have done it either way. However, @gvalentinivmw might know more. We could have still run into this issue as the config modifiers don't constrain IP addresses AFAIK.

OK that's fine. It's not a big deal either way, though I think in general it's better to use the stock config in case that changes in the future.

For the record, I think the problem here is related to a bug I reported a while back: #5556

it'd be nice to make tests that do not care about ports not have to specify anything about them, or care about ipv6 vs ipv4.

Yea, that'd definitely be helpful.

* master: (59 commits) http fault: add response rate limit injection (envoyproxy#6267) xds: introduce initial_fetch_timeout option to limit initialization time (envoyproxy#6048) test: fix cpuset-threads tests (envoyproxy#6278) server: add an API for registering for notifications for server instance life… (envoyproxy#6254) remove remains of TestBase (envoyproxy#6286) dubbo_proxy: Implement the routing of Dubbo requests (envoyproxy#5973) Revert "stats: add new BoolIndicator stat type (envoyproxy#5813)" (envoyproxy#6280) runtime: codifying runtime guarded features (envoyproxy#6134) mysql_filter: fix integration test flakes (envoyproxy#6272) tls: update BoringSSL to debed9a4 (3683). (envoyproxy#6273) rewrite buffer implementation to eliminate evbuffer dependency (envoyproxy#5441) Remove the dependency from TimeSystem to libevent by using the Event::Scheduler abstraction as a delegate. (envoyproxy#6240) fuzz: fix use of literal in default initialization. (envoyproxy#6268) http: add HCM functionality required for rate limiting (envoyproxy#6242) Disable mysql_integration_test until it is deflaked. (envoyproxy#6250) test: use ipv6_only IPv6 addresses in custom cluster integration tests. (envoyproxy#6260) tracing: If parent span is propagated with empty string, it causes th… (envoyproxy#6263) upstream: fix oss-fuzz issue envoyproxy#11095. (envoyproxy#6220) Wire up panic mode subset to receive updates (envoyproxy#6221) docs: clarify xds docs with warming information (envoyproxy#6236) ...

dnoe assigned jmarantz Mar 12, 2019

venilnoronha changed the title ~~mysql_filter: fix integration test~~ [WIP] mysql_filter: fix integration test Mar 12, 2019

venilnoronha force-pushed the mysql-fix branch from 97f617b to 24745ef Compare March 13, 2019 00:51

mysql_filter: fix integration test flakes

6331aad

This fixes flakes in the MySQL integration tests by introducing an exact match for server responses. Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha force-pushed the mysql-fix branch from 24745ef to 6331aad Compare March 13, 2019 00:52

venilnoronha marked this pull request as ready for review March 13, 2019 00:52

venilnoronha requested a review from mattklein123 as a code owner March 13, 2019 00:52

venilnoronha changed the title ~~[WIP] mysql_filter: fix integration test~~ mysql_filter: fix integration test flakes Mar 13, 2019

dkalani previously approved these changes Mar 13, 2019

View reviewed changes

Fix flakes in MySQL integration test with IPv6

9ed7dd0

This fixes the flakes in the MySQL integration test when running with IPv6 by switching IPv4 addresses with addresses determined at runtime. Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha dismissed dkalani’s stale review via 9ed7dd0 March 13, 2019 06:02

jmarantz reviewed Mar 13, 2019

View reviewed changes

test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc Outdated Show resolved Hide resolved

jmarantz reviewed Mar 13, 2019

View reviewed changes

test/extensions/filters/network/mysql_proxy/mysql_integration_test.cc Outdated Show resolved Hide resolved

Drop string initializers

608507d

Signed-off-by: Venil Noronha <veniln@vmware.com>

dnoe assigned mattklein123 Mar 13, 2019

mattklein123 reviewed Mar 13, 2019

View reviewed changes

mattklein123 approved these changes Mar 13, 2019

View reviewed changes

mattklein123 merged commit 21130ad into envoyproxy:master Mar 13, 2019

Conversation

venilnoronha commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarantz commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

venilnoronha commented Mar 12, 2019

Uh oh!

venilnoronha commented Mar 12, 2019

Uh oh!

venilnoronha commented Mar 13, 2019

Uh oh!

jmarantz commented Mar 13, 2019

Uh oh!

jmarantz commented Mar 13, 2019

Uh oh!

jmarantz commented Mar 13, 2019

Uh oh!

venilnoronha commented Mar 13, 2019

Uh oh!

jmarantz commented Mar 13, 2019

Uh oh!

Uh oh!

Uh oh!

mattklein123 Mar 13, 2019

Choose a reason for hiding this comment

Uh oh!

venilnoronha Mar 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 Mar 13, 2019

Choose a reason for hiding this comment

Uh oh!

jmarantz Mar 13, 2019

Choose a reason for hiding this comment

Uh oh!

venilnoronha Mar 13, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

venilnoronha commented Mar 12, 2019 •

edited

Loading

jmarantz commented Mar 12, 2019 •

edited

Loading

venilnoronha Mar 13, 2019 •

edited

Loading