From 1b93983bf5c8460b2e5741f5efe88c0adbed4a79 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 9 Sep 2025 15:12:22 +0200 Subject: [PATCH 1/2] test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition Instead of requiring a run with an explicit `--ihave1111and2222`, detect whether the routable addresses are set up and if not, then skip the test. To detect whether the addresses are set use `bitcoind` - start it and ask it to bind on them and see if it will error with "Unable to bind". Since this is what the tests do anyway, just start the nodes and see if an exception will be raised like `FailedToStartError` / "Unable to bind". This makes it possible for the CI to run `feature_bind_port_discover.py` and `feature_bind_port_externalip.py` by just setting up the addresses, without having to explicitly provide `--ihave1111and2222`. Co-authored-by: willcl-ark --- test/functional/feature_bind_port_discover.py | 43 ++++++++-------- .../feature_bind_port_externalip.py | 51 ++++++++++++------- .../test_framework/test_framework.py | 21 ++++++++ 3 files changed, 74 insertions(+), 41 deletions(-) diff --git a/test/functional/feature_bind_port_discover.py b/test/functional/feature_bind_port_discover.py index 613b111b33f..407b1364a50 100755 --- a/test/functional/feature_bind_port_discover.py +++ b/test/functional/feature_bind_port_discover.py @@ -7,6 +7,9 @@ Test that -discover does not add all interfaces' addresses if we listen on only """ from test_framework.test_framework import BitcoinTestFramework, SkipTest +from test_framework.test_node import ( + FailedToStartError, +) from test_framework.util import ( assert_equal, assert_not_equal, @@ -20,18 +23,14 @@ from test_framework.util import ( # Linux: # First find your interfaces: ip addr show # Then use your actual interface names (replace INTERFACE_NAME with yours): -# ip addr add 1.1.1.1/32 dev INTERFACE_NAME && ip addr add 2.2.2.2/32 dev INTERFACE_NAME # to set up -# ip addr del 1.1.1.1/32 dev INTERFACE_NAME && ip addr del 2.2.2.2/32 dev INTERFACE_NAME # to remove it +# ip addr add 1.1.1.5/32 dev INTERFACE_NAME && ip addr add 1111:1111::5/128 dev INTERFACE_NAME # to set up +# ip addr del 1.1.1.5/32 dev INTERFACE_NAME && ip addr del 1111:1111::5/128 dev INTERFACE_NAME # to remove it # -# macOS: -# ifconfig en0 alias 1.1.1.1 && ifconfig en0 alias 2.2.2.2 # to set up -# ifconfig en0 1.1.1.1 -alias && ifconfig en0 2.2.2.2 -alias # to remove it, after the test -# -# FreeBSD: -# ifconfig em0 1.1.1.1/32 alias && ifconfig wlan0 2.2.2.2/32 alias # to set up -# ifconfig em0 1.1.1.1 -alias && ifconfig wlan0 2.2.2.2 -alias # to remove it, after the test -ADDR1 = '1.1.1.1' -ADDR2 = '2.2.2.2' +# FreeBSD and MacOS: +# ifconfig INTERFACE_NAME 1.1.1.5/32 alias && ifconfig INTERFACE_NAME inet6 1111:1111::5/128 alias # to set up +# ifconfig INTERFACE_NAME 1.1.1.5 -alias && ifconfig INTERFACE_NAME inet6 1111:1111::5 -alias # to remove it, after the test +ADDR1 = '1.1.1.5' # This and the address below are set in the CI environment, don't change it just here (keep them in sync). +ADDR2 = '1111:1111::5' class BindPortDiscoverTest(BitcoinTestFramework): def set_test_params(self): @@ -69,19 +68,17 @@ class BindPortDiscoverTest(BitcoinTestFramework): # False. We do not want any -bind= thus set has_explicit_bind to True. for node in self.nodes: node.has_explicit_bind = True - self.start_nodes() - def add_options(self, parser): - parser.add_argument( - "--ihave1111and2222", action='store_true', dest="ihave1111and2222", - help=f"Run the test, assuming {ADDR1} and {ADDR2} are configured on the machine", - default=False) - - def skip_test_if_missing_module(self): - if not self.options.ihave1111and2222: - raise SkipTest( - f"To run this test make sure that {ADDR1} and {ADDR2} (routable addresses) are " - "assigned to the interfaces on this machine and rerun with --ihave1111and2222") + try: + self.start_nodes() + except FailedToStartError as e: + self.cleanup_partially_started_nodes() + if 'Unable to bind to ' in str(e): + raise SkipTest( + f'To run this test make sure that {ADDR1} and {ADDR2} ' + '(routable addresses) are assigned to non-loopback ' + 'interfaces on this machine') + raise def run_test(self): self.log.info( diff --git a/test/functional/feature_bind_port_externalip.py b/test/functional/feature_bind_port_externalip.py index 0e7764dbf2e..074797acf32 100755 --- a/test/functional/feature_bind_port_externalip.py +++ b/test/functional/feature_bind_port_externalip.py @@ -7,17 +7,24 @@ Test that the proper port is used for -externalip= """ from test_framework.test_framework import BitcoinTestFramework, SkipTest +from test_framework.test_node import ( + FailedToStartError, +) from test_framework.util import assert_equal, p2p_port -# We need to bind to a routable address for this test to exercise the relevant code. -# To set a routable address on the machine use: +# We need to bind to a routable address for this test to exercise the relevant +# code. Those addresses must be on an interface that is UP and is not a loopback +# interface (IFF_LOOPBACK). +# To set these routable addresses on the machine, use: # Linux: -# ifconfig lo:0 1.1.1.1/32 up # to set up -# ifconfig lo:0 down # to remove it, after the test -# FreeBSD: -# ifconfig lo0 1.1.1.1/32 alias # to set up -# ifconfig lo0 1.1.1.1 -alias # to remove it, after the test -ADDR = '1.1.1.1' +# First find your interfaces: ip addr show +# Then use your actual interface names (replace INTERFACE_NAME with yours): +# ip addr add 1.1.1.5/32 dev INTERFACE_NAME # to set up +# ip addr del 1.1.1.5/32 dev INTERFACE_NAME # to remove it +# FreeBSD and MacOS: +# ifconfig INTERFACE_NAME 1.1.1.5/32 alias # to set up +# ifconfig INTERFACE_NAME 1.1.1.5 -alias # to remove it, after the test +ADDR = '1.1.1.5' # This is set in the CI environment, don't change it just here (keep them in sync). # array of tuples [arguments, expected port in localaddresses] EXPECTED = [ @@ -45,17 +52,25 @@ class BindPortExternalIPTest(BitcoinTestFramework): self.num_nodes = len(EXPECTED) self.extra_args = list(map(lambda e: e[0], EXPECTED)) - def add_options(self, parser): - parser.add_argument( - "--ihave1111", action='store_true', dest="ihave1111", - help=f"Run the test, assuming {ADDR} is configured on the machine", - default=False) + def setup_network(self): + self.setup_nodes() - def skip_test_if_missing_module(self): - if not self.options.ihave1111: - raise SkipTest( - f"To run this test make sure that {ADDR} (a routable address) is assigned " - "to one of the interfaces on this machine and rerun with --ihave1111") + def setup_nodes(self): + self.add_nodes(self.num_nodes, self.extra_args) + # TestNode.start() will add -bind= to extra_args if has_explicit_bind is + # False. We do not want any -bind= thus set has_explicit_bind to True. + for node in self.nodes: + node.has_explicit_bind = True + try: + self.start_nodes() + except FailedToStartError as e: + self.cleanup_partially_started_nodes() + if 'Unable to bind to ' in str(e): + raise SkipTest( + f'To run this test make sure that {ADDR} ' + '(routable address) is assigned to non-loopback ' + 'interface on this machine') + raise def run_test(self): self.log.info("Test the proper port is used for -externalip=") diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index b809e68f4df..8cc7121f102 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -535,6 +535,27 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Wait for nodes to stop node.wait_until_stopped() + def cleanup_partially_started_nodes(self): + """Tear down nodes left running after a failed start_nodes(). + + After start_nodes() raises (e.g. FailedToStartError), some nodes may be + RPC-connected, some may have a live process without RPC, and some may + already have exited. Stop the connected ones cleanly and force-kill the + rest so the framework's teardown can proceed. + """ + for node in self.nodes: + if not node.running: + continue + if node.rpc_connected: + node.stop_node(wait=node.rpc_timeout) + else: + node.process.kill() + node.process.wait(timeout=node.rpc_timeout) + node.process = None + node.stdout.close() + node.stderr.close() + node.running = False + def restart_node(self, i, extra_args=None, clear_addrman=False, *, expected_stderr=''): """Stop and start a test node""" self.stop_node(i, expected_stderr=expected_stderr) From 75cf9708a05d08c3de5dffd91228444d942254b8 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 11 Sep 2025 15:39:14 +0200 Subject: [PATCH 2/2] ci: add one more routable address to the VMs (docker containers) Also explicitly specify which addresses from the docker network to assign to the VM. With `1.1.1.5` and `1111:1111::5` set on the machine, the tests `feature_bind_port_discover.py` and `feature_bind_port_externalip.py` will run. --- ci/test/02_run_container.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index abaa535553d..05887000a2b 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -115,6 +115,7 @@ def main(): CI_CCACHE_MOUNT = f"type=bind,src={os.environ['CCACHE_DIR']},dst={os.environ['CCACHE_DIR']}" run(["docker", "network", "create", "--ipv6", "--subnet", "1111:1111::/112", "ci-ip6net"], check=False) + run(["docker", "network", "create", "--subnet", "1.1.1.0/24", "ci-ip4net"], check=False) if os.getenv("RESTART_CI_DOCKER_BEFORE_RUN"): print("Restart docker before run to stop and clear all containers started with --rm") @@ -144,6 +145,7 @@ def main(): f"--env-file={env_file}", f"--name={os.environ['CONTAINER_NAME']}", "--network=ci-ip6net", + "--ip6=1111:1111::5", # Used by some of the tests, don't change it just here (keep them in sync). f"--platform={os.environ['CI_IMAGE_PLATFORM']}", os.environ["CONTAINER_NAME"], ] @@ -154,6 +156,8 @@ def main(): text=True, ).stdout.strip() + run(["docker", "network", "connect", "--ip=1.1.1.5", "ci-ip4net", container_id]) # The IP address is used by some of the tests, don't change it just here (keep them in sync). + def ci_exec(cmd_inner, **kwargs): if os.getenv("DANGER_RUN_CI_ON_HOST"): prefix = []