Fix Pilot port number config for Ingress and ConfigMap#4066
Fix Pilot port number config for Ingress and ConfigMap#4066ldemailly merged 1 commit intoistio:masterfrom
Conversation
install/updateVersion.sh
Outdated
There was a problem hiding this comment.
can you change it to optional: false instead of removing it fully. ?
There was a problem hiding this comment.
"s/optional: \(.*\)/optional: false/"
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wattli The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test e2e-suite-rbac-auth |
Codecov Report
@@ Coverage Diff @@
## master #4066 +/- ##
=======================================
+ Coverage 75% 75% +1%
=======================================
Files 305 305
Lines 27871 27962 +91
=======================================
+ Hits 20726 20940 +214
+ Misses 5827 5699 -128
- Partials 1318 1323 +5
Continue to review full report at Codecov.
|
|
/test all [submit-queue is verifying that this PR is safe to merge] |
@myidpt @wattli do not merge ? |
|
@mandarjog How did you get the Envoy log? |
|
@myidpt it has not changed (and thankfully is still working well on prow... for now...): go to the failed build link, click artifact and click on the namespacename and then the pod's log |
|
@ldemailly I found it on prow, thanks. |
|
I saw the TestSimpleIngress still exhaust the retries. I will try to flip "optional: true" for secret mount on ingress (and maybe also others) to see the result. |
|
I can exclude the Envoy restarting as an issue. This test fails without Pilot's Envoy restarting: Still investigating. |
|
You could dump bootstrap and certs, and start it locally, right?
…On Thu, Mar 8, 2018 at 4:39 PM Oliver Liu ***@***.***> wrote:
I can exclude the Envoy restarting as an issue. This test fails without
Pilot's Envoy restarting:
https://k8s-gubernator.appspot.com/build/istio-prow/pull/istio_istio/4066/e2e-simpleTests/834/
Pilot's Envoy log
https://storage.googleapis.com/istio-prow/pull/istio_istio/4066/e2e-simpleTests/834/artifacts/simple-auth-test-e544e2a624f94902b4f9/istio-pilot-6d87788d8c-ckp8q_container:istio-proxy.log
Still investigating.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#4066 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJGIxji1kst31NPV4op2y4CRDqVkoPIYks5tcc84gaJpZM4Sh9Xh>
.
|
pilot/pkg/proxy/envoy/v1/watcher.go
Outdated
There was a problem hiding this comment.
this needs to be a wait for the cert not a sleep
There was a problem hiding this comment.
that's just experimental :) I fixed it.
|
Thanks to Kuat. We found the issue is due to --discoveryAddress is set to 8080 on ingress: It's a problem in updateVersion.sh |
|
the cert and startup order still is an issue, glad you found both, let's fix both |
|
That is great sleuth work @Kuat and @myipdt. |
|
/test e2e-suite-rbac-auth |
|
Having one way is always better, so helm or not helm, get rid of the
multitude of ways we create YAMLs :)
…On Thu, Mar 8, 2018, 7:44 PM mandarjog ***@***.***> wrote:
/test e2e-suite-rbac-auth
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#4066 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJGIxhSNUjcqLYjPO8zpndWXTKDIK-h4ks5tcfqsgaJpZM4Sh9Xh>
.
|
|
While the port number is clearly the most egregious bug and it's fantastic to solve it asap I'd like to not lose the fact that #3757 is how that bug sneaked in undetected behind the "oh ingress fails already anyway" - so we do need to start pilot's envoy only when the certs are there and get those certs there faster by starting CA + service accounts first. |
|
@ldemailly I will have a PR to fix the proxy bootstrapping issue as well. |
|
/retest |
|
@myidpt: The following tests failed, say
DetailsInstructions 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. |
Partially fixes #3757.
Pilot port number in mesh config and ingress are wrongly configured as 8080 when mTLS is configured. It should be 15005.