wasm: fix fail-close streams on VM failure.#16112
Conversation
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
repokitteh-read-only
bot
added
the
deps
|
will add test cases.. |
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
/retest |
|
Retrying Azure Pipelines: |
mathetake
force-pushed
the
wasm-fail-close
branch
from
1cdff7b to
df8b9ab
Compare
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
mathetake
force-pushed
the
wasm-fail-close
branch
from
df8b9ab to
61908d5
Compare
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
|
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
PiotrSikora
left a comment
PiotrSikora
left a comment
There was a problem hiding this comment.
Awesome work, thanks!
|
|
||
| TEST_P(WasmNetworkFilterTest, CloseUpstream) { | ||
| if (std::get<1>(GetParam()) == "rust") { | ||
| // TODO(mathetake): not yet supported in the Rust SDK. |
There was a problem hiding this comment.
This is blocked on proxy-wasm/proxy-wasm-rust-sdk#64, I'll add this once it's merged, so feel free to re-assign to me (if there are other changes, but don't waste CI cycle on it).
|
|
||
| TEST_P(WasmNetworkFilterTest, CloseDownstream) { | ||
| if (std::get<1>(GetParam()) == "rust") { | ||
| // TODO(mathetake): not yet supported in the Rust SDK. |
There was a problem hiding this comment.
This is blocked on proxy-wasm/proxy-wasm-rust-sdk#64, I'll add this once it's merged, so feel free to re-assign to me (if there are other changes, but don't waste CI cycle on it).
|
kindly ping @envoyproxy/dependency-shepherds PTAL! :) |
|
@moderation PTAL for deps. Also, is there a way we could automate those dependency checks? We've been stuck many times for a multiple days on what appears to be a manual check to verify that the commit is merged into |
|
/lgtm deps |
repokitteh-read-only
bot
removed
the
deps
Fixes envoyproxy#14947 and properly closes streams. This commit differentiates `failStream` from `closeStream` where the former is called when a VM fails, and the latter is called via `close_stream` or `grpc_close` by user Wasm codes. Notably, we try to send local response with 503 for http streams as expected by the description of `fail_open` api. The change here is a little and mostly done in Proxy-Wasm C++ host implementation(proxy-wasm/proxy-wasm-cpp-host#155). Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io> Signed-off-by: Gokul Nair <gnair@twitter.com>
4 participants
Fixes #14947 and properly closes streams. This commit differentiates
failStreamfromcloseStreamwhere the former is called when a VM fails, and the latter is called viaclose_streamorgrpc_closeby user Wasm codes. Notably, we try to send local response with 503 for http streams as expected by the description offail_openapi.The change here is a little and mostly done in Proxy-Wasm C++ host implementation(proxy-wasm/proxy-wasm-cpp-host#155).
Signed-off-by: Takeshi Yoneda takeshi@tetrate.io