Lazily evaluate Ingress address #4118

mandarjog · 2018-03-08T23:42:59Z

Ingress takes up to 2 minutes to get an address.
We could be doing other work during that time.

This PR updates code to wait for ingress address only when the code is about to use that address.

ldemailly · 2018-03-09T00:06:01Z

in theory it's great to do as much stuff as possible in parallel but getting the ingress really up is gating every e2e tests anyway so I'm not sure of the gain here - we just discussed in the other PR (#4050 (comment)) that we need to do more ingress checks before spending time starting other tests doomed to fail

mandarjog · 2018-03-09T00:08:36Z

Ingress gates the test, but it does not gate setting up the applications.
There are tests that do kubeexec and do not do use ingress. If those are running first, you don't need to wait for ingress adress to appear before starting tests. I expect it to shave off a minute or two from these tests.

ldemailly · 2018-03-09T00:09:59Z

that or our ingress could come up in 10s like it should :-) but ok, thanks for this! (if it works and doesn't have weird side effects)

codecov · 2018-03-09T00:11:04Z

Codecov Report

Merging #4118 into master will increase coverage by 1%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #4118   +/-   ##
======================================
+ Coverage      73%     73%   +1%     
======================================
  Files         313     313           
  Lines       28812   28748   -64     
======================================
+ Hits        20928   20934    +6     
+ Misses       6559    6493   -66     
+ Partials     1325    1321    -4

Impacted Files	Coverage Δ
mixer/adapter/stackdriver/stackdriver.go	`55% <0%> (-15%)`	⬇️
pilot/pkg/config/memory/monitor.go	`82% <0%> (-9%)`	⬇️
mixer/adapter/solarwinds/log_handler.go	`50% <0%> (-7%)`	⬇️
pilot/pkg/serviceregistry/kube/controller.go	`64% <0%> (-4%)`	⬇️
pilot/pkg/serviceregistry/kube/queue.go	`86% <0%> (-3%)`	⬇️
mixer/adapter/fluentd/fluentd.go	`76% <0%> (-1%)`	⬇️
mixer/tools/adapterlinter/main.go	`81% <0%> (-1%)`	⬇️
mixer/adapter/memquota/rollingWindow.go	`100% <0%> (ø)`	⬆️
mixer/adapter/memquota/keys.go	`100% <0%> (ø)`	⬆️
broker/pkg/model/config/store.go	`100% <0%> (ø)`	⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 328a64b...be41dff. Read the comment docs.

mandarjog · 2018-03-09T00:13:55Z

/test e2e-suite-rbac-no_auth

ldemailly · 2018-03-09T00:20:41Z

want to test your theory by swapping the order of some tests ?

mandarjog · 2018-03-09T00:47:44Z

Old test 173 sec from start of process to starting running real test

Mixer test starts
2018-03-07T21:33:16.890210Z	info	Logging initialized
.
2018-03-07T21:34:45.042793Z	info	Istio ingress: 35.227.55.149
.
2018-03-07T21:36:03.786502Z	info	Get all pods running!
2018-03-07T21:36:05.650886Z	info	Get from http://35.227.55.149/productpage: 200 OK (200)
2018-03-07T21:36:05.651228Z	info	Got 200 response from product page!

New test 120 sec

I0309 00:20:02.799] 2018-03-09T00:20:02.799287Z	info	Logging initialized
.
.
I0309 00:22:00.560] 2018-03-09T00:22:00.560314Z	info	Istio ingress: 35.224.39.44
I0309 00:22:02.218] 2018-03-09T00:22:02.217755Z	info	Get from http://35.224.39.44/productpage: 200 OK (200)
I0309 00:22:02.218] 2018-03-09T00:22:02.217951Z	info	Got 200 response from product page!

mandarjog · 2018-03-09T17:12:39Z

/cc @ajy

istio-testing · 2018-03-09T17:12:40Z

@mandarjog: GitHub didn't allow me to request PR reviews from the following users: ajy.

Note that only istio members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @ajy

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mandarjog · 2018-03-09T18:19:42Z

@costinm @ldemailly all jobs are succeeding. /cc @ayj

douglas-reid · 2018-03-09T18:44:16Z

/lgtm

ldemailly · 2018-03-09T18:45:37Z

install/updateVersion.sh

@@ -173,6 +173,7 @@ function merge_files() {
  execute_sed "s/# authPolicy: MUTUAL_TLS/authPolicy: MUTUAL_TLS/" $ISTIO_AUTH
  execute_sed "s/# controlPlaneAuthPolicy: MUTUAL_TLS/controlPlaneAuthPolicy: MUTUAL_TLS/" $ISTIO_AUTH
  execute_sed "s/NONE #--controlPlaneAuthPolicy/MUTUAL_TLS/" $ISTIO_AUTH
+  execute_sed "s/8080 #--controlPlaneAuthPolicy/15005/" $ISTIO_AUTH


these changes look like from the auth fix PR - is this PR not rebased or some such ?

istio-merge-robot · 2018-03-09T18:46:04Z

/test all [submit-queue is verifying that this PR is safe to merge]

ldemailly · 2018-03-09T18:55:39Z

please rebase, this seemingly includes #4066
/hold

istio-merge-robot · 2018-03-09T18:56:40Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @douglas-reid @mandarjog

mandarjog · 2018-03-09T18:57:33Z

@ldemailly I had cherry picked those changes while the other PR was still in flight.
When you merge into master those changes go away.
Any way I have rebased.

ldemailly · 2018-03-09T19:02:41Z

tests/e2e/framework/kubernetes.go

+func (k *KubeInfo) IngressOrFail(t *testing.T) string {
+	gw, err := k.Ingress()
+	if err != nil {
+		t.Fatalf("Unable to get ingress: %v", err)


does it dump the ingress logs and/or pod status when that happens?

ldemailly · 2018-03-09T19:04:43Z

thx
/lgtm
what fails on circle now ?

istio-merge-robot · 2018-03-09T19:05:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: douglas-reid, ldemailly

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~tests/OWNERS~~ [douglas-reid,ldemailly]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

ayj · 2018-03-09T19:27:39Z

0309 19:02:50.404] --- FAIL: TestVerifyCert (0.00s)
I0309 19:02:50.405] 	verify_cert_test.go:367: Succeeded: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid
I0309 19:02:50.405] 	verify_cert_test.go:369: extKeyUsage error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'ExtKeyUsage' field
I0309 19:02:50.405] 	verify_cert_test.go:369: KeyUsage Error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'KeyUsage' field
I0309 19:02:50.405] 	verify_cert_test.go:369: Org error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'Organization' field
I0309 19:02:50.406] 	verify_cert_test.go:369: TTL error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'NotAfter' - 'NotBefore'
I0309 19:02:50.406] 	verify_cert_test.go:369: IsCA error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'IsCA' field
I0309 19:02:50.407] 	verify_cert_test.go:369: Failed to verify key: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) invalid PEM-encoded key
I0309 19:02:50.407] 	verify_cert_test.go:369: Failed to match key/cert: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) the generated private key and cert doesn't match
I0309 19:02:50.407] 	verify_cert_test.go:369: Wrong SAN: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) the certificate doesn't have the expected SAN for: spiffe
I0309 19:02:50.408] 	verify_cert_test.go:369: Timestamp error: Unexpected error: failed to verify certificate: x509: certificate has expired or is not yet valid VS (expected) unexpected value for 'NotBefore' field

@wattli is working on a fix.

istio-merge-robot · 2018-03-09T21:39:44Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @douglas-reid @ldemailly @mandarjog

istio-testing · 2018-03-09T21:47:28Z

@mandarjog: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/e2e-suite-rbac-no_auth.sh	8cc1c1bf2ca2b0d460b34935c8e193a301e3d744	link	`/test e2e-suite-rbac-no_auth`
prow/istio-unit-tests.sh	`be41dff`	link	`/test istio-unit-tests`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: 2656f34 Update Envoy SHA to latest with MetricImpl optimizations (release-1.0). (istio#1939) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (istio#4190) 36809d80a fuzz: coverage profile generation instructions. (istio#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (istio#4075) 05c0d52d3 build: use clang-6.0. (istio#4168) 01f403ec4 thrift_proxy: introduce header transport (istio#4082) 564d256fb tcp: allow connection pool callers to store protocol state (istio#4131) 3e1d643b9 configs: match latest API changes (istio#4185) bc6a10c2f Fix wrong mock function name. (istio#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (istio#4182) 3d1325e89 Converting envoy configs to V2 (istio#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (istio#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (istio#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (istio#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (istio#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (istio#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (istio#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (istio#4157) 71152b710 ratelimit: Add ratelimit custom response headers (istio#4015) 306287418 ssl: make Ssl::Connection const everywhere (istio#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (istio#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (istio#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (istio#4130) 8c189a552 Make over provisioning factor configurable (istio#4003) 6c08fb43c Making test less flaky (istio#4149) 592775b7b fuzz: bare bones HCM fuzzer. (istio#4118) For istio#7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: c9d2230 Update Envoy SHA to latest with MetricImpl optimizations. (istio#1938) a32a587 Add a check cache test for string map sub keys (istio#1931) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (istio#4190) 36809d80a fuzz: coverage profile generation instructions. (istio#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (istio#4075) 05c0d52d3 build: use clang-6.0. (istio#4168) 01f403ec4 thrift_proxy: introduce header transport (istio#4082) 564d256fb tcp: allow connection pool callers to store protocol state (istio#4131) 3e1d643b9 configs: match latest API changes (istio#4185) bc6a10c2f Fix wrong mock function name. (istio#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (istio#4182) 3d1325e89 Converting envoy configs to V2 (istio#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (istio#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (istio#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (istio#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (istio#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (istio#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (istio#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (istio#4157) 71152b710 ratelimit: Add ratelimit custom response headers (istio#4015) 306287418 ssl: make Ssl::Connection const everywhere (istio#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (istio#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (istio#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (istio#4130) 8c189a552 Make over provisioning factor configurable (istio#4003) 6c08fb43c Making test less flaky (istio#4149) 592775b7b fuzz: bare bones HCM fuzzer. (istio#4118) For istio#7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

…). (#8161) This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: 2656f34 Update Envoy SHA to latest with MetricImpl optimizations (release-1.0). (#1939) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (#4190) 36809d80a fuzz: coverage profile generation instructions. (#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (#4075) 05c0d52d3 build: use clang-6.0. (#4168) 01f403ec4 thrift_proxy: introduce header transport (#4082) 564d256fb tcp: allow connection pool callers to store protocol state (#4131) 3e1d643b9 configs: match latest API changes (#4185) bc6a10c2f Fix wrong mock function name. (#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (#4182) 3d1325e89 Converting envoy configs to V2 (#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (#4157) 71152b710 ratelimit: Add ratelimit custom response headers (#4015) 306287418 ssl: make Ssl::Connection const everywhere (#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (#4130) 8c189a552 Make over provisioning factor configurable (#4003) 6c08fb43c Making test less flaky (#4149) 592775b7b fuzz: bare bones HCM fuzzer. (#4118) For #7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: c9d2230 Update Envoy SHA to latest with MetricImpl optimizations. (#1938) a32a587 Add a check cache test for string map sub keys (#1931) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (#4190) 36809d80a fuzz: coverage profile generation instructions. (#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (#4075) 05c0d52d3 build: use clang-6.0. (#4168) 01f403ec4 thrift_proxy: introduce header transport (#4082) 564d256fb tcp: allow connection pool callers to store protocol state (#4131) 3e1d643b9 configs: match latest API changes (#4185) bc6a10c2f Fix wrong mock function name. (#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (#4182) 3d1325e89 Converting envoy configs to V2 (#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (#4157) 71152b710 ratelimit: Add ratelimit custom response headers (#4015) 306287418 ssl: make Ssl::Connection const everywhere (#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (#4130) 8c189a552 Make over provisioning factor configurable (#4003) 6c08fb43c Making test less flaky (#4149) 592775b7b fuzz: bare bones HCM fuzzer. (#4118) For #7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

mandarjog requested a review from a team March 8, 2018 23:42

googlebot added the cla: yes label Mar 8, 2018

mandarjog added the kind/fixit label Mar 8, 2018

mandarjog force-pushed the faster_test branch from 8cc1c1b to f1a0c2e Compare March 9, 2018 05:04

mandarjog requested a review from a team March 9, 2018 05:11

mandarjog requested review from ayj and douglas-reid March 9, 2018 18:26

istio-testing assigned douglas-reid Mar 9, 2018

istio-testing added the lgtm label Mar 9, 2018

istio-merge-robot added the approved label Mar 9, 2018

ldemailly reviewed Mar 9, 2018

View reviewed changes

istio-testing added the do-not-merge/hold Block automatic merging of a PR. label Mar 9, 2018

mandarjog force-pushed the faster_test branch from 96b42e0 to 519e2ca Compare March 9, 2018 18:56

istio-merge-robot removed the lgtm label Mar 9, 2018

ldemailly reviewed Mar 9, 2018

View reviewed changes

ldemailly approved these changes Mar 9, 2018

View reviewed changes

istio-testing assigned ldemailly Mar 9, 2018

istio-testing added the lgtm label Mar 9, 2018

Lazily evaluate Ingress address

be41dff

mandarjog force-pushed the faster_test branch from 519e2ca to be41dff Compare March 9, 2018 21:38

istio-merge-robot removed the lgtm label Mar 9, 2018

mandarjog merged commit 9fd19e3 into istio:master Mar 9, 2018

This was referenced Aug 23, 2018

Update Proxy SHA to latest with MetricImpl optimizations. #8160

Merged

Update Proxy SHA to latest with MetricImpl optimizations (release-1.0). #8161

Merged

Lazily evaluate Ingress address #4118

Lazily evaluate Ingress address #4118

Uh oh!

Conversation

mandarjog commented Mar 8, 2018

Uh oh!

ldemailly commented Mar 9, 2018

Uh oh!

mandarjog commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldemailly commented Mar 9, 2018

Uh oh!

codecov bot commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mandarjog commented Mar 9, 2018

Uh oh!

ldemailly commented Mar 9, 2018

Uh oh!

mandarjog commented Mar 9, 2018

Uh oh!

mandarjog commented Mar 9, 2018

Uh oh!

istio-testing commented Mar 9, 2018

Uh oh!

mandarjog commented Mar 9, 2018

Uh oh!

douglas-reid commented Mar 9, 2018

Uh oh!

ldemailly Mar 9, 2018

Choose a reason for hiding this comment

Uh oh!

istio-merge-robot commented Mar 9, 2018

Uh oh!

ldemailly commented Mar 9, 2018

Uh oh!

istio-merge-robot commented Mar 9, 2018

Uh oh!

mandarjog commented Mar 9, 2018

Uh oh!

ldemailly Mar 9, 2018

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Mar 9, 2018

Uh oh!

istio-merge-robot commented Mar 9, 2018

Uh oh!

ayj commented Mar 9, 2018

Uh oh!

istio-merge-robot commented Mar 9, 2018

Uh oh!

istio-testing commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mandarjog commented Mar 9, 2018 •

edited

Loading

codecov bot commented Mar 9, 2018 •

edited

Loading

istio-testing commented Mar 9, 2018 •

edited

Loading