libnetwork: fan out firewall reload top-down#46666
Draft
corhere wants to merge 2 commits intomoby:masterfrom
Draft
libnetwork: fan out firewall reload top-down#46666corhere wants to merge 2 commits intomoby:masterfrom
corhere wants to merge 2 commits intomoby:masterfrom
Conversation
Make one global variable go away. Signed-off-by: Cory Snider <csnider@mirantis.com>
The pattern of having low-level code that sets up iptables rules register reload callbacks for itself is not ideal. Callbacks can not be unregistered, so any values captured by the closures can never be GC'ed. And the callback registry is a singleton. While these issues could be overcome by iterating on the API, there are some other insurmountable issues which cannot be overcome without a complete overhaul of how the firewall reload logic is plumbed through. Imperative callback registration makes the control flow hard to reason about and the order of callbacks is implied by the order they were registered. Make the libnetwork Controller ultimately responsible for replaying firewall configuration when the firewall rulesets have been flushed externally. It fans out the reload signal to the drivers, which in turn fan out to the individual networks. Signed-off-by: Cory Snider <csnider@mirantis.com>
d8d0cea to
221d1ab
Compare
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.
The pattern of having low-level code that sets up iptables rules register reload callbacks for itself is not ideal. Callbacks can not be unregistered, so any values captured by the closures can never be GC'ed. And the callback registry is a singleton. While these issues could be overcome by iterating on the API, there are some other insurmountable issues which cannot be overcome without a complete overhaul of how the firewall reload logic is plumbed through. Imperative callback registration makes the control flow hard to reason about and the order of callbacks is implied by the order they were registered.
- What I did
Make the libnetwork Controller ultimately responsible for replaying firewall configuration when the firewall rulesets have been flushed externally. It fans out the reload signal to the drivers, which in turn fan out to the individual networks.
- How I did it
Made
(*Controller).handleFirewallReloadthe only registerediptables.OnReloadedcallback. It handles replaying firewall configuration for the controller, then calls into all the drivers which implementFirewallReplayerto handle replaying their own firewall configuration.- How to verify it
TODO - UNVERIFIED
- Description for the changelog
N/A
- A picture of a cute animal (not mandatory but encouraged)