Skip to content

[DO NOT SUBMIT] Update sha for upstream envoy (latest)#531

Closed
varungbt wants to merge 4 commits intoGoogleCloudPlatform:masterfrom
varungbt:vmanohar-pull-latest-envoy
Closed

[DO NOT SUBMIT] Update sha for upstream envoy (latest)#531
varungbt wants to merge 4 commits intoGoogleCloudPlatform:masterfrom
varungbt:vmanohar-pull-latest-envoy

Conversation

@varungbt
Copy link

@varungbt varungbt commented Jun 9, 2021

This pull request updates the sha version for the envoy repo's latest commit.

@google-oss-robot
Copy link
Collaborator

Hi @varungbt. Thanks for your PR.

I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

/assign @nareddyt

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

/ok-to-test

@nareddyt nareddyt changed the title Update sha for upstream envoy (latest) [DO NOT SUBMIT] Update sha for upstream envoy (latest) Jun 9, 2021
@nareddyt
Copy link
Contributor

nareddyt commented Jun 9, 2021

Oh man, it looks like the latest Envoy has some breaking changes that we need to fix.

(19:06:59) ERROR: /home/prow/go/src/github.com/GoogleCloudPlatform/esp-v2/src/envoy/http/service_control/BUILD:117:17: no such package '@envoy//include/envoy/server': BUILD file not found in directory 'include/envoy/server' of external repository @envoy. Add a BUILD file to a directory to mark it as a package. and referenced by '//src/envoy/http/service_control:service_control_call_impl_lib'
(19:06:59) ERROR: Analysis of target '//src/envoy:envoy' failed; build aborted: Analysis failed

This is announced here: https://groups.google.com/g/envoy-announce/c/MZU4vp3nf5U

Maybe you can just run the shell script in the post above to fix all our files, and then update this PR with those changes?

If it is too much work I can attempt to fix our side of things this week.

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@nareddyt I actually want this commit to be consumed
envoyproxy/envoy@db16625

So let me try to update the sha for this commit and see if it works fine

@google-oss-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: varungbt
To complete the pull request process, please ask for approval from nareddyt after the PR has been reviewed.

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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@nareddyt the build failed again due to some protobuf issue.
https://oss-prow.knative.dev/view/gs/oss-prow/pr-logs/pull/GoogleCloudPlatform_esp-v2/531/ESPv2-build/1402707919362854912

This is still due to a breaking change on envoy?

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@nareddyt let me try running the shellscript and commit a few more changes.

@nareddyt
Copy link
Contributor

nareddyt commented Jun 9, 2021

@nareddyt the build failed again due to some protobuf issue.
https://oss-prow.knative.dev/view/gs/oss-prow/pr-logs/pull/GoogleCloudPlatform_esp-v2/531/ESPv2-build/1402707919362854912

This is still due to a breaking change on envoy?

No, it seems unrelated. Envoy always have some breaking changes we have to fix. These shouldn't be too difficult, but it unrelated to the shell script and announcement.

I can work on it this week for you.

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@nareddyt sure let me know. We are a blocked due to the jwks refetch issue on envoy.
Our long running jobs under load are failing due to the envoy's missing jwks refetch. So wanted to get the espv2 built with the latest version of envoy.

I tried the shell script to update the includes but still the C++ compilation is unable to complete.

If you could release out a new version of espv2 with these changes, it would be greatly appreciated. Thank you.

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

Hi @varungbt , is this pr that you want to get?

If so, it will not help. The feature is not on by default. ESPv2 needs to add a flag to turn it on.

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@qiwzhang you are right I would like the commit you shared to be used in the ESPV2 runtime build. Can you share details on how to use the flag?

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

The flag is not added yet. @varungbt

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

Hi @nareddyt The envoy pr was submitted 13 days ago. My original plan was to wait for it to be officially released by upstream envoy into next envoy release.

Envoy 1.18.3 is already 29 days old. I am not sure about its release schedule. If it is about to have a new release. I like to wait for its new release to get that change.

@google-oss-robot
Copy link
Collaborator

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

Test name Commit Details Rerun command
ESPv2-presubmit-asan b351937 link /test ESPv2-presubmit-asan
ESPv2-presubmit b351937 link /test ESPv2-presubmit
ESPv2-build b351937 link /test ESPv2-build
ESPv2-presubmit-tsan b351937 link /test ESPv2-presubmit-tsan
ESPv2-API-regression-test b351937 link /test ESPv2-API-regression-test
ESPv2-cloud-run-e2e-cloud-run-grpc-echo b351937 link /test ESPv2-cloud-run-e2e-cloud-run-grpc-echo
ESPv2-cloud-run-e2e-cloud-function-http-bookstore b351937 link /test ESPv2-cloud-run-e2e-cloud-function-http-bookstore
ESPv2-cloud-run-e2e-cloud-run-http-bookstore b351937 link /test ESPv2-cloud-run-e2e-cloud-run-http-bookstore
ESPv2-cloud-run-e2e-app-engine-http-bookstore b351937 link /test ESPv2-cloud-run-e2e-app-engine-http-bookstore
ESPv2-gke-e2e-grpc-interop-managed b351937 link /test ESPv2-gke-e2e-grpc-interop-managed
ESPv2-gke-e2e-grpc-echo-managed b351937 link /test ESPv2-gke-e2e-grpc-echo-managed
ESPv2-gke-e2e-http-bookstore-managed b351937 link /test ESPv2-gke-e2e-http-bookstore-managed
ESPv2-gke-e2e-http-bookstore-managed-using-sa-cred b351937 link /test ESPv2-gke-e2e-http-bookstore-managed-using-sa-cred
ESPv2-anthos-cloud-run-e2e-anthos-cloud-run-http-bookstore b351937 link /test ESPv2-anthos-cloud-run-e2e-anthos-cloud-run-http-bookstore
Details

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.

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

Hmm, envoy 1.19.0 will be released at 6/30/2021, according to this doc

Is that too late for you?

In general, we only sync to a release version. Unless it is a critical bug fix or important new features.

@varungbt
Copy link
Author

varungbt commented Jun 9, 2021

@qiwzhang our long running jobs are failing due to the envoy's jwks fetch errors. It would be very helpful if we could get this commit into espv2 with a new runtime pushed to the global repo.

@qiwzhang
Copy link
Contributor

qiwzhang commented Jun 9, 2021

OK, I will work on this. will sync to the latest Envoy and add a flag to enable fetch_jwks_in_background feature.

@qiwzhang
Copy link
Contributor

Replaced this with #532

@qiwzhang qiwzhang closed this Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants