Update Envoy to 124c160 (March 8th 2021) #641

eric846 · 2021-03-08T22:14:21Z

Sync .bazelrc
Update Docker image version in .circleci/config.yml
New way to declare Bazel pip imports in WORKSPACE (the requirement("...") side in test/integration/BUILD is unchanged)
ci/run_envoy_docker.sh was unchanged
Code changes
- Relax some byte count checks in integration tests
  - The counters started to come out lower after updating Envoy, e.g. 1450 decreased to 900. This was because the underlying Envoy infrastructure in the Nighthawk client stopped sending content-length: 0 in the requests.
  - Updating the test to only check if the count is >=500, rather than checking for a specific value
    - This should still catch failure modes where Nighthawk fails after 1 request or less
    - test_http_h2 already has such relaxed checks
- Pass through new TimeSource arg in allocateConnPool()
- factoryForGrpcService() changed the skip_cluster_check boolean to an enum Envoy::Grpc::AsyncClientFactoryClusterChecks::Skip
- Empty dumpState() override added to StreamDecoder
- UUID generator for tracing was moved
- Ignore warning "Unable to use runtime singleton for feature envoy.reloadable_features.header_map_correctly_coalesce_cookies"

update from master

merge from upstream

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

…double Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

…variable_setter Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

merge from upstream

…ent.Output turns out not to include the status Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

…plugin names, log thresholds only once per session Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

…uuid generator location Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

…oxy/envoy#13504 Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 · 2021-03-10T05:36:50Z

/retest

repokitteh-read-only · 2021-03-10T05:36:54Z

🔨 rebuilding ci/circleci: integration_test_coverage (failed build)
🔨 rebuilding ci/circleci: test (failed build)
🔨 rebuilding ci/circleci: tsan (failed build)
🔨 rebuilding ci/circleci: asan (failed build)

🐱

Caused by: a #641 (comment) was created by @eric846.

see: more, trace.

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 · 2021-03-10T21:23:44Z

/retest

repokitteh-read-only · 2021-03-10T21:23:47Z

🔨 rebuilding ci/circleci: tsan (failed build)

🐱

Caused by: a #641 (comment) was created by @eric846.

see: more, trace.

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 · 2021-03-10T22:19:09Z

/retest

repokitteh-read-only · 2021-03-10T22:19:12Z

🔨 rebuilding ci/circleci: integration_test_coverage (failed build)

🐱

Caused by: a #641 (comment) was created by @eric846.

see: more, trace.

mum4k

Thank you for putting this together @eric846.

mum4k · 2021-03-11T00:57:56Z

test/integration/integration_test_fixtures.py

@@ -153,6 +153,8 @@ def tearDown(self, caplog):
      for record in caplog.get_records(when):
        if record.levelno not in (logging.WARNING, logging.ERROR):
          continue
+        if "Unable to use runtime singleton for feature" in record.message:


We already have a way of skipping some warnings:

nighthawk/test/integration/nighthawk_test_server.py

Lines 63 to 69 in e2f5265

# A list of _TestCaseWarnErrorIgnoreList instances, message pieces that should

# be ignored even if logged by the test server at a WARNING or an ERROR

# severity.

#

# If multiple test_case_regexp entries match the current test case name, all the

# corresponding ignore lists will be used.

_TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([

Can we utilize this one?

…s warning Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

oschaaf

Re: "The counters started to come out lower after updating Envoy, e.g. 1450 decreased to 900. Is this expected?"
It might be worth comparing headers to have a look-see at what precisely changed. This seems significant enough to make an actual real difference?

oschaaf

Yeah, thinking about it more, the strict byte count testing may seem overly strict, but it might just be worth it as it will prompt investigation in cases like this. In headers-only tests we will transfer a significant % less bytes, possibly impacting packet counts required for transmitting a single request; so not requesting changes in this PR right away but I do feel it is worth discussing.

eric846 · 2021-03-11T18:07:43Z

Re: "The counters started to come out lower after updating Envoy, e.g. 1450 decreased to 900. Is this expected?"
It might be worth comparing headers to have a look-see at what precisely changed. This seems significant enough to make an actual real difference?

Bingo: The Envoy infrastructure inside the Nighthawk client seems to have stopped sending content-length: 0 in the request.

This PR

$ nc -l 127.0.0.1 12345
GET / HTTP/1.1
host: 127.0.0.1:12345

$ bazel-bin/nighthawk_client --rps 1 --duration 5 http://127.0.0.1:12345
...
upstream_cx_tx_bytes_total              123         24.60
...

Main branch

$ nc -l 127.0.0.1 12345
GET / HTTP/1.1
host: 127.0.0.1:12345
content-length: 0

$ bazel-bin/nighthawk_client --rps 1 --duration 5 http://127.0.0.1:12345
...
upstream_cx_tx_bytes_total              180         36.00
...

eric846 · 2021-03-11T18:16:03Z

Yeah, thinking about it more, the strict byte count testing may seem overly strict, but it might just be worth it as it will prompt investigation in cases like this. In headers-only tests we will transfer a significant % less bytes, possibly impacting packet counts required for transmitting a single request; so not requesting changes in this PR right away but I do feel it is worth discussing.

Agreed, opened #644 to discuss. Depending on the schedule, we might want to move forward with these relaxed assumptions for now, rather than adjust the strict counts. Or on the other hand, we might want to make the counts in the h2 test more strict to match. There's a chance Envoy could change its header behavior again.

oschaaf

Approving; while it seems odd to have a content-length header appear in a GET request, apparently it causes no trouble as all integration tests past — modulo the strict byte count check.
We may want to file an issue upstream though?

eric846 added 30 commits June 1, 2020 17:23

Merge pull request #5 from envoyproxy/master

8ea442d

update from master

Merge pull request #6 from envoyproxy/master

5ac755a

merge from upstream

Merge pull request #7 from envoyproxy/master

b8c25a5

merge from upstream

initial commit

1c19c68

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix comments

7050686

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix format

0776563

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

rename adaptive_rps to adaptive_load

16fd8f6

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add field_selector in example

c383010

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix example comment

6e1a483

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix format

4ef1140

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add support for fault injection headers

4111bf4

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

replace linear and binary search with exponential search

871a959

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add InputVariableSetter mechanism

1fd77c1

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add input variable setter to build file

edc36b2

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix syntax errors

4d0364e

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

rename samples/adaptive_rps

aed6d94

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

improve comments, change step controller initial value from int64 to …

d9ae87d

…double Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add proto validation rules, fix comments, make rps the default input_…

a05a6f5

…variable_setter Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix comment wording

8cd4d21

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

simplify protos, add defaults, specify required or optional

d814a96

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

add missing newline

5f5a885

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Kick CI

7e20a78

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

simplify protos

9048267

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix format

306c0ec

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

fix some optional field comments and rules

d33f543

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Merge pull request #10 from envoyproxy/master

442cca9

merge from upstream

add Nighthawk status field in BenchmarkResult as nested nighthawk.cli…

677b783

…ent.Output turns out not to include the status Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

switch to standard Envoy plugin config proto, add prefix to internal …

cefb366

…plugin names, log thresholds only once per session Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into adaptive-rps-protos2

f3684df

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

create headers

5463051

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 added 3 commits March 8, 2021 22:18

fix allocateConnPool extra arg, factoryForGrpcService skip mode, new …

6b3573a

…uuid generator location Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

regenerate readme

635d41d

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

ignore warnings: Unable to use runtime singleton for feature: envoypr…

272fc34

…oxy/envoy#13504 Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 added 3 commits March 10, 2021 11:42

Merge remote-tracking branch 'upstream/main'

7da182a

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

Merge branch 'main' into envoy-update

9181d31

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

relax integration test tx bytes requirements

7d90b41

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

change grpc behavior to skip, as it was before this PR

6db715e

Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 marked this pull request as ready for review March 10, 2021 22:44

eric846 added the waiting-for-review A PR waiting for a review. label Mar 10, 2021

mum4k requested a review from oschaaf March 11, 2021 00:53

mum4k requested changes Mar 11, 2021

View reviewed changes

mum4k added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Mar 11, 2021

use existing mechanism to ignore header_map_correctly_coalesce_cookie…

a4415e4

…s warning Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

eric846 added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Mar 11, 2021

oschaaf reviewed Mar 11, 2021

View reviewed changes

eric846 mentioned this pull request Mar 11, 2021

integration tests with specific header expectations #644

Open

oschaaf approved these changes Mar 11, 2021

View reviewed changes

mum4k approved these changes Mar 11, 2021

View reviewed changes

mum4k merged commit 3881261 into envoyproxy:main Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update Envoy to 124c160 (March 8th 2021) #641

Update Envoy to 124c160 (March 8th 2021) #641

Uh oh!

eric846 commented Mar 8, 2021 •

edited

Loading

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

mum4k left a comment

Uh oh!

mum4k Mar 11, 2021

Uh oh!

eric846 Mar 11, 2021

Uh oh!

oschaaf left a comment

Uh oh!

oschaaf left a comment

Uh oh!

eric846 commented Mar 11, 2021 •

edited

Loading

Uh oh!

eric846 commented Mar 11, 2021 •

edited

Loading

Uh oh!

oschaaf left a comment

Uh oh!

Uh oh!

	# A list of _TestCaseWarnErrorIgnoreList instances, message pieces that should
	# be ignored even if logged by the test server at a WARNING or an ERROR
	# severity.
	#
	# If multiple test_case_regexp entries match the current test case name, all the
	# corresponding ignore lists will be used.
	_TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([

Update Envoy to 124c160 (March 8th 2021) #641

Update Envoy to 124c160 (March 8th 2021) #641

Uh oh!

Conversation

eric846 commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

eric846 commented Mar 10, 2021

Uh oh!

repokitteh-read-only bot commented Mar 10, 2021

Uh oh!

mum4k left a comment

Choose a reason for hiding this comment

Uh oh!

mum4k Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

eric846 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

oschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

oschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

eric846 commented Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR

Main branch

Uh oh!

eric846 commented Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eric846 commented Mar 8, 2021 •

edited

Loading

eric846 commented Mar 11, 2021 •

edited

Loading

eric846 commented Mar 11, 2021 •

edited

Loading