Skip to content

Wasm upstreaming: required bazel/* support.#12116

Merged
lizan merged 18 commits intoenvoyproxy:masterfrom
jplevyak:upstream-wasm-bazel
Jul 29, 2020
Merged

Wasm upstreaming: required bazel/* support.#12116
lizan merged 18 commits intoenvoyproxy:masterfrom
jplevyak:upstream-wasm-bazel

Conversation

@jplevyak
Copy link
Copy Markdown
Contributor

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: Merge in changes from envoyproxy/envoy-wasm in the bazel/ directory as part of upstreaming Wasm.
Risk Level: Low
Testing: Unit tests pass.
Docs Changes: N/A
Release Notes: N/A
[Optional Fixes #Issue]
[Optional Deprecated:]

@jplevyak jplevyak marked this pull request as draft July 15, 2020 21:30
@lizan lizan self-assigned this Jul 15, 2020
@jplevyak jplevyak force-pushed the upstream-wasm-bazel branch from 80a2fb1 to 69ccf0f Compare July 16, 2020 23:20
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak force-pushed the upstream-wasm-bazel branch from 69ccf0f to 5a2a381 Compare July 16, 2020 23:39
@jplevyak jplevyak marked this pull request as ready for review July 16, 2020 23:42
@jplevyak jplevyak requested review from kyessenov and lizan July 16, 2020 23:42
@jplevyak
Copy link
Copy Markdown
Contributor Author

ERROR: Codecs are not synced: ./source/common/http/http2/codec_impl.cc does not match ./source/common/http/http2/codec_impl_legacy.cc. Update codec implementations to sync and/or update the diff manually to:

What is this?

@jplevyak jplevyak requested a review from PiotrSikora as a code owner July 20, 2020 23:01
@jplevyak jplevyak requested a review from PiotrSikora July 21, 2020 17:24
@jplevyak jplevyak requested a review from lizan July 22, 2020 04:29
jplevyak added 3 commits July 22, 2020 04:30
Signed-off-by: John Plevyak <jplevyak@gmail.com>
Signed-off-by: John Plevyak <jplevyak@gmail.com>
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak requested a review from PiotrSikora July 22, 2020 04:40
Signed-off-by: John Plevyak <jplevyak@gmail.com>
PiotrSikora
PiotrSikora previously approved these changes Jul 22, 2020
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.

LGTM sans the nit.

Signed-off-by: John Plevyak <jplevyak@gmail.com>
PiotrSikora
PiotrSikora previously approved these changes Jul 22, 2020
@jplevyak
Copy link
Copy Markdown
Contributor Author

//test/extensions/filters/network/rbac:integration_test FAILED

Suggestions?

@jplevyak
Copy link
Copy Markdown
Contributor Author

Updated wee8 => v8. PTAL

@jplevyak jplevyak requested a review from PiotrSikora July 24, 2020 19:30
@jplevyak
Copy link
Copy Markdown
Contributor Author

Could someone with permission kick circleci ? The go_control_plain_mirror failure seems like a flake.

@lizan
Copy link
Copy Markdown
Member

lizan commented Jul 24, 2020

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🔨 rebuilding ci/circleci: go_control_plane_mirror (failed build)

🐱

Caused by: a #12116 (comment) was created by @lizan.

see: more, trace.

…tream-wasm-bazel

Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak force-pushed the upstream-wasm-bazel branch from e4995f5 to 1a66ea1 Compare July 27, 2020 20:47
**kwargs
)

wasm_binary(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need proxy_wasm_cpp_sdk, wasm_binary in the envoy repo?

mainly asking as this pulls in emscripten as a dependency, which adds bloat to building envoy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not only is it intentional it is the title of the PR. Do you have some suggestion as to how we can support Wasm without adding the sdk and wasm binary to the the envoy repo?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAIK envoy only needs proxy_wasm_cpp_host, and does not need to build wasm binaries.

proxy_wasm_cpp_sdk is for building filters, which is not done by envoy. so not sure why is it needed.
if i want to build a filter with the cpp-sdk, I can add the proxy_wasm_cpp_host to my filter project directly.
is it the intention to fold https://github.com/proxy-wasm/proxy-wasm-cpp-sdk back into this repo?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is used for building tests in Envoy (vs checking-in compiled .wasm files).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah this is needed for test (without checking-in binaries).

@jplevyak jplevyak requested a review from lizan July 28, 2020 16:52
@lizan
Copy link
Copy Markdown
Member

lizan commented Jul 28, 2020

@jplevyak fix the typo? there's bunch of NDBUG vs NDEBUG. otherwise LGTM

Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
Copy link
Copy Markdown
Contributor Author

Could someone kick this? Or perhaps I should sync again?

@lizan
Copy link
Copy Markdown
Member

lizan commented Jul 28, 2020

re-kicked

@lizan lizan merged commit 7f6a716 into envoyproxy:master Jul 29, 2020
lizan pushed a commit that referenced this pull request Aug 4, 2020
Add authority field in envoy grpc message to override the default host name as cluster name.

Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Fix #12116 

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
Merge in changes from envoyproxy/envoy-wasm in the bazel/ directory as part of upstreaming Wasm.
Risk Level: Low
Testing: Unit tests pass.
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: John Plevyak <jplevyak@gmail.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
Add authority field in envoy grpc message to override the default host name as cluster name.

Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Fix envoyproxy#12116 

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
Merge in changes from envoyproxy/envoy-wasm in the bazel/ directory as part of upstreaming Wasm.
Risk Level: Low
Testing: Unit tests pass.
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: John Plevyak <jplevyak@gmail.com>
Signed-off-by: chaoqinli <chaoqinli@google.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
Add authority field in envoy grpc message to override the default host name as cluster name.

Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Fix envoyproxy#12116

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: chaoqinli <chaoqinli@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants