Skip to content

update Hyper and tokio::{io, net} dependencies#510

Merged
hawkw merged 57 commits intomaster-tokio-0.2from
eliza/0.2-clients
May 13, 2020
Merged

update Hyper and tokio::{io, net} dependencies#510
hawkw merged 57 commits intomaster-tokio-0.2from
eliza/0.2-clients

Conversation

@hawkw
Copy link
Contributor

@hawkw hawkw commented May 8, 2020

This branch updates the Hyper dependency to 0.13 (and thus also updates
http/http-body), and updates all uses of AsyncRead and
AsyncWrite and tokio::net types to use the Tokio 0.2 versions.This
also implies updating bytes from 0.4 to 0.5. Unfortunately, this
change is rather massive, but all these dependencies are pretty tightly
coupled. It wasn't really possible to upgrade them incrementally the way
it was for code in Linkerd, at least not without forking and patching the
dependencies, which I wanted to avoid.

Note on Test Support

This branch also makes a great deal of changes to the test support code
in linkerd2-app-integration. This is necessary because this code no
longer compiled after updating Hyper and the IO traits. When practical,
I've tried to make this a direct translation of the existing code.

I think there's a lot of opportunity to improve this code and reduce
complexity. In particular, the test-support clients work by spawning a
background thread with a Tokio runtime on it, and sending work to that
task and responses back over channels. This is so we can present a
synchronous API to the tests themselves, and write straight-line code in
tests rather than nested futures. In an async/await world, that's kind
of unnecessary. We could switch the integration tests to be async
functions written using #[tokio::test], and remove the complexity of
managing multiple background threads and runtimes for each test client
(and server). We would probably still want to isolate the test proxy
in its own runtime as we do currently, but we could run all the test
support code in the main test thread's runtime and not have to do all
this messing around with threads and channels.

However, I didn't do this kind of refactoring in this PR, beyond adding
async versions of some of the existing test support APIs (which still
use channels internally). These were added for use in tests which have
to call into async APIs from other crates and therefore need to be
async fns. I wanted to avoid totally rewriting the test support code
in this branch, however, since it's already a huge change.

Note on IO Trait Changes

The BoxedIo trait used to have a special shutdown_write method that
would call std::net::TcpStream::shutdown(Shutdown::Write). This was
added because Tokio 0.1's <TcpStream as AsyncWrite>::shutdown did
not call this method; it simply returned Ok(()). However, Tokio
0.2's <TcpStream as AsyncWrite>::poll_shutdown does call
std::net's TcpStream::shutdown. Since BoxedIo was ported directly
from the 0.1 code and still called both AsyncWrite::poll_shutdown
and Io::shutdown_write, we would try to shut down the stream twice,
and the second call would fail with a "transport endpoint is not
connected" error. This broke the tls_accept tests, which expected
there to be no error.

This commit removes the Io::shutdown_write method, since Tokio's
TcpStream now does what we want.

Signed-off-by: Eliza Weisman [email protected]

hawkw added 30 commits May 5, 2020 09:39
This does **not** update the payload module to `std::future`, since we
are still on the futures 0.1 version of Hyper. It only updates the
`tower-service` impls.

This will be necessary to bring back the HTTP clients.

Signed-off-by: Eliza Weisman <[email protected]>
This does **not** update the payload module to `std::future`, since we
are still on the futures 0.1 version of Hyper. It only updates the
`tower-service` impls.

This will be necessary to bring back the HTTP clients.

Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
proxy still not compile-y; uses old hyper in admin& elsewhere and also
some stuff is just broken, but the io & transport crates mostly build on
their own.

Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
hawkw added 16 commits May 11, 2020 11:17
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
whoops, this broke "everything"

Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
The `BoxedIo` trait used to have a special `shutdown_write` method that
would call `std::net::TcpStream::shutdown(Shutdown::Write)`. This was
added because Tokio 0.1's `<TcpStream as AsyncWrite>::shutdown` did
_not_ call this method; it simply returned `Ok(())`. However, Tokio
0.2's `<TcpStream as AsyncWrite>::poll_shutdown` *does* call
`std::net`'s `TcpStream::shutdown`. Since `BoxedIo` was ported directly
from the 0.1 code and still called both `AsyncWrite::poll_shutdown`
_and_ `Io::shutdown_write`, we would try to shut down the stream twice,
and the second call would fail with a "transport endpoint is not
connected" error. This broke the `tls_accept` tests, which expected
there to be no error.

This commit removes the `Io::shutdown_write` method, since Tokio's
`TcpStream` now does what we want.

Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw changed the title [WIP] Update Hyper and tokio::{io, net} dependencies update Hyper and tokio::{io, net} dependencies May 12, 2020
@hawkw hawkw requested review from a team and olix0r May 12, 2020 18:46
@hawkw hawkw marked this pull request as ready for review May 12, 2020 18:46
@hawkw
Copy link
Contributor Author

hawkw commented May 12, 2020

I know this branch is kind of massive. However, this should unblock pretty much all the remaining update work. Now that we're on the std::future versions of Hyper and tokio::{io, net}, we basically don't need any more compat layers between futures 0.1 and std::future. It should now be quite easy to turn the proxy back into an actual proxy, and start re-enabling functionality layer-by-layer.

hawkw added 3 commits May 12, 2020 15:02
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw requested a review from a team May 13, 2020 16:35
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

Looks good! Left a few non-blocking comments

@hawkw hawkw merged commit d72fe6f into master-tokio-0.2 May 13, 2020
@olix0r olix0r deleted the eliza/0.2-clients branch May 25, 2021 15:49
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.

3 participants