Skip to content

kafka: fix integration test#17764

Merged
mattklein123 merged 2 commits intoenvoyproxy:mainfrom
adamkotwasinski:kafka-broker-intttest
Aug 19, 2021
Merged

kafka: fix integration test#17764
mattklein123 merged 2 commits intoenvoyproxy:mainfrom
adamkotwasinski:kafka-broker-intttest

Conversation

@adamkotwasinski
Copy link
Contributor

@adamkotwasinski adamkotwasinski commented Aug 18, 2021

Commit Message: kafka: fix integration test
Additional Description: fix broker integration test (it was referencing the normal binary) + make integration test run automatic (as it's now in contrib/)
FWIW inttest duration:
Mac i7-7820HQ + 8GB ram -> 78 seconds
Ubuntu i5-9400? + 8GB ram -> 36 seconds
CI -> 43s
Risk Level: Low
Testing: Integration tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Adam Kotwasinski <adam.kotwasinski@gmail.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #17764 was opened by adamkotwasinski.

see: more, trace.

Signed-off-by: Adam Kotwasinski <adam.kotwasinski@gmail.com>
@adamkotwasinski
Copy link
Contributor Author

The release builds invoke the int test and pass, e.g. https://github.com/envoyproxy/envoy/runs/3365728636 :

2021-08-18T21:13:15.5353838Z //contrib/kafka/filters/network/test/broker/integration_test:kafka_broker_integration_test �[0m�[32mPASSED�[0m in 42.1s

@adamkotwasinski
Copy link
Contributor Author

/assign @mattklein123

@adamkotwasinski adamkotwasinski marked this pull request as ready for review August 19, 2021 01:19
flaky = True,
python_version = "PY3",
srcs_version = "PY3",
tags = ["manual"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that Kafka has moved to contrib, why not give this a try, so far it appears to be stable.
In the worst case we'd go back to manual mode (and we're still keeping flaky just in case of slow machines, but the default of moderate = 5 minutes should be a good first idea).

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Sure happy to give it a shot, thanks.

@mattklein123 mattklein123 merged commit dee7021 into envoyproxy:main Aug 19, 2021
@adamkotwasinski adamkotwasinski deleted the kafka-broker-intttest branch August 19, 2021 06:26
mpuncel added a commit to mpuncel/envoy that referenced this pull request Aug 19, 2021
* main:
  Fix for fuzz tests failing due to invalid corpus paths (envoyproxy#17767)
  kafka: fix integration test (envoyproxy#17764)
  Fix typo in cluster.proto (envoyproxy#17755)
  cluster manager: add drainConnections() API (envoyproxy#17747)
  kafka broker filter: move to contrib (envoyproxy#17750)
  quiche: switch external dependency to github (envoyproxy#17732)
  quiche: implement stream idle timeout in codec (envoyproxy#17674)
  Update c-ares to 1.17.2 (envoyproxy#17704)
  Fix dns resolve fuzz bug (envoyproxy#17107)
  Remove members that shadow members of the base class (envoyproxy#17713)
  thrift proxy: missing parts from the previous PR (envoyproxy#17668)
  thrift-proxy: cleanup ConnectionManager::ActiveRpc (envoyproxy#17734)
  listener: extra warning for deprecated use_proxy_proto field (envoyproxy#17736)
  kafka: add support for metadata request in mesh-filter (envoyproxy#17597)
  upstream: add all host map to priority set for fast host searching (envoyproxy#17290)
  Remove the support for `hidden_envoy_deprecated_per_filter_config` (envoyproxy#17725)
  tls: SDS support for private key providers (envoyproxy#16737)
  bazel: update rules_foreign_cc (envoyproxy#17445)

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
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.

2 participants