From bae1b7f41331073d59a199cd1835e2c5a9889b49 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Thu, 29 Oct 2020 21:20:24 +0100
Subject: [PATCH] netns test: improve ping test

- Use fping for pinging multiple hosts in parallel.
  Significantly improves test runtime:
  >13 s -> ~200 ms for the negative ping tests.
- Only test network namespaces that are enabled.
  This allows running the netns test with a reduced service set for debugging.
- Remove deprecated services, instead add btcpayserver, spark-wallet
---
 test/tests.nix |  1 +
 test/tests.py  | 50 +++++++++++++++++++-------------------------------
 2 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/test/tests.nix b/test/tests.nix
index 634b97f..9d2e3a8 100644
--- a/test/tests.nix
+++ b/test/tests.nix
@@ -115,6 +115,7 @@ let testEnv = rec {
       nix-bitcoin.netns-isolation.enable = true;
       test.data.netns = config.nix-bitcoin.netns-isolation.netns;
       tests.netns-isolation = true;
+      environment.systemPackages = [ pkgs.fping ];
 
       # This test is rather slow and unaffected by netns settings
       tests.backups = mkForce false;
diff --git a/test/tests.py b/test/tests.py
index ea3b3ad..931d1ce 100644
--- a/test/tests.py
+++ b/test/tests.py
@@ -237,39 +237,27 @@ def _():
 # (and their corresponding network namespaces).
 @test("netns-isolation")
 def _():
-    ping_bitcoind = "ip netns exec nb-bitcoind ping -c 1 -w 1"
-    ping_nanopos = "ip netns exec nb-nanopos ping -c 1 -w 1"
-    ping_nbxplorer = "ip netns exec nb-nbxplorer ping -c 1 -w 1"
+    def get_ips(services):
+        enabled = enabled_tests.intersection(services)
+        return " ".join(ip(service) for service in enabled)
 
-    # Positive ping tests (non-exhaustive)
-    machine.succeed(
-        "%s %s &&" % (ping_bitcoind, ip("bitcoind"))
-        + "%s %s &&" % (ping_bitcoind, ip("clightning"))
-        + "%s %s &&" % (ping_bitcoind, ip("lnd"))
-        + "%s %s &&" % (ping_bitcoind, ip("liquidd"))
-        + "%s %s &&" % (ping_bitcoind, ip("nbxplorer"))
-        + "%s %s &&" % (ping_nbxplorer, ip("btcpayserver"))
-        + "%s %s &&" % (ping_nanopos, ip("lightning-charge"))
-        + "%s %s &&" % (ping_nanopos, ip("nanopos"))
-        + "%s %s" % (ping_nanopos, ip("nginx"))
-    )
+    def assert_reachable(src, dests):
+        dest_ips = get_ips(dests)
+        if src in enabled_tests and dest_ips:
+            machine.succeed(f"ip netns exec nb-{src} fping -c1 -t100 {dest_ips}")
 
-    # Negative ping tests (non-exhaustive)
-    machine.fail(
-        "%s %s ||" % (ping_bitcoind, ip("spark-wallet"))
-        + "%s %s ||" % (ping_bitcoind, ip("lightning-loop"))
-        + "%s %s ||" % (ping_bitcoind, ip("lightning-charge"))
-        + "%s %s ||" % (ping_bitcoind, ip("nanopos"))
-        + "%s %s ||" % (ping_bitcoind, ip("nginx"))
-        + "%s %s ||" % (ping_nanopos, ip("bitcoind"))
-        + "%s %s ||" % (ping_nanopos, ip("clightning"))
-        + "%s %s ||" % (ping_nanopos, ip("lnd"))
-        + "%s %s ||" % (ping_nanopos, ip("lightning-loop"))
-        + "%s %s ||" % (ping_nanopos, ip("liquidd"))
-        + "%s %s ||" % (ping_nanopos, ip("electrs"))
-        + "%s %s ||" % (ping_nanopos, ip("spark-wallet"))
-        + "%s %s" % (ping_nanopos, ip("btcpayserver"))
-    )
+    def assert_unreachable(src, dests):
+        dest_ips = get_ips(dests)
+        if src in enabled_tests and dest_ips:
+            machine.fail(
+                # This fails when no host is reachable within 100 ms
+                f"ip netns exec nb-{src} fping -c1 -t100 --reachable=1 {dest_ips}"
+            )
+
+    # These reachability tests are non-exhaustive
+    assert_reachable("bitcoind", ["clightning", "lnd", "liquidd"])
+    assert_unreachable("bitcoind", ["btcpayserver", "spark-wallet", "lightning-loop"])
+    assert_unreachable("btcpayserver", ["bitcoind", "lightning-loop", "liquidd"])
 
     # test that netns-exec can't be run for unauthorized namespace
     machine.fail("netns-exec nb-electrs ip a")