Skip to content

Prevent test network lock convoys - #470

Merged
yummybomb merged 3 commits into
mainfrom
hypeship/fix-test-network-lock
Sep 1, 2026
Merged

Prevent test network lock convoys#470
yummybomb merged 3 commits into
mainfrom
hypeship/fix-test-network-lock

Conversation

@yummybomb

@yummybomb yummybomb commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

This pr was made using the flake testing skill in hypeman, from this action, which failed in main: https://github.com/kernel/hypeman/actions/runs/33411612595/attempts/1

  • keep the shared subnet lock limited to route selection and lease-file updates
  • run stale network cleanup behind a separate non-blocking lock
  • delete iptables entries by exact rule specification and reuse the orphan sweep's rule listing

Why

Concurrent Linux test suites could hold the shared subnet lock while performing slow bridge and iptables cleanup. Other integration tests then blocked on lease allocation or release until the package-level timeout fired.

Tests

  • go test -count=20 -run '^(TestTryWithTestFileLock|TestReleaseRemovesLeaseBeforeNetworkArtifacts|TestParseIPTablesAppendRule)$' ./lib/instances
  • three consecutive no-cache full-suite runs with CI-equivalent Linux settings: 285s, 443s, 369s

Note

Medium Risk
Changes shared-host test networking locks and iptables teardown used by parallel integration tests; incorrect ordering or parsing could cause lease races or stray rules, though scoped to the test harness.

Overview
Addresses parallel Linux integration tests timing out when many suites block on the same subnet lease lock while one holder runs slow bridge/iptables cleanup.

The blocking subnet lock now covers only lease-file reads/writes and subnet selection. Stale route and orphan iptables sweeps run under a separate non-blocking file lock (tryWithTestFileLock), so allocators skip cleanup if another process is already sweeping. Test teardown is centralized in releaseTestNetworkLease: drop the lease under the subnet lock, then run host artifact cleanup outside that lock.

iptables cleanup no longer uses mutable line numbers; it parses iptables -S append lines via parseIPTablesAppendRule and deletes by exact rule spec, including all duplicate orphan rules. Test harness comment matching is anchored to full hm* comments.

Adds unit tests for the non-blocking lock, release ordering (lease gone before route delete), and rule parsing; NOTES.md documents the CI flake and validation.

Reviewed by Cursor Bugbot for commit 44e1f8a. Bugbot is set up for automated code reviews on this repo. Configure here.

@yummybomb
yummybomb marked this pull request as ready for review August 31, 2026 20:30

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 334ad51. Configure here.

Comment thread lib/instances/test_network_config_test.go Outdated
@yummybomb
yummybomb force-pushed the hypeship/fix-test-network-lock branch from 7d92a8a to 44e1f8a Compare August 31, 2026 21:18
@yummybomb
yummybomb requested a review from sjmiller609 August 31, 2026 21:31

@sjmiller609 sjmiller609 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. thanks!

@yummybomb
yummybomb merged commit 49cffe1 into main Sep 1, 2026
9 checks passed
@yummybomb
yummybomb deleted the hypeship/fix-test-network-lock branch September 1, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants