ci: Fix coverage workflow (#1382)

* Avoid building the binary target, since it slows down the build
* Use --skip-clean to use the cached build
* Fetch a pre-build tarpaulin binary (saves ~5m)
* Rename `flaky_tests` feature to `flakey-in-ci`
* Add a `flakey-in-coverage` feature
* Mark broken tests with `ignore` (not feature-gated)
* Enable all tests by default

.github/workflows/coverage.yml

@@ -9,6 +9,7 @@ on:
 env:
   CARGO_INCREMENTAL: 0
   CARGO_NET_RETRY: 10
+  CARGO_TARPAULIN_VERSION: 0.18.5
   RUST_BACKTRACE: short
   RUSTUP_MAX_RETRIES: 10
 
@@ -26,7 +27,12 @@ jobs:
     steps:
       - run: apt update && apt install -y cmake clang golang # for boring
       - uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579
-      - run: cargo install cargo-tarpaulin
-      - run: cargo tarpaulin --verbose --workspace --no-run
-      - run: cargo tarpaulin --verbose --workspace --out Xml --ignore-tests --no-fail-fast
+      - name: install cargo-tarpaulin ${{ env.CARGO_TARPAULIN_VERSION }}
+        run: |
+          cd "${CARGO_HOME}/bin"
+          curl -sL https://github.com/xd009642/tarpaulin/releases/download/${CARGO_TARPAULIN_VERSION}/cargo-tarpaulin-${CARGO_TARPAULIN_VERSION}-travis.tar.gz | tar xzvf -
+      - run: cargo tarpaulin --locked --workspace --exclude=linkerd2-proxy --exclude=linkerd-app-integration --no-run
+      - run: cargo tarpaulin --locked --workspace --exclude=linkerd2-proxy --exclude=linkerd-app-integration --skip-clean --ignore-tests --no-fail-fast --out=Xml
+      - run: cargo tarpaulin --locked --workspace --package=linkerd-app-integration --no-default-features --skip-clean --no-run
+      - run: cargo tarpaulin --locked --workspace --package=linkerd-app-integration --no-default-features --skip-clean --ignore-tests --no-fail-fast --out=Xml
       - uses: codecov/codecov-action@f32b3a3741e1053eb607407145bc9619351dc93b

.github/workflows/slow.yml

@@ -61,7 +61,6 @@ jobs:
             --package=linkerd-app-core \
             --package=linkerd-app-gateway \
             --package=linkerd-app-inbound \
-            --package=linkerd-app-integration \
             --package=linkerd-app-outbound \
             --package=linkerd-app-test
       - run: |
@@ -71,7 +70,16 @@ jobs:
             --package=linkerd-app-core \
             --package=linkerd-app-gateway \
             --package=linkerd-app-inbound \
-            --package=linkerd-app-integration \
             --package=linkerd-app-outbound \
             --package=linkerd-app-test
 
+      # Integration: enable tests that are flakey in coverage, but disable tests
+      # that can be flakey in CI...
+      - run: |
+          cargo test --no-run \
+            --package=linkerd-app-integration \
+            --no-default-features --features=flakey-in-coverage
+      - run: |
+          cargo test \
+            --package=linkerd-app-integration \
+            --no-default-features --features=flakey-in-coverage

Makefile

@@ -103,10 +103,6 @@ shellcheck:
 test: fetch
 	$(CARGO_TEST)
 
-.PHONY: test-flakey
-test-flakey: fetch
-	$(CARGO_TEST) --features linkerd-app-integration/flaky_tests
-
 .PHONY: package
 package: $(PKG_ROOT)/$(PKG)
 

linkerd/app/integration/Cargo.toml

@@ -13,7 +13,9 @@ a dedicated crate to help the compiler cache dependencies properly.
 """
 
 [features]
-flaky_tests = [] # Disable to skip certain tests that should not be run on CI.
+default = ["flakey-in-ci"]
+flakey-in-ci = ["flakey-in-coverage"]
+flakey-in-coverage = []
 rustfmt = ["linkerd2-proxy-api/rustfmt"]
 
 [dependencies]

linkerd/app/integration/src/tests/identity.rs

@@ -245,8 +245,9 @@ mod require_id_header {
             server: $make_server:path,
             tls_server: $make_tls_server:path,
         ) => {
+            // FIXME(ver) this test was marked flakey, but now it consistently fails.
+            #[ignore]
             #[tokio::test]
-            #[cfg_attr(not(feature = "flaky_tests"), ignore)]
             async fn orig_dst_client_connects_to_tls_server() {
                 let _trace = trace_init();
 
@@ -313,8 +314,9 @@ mod require_id_header {
                 );
             }
 
+            // FIXME(ver) this test was marked flakey, but now it consistently fails.
+            #[ignore]
             #[tokio::test]
-            #[cfg_attr(not(feature = "flaky_tests"), ignore)]
             async fn disco_client_connects_to_tls_server() {
                 let _trace = trace_init();
 
@@ -391,8 +393,9 @@ mod require_id_header {
                 );
             }
 
+            // FIXME(ver) this test was marked flakey, but now it consistently fails.
+            #[ignore]
             #[tokio::test]
-            #[cfg_attr(not(feature = "flaky_tests"), ignore)]
             async fn orig_dst_client_cannot_connect_to_plaintext_server() {
                 let _trace = trace_init();
 

linkerd/app/integration/src/tests/tap.rs

@@ -74,10 +74,9 @@ async fn rejects_incorrect_identity_when_identity_is_expected() {
     assert!(events.next().await.expect("next1").is_err());
 }
 
-// Flaky: sometimes the admin thread hasn't had a chance to register
-// the Taps before the `client.get` is called.
+// FIXME(ver) this test was marked flakey, but now it consistently fails.
+#[ignore]
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
 async fn inbound_http1() {
     let _trace = trace_init();
 
@@ -158,8 +157,9 @@ async fn inbound_http1() {
     assert_eq!(ev3.response_end_bytes(), 5);
 }
 
+// FIXME(ver) this test was marked flakey, but now it consistently fails.
+#[ignore]
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
 async fn grpc_headers_end() {
     let _trace = trace_init();
 

linkerd/app/integration/src/tests/telemetry.rs

@@ -493,7 +493,7 @@ where
 // Eventually, we can add some kind of mock timer system for simulating latency
 // more reliably, and re-enable this test.
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
+#[cfg_attr(not(feature = "flakey-in-ci"), ignore)]
 async fn inbound_response_latency() {
     test_response_latency(Fixture::inbound_with_server).await
 }
@@ -503,7 +503,7 @@ async fn inbound_response_latency() {
 // Eventually, we can add some kind of mock timer system for simulating latency
 // more reliably, and re-enable this test.
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
+#[cfg_attr(not(feature = "flakey-in-ci"), ignore)]
 async fn outbound_response_latency() {
     test_response_latency(Fixture::outbound_with_server).await
 }
@@ -676,7 +676,7 @@ mod outbound_dst_labels {
     // the mock controller before the first request has finished.
     // See linkerd/linkerd2#751
     #[tokio::test]
-    #[cfg_attr(not(feature = "flaky_tests"), ignore)]
+    #[cfg_attr(not(feature = "flakey-in-ci"), ignore)]
     async fn controller_updates_addr_labels() {
         let _trace = trace_init();
         info!("running test server");
@@ -755,12 +755,9 @@ mod outbound_dst_labels {
         }
     }
 
-    // Ignore this test on CI, as it may fail due to the reduced concurrency
-    // on CI containers causing the proxy to see both label updates from
-    // the mock controller before the first request has finished.
-    // See linkerd/linkerd2#751
+    // FIXME(ver) this test was marked flakey, but now it consistently fails.
+    #[ignore]
     #[tokio::test]
-    #[cfg_attr(not(feature = "flaky_tests"), ignore)]
     async fn controller_updates_set_labels() {
         let _trace = trace_init();
         info!("running test server");
@@ -1191,6 +1188,7 @@ mod transport {
         test_tcp_connect(TcpFixture::outbound()).await;
     }
 
+    #[cfg_attr(not(feature = "flakey-in-coverage"), ignore)]
     #[tokio::test]
     async fn outbound_tcp_accept() {
         test_tcp_accept(TcpFixture::outbound()).await;
@@ -1294,6 +1292,7 @@ mod transport {
         test_read_bytes_total(TcpFixture::outbound()).await
     }
 
+    #[cfg_attr(not(feature = "flakey-in-coverage"), ignore)]
     #[tokio::test]
     async fn outbound_tcp_open_connections() {
         test_tcp_open_conns(TcpFixture::outbound()).await

linkerd/app/integration/src/tests/telemetry/tcp_errors.rs

@@ -231,6 +231,7 @@ async fn inbound_multi() {
 }
 
 /// Tests that TLS detect failure metrics are collected for the direct stack.
+#[cfg_attr(not(feature = "flakey-in-coverage"), ignore)]
 #[tokio::test]
 async fn inbound_direct_multi() {
     let _trace = trace_init();
@@ -314,6 +315,7 @@ async fn inbound_invalid_ip() {
 
 /// Tests that the detect metric is not incremented when TLS is successfully
 /// detected by the direct stack.
+#[cfg_attr(not(feature = "flakey-in-coverage"), ignore)]
 #[tokio::test]
 async fn inbound_direct_success() {
     let _trace = trace_init();

linkerd/app/integration/src/tests/transparency.rs

@@ -153,8 +153,9 @@ async fn inbound_tcp() {
     proxy.join_servers().await;
 }
 
+// FIXME(ver) this test was marked flakey, but now it consistently fails.
+#[ignore]
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
 async fn loop_outbound_http1() {
     let _trace = trace_init();
 
@@ -174,8 +175,9 @@ async fn loop_outbound_http1() {
     assert_eq!(rsp.status(), http::StatusCode::BAD_GATEWAY);
 }
 
+// FIXME(ver) this test was marked flakey, but now it consistently fails.
+#[ignore]
 #[tokio::test]
-#[cfg_attr(not(feature = "flaky_tests"), ignore)]
 async fn loop_inbound_http1() {
     let _trace = trace_init();
 
@@ -245,6 +247,8 @@ async fn tcp_server_first() {
     test_server_speaks_first(TestEnv::default()).await;
 }
 
+// FIXME(ver) this test doesn't actually test TLS functionality.
+#[ignore]
 #[tokio::test]
 async fn tcp_server_first_tls() {
     use std::path::PathBuf;
@@ -289,18 +293,18 @@ async fn tcp_server_first_tls() {
 }
 
 #[tokio::test]
-#[allow(warnings)]
 async fn tcp_connections_close_if_client_closes() {
     let _trace = trace_init();
 
     let msg1 = "custom tcp hello\n";
     let msg2 = "custom tcp bye";
 
-    let (mut tx, mut rx) = mpsc::channel(1);
+    let (tx, mut rx) = mpsc::channel::<()>(1);
 
     let srv = server::tcp()
         .accept_fut(move |mut sock| {
             async move {
+                let _tx = tx;
                 let mut vec = vec![0; 1024];
                 let n = sock.read(&mut vec).await?;
 
@@ -309,10 +313,8 @@ async fn tcp_connections_close_if_client_closes() {
                 let n = sock.read(&mut [0; 16]).await?;
                 assert_eq!(n, 0);
                 panic!("lol");
-                tx.send(()).await.unwrap();
-                Ok::<(), io::Error>(())
             }
-            .map(|res| res.expect("TCP server must not fail"))
+            .map(|res: io::Result<()>| res.expect("TCP server must not fail"))
         })
         .run()
         .await;

linkerd/app/test/Cargo.toml

@@ -6,16 +6,9 @@ license = "Apache-2.0"
 edition = "2021"
 publish = false
 description = """
-Proxy integration tests
-
-The test utilities can be very costly to compile, so they are extracted into
-a dedicated crate to help the compiler cache dependencies properly.
+Proxy test helpers
 """
 
-[features]
-# Disable to skip certain tests that should not be run on CI.
-flaky_tests = []
-
 [dependencies]
 futures = { version = "0.3", default-features = false }
 h2 = "0.3"