Avoid python-iptables stdout race in partition tests - #8292
Open
Amaury Chamayou (achamayou) wants to merge 3 commits into
Open
Avoid python-iptables stdout race in partition tests#8292Amaury Chamayou (achamayou) wants to merge 3 commits into
Amaury Chamayou (achamayou) wants to merge 3 commits into
Conversation
Run partition firewall operations through the iptables CLI so its output is isolated from concurrent test logging. Use --wait for cross-process updates and remove the unused Python binding. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e6066b0-464e-4b74-9d4e-29953fa3de77
Copilot started reviewing on behalf of
Amaury Chamayou (achamayou)
September 4, 2026 19:40
View session
Each Partitioner owns a private chain, while iptables --wait serializes individual table updates across every process. No worker threads share a Partitioner, so the old process-local lock no longer protects shared state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e6066b0-464e-4b74-9d4e-29953fa3de77
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The migration to iptables subprocess calls is consistent throughout the partitioning helper, and no remaining python-iptables/iptc usage was found in the repository.
Pull request overview
This PR updates the e2e partition test infrastructure to avoid python-iptables’ process-wide stdout capture race by switching to iptables --wait subprocess calls, and removes the now-unused python-iptables dependency from the test requirements.
Changes:
- Replaced in-process
python-iptablesusage withiptablesCLI subprocess helpers (with--wait) to avoid stdout corruption under concurrent logging. - Kept per-process atomicity of related rule updates via a re-entrant lock while relying on xtables locking to serialize individual iptables operations across concurrent ctest processes.
- Removed
python-iptablesfromtests/requirements.txt.
Custom instructions used: None (no .github/copilot-instructions.md / .github/instructions/* files were explicitly loaded during this review).
File summaries
| File | Description |
|---|---|
tests/requirements.txt |
Drops python-iptables dependency now that tests invoke iptables via subprocess. |
tests/infra/partitions.py |
Reworks partition rule manipulation to call iptables --wait in subprocesses and removes iptc/JSON-based dumping. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Remove unused dataclass fields from the non-dataclass Rules type, fix its fallback label, and make unsupported-field diagnostics deterministic. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e6066b0-464e-4b74-9d4e-29953fa3de77
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
python-iptablescalls withiptables --waitsubprocessesPartitionerchains and the xtables cross-process lock instead of a redundant process-local lockpython-iptablesdependencyContext
After #8254 made partition groups concurrent, #8249's CI failed with
ValueError: No closing quotation.python-iptablestemporarily replaces process-wide stdout while serialising a rule; a concurrent{in-place-restart}Loguru line entered that captured buffer and brokeshlex.split.Running the CLI in a subprocess isolates its output from all parent test threads rather than relying on every stdout writer to coordinate with the iptables operation.
--waittakes the exclusive xtables lock for each update, and concurrent sub-tests operate on distinct chains.Testing