From 174b6b4c29f4b8e59868bab68e380ed7ea551b69 Mon Sep 17 00:00:00 2001 From: Viorel Munteanu Date: Sat, 5 Sep 2026 14:03:14 +0000 Subject: [PATCH] Use explicit test-only subnets and test- prefixed names in the test suite Running the full device test suite on the real Android target found a genuine test-harness gap: every extern-network test in test_network_join_scenarios.cpp failed with "RTNETLINK answers: File exists" adding its own uplink route. Root cause: each test runs under its own ScratchXdgDirs, so its own persistent.yaml starts empty every time -- allocate_ipv4_subnet()'s own auto-allocation always picks the very first slot (10.168.0.0/24) with no way to see whatever real, non-test networks already exist on the host. The device happens to have a real, long-lived "extern" network already occupying exactly that subnet from prior manual testing, and since extern's uplink adds a route back into the (necessarily shared, host-root) routing table -- unlike intern, whose routing lives entirely inside its own isolated per-network namespace -- every extern test collided with it. Not a production bug: a real end user only ever has one persistent.yaml where allocation correctly sees every existing entry. Fixed by giving each of the 10 test networks in test_network_join_scenarios.cpp its own fixed, explicit subnet (--subnet) in 10.169.0.0/16 -- a different /16 than production's own default 10.168.0.0/16 range, so a test run can't collide with a real network regardless of how many the host already has. Also renamed every network/hostname/container-name string literal used across the test suite (test_network_join_scenarios.cpp, test_root_networking.cpp, test_rootless_run.cpp, test_session_cleanup.cpp) from "selftest*" to "test-*", to further reduce the chance of colliding with anything a real invocation might already be using. Low-level interface device literals (slkselftest0/thselftest0/ ethselftest in test_root_networking.cpp) are left as-is -- they're internal identifiers for a throwaway unit test, not network or container names. Verified: clean rebuild, meson test, and 3 consecutive [integration][root] suite runs (61 assertions, 14 test cases) with no failures. --- .../test_network_join_scenarios.cpp | 63 +++++++++++++------ tests/integration/test_root_networking.cpp | 16 ++--- tests/integration/test_rootless_run.cpp | 2 +- tests/integration/test_session_cleanup.cpp | 2 +- 4 files changed, 55 insertions(+), 28 deletions(-) diff --git a/tests/integration/test_network_join_scenarios.cpp b/tests/integration/test_network_join_scenarios.cpp index b6839de..df5fb0b 100644 --- a/tests/integration/test_network_join_scenarios.cpp +++ b/tests/integration/test_network_join_scenarios.cpp @@ -27,6 +27,12 @@ // // Needs root (real bridges/namespaces/iptables) and a real busybox fixture // (find_busybox_fixture()) -- SKIPs cleanly on either missing. +// +// Every network created here is named "test-..." and given an explicit +// subnet in 10.169.0.0/16 (see create_test_network()'s own doc comment +// below for why the explicit subnet matters) -- both deliberately distinct +// from anything a real invocation would plausibly already have in use, so +// a test run can't collide with pre-existing host state. #include #include @@ -69,8 +75,28 @@ AppConfig reload_config() { // Creates `name` via the real -n/--network (create) CLI path // (dispatch_command(), Mode::network) -- --with-ipv6=false always (see this -// file's own top comment for why). Returns true on success. -bool create_test_network(const std::string& name, NetworkKind kind, bool veth) { +// file's own top comment for why). `subnet` is always passed explicitly +// (--subnet) rather than left to allocate_ipv4_subnet()'s own auto-allocation: +// each test runs under its own ScratchXdgDirs, so its own persistent.yaml +// starts empty every time -- auto-allocation would always pick the very +// first slot (10.168.0.0/24) with no way to see whatever real, non-test +// networks already exist on the host it's running on. Confirmed by direct +// testing on the real Android target device, which happens to have a real, +// long-lived "extern" network already occupying exactly that subnet: every +// extern-network test failed with "RTNETLINK answers: File exists" adding +// its own uplink route for the identical 10.168.0.0/24 destination into the +// (necessarily shared, host-root) routing table -- not a production bug, +// since a real end user only ever has one persistent.yaml where allocation +// correctly sees every existing entry, but a real, reproducible test-harness +// gap once tests run alongside pre-existing host state. intern networks +// never hit this (their routing lives entirely inside their own isolated, +// per-network namespace, invisible to and unaffected by anything in host +// root), which is exactly why only extern tests failed. Fixed by giving each +// test network below its own fixed, explicit subnet in 10.169.0.0/16 -- +// deliberately a different /16 than production's own default 10.168.0.0/16 +// range, so test runs can never collide with a real network regardless of +// how many the host already has. +bool create_test_network(const std::string& name, NetworkKind kind, bool veth, const std::string& subnet) { ParsedArgs args; args.mode = Mode::network; args.network_specs = {name}; @@ -78,6 +104,7 @@ bool create_test_network(const std::string& name, NetworkKind kind, bool veth) { args.network_intern_flag = (kind == NetworkKind::intern); args.network_with_veth_flag = veth; args.network_with_ipv6_flag = false; + args.network_subnet_flag = subnet; AppConfig config = reload_config(); CapturedStdout capture; return dispatch_command(args, "/nonexistent/unused-config.yaml", config) == 0; @@ -114,8 +141,8 @@ std::optional test_network_subnet(const std::string& name) { // REQUIRE convention exists for, just via RAII instead. class TestNetwork { public: - TestNetwork(std::string name, NetworkKind kind, bool veth) : name_(std::move(name)) { - created_ = create_test_network(name_, kind, veth); + TestNetwork(std::string name, NetworkKind kind, bool veth, const std::string& subnet) : name_(std::move(name)) { + created_ = create_test_network(name_, kind, veth, subnet); } ~TestNetwork() { if (created_) { @@ -319,7 +346,7 @@ TEST_CASE("network join: two intern peers can ping each other by IP (veth)", "[i } ScratchXdgDirs scratch; - TestNetwork network("selftest-intern-ping-veth", NetworkKind::intern, /*veth=*/true); + TestNetwork network("test-intern-ping-veth", NetworkKind::intern, /*veth=*/true, "10.169.0.0/24"); REQUIRE(network.created()); auto subnet = test_network_subnet(network.name()); @@ -356,7 +383,7 @@ TEST_CASE("network join: two extern peers can ping each other by IP (veth)", "[i } ScratchXdgDirs scratch; - TestNetwork network("selftest-extern-ping-veth", NetworkKind::extern_, /*veth=*/true); + TestNetwork network("test-extern-ping-veth", NetworkKind::extern_, /*veth=*/true, "10.169.1.0/24"); REQUIRE(network.created()); auto subnet = test_network_subnet(network.name()); @@ -391,7 +418,7 @@ TEST_CASE("network join: two extern peers can ping each other by IP (tap+relay)" } ScratchXdgDirs scratch; - TestNetwork network("selftest-extern-ping-tap", NetworkKind::extern_, /*veth=*/false); + TestNetwork network("test-extern-ping-tap", NetworkKind::extern_, /*veth=*/false, "10.169.2.0/24"); REQUIRE(network.created()); auto subnet = test_network_subnet(network.name()); @@ -426,7 +453,7 @@ TEST_CASE("network join: extern network can reach the outside (veth)", "[integra } ScratchXdgDirs scratch; - TestNetwork network("selftest-extern-outside-veth", NetworkKind::extern_, /*veth=*/true); + TestNetwork network("test-extern-outside-veth", NetworkKind::extern_, /*veth=*/true, "10.169.3.0/24"); REQUIRE(network.created()); // 8.8.8.8 -- real internet access confirmed available in this sandbox @@ -455,7 +482,7 @@ TEST_CASE("network join: extern network can reach the outside (tap+relay)", "[in } ScratchXdgDirs scratch; - TestNetwork network("selftest-extern-outside-tap", NetworkKind::extern_, /*veth=*/false); + TestNetwork network("test-extern-outside-tap", NetworkKind::extern_, /*veth=*/false, "10.169.4.0/24"); REQUIRE(network.created()); auto output = run_networked(*image, {network.name()}, @@ -480,7 +507,7 @@ TEST_CASE("network join: intern network has no route to the outside (veth)", "[i } ScratchXdgDirs scratch; - TestNetwork network("selftest-intern-isolation-veth", NetworkKind::intern, /*veth=*/true); + TestNetwork network("test-intern-isolation-veth", NetworkKind::intern, /*veth=*/true, "10.169.5.0/24"); REQUIRE(network.created()); // 8.8.8.8 is real, well-known and reachable outside this project's own @@ -513,7 +540,7 @@ TEST_CASE("network join: intern network has no route to the outside (tap+relay)" } ScratchXdgDirs scratch; - TestNetwork network("selftest-intern-isolation-tap", NetworkKind::intern, /*veth=*/false); + TestNetwork network("test-intern-isolation-tap", NetworkKind::intern, /*veth=*/false, "10.169.6.0/24"); REQUIRE(network.created()); auto output = run_networked(*image, {network.name()}, @@ -541,7 +568,7 @@ TEST_CASE("network join: two intern peers can ping each other by IP (tap+relay)" } ScratchXdgDirs scratch; - TestNetwork network("selftest-intern-ping-tap", NetworkKind::intern, /*veth=*/false); + TestNetwork network("test-intern-ping-tap", NetworkKind::intern, /*veth=*/false, "10.169.7.0/24"); REQUIRE(network.created()); auto subnet = test_network_subnet(network.name()); @@ -579,15 +606,15 @@ TEST_CASE("network join: two peers can resolve and ping each other by hostname ( } ScratchXdgDirs scratch; - TestNetwork network("selftest-dns-ping-veth", NetworkKind::intern, /*veth=*/true); + TestNetwork network("test-dns-ping-veth", NetworkKind::intern, /*veth=*/true, "10.169.8.0/24"); REQUIRE(network.created()); - BackgroundPeer peer_a(*image, {network.name()}, std::string("peer-a"), 20); + BackgroundPeer peer_a(*image, {network.name()}, std::string("test-peer-a"), 20); REQUIRE(peer_a.ready()); auto output = run_networked( *image, {network.name()}, - {"sh", "-c", wait_for_hostname_then("peer-a", "ping -c 2 -W 2 peer-a; echo RESULT=$?")}); + {"sh", "-c", wait_for_hostname_then("test-peer-a", "ping -c 2 -W 2 test-peer-a; echo RESULT=$?")}); auto lines = extract_marked_lines(output); bool found_success = false; for (const auto& line : lines) { @@ -612,15 +639,15 @@ TEST_CASE("network join: two peers can resolve and ping each other by hostname ( } ScratchXdgDirs scratch; - TestNetwork network("selftest-dns-ping-tap", NetworkKind::intern, /*veth=*/false); + TestNetwork network("test-dns-ping-tap", NetworkKind::intern, /*veth=*/false, "10.169.9.0/24"); REQUIRE(network.created()); - BackgroundPeer peer_a(*image, {network.name()}, std::string("peer-a"), 20); + BackgroundPeer peer_a(*image, {network.name()}, std::string("test-peer-a"), 20); REQUIRE(peer_a.ready()); auto output = run_networked( *image, {network.name()}, - {"sh", "-c", wait_for_hostname_then("peer-a", "ping -c 2 -W 2 peer-a; echo RESULT=$?")}); + {"sh", "-c", wait_for_hostname_then("test-peer-a", "ping -c 2 -W 2 test-peer-a; echo RESULT=$?")}); auto lines = extract_marked_lines(output); bool found_success = false; for (const auto& line : lines) { diff --git a/tests/integration/test_root_networking.cpp b/tests/integration/test_root_networking.cpp index c211249..e956142 100644 --- a/tests/integration/test_root_networking.cpp +++ b/tests/integration/test_root_networking.cpp @@ -83,7 +83,7 @@ TEST_CASE("persistent network namespace create/verify/remove", "[integration][ro SKIP("requires root"); } - constexpr std::string_view test_netns_name = "selftest"; + constexpr std::string_view test_netns_name = "test-netns"; // Clean up a leftover from a previous interrupted run, if any -- // create_persistent_netns() refuses to overwrite an existing live @@ -122,7 +122,7 @@ TEST_CASE("tap-relay create/attach/teardown", "[integration][root][net]") { SKIP("requires root"); } - const std::string test_network_name = "selftest-tap-relay"; + const std::string test_network_name = "test-tap-relay"; const std::string test_bridge = "slkselftest0"; const std::string host_tap = "thselftest0"; const std::string container_if = "ethselftest"; @@ -394,8 +394,8 @@ TEST_CASE("dns-resolver create/answer/teardown", "[integration][root][net]") { SKIP("dnsmasq not found in PATH"); } - const std::string test_network_name = "selftest-dns"; - const std::string test_hostname = "selftest-peer"; + const std::string test_network_name = "test-dns"; + const std::string test_hostname = "test-peer"; const std::string test_ip = "10.99.99.2"; pid_t container_pid = fork(); @@ -419,14 +419,14 @@ TEST_CASE("dns-resolver create/answer/teardown", "[integration][root][net]") { std::error_code ec; std::filesystem::remove_all(dns_hosts_dir(test_network_name), ec); if (isolated) { - record_dns_host(test_network_name, "selftest-peer-container", 1, test_ip, test_hostname); + record_dns_host(test_network_name, "test-peer-container", 1, test_ip, test_hostname); } std::optional resolver; if (isolated) { NetworkEntry test_network{test_network_name, NetworkKind::intern, "", false, "", true}; JoinedNetwork joined_network{test_network, test_ip, std::nullopt}; - resolver = start_dns_resolver("selftest-dns-session", container_pid, container_pid, {joined_network}); + resolver = start_dns_resolver("test-dns-session", container_pid, container_pid, {joined_network}); CHECK(resolver.has_value()); } @@ -440,8 +440,8 @@ TEST_CASE("dns-resolver create/answer/teardown", "[integration][root][net]") { stop_dns_resolver(*resolver); } - remove_dns_resolver_record("selftest-dns-session", container_pid); - remove_dns_host_record(test_network_name, "selftest-peer-container", 1); + remove_dns_resolver_record("test-dns-session", container_pid); + remove_dns_host_record(test_network_name, "test-peer-container", 1); std::filesystem::remove_all(dns_hosts_dir(test_network_name), ec); if (container_pid > 0) { diff --git a/tests/integration/test_rootless_run.cpp b/tests/integration/test_rootless_run.cpp index 68b5afa..1e38d86 100644 --- a/tests/integration/test_rootless_run.cpp +++ b/tests/integration/test_rootless_run.cpp @@ -234,7 +234,7 @@ bool session_cgroup_would_work() { if (!cgroup_v2_available()) { return false; } - auto probe_path = session_cgroup_path("selftest-cgroup-probe", getpid()); + auto probe_path = session_cgroup_path("test-cgroup-probe", getpid()); std::error_code ec; std::filesystem::create_directories(probe_path, ec); if (ec) { diff --git a/tests/integration/test_session_cleanup.cpp b/tests/integration/test_session_cleanup.cpp index 19d1d7c..2218ba6 100644 --- a/tests/integration/test_session_cleanup.cpp +++ b/tests/integration/test_session_cleanup.cpp @@ -46,7 +46,7 @@ TEST_CASE("session cgroup sweep: kill_via_cgroup reaps a reparented straggler", SKIP("requires root"); } - const std::string container_name = "selftest-cgroup-sweep"; + const std::string container_name = "test-cgroup-sweep"; // sync_pipe makes the child wait until it's actually been moved into the // session cgroup before it forks its own straggler -- otherwise the