Update our Envoy dependency by oschaaf · Pull Request #101 · envoyproxy/nighthawk

oschaaf · 2019-07-26T12:40:57Z

Updates Envoy to envoyproxy/envoy@8246167
Swap out Envoy's integration test server to use our own python integration wrappers to fire up a backend in the cpp benchmark client tests. This was needed to resolve the singleton runtime conflict with Envoy's integration test server rearing its head again. As an a-side this sets us up for testing against multiple types of backends later on.
Coverage refresher: syncs the way we do coverage testing up with how Envoy pulls it off, greatly improving the time needed to generate a coverage report as well as improving accuracy tracking coverage of inlined code in headers.
Notes:
Fixed argument handling (as with the new coverage test method the test around it would fail).

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

- It caused x-version trouble. - gperftools reported leaks , seemingly unfixable - gcovr ran into trouble In short, probably more trouble then it's worth, though it would have been a nice alternative to the pipe IPC we rely on with this change. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

- Update CI image: 8246167b9d238797cbc6c03dccc9e3921c37617d - Update Envoy: bcc66c6b74c365d1d2834cfe15b847ae13be0eb6 Prelude to updating coverage testing with lcov Benchmark http client still needs to be fixed Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

As per Harvey's recommendation, use py_binary and refer to it as a tool in the cpp_tests to get the pip deps sorted. Also, clean up integration/BUILD Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

- sync the way we do coverage testing with Envoy upstream - sync .bazelrc with Envoy upstream Notes: disabled `LinearRateLimiterInvalidArgumentTest` as that somehow specifically failed with the new coverage testing. Visibility of this target is the single thing from barring this to work: https://github.com/envoyproxy/envoy/blob/master/bazel/envoy_test.bzl#L147 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

…e-dump Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

In envoyproxy/nighthawk#101 we update coverage to the new methodology. For that to work we need to be able to consume `_lib_internal_only` targets which aren't visible today. This proposes a change so we can make that happen. The naming is fairly clear that these targets shouldn't be used otherwise. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

In anticipation of envoyproxy/envoy#7749 it makes sense to further upgrade the Envoy dep, which hereby is done. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

In envoyproxy/nighthawk#101 we update coverage to the new methodology. For that to work we need to be able to consume _lib_internal_only targets which aren't visible externally today. This proposes a change so we can make that happen. The naming is fairly clear that these targets shouldn't be used otherwise. Risk Level: low Testing: N/A Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

- the updated envoy fixes a clang-tidy issue we run in to - had to introduce some shim code which reduces the coverage % Let's see if CI is happy with these changes included. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

…k into update-envoy-state-dump Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

htuch · 2019-08-01T20:07:22Z

test/ssl_test.cc

@@ -0,0 +1,90 @@
+#include <chrono>


Thanks @oschaaf . How much of this can be split into separate PRs? I.e. there is a ton of churn, around Python test infra, SSL objects, coverage, general upgrade? Ideally our Envoy upgrades are lightweight and some of these changes, e.g. test infra, are one-offs. Could you split out some of this easily?

Yes, this can be split up into multiple PR's easily; be we may have to temporarily allow a lower coverage threshold at some point. I'll carve off a more fun-sized piece right away into a separate PR.

Split off from envoyproxy#101 - As a step towards upgrading our Envoy dependency, this swaps out Envoy's c++ integration test server with our own python wrappers around our own test server. This permanently avoids future runtime singleton conflicts. - Also starts setting `--base-id` when starting up our test-server. This is to support the upcoming test-sharding that will be performed by updated coverage test methodoly which will land in a future update. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf · 2019-08-10T21:13:50Z

Closing this PR as all has been either split out into separate PR's or been superseded.

oschaaf added 22 commits July 15, 2019 16:43

save state

732b1be

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

save state

18197fb

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

save state

8a14b8b

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

envoy 9e3871ade779664ffb0a8f09b11688878253a082

1f43db6

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Update to Envoy bcc66c6b74c365d1d2834cfe15b847ae13be0eb6

d1d7f78

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

poc for embedding python in the c++ bm test

c84d146

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

save state

fe671c8

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

save state

9dd32b4

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Formatting fixes

b05575c

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

accidental whitespace

4cc57bf

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

revert whitespace

b84f137

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Dep & CI update

b50fda5

- Update CI image: 8246167b9d238797cbc6c03dccc9e3921c37617d - Update Envoy: bcc66c6b74c365d1d2834cfe15b847ae13be0eb6 Prelude to updating coverage testing with lcov Benchmark http client still needs to be fixed Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Enable all tests

dd55512

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

do_ci.sh: add install_virtualenv()

f0b16f3

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Fix cpp vs python vs pip in CI

a80e89e

As per Harvey's recommendation, use py_binary and refer to it as a tool in the cpp_tests to get the pip deps sorted. Also, clean up integration/BUILD Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Tidy up

d4e8442

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Merge remote-tracking branch 'upstream/master' into update-envoy-stat…

a62e152

…e-dump Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Bump coverage threshold to 97.5

32d2d4c

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Fix asan issue + a couple of tweaks

401306e

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

back out remote cache in .bazelrc

95166d0

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf mentioned this pull request Jul 29, 2019

Consuming coverage tests envoyproxy/envoy#7749

Merged

Update Envoy to the latest.

07d6f65

In anticipation of envoyproxy/envoy#7749 it makes sense to further upgrade the Envoy dep, which hereby is done. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf added 3 commits July 29, 2019 23:56

Update Envoy

9b9b567

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Coverage: explicit ipv4 only (test for fixing CI)

d4c8857

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Update Envoy & coverage threshold

958d193

- the updated envoy fixes a clang-tidy issue we run in to - had to introduce some shim code which reduces the coverage % Let's see if CI is happy with these changes included. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf added 7 commits July 31, 2019 11:04

Merge branch 'update-envoy-state-dump' of github.com:oschaaf/nighthaw…

39f11d2

…k into update-envoy-state-dump Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Add include

53ae490

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Cover ssl shims in tests

e7f33c4

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Adress CI issues

e2ff84d

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Coverage fixes

b95ce2d

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Set coverage threshold to 97.0

4f44f51

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

A couple of small cleanups

0d73cfe

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf marked this pull request as ready for review August 1, 2019 19:46

oschaaf requested a review from htuch August 1, 2019 19:46

htuch reviewed Aug 1, 2019

View reviewed changes

oschaaf mentioned this pull request Aug 1, 2019

Integration test update #104

Closed

oschaaf closed this Aug 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update our Envoy dependency#101

Update our Envoy dependency#101
oschaaf wants to merge 33 commits intoenvoyproxy:masterfrom
oschaaf:update-envoy-state-dump

oschaaf commented Jul 26, 2019 •

edited

Loading

Uh oh!

htuch Aug 1, 2019

Uh oh!

oschaaf Aug 1, 2019

Uh oh!

oschaaf commented Aug 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

oschaaf commented Jul 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

htuch Aug 1, 2019

Choose a reason for hiding this comment

Uh oh!

oschaaf Aug 1, 2019

Choose a reason for hiding this comment

Uh oh!

oschaaf commented Aug 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oschaaf commented Jul 26, 2019 •

edited

Loading