refactor(http/retry): port remaining `ReplayBody` tests to `Frame<T>` #3567

cratelyn · 2025-01-27T18:53:11Z

based on #3564. see linkerd/linkerd2#8733.

this branch upgrades the remaining parts of the ReplayBody<B> test suite to poll bodies in terms of Frame<T>s.

see linkerd/linkerd2#8733. this commit upgrades a test that uses defunct `data()` and `trailers()` futures. Signed-off-by: katelyn martin <[email protected]>

this commit adds a method that exposes the inner `B`-typed body's `is_end_stream()` trait method, gated for use in tests. Signed-off-by: katelyn martin <[email protected]>

this is a refactoring commit, upgrading more of the replay body test to work in terms of `Frame<T>`. this updates the `body_to_string()` helper in particular. Signed-off-by: katelyn martin <[email protected]>

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

cratelyn

review notes...

cratelyn · 2025-01-27T18:56:17Z

linkerd/http/retry/src/replay.rs

    }

    #[tokio::test(flavor = "current_thread")]
    async fn eos_only_when_fully_replayed() {


this test was marginally tricky to port.

i discovered a small, interesting tidbit in that the replay body won't report itself as finished until the trailers are (not) yielded. that matches the existing behavior, it's just slightly more noticeable in the poll_frame(..) era.

nb: that's fine, in fact.

A return value of false does not guarantee that a value will be returned from poll_frame.

it's okay if we are slightly pessimistic about returning true; a false negative is acceptable, a false positive is not.

cratelyn · 2025-01-27T18:58:58Z

linkerd/http/retry/src/replay.rs

-        initial.trailers().await.expect("trailers should not error");
+        // Read the initial body, show that the replay does not consider itself to have reached the
+        // end-of-stream. Then drop the initial body, show that the replay is still not done.
+        assert!(!initial.is_end_stream());
+        initial
+            .frame()
+            .await
+            .expect("yields a result")
+            .expect("yields a frame")
+            .into_data()
+            .expect("yields a data frame");


nb: we don't use the body_to_string() helper, or a closure, in this test because we're explicitly concerned with the outcome of is_end_stream() before/after a previous replay is dropped.

cratelyn · 2025-01-27T19:00:55Z

linkerd/http/retry/src/replay.rs

-    async fn body_to_string<T>(mut body: T) -> String
+    async fn body_to_string<B>(body: B) -> (String, Option<HeaderMap>)


now that we have a single frame() method, this function knows the body is done when it gets either a None or a Some(Ok(trailers)). as a result, we return a (data, trls) tuple here.

pr's #3564 and #3567, 1eb822f and 3204278 respectively, replaced uses of defunct `http_body::Body` trait methods — namely, `data()` and `trailers()`. this commit updates two remaining uses of `data()` that were missed in this initial pass. see linkerd/linkerd2#8733 for more information. 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]>

cratelyn added 5 commits January 24, 2025 16:31

refactor(http/retry): replays_trailers() uses Frame<T>

28c80b8

see linkerd/linkerd2#8733. this commit upgrades a test that uses defunct `data()` and `trailers()` futures. Signed-off-by: katelyn martin <[email protected]>

refactor(http/retry): trailers_only() uses Frame<T>

cc7adcb

see linkerd/linkerd2#8733. this commit upgrades a test that uses defunct `data()` and `trailers()` futures. Signed-off-by: katelyn martin <[email protected]>

feat(http/retry): ForwardCompatibleBody::is_end_stream()

29aa70e

this commit adds a method that exposes the inner `B`-typed body's `is_end_stream()` trait method, gated for use in tests. Signed-off-by: katelyn martin <[email protected]>

refactor(http/retry): body_to_string() helper uses Frame<T>

af035aa

this is a refactoring commit, upgrading more of the replay body test to work in terms of `Frame<T>`. this updates the `body_to_string()` helper in particular. Signed-off-by: katelyn martin <[email protected]>

refactor(http/retry): chunk() helper uses Frame<T>

0b83e90

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

cratelyn commented Jan 27, 2025

View reviewed changes

cratelyn marked this pull request as ready for review January 27, 2025 19:02

cratelyn requested a review from a team as a code owner January 27, 2025 19:02

cratelyn mentioned this pull request Jan 27, 2025

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

Closed

olix0r approved these changes Jan 27, 2025

View reviewed changes

Base automatically changed from kate/http-retry.upgrade-replay-body-tests.pt-1 to main January 27, 2025 20:44

cratelyn merged commit 3204278 into main Jan 27, 2025
15 checks passed

cratelyn deleted the kate/http-retry.upgrade-replay-body-tests.pt-2 branch January 27, 2025 20:45

cratelyn mentioned this pull request Jan 28, 2025

refactor(http/retry): update more Body::data() calls #3573

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(http/retry): port remaining `ReplayBody` tests to `Frame<T>` #3567

refactor(http/retry): port remaining `ReplayBody` tests to `Frame<T>` #3567

Uh oh!

cratelyn commented Jan 27, 2025

Uh oh!

cratelyn left a comment

Uh oh!

cratelyn Jan 27, 2025

Uh oh!

cratelyn Jan 27, 2025

Uh oh!

cratelyn Jan 27, 2025

Uh oh!

cratelyn Jan 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		async fn body_to_string<T>(mut body: T) -> String
		async fn body_to_string<B>(body: B) -> (String, Option<HeaderMap>)

refactor(http/retry): port remaining ReplayBody tests to Frame<T> #3567

refactor(http/retry): port remaining ReplayBody tests to Frame<T> #3567

Uh oh!

Conversation

cratelyn commented Jan 27, 2025

Uh oh!

cratelyn left a comment

Choose a reason for hiding this comment

Uh oh!

cratelyn Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

cratelyn Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

cratelyn Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

cratelyn Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

refactor(http/retry): port remaining `ReplayBody` tests to `Frame<T>` #3567

refactor(http/retry): port remaining `ReplayBody` tests to `Frame<T>` #3567