introduce tests for isolated services by hawkw · Pull Request #655 · linkerd/linkerd2-proxy

hawkw · 2020-09-11T18:31:24Z

This branch introduces a new style of tests to the proxy codebase, and
an initial implementation of new test support code for writing tests in
this style. Rather than running an entire proxy alongside a simulated
control plane, and binding actual network socket, as the present
integration tests do, the new tests run individual proxy components in
isolation, using simpler mock implementations of components like name
resolution and IO.

This approach has a few advantages. It should reduce flakiness
significantly, since we don't perform any IO and don't need to
synchronize events between multiple threads running test support
servers/controllers (the new mocks all just synchronously provide values
immediately). We can run everything in a single thread using Tokio's
basic scheduler. Additionally, since we are testing individual
components in isolation, these tests can live within the crate for the
part of the proxy being tested. This means we have access to more
internal state to make assertions on, rather than having to make
assertions on side effects like metrics.

I've added a new linkerd2-app-test crate that adds some initial mocks
for this kind of testing, including a simple mock resolver and a mock
connector using tokio-test's mock IO. I've also added a simple "hello
world" test for the outbound TCP stack written in this style. The
intention is to use the new test support code for testing the recent
changes adding TCP mTLS and load-balancing.

In order to make this code more testable, it was necessary to move a few
things around. In particular, the outbound TCP balancer stack is now
constructed in a separate method, rather than in Config::build_server,
so that it can be tested without requiring all the dependencies of the
full HTTP server stack. A few other similar changes were also necessary.

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

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

linkerd/app/outbound/src/lib.rs

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

olix0r

Tests look great. Some superficial feedback

olix0r · 2020-09-16T14:54:17Z

linkerd/app/test/src/lib.rs

+
+pub use futures::{future, FutureExt, TryFuture, TryFutureExt};
+
+pub use linkerd2_app_core::{self as app_core, Addr, Error};


just call this core? or are there other cores we should be aware of?

IIRC it clashed with libcore. will double check if this is the case.

yup:

error[E0659]: `core` is ambiguous (name vs any other name during import resolution) --> linkerd/app/outbound/src/tests.rs:12:9 | 12 | use core::{ | ^^^^ ambiguous name | = note: `core` could refer to a built-in crate = help: use `::core` to refer to this crate unambiguously note: `core` could also refer to the crate imported here --> linkerd/app/outbound/src/tests.rs:3:25 | 3 | use linkerd2_app_core::{self as core, metrics::Metrics, Addr}; | ^^^^^^^^^^^^ = help: use `self::core` to refer to this crate unambiguously error: aborting due to previous error

linkerd/app/test/src/lib.rs

linkerd/app/outbound/Cargo.toml

linkerd/app/outbound/src/lib.rs

linkerd/app/integration/src/lib.rs

linkerd/app/outbound/src/tests.rs

Co-authored-by: Oliver Gould <[email protected]>

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

kleimkuhler

Looks great!

kleimkuhler · 2020-09-16T19:24:11Z

linkerd/app/test/src/resolver.rs

+    atomic::{AtomicBool, Ordering},
+    Arc, Mutex,
+};
+// use std::future::Future;


Can remove this

kleimkuhler · 2020-09-16T19:24:17Z

linkerd/app/test/src/resolver.rs

+use std::net::SocketAddr;
+use std::task::{Context, Poll};
+use tokio::sync::mpsc;
+// use tower::Service;


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

This release fixes a recent regression in multicluster gateway configurations that would forbid inbound gateway traffic. It also fixes URI normalization for orig-proto-upgrade requests that do not include a `Host` header. --- * http: Simplify stacks and target types (linkerd/linkerd2-proxy#656) * Make SkipDetect more generic as stack::MakeSwitch (linkerd/linkerd2-proxy#657) * introduce tests for isolated services (linkerd/linkerd2-proxy#655) * http: Put normalize_uri back on the stack (linkerd/linkerd2-proxy#659) * inbound: Apply loop detection on the connect stack (linkerd/linkerd2-proxy#660) * tracing: Elide redundant info in tracing contexts (linkerd/linkerd2-proxy#661) * outbound: Reorganize outbound stacks (linkerd/linkerd2-proxy#662) * app: Decouple stacks from listeners (linkerd/linkerd2-proxy#663) * inbound: Split HTTP detection stack from TLS (linkerd/linkerd2-proxy#664) * integration: Bundle tests in src (linkerd/linkerd2-proxy#665)

hawkw added 12 commits September 11, 2020 11:28

move test support to yet another crate

56873e0

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

wip

4b6350b

wip

7fa62ff

outbound: build TCP stack separately for tests

bf83988

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

oh my god it basically works!!!

62a63a4

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

attempt to explain what's going on

e3bd9fd

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

put integration test stuff back where it belongs

bf972bd

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

move tests to outbound (where they belong)

a654c52

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

no longer needs to be pub

299ea50

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

add a little tracing to tests

3aead83

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

cleanup

40fb04a

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

use basic scheduler

6fad739

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

hawkw requested review from a team and olix0r September 11, 2020 18:31

hawkw changed the title ~~Eliza/tcp lb tests~~ introduce tests for isolated services Sep 11, 2020

olix0r reviewed Sep 15, 2020

View reviewed changes

linkerd/app/outbound/src/lib.rs Outdated Show resolved Hide resolved

use SocketAddr in balance stack

483d6d5

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

hawkw requested a review from olix0r September 15, 2020 18:25

olix0r requested changes Sep 16, 2020

View reviewed changes

hawkw and others added 3 commits September 16, 2020 10:17

Update linkerd/app/test/src/lib.rs

eefbd26

Co-authored-by: Oliver Gould <[email protected]>

fix whitespace

0730825

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

style fixup

51d4d8a

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

hawkw requested a review from olix0r September 16, 2020 17:24

olix0r approved these changes Sep 16, 2020

View reviewed changes

hawkw added 2 commits September 16, 2020 12:25

Merge branch 'main' into eliza/tcp-lb-tests

1a4248c

rustfmt

a771561

kleimkuhler approved these changes Sep 16, 2020

View reviewed changes

rm commented out imports

c059cd8

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

hawkw merged commit 445e7dc into main Sep 16, 2020

hawkw deleted the eliza/tcp-lb-tests branch September 16, 2020 20:08

hawkw mentioned this pull request Sep 16, 2020

outbound: initial tests for TCP mTLS #658

Closed

olix0r mentioned this pull request Sep 18, 2020

proxy: v2.110.0 linkerd/linkerd2#4987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce tests for isolated services#655

introduce tests for isolated services#655
hawkw merged 19 commits intomainfrom
eliza/tcp-lb-tests

hawkw commented Sep 11, 2020

Uh oh!

Uh oh!

olix0r left a comment

Uh oh!

olix0r Sep 16, 2020

Uh oh!

hawkw Sep 16, 2020

Uh oh!

hawkw Sep 16, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kleimkuhler left a comment

Uh oh!

kleimkuhler Sep 16, 2020

Uh oh!

kleimkuhler Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		pub use futures::{future, FutureExt, TryFuture, TryFutureExt};

		pub use linkerd2_app_core::{self as app_core, Addr, Error};

Conversation

hawkw commented Sep 11, 2020

Uh oh!

Uh oh!

olix0r left a comment

Choose a reason for hiding this comment

Uh oh!

olix0r Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

hawkw Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

hawkw Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kleimkuhler left a comment

Choose a reason for hiding this comment

Uh oh!

kleimkuhler Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

kleimkuhler Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants