feat(http/retry): add more replay body test coverage #3583

cratelyn · 2025-01-30T18:35:34Z

we are currently in the process of upgrading our hyper, http, and http-body dependencies to their 1.0 major releases.

this branch introduces additional test coverage, and further refines existing test coverage, concerning how a ReplayBody<B> behaves when it reaches the "end of stream" of its internal B-typed body.

additional assertions are added to show that bodies with trailers may be replayed an arbitrary number of times, and that capacity errors occur precisely at their expected boundary. additional assertions are added to confirm that ReplayBody::is_capped() reports these conditions properly.

this branch also notably outlines the unit test suite into a separate file, due to its size. as a result, reviewers are encouraged to walk through this branch on a commit-by-commit basis when reading these changes.

i noticed some relatively minor issues with is_end_stream() and size_hint() while i was reviewing this middleware, in preparation to port it to http-body 1.0. i have left TODO comments noting where today's behavior is slightly askew, but intentionally avoided fixing them here. my goal on that front is to highlight those wrinkles so that later fixes to these edges are more easily reviewable.

there are more than 500 lines of unit tests. let's move them into a submodule, for convenience. Signed-off-by: katelyn martin <[email protected]>

this is a small cosmetic change reording some test helpers. there is a common convention of affixing a banner comment above groups of `impl T {}` blocks, which is useful when top-level blocks are folded in an editor. similarly, there is a convention of defining structures at the top of a file. this commit reorganizes the replay body tests to follow each of these conventions. Signed-off-by: katelyn martin <[email protected]>

just to be extra sure! Signed-off-by: katelyn martin <[email protected]>

this is part of a family of other tests called `replays_one_chunk()`, `replays_several_chunks()`, and `replays_trailers()`. let's name this something that lines up with this convention. Signed-off-by: katelyn martin <[email protected]>

we have a unit test, called `eos_only_when_fully_replayed` that confirms `Body::end_of_stream()` reports the stream ending properly. soon, we aim to introduce additional test coverage that exercises this when a body has trailers, as well. this will be useful for assurance related to upgrading to http-body v1.x. see linkerd/linkerd2#8733 for more information. unfortunately, hyper 0.14's channel-backed body does not report itself as having reached the end of the stream. this is an unfortunate quality that prevents us from using `Test::new()`. this commit adds a `TestBody` type that we can use in place of `BoxBody::from_static(..)`, which boxes a static string, but does not send trailers. Signed-off-by: katelyn martin <[email protected]>

this commit introduces additional test coverage that exercises `is_end_stream()` when a replay body is wrapping a body with trailers. this will be useful for assurance related to upgrading to http-body v1.x. see linkerd/linkerd2#8733 for more information. Signed-off-by: katelyn martin <[email protected]>

Signed-off-by: katelyn martin <[email protected]>

we want to show that exceeding the capacity is the point at which replays will fail. this commit defines some constants to further communicate and encode this relationship between the bytes sent, and the capacity of the replay body. further, it shortens the second frame sent so that we ensure precisely when a body becomes capped. Signed-off-by: katelyn martin <[email protected]>

Signed-off-by: katelyn martin <[email protected]>

for now, a replaying body that will not yield trailers must be polled to the `None` before reporting itself as reaching the end of the stream. this isn't hugely important, but does affect some test control flow. leave two todo comments so that if/when upgrading to hyper 1.0, it is clear that these are not load-bearing or otherwise expected behavior, should this behavior be rectified. Signed-off-by: katelyn martin <[email protected]>

note: this commit will not compile, code changes are intentionally elided from this commit. this commit upgrades hyper, http, tonic, prost, related dependencies, and their assorted cargo features. see <linkerd/linkerd2#8733>. see also: * #3379 * #3380 * #3382 * #3405 * hyperium/hyper#3796 * #3411 * #3421 * #3427 * #3428 * #3432 * #3433 * #3444 * #3445 * #3454 * #3455 * #3456 * #3457 * #3461 * #3459 * #3465 * #3466 * #3467 * #3468 * linkerd/linkerd2-proxy-api#421 * linkerd/linkerd2#13492 * linkerd/linkerd2#13493 * hyperium/hyper#3816 * #3472 * #3473 * #3479 * tokio-rs/tokio#7059 * #3509 * hyperium/http-body#140 * #3515 * hyperium/http-body#141 * #3530 * #3531 * #3540 * #3556 * #3558 * #3559 * #3564 * #3567 * #3573 * #3583 * hyperium/http-body#144 * #3585 * #3586 * #3597 * #3598 * #3611 * #3614 * #3615 * #3616 * #3647 * #3651 * #3653 * #3654 * #3655 * #3656 * #3657 * #3660 * #3671 * #3672 * #3673 * #3676 * hyperium/http-body#147 * #3692 * #3699 * #3700 * #3701 * #3708 * linkerd/drain-rs#36 * #3715 * #3717 * eminence/procfs#340 --- squash: chore(deps): add hyper-util workspace dependency chore(deps): add http-body-util workspace dependency chore(deps): upgrade linkerd2-proxy-api this commit represents main as of linkerd/linkerd2-proxy-api#421. Signed-off-by: katelyn martin <[email protected]>

## ✨ chore(deps): upgrade to hyperium 1.x crates this branch performs an exciting upgrade for our proxy. this branch upgrades a number of our dependencies so that we use the 1.0 release family of the `hyper` http framework, and its ecosystem. see the [v1.0 announcement][hyper-v1] for more information. this branch upgrades the following dependencies: * `h2`: 0.3 -> 0.4 * `http`: 0.2 -> 1 * `http-body`: 0.4 -> 1 * `hyper`: 0.14.32 -> 1 * `prost`: 0.12 -> 0.13 * `prost-build`: 0.12 -> 0.13 * `prost-types`: 0.12 -> 0.13 * `tonic`: 0.10 -> 0.12 * `tonic-build`: 0.10 -> 0.12 a `hyper-util` dependency is added, which provides among other things, legacy-compatible interfaces such as `hyper_util::client::legacy::Client`, or glue to use `hyper` with the tokio runtime. see <https://docs.rs/hyper-util/latest/hyper_util/> for more information. a `http-body-util` dependency is added, which provides a `BodyExt` trait and a channel-backed body for use in unit tests. the `deprecated` feature flag that was active on our `0.14` hyper dependency has been removed, along with the `stream` and `runtime` feature flags. the `linkerd2-proxy-api` dependency is updated. see: <linkerd/linkerd2-proxy-api#421> ### 📝 notes for review bear particular attention to changes involving `http_body::Body` middleware. the change from two separate `poll_data()` and `poll_trailers()` functions, to a single `poll_frame()` method, induces some subtle changes to various pieces of middleware. also bear in mind that failing to set a timer, in our case `hyper_util::rt::TokioTimer`, can cause http/2 clients, or http/1 and http/2 servers, to panic. make sure that any uses of `hyper::server::conn::http1::Builder`, `hyper::client::conn::http1::Builder`, or `hyper::client::conn::http2::Builder` install a timer. ### ❗ breaking change: `l5d-proxy-error` values the `l5d-proxy-error` header can be examined to observe the cause of proxy errors encountered when sending meshed traffic. by virtue of this using a newer `hyper` client in the proxy, some error messages may in turn look different. for example, an error like `"connect timed out after 1s"` may now appear as `"client error (Connect)"`. ### 📚 other notes this work, by virtue of touching so many parts of the system, is carried out in distinct commits. an initial commit upgrades the dependencies at th workspace level. subsequent commits will not compile if the `--workspace` flag is provided, but the intent of this branch is to update each crate individually. use commands like, e.g. `cargo check --tests -p linkerd-proxy-http` to build particular crates at intermediate commits within this branch. this commit is also only the final leaf in an _extended_ line of work. this has been done to mitigate the effort of reviewing this change, and the risk of churn in the event of any unanticipated errors. see the top-level comment in linkerd/linkerd2#8733 for an overview of all of the work that brought us to this juncture. [hyper-v1]: https://seanmonstar.com/blog/hyper-v1/ --- * chore(deps): upgrade to hyper 1.x note: this commit will not compile, code changes are intentionally elided from this commit. this commit upgrades hyper, http, tonic, prost, related dependencies, and their assorted cargo features. see <linkerd/linkerd2#8733>. see also: * #3379 * #3380 * #3382 * #3405 * hyperium/hyper#3796 * #3411 * #3421 * #3427 * #3428 * #3432 * #3433 * #3444 * #3445 * #3454 * #3455 * #3456 * #3457 * #3461 * #3459 * #3465 * #3466 * #3467 * #3468 * linkerd/linkerd2-proxy-api#421 * linkerd/linkerd2#13492 * linkerd/linkerd2#13493 * hyperium/hyper#3816 * #3472 * #3473 * #3479 * tokio-rs/tokio#7059 * #3509 * hyperium/http-body#140 * #3515 * hyperium/http-body#141 * #3530 * #3531 * #3540 * #3556 * #3558 * #3559 * #3564 * #3567 * #3573 * #3583 * hyperium/http-body#144 * #3585 * #3586 * #3597 * #3598 * #3611 * #3614 * #3615 * #3616 * #3647 * #3651 * #3653 * #3654 * #3655 * #3656 * #3657 * #3660 * #3671 * #3672 * #3673 * #3676 * hyperium/http-body#147 * #3692 * #3699 * #3700 * #3701 * #3708 * linkerd/drain-rs#36 * #3715 * #3717 * eminence/procfs#340 --- squash: chore(deps): add hyper-util workspace dependency chore(deps): add http-body-util workspace dependency chore(deps): upgrade linkerd2-proxy-api this commit represents main as of linkerd/linkerd2-proxy-api#421. Signed-off-by: katelyn martin <[email protected]> * chore(http/box): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(hyper-balance): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retain): ugrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/stream-timeouts): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/classify): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/upgrade): upgrade to hyper 1.x NOTE: there is a comment noting that the upgrade middleware does not expect to be cloneable. it is unfortunately, however, at odds with the new bounds expected of extensions. so, `Http11Upgrade` is now Clone'able, but a comment is left in place noting this weakened invariant. it's worth investigating how upgrades have changed since, in more detail, but for the current moment we are interested in being especially conservative about changing behavior, and focusing on api changes like `Body::poll_frame(..)`. Signed-off-by: katelyn martin <[email protected]> * chore(metrics): upgrade to hyper 1.x a brief note; this commit happened to tickle an unfortunate sharp edge in `BoxBody` and `Full`'s respective constructors. type inference could not figure out how to construct the body, so we refrain from boxing the response body now. Signed-off-by: katelyn martin <[email protected]> * chore(http/metrics): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/prom): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/insert): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retry): deprecate linkerd-http-body-compat Signed-off-by: katelyn martin <[email protected]> * chore(mock/http-body): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retry): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(proxy/tap): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(proxy/http): update to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/core): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/test): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/admin): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/outbound): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/inbound): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/integration): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(transport-header): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(spiffe-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(opencensus-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(opentelemetry-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(deny.toml): update cargo-deny directives this commit updates the contents of `deny.toml`. Signed-off-by: katelyn martin <[email protected]> * chore: `compile` has been renamed to `compile_protos` this addresses deprecation warnings, updating calls to a function that has since been renamed. Signed-off-by: katelyn martin <[email protected]> * chore(deps): remove `linkerd-http-body-compat` dependencies this commit removes this crate, which we added to future proof code for this upgrade, from its dependents. Signed-off-by: katelyn martin <[email protected]> * chore(http/body-compat): remove `linkerd-http-body-compat` crate Signed-off-by: katelyn martin <[email protected]> * chore(deps): update to drain 0.2.1 see linkerd/drain-rs#41. Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>

the initial replay body, circa the usage of our "compatibility" layer (4b53081, #3598), used to need an extra poll to confirm the absence of trailers before it would report itself as reaching the end of the stream. these tests were added in (afda8a7, #3583). this was an artifact of how the compatibility middleware masked the previous `poll_data()` and `poll_trailer()` methods behind a forward-compatible `poll_frame()`- and `frame()`-oriented interface. this commit removes these extra calls to `initial.frame().await`, now that the initial body will report the end of stream without an extra call to await a `None`. X-ref: #3598 X-ref: #3583 Signed-off-by: katelyn martin <[email protected]>

cratelyn added 7 commits January 30, 2025 13:16

refactor(http/retry): outline ReplayBody<B> unit tests

154986d

there are more than 500 lines of unit tests. let's move them into a submodule, for convenience. Signed-off-by: katelyn martin <[email protected]>

nit(http/retry): test replays trailers twice

f70482c

just to be extra sure! Signed-off-by: katelyn martin <[email protected]>

nit(http/retry): rename trailers_only() test

21db80e

this is part of a family of other tests called `replays_one_chunk()`, `replays_several_chunks()`, and `replays_trailers()`. let's name this something that lines up with this convention. Signed-off-by: katelyn martin <[email protected]>

feat(http/retry): add is_capped() test coverage

ef76ace

Signed-off-by: katelyn martin <[email protected]>

cratelyn mentioned this pull request Jan 30, 2025

proxy: Figure out the plan for Hyper 1.0 migration linkerd/linkerd2#8733

Open

cratelyn added 3 commits January 30, 2025 13:43

feat(http/retry): add size_hint() test coverage

8dda02d

Signed-off-by: katelyn martin <[email protected]>

cratelyn force-pushed the kate/http-retry.add-more-replay-body-test-coverage branch from e246186 to 50c2d6f Compare January 30, 2025 18:43

cratelyn marked this pull request as ready for review January 30, 2025 18:46

cratelyn requested a review from a team as a code owner January 30, 2025 18:46

olix0r approved these changes Jan 30, 2025

View reviewed changes

cratelyn merged commit afda8a7 into main Jan 30, 2025
15 checks passed

cratelyn deleted the kate/http-retry.add-more-replay-body-test-coverage branch January 30, 2025 21:01

cratelyn mentioned this pull request Apr 21, 2025

nit(http/retry): remove extra body poll in tests #3877

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(http/retry): add more replay body test coverage #3583

feat(http/retry): add more replay body test coverage #3583

Uh oh!

cratelyn commented Jan 30, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

feat(http/retry): add more replay body test coverage #3583

feat(http/retry): add more replay body test coverage #3583

Uh oh!

Conversation

cratelyn commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cratelyn commented Jan 30, 2025 •

edited

Loading