Modularize iptables manager #28746
Merged
Modularize iptables manager #28746
Conversation
This file contains 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
pippolo84
added
release-note/misc
|
/test |
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
2 times, most recently
from
c15bed0
to
aa4c0b0
Compare
joamaki
requested changes
Oct 24, 2023
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
from
aa4c0b0
to
5eaef6a
Compare
|
I've addressed all the feedbacks (the ones regarding modules manager have been pushed in the specific PR as well) but I'm planning to rework this quite a lot, so still not ready for review yet. |
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
5 times, most recently
from
5b7d4df
to
863ec7d
Compare
dylandreimerink
requested changes
Oct 26, 2023
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
from
863ec7d
to
7ae9016
Compare
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
2 times, most recently
from
dbca706
to
0d58076
Compare
mhofstetter
reviewed
Nov 1, 2023
mhofstetter
reviewed
Nov 1, 2023
dylandreimerink
approved these changes
Nov 6, 2023
joamaki
approved these changes
Nov 7, 2023
Fix iptables manager name to avoid the repetion of "iptables" in both package and type names (as suggested in "Effective Go" guidelines about naming: https://go.dev/doc/effective_go#names) Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Modularize iptables configuration manager into its own cell. The previous Init method is replaced with an Invoke function in order to be executed when starting the iptables manager cell. Also, enableIPForwarding is moved in the iptables package and kept unexported, since it is not used outside of the package. Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
iptables has been modularized into a cell, but still depends directly on option.DaemonConfig global variables and helpers. The configuration flags that are "private" to the cell are provided through cell.Config. Regarding the flags and the helpers shared with other parts of the code, a SharedConfig struct is provided in the cell itself. Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
from
0d58076
to
6055a83
Compare
Move AddToNodeIpset and RemoveFromNodeIpset into the iptables manager, so that other cells depending on them will import the iptables config manager through dependency injection. Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84
force-pushed
the
pr/pippolo84/iptables-manager-modularization
branch
from
6055a83
to
c9fa608
Compare
|
Force-pushed to fix controlplane tests: |
|
/test |
|
Conformance Cluster Mesh failure tracked here, rerunning. |
maintainer-s-little-helper
bot
added
the
ready-to-merge
2 tasks
qmonnet
added
the
backport-pending/1.14
2 tasks
github-actions
bot
added
backport-done/1.14
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.
Modularize the iptables configuration manager in a cell.
Direct dependency to the DaemonConfig global object is avoided through a SharedConfig privately provided to the cell. This config gets the values from all the referenced DaemonConfig helpers at startup, in order to be used by the new cell.
Also, cells depending on iptables (like the node manager one) now takes a reference to it through dependency injection instead of directly calling side effectful exported functions.
Depends on #28713