Skip to content

Merge envoy-wasm for v1.13.0.#2631

Merged
istio-testing merged 5 commits intoistio:masterfrom
jplevyak:merge-1.13.0
Jan 24, 2020
Merged

Merge envoy-wasm for v1.13.0.#2631
istio-testing merged 5 commits intoistio:masterfrom
jplevyak:merge-1.13.0

Conversation

@jplevyak
Copy link
Copy Markdown
Contributor

@jplevyak jplevyak commented Jan 24, 2020

Signed-off-by: John Plevyak jplevyak@gmail.com

@jplevyak
Merge envoy-wasm for v1.13.0.
4f4b621 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak requested a review from a team January 24, 2020 04:26
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jan 24, 2020
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 24, 2020
@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

There's a crash in WasmHandle destructor.

@jplevyak
Copy link
Copy Markdown
Contributor Author

jplevyak commented Jan 24, 2020

There are a number of changes in the underlying envoy repository that I am not familiar with.
Suggestions appreciated.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

It looks like a Wasm specific crash:

[2020-01-24 04:55:22.017][52058][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #9: Envoy::Extensions::Common::Wasm::Context::onDone() [0x558b75aab736]
[2020-01-24 04:55:22.026][52152][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #8: std::__1::function<>::operator()() [0x558b75ab612a]
[2020-01-24 04:55:22.060][52058][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #10: Envoy::Extensions::Common::Wasm::Wasm::shutdown() [0x558b75af2639]
[2020-01-24 04:55:22.068][52152][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #9: Envoy::Extensions::Common::Wasm::Context::onConfigure() [0x558b75aa8e73]
[2020-01-24 04:55:22.101][52058][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #11: Envoy::Extensions::Common::Wasm::WasmHandle::~WasmHandle()::{lambda()#1}::operator()() [0x558b75b1247d]
[2020-01-24 04:55:22.109][52152][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #10: Envoy::Extensions::Common::Wasm::Wasm::configure() [0x558b75af1c03]
[2020-01-24 04:55:22.142][52058][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #12: std::__1::__invoke<>() [0x558b75b1243d]
[2020-01-24 04:55:22.149][52152][critical][backtrace] [bazel-out/k8-fastbuild/bin/external/envoy/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:75] #11: Envoy::Extensions::Common::Wasm::createThreadLocalWasm() [0x558b75af3788]

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

You also need to re-generate Wasm files here I think.

douglas-reid
douglas-reid reviewed Jan 24, 2020
View reviewed changes
@jplevyak
Update .wasm files.
8d2c02f 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
WASM SDK v2 -> v3.
fc2ad9c 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
Address comments.
109ded4 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
PiotrSikora
PiotrSikora suggested changes Jan 24, 2020
View reviewed changes
WORKSPACE
# 1. Determine SHA256 `wget https://github.com/envoyproxy/envoy-wasm/archive/$COMMIT.tar.gz && sha256sum $COMMIT.tar.gz`
# 2. Update .bazelversion, envoy.bazelrc and .bazelrc if needed.
#
# envoy-wasm commit date: 12/19/19
Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the text above, you still need to update .bazelversion to 2.0.0.

@PiotrSikora
Copy link
Copy Markdown
Contributor

PiotrSikora commented Jan 24, 2020

I kicked off the build on my latop for the macOS, to make sure we don't release from a broken thing.

This was referenced Jan 24, 2020
Closed
Merged
@jplevyak
Bump bazelversion.
129c167 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

@PiotrSikora @jplevyak I'd like to get envoyproxy/envoy-wasm#345 in. I can send a follow-up SHA bump or you can include it here.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

Btw I assume the crash was due to ABI change. Should I file an issue to harden host once more, since it's likely to happen someone is going to run mis-matched Wasm file anyways?

@jplevyak
Copy link
Copy Markdown
Contributor Author

jplevyak commented Jan 24, 2020

Kuat, do we need that for the release? It isn't currently used AFAICT

@jplevyak
Copy link
Copy Markdown
Contributor Author

jplevyak commented Jan 24, 2020

We should probably just get this one in.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Jan 24, 2020

I think so. Why not? It's fairly self-contained from the rest. I can make a plugin PR to use it quickly, since I was waiting for it to make through. It would be nice to test out de-coupling plugin release from envoy release actually.

@PiotrSikora
Copy link
Copy Markdown
Contributor

PiotrSikora commented Jan 24, 2020

Yes, sorry, still waiting for one of the macOS builds to finish (either on CircleCI or my laptop, whichever is first).

PiotrSikora
PiotrSikora approved these changes Jan 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@istio-testing istio-testing merged commit 1d997c1 into istio:master Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyessenov kyessenov Awaiting requested review from kyessenov

@mandarjog mandarjog Awaiting requested review from mandarjog

3 more reviewers

@douglas-reid douglas-reid douglas-reid left review comments

@PiotrSikora PiotrSikora PiotrSikora approved these changes

@fpesce fpesce fpesce approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jplevyak @kyessenov @PiotrSikora @fpesce @douglas-reid @googlebot @istio-testing