fix(network_filter_ng): ensure rollback for node and pod network filters#1462
Open
cynox-66 wants to merge 2 commits into
Open
fix(network_filter_ng): ensure rollback for node and pod network filters#1462cynox-66 wants to merge 2 commits into
cynox-66 wants to merge 2 commits into
Conversation
NodeNetworkFilterModule and PodNetworkFilterModule left the privileged chaos pod and any applied iptables rules orphaned on the cluster if an exception occurred after the pod was deployed (including an interrupted time.sleep) or, for the pod filter, on the no-interface early return. Add a private _rollback() helper to each module and track application state with a rules_applied flag set only after apply_network_rules() completes. Because iptables cleanup is destructive and not idempotent, the guard is disarmed immediately before the happy-path cleanup so the exception handler can never run a second cleanup pass, and cleanup is never attempted for rules that were only generated but not applied. Cleanup stays inside the existing try/except so its failures continue to flow through the error_queue / raise contract unchanged; the exception- and early-return-path rollbacks are best-effort so a cleanup failure can never mask the original error or turn the graceful no-interface skip into a reported scenario failure. Behaviour is identical to before except that the previously-leaked chaos pod and iptables rules are now cleaned up. Add unit tests for both modules covering the success, early-return, exception, cleanup-failure, exactly-once, and parallel/error_queue paths. Signed-off-by: cynox-66 <devj2311@gmail.com>
cynox-66
requested review from
chaitanyaenr,
ddjain,
paigerube14 and
tsebastiani
as code owners
July 10, 2026 07:47
Contributor
|
👀 Human Input Needed → Pair Review Briefing |
The exactly-once cleanup docstrings claimed rollback runs in `finally`. It actually runs in the `except` block; the exactly-once guarantee comes from disarming rules_applied (set False) before the happy-path clean, not from finally semantics. Correct the two docstrings to match the code. Documentation-only: no implementation or test logic changed. Signed-off-by: cynox-66 <devj2311@gmail.com>
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
1 participant
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
This PR adds exception-safe rollback to the
network_filter_ngnode and pod filter modules.It follows the same goal as #1461 by ensuring temporary resources are cleaned up on exception and early-return paths, preventing leaked chaos pods while preserving the existing behaviour of successful executions.
Changes
NodeNetworkFilterModule
_rollback()helperrules_appliedflagPodNetworkFilterModule
_rollback()helperrules_appliedflagWhy
rules_appliedis necessaryUnlike the
tccleanup used by the network chaos modules, the filter modules remove iptables rules using positional deletion (iptables -D ...).The rule lists are generated before
apply_network_rules()executes. If cleanup were triggered simply because rules had been generated, an exception before rule application could remove unrelated pre-existing firewall rules.To avoid that, rollback is gated by a dedicated
rules_appliedflag that is set only afterapply_network_rules()completes successfully.Behaviour
The successful execution path is unchanged.
This PR only changes previously-unhandled cleanup paths:
These paths now perform best-effort cleanup of temporary resources while preserving the existing error-handling behaviour.
Tests
This PR introduces comprehensive unit tests for both filter modules, including coverage for:
error_queue) executionAll existing network-related regression tests continue to pass.