Skip to content

Commit 7de6971

Browse files
achamayouCopilot
andcommitted
Stop networks that a test recovers into
Two governance_test nodes outlived their test by about 8.5 minutes in CI, logging roughly 660 lines a minute on a join retry loop through the rest of the bucket. `test_all_members` stops the network it is given and recovers into a new one, which it returns. `gov` rebound its local `network` to that, but the enclosing `infra.network.network` context manager still owns the original, so nothing ever stopped the recovered network. The nodes that leaked were the ones `test_no_quote` and `test_node_data` deliberately add without ever trusting, which is why they sat retrying rather than exiting. Give `gov` explicit ownership of the recovered network and stop it in a finally block. The node logs of the leaked nodes were clean, so this reaps them without newly failing on anything. `partitions_test.run_recovery_elections` has the same shape: `test_recovery_elections` stops both the original and its intermediate network but returns a third that nobody stopped. Fix it the same way, which matters more now that the partitions groups run concurrently and a stray node is a neighbour of a live test rather than of an idle runner. The pattern is wider than these two: any test that recovers into a new network and returns it leaks unless its caller takes ownership, and recovery.py does this in several places. Left alone here. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 24ad6978-a80f-45bc-b73b-268aeaf2b30e
1 parent c2f76f2 commit 7de6971

2 files changed

Lines changed: 30 additions & 12 deletions

File tree

tests/governance.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -568,16 +568,26 @@ def gov(args):
568568
test_consensus_status(network, args)
569569
test_member_data(network, args)
570570
test_ack_state_digest_update(network, args)
571-
network = test_all_members(network, args)
572-
test_user(network, args)
573-
test_jinja_templates(network, args)
574-
test_no_quote(network, args)
575-
test_node_data(network, args)
576-
test_each_node_cert_renewal(network, args)
577-
test_binding_proposal_to_service_identity(network, args)
578-
test_all_nodes_cert_renewal(network, args)
579-
test_service_cert_renewal(network, args)
580-
test_service_cert_renewal_extended(network, args)
571+
572+
# test_all_members stops this network and recovers into a new one, which
573+
# the enclosing context manager does not own: it still holds the
574+
# original. Stop the recovered network here, or its nodes outlive the
575+
# test. That includes the deliberately untrusted nodes added by
576+
# test_no_quote and test_node_data, which then sit in a join retry loop
577+
# for the rest of the CI job.
578+
recovered_network = test_all_members(network, args)
579+
try:
580+
test_user(recovered_network, args)
581+
test_jinja_templates(recovered_network, args)
582+
test_no_quote(recovered_network, args)
583+
test_node_data(recovered_network, args)
584+
test_each_node_cert_renewal(recovered_network, args)
585+
test_binding_proposal_to_service_identity(recovered_network, args)
586+
test_all_nodes_cert_renewal(recovered_network, args)
587+
test_service_cert_renewal(recovered_network, args)
588+
test_service_cert_renewal_extended(recovered_network, args)
589+
finally:
590+
recovered_network.stop_all_nodes(skip_verification=True)
581591

582592

583593
# These tests requiring starting up + shutting down a node with specific

tests/partitions_test.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,8 +1607,16 @@ def run_forwarding_and_sessions(args):
16071607

16081608
def run_recovery_elections(args):
16091609
with partitioned_network(args) as network:
1610-
network = test_recovery_elections(network, args)
1611-
test_ledger_invariants(network, args)
1610+
# test_recovery_elections stops this network and recovers into a new
1611+
# one, which the context manager does not own: it still holds the
1612+
# original. Stop the returned network here, or its nodes outlive the
1613+
# test.
1614+
recovery_network = test_recovery_elections(network, args)
1615+
try:
1616+
test_ledger_invariants(recovery_network, args)
1617+
finally:
1618+
if recovery_network is not network:
1619+
recovery_network.stop_all_nodes(skip_verification=True)
16121620

16131621

16141622
if __name__ == "__main__":

0 commit comments

Comments
 (0)