Nireconciler #9
Open
Nireconciler #9
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
This commit contains few improvements for the state reconciliation of network routes. They are mostly aimed at avoiding unnecessary Route Modify operations (which sometimes cause routes to be re-created even when config has not actually changed but only some runtime route flags make Route.Equal() to return false) and also to avoid some false-negative error log messages (e.g. when trying to delete route which was already automatically deleted by kernel). Additionally, there is a change in the description outputted for WLAN config in the dependency graph to NOT include the hashed password. This is stored in /run/nim-current-state.dot and /run/nim-intended-state.dot. Even though this filesystem is only in memory (just like /run/wlan/wpa_supplicant.conf), it is safer to avoid spreading this secret information into multiple files, because it increases the chance of the password being accidentally leaked (e.g. by someone debugging a connectivity issue and publishing this file to recipients who are not intended to know the password, even if it is already hashed - enough to connect to the WiFi network). Signed-off-by: Milan Lenco <[email protected]>
The process of converting the declarative high-level network instance config coming from the controller into the corresponding low-level (Linux) network stack config items (routes, bridges, iptables, etc.) has been previously scattered all across the zedrouter codebase and it was tightly coupled to the Linux network stack. Given the considerable amount of config items that even a single network instance with only a couple of ACL rules translates into, the original implementation ended up being quite complicated to understand and debug/maintain, not to mention extend. In order to avoid adding even more complexity, it didn't support config changes (it was necessary to recreate network instances). Lastly, since the original code would make netlink and other syscalls directly, without hiding them behind some interface, it was not possible to "mock" the host system and write some unit tests for zedrouter. In this commit we introduce a new component NIReconciler, which, just like NIM's DPCReconciler, is based on the Reconciler (libs/reconciler). NIReconciler is an interface and LinuxNIReconciler is just one (and currently the only one) implementation. By using the generic Reconciler, NIReconciler only needs to determine what the intended state of the network stack should be for the given EdgeDevConfig config received from the controller wrt. app connectivity (specifically it reads the config for network instances and app interfaces). The problem of intended<->current state reconciliation is taken care of by the Reconciler, making NIReconciler considerably less complex than the original zedrouter's approach, while additionally supporting any config change (that makes sense). LinuxNIReconciler is covered by unit tests (around 86% coverage, excluding "mocked" parts). This commit only introduces NIReconciler as another package under pillar. It is not yet integrated into zedrouter. This will be done in the next commit. The only changes outside of introducing a new package are: * Small improvements in iptables - e.g. adding support for ipsets * Split base routing table index between DPC and network instances for more clarity * change HostSubnet to return net.IPNet with pointer, not as a value (in net and other standard packages IPNet is always used with pointer) Signed-off-by: Milan Lenco <[email protected]>
milan-zededa
force-pushed
the
nireconciler
branch
from
92a621a to
0331a11
Compare
This commit untangles the close coupling between zedrouter and Linux by replacing all remaining netlink and system calls with the calls to NIReconciler (which has generic interface and replaceable implementations). Moreover, this commit contains a substantial cleanup of the zedrouter code-base - some of the source files had too many lines, some functions were way overcomplicated, had strange names, obsolete comments, used strings instead of net.IP/net.HardwareAddr, etc. Lastly, VLANs for switch network instances are now configured by zedrouter and not domainmgr - where it does not belong. Signed-off-by: Milan Lenco <[email protected]>
Signed-off-by: Milan Lenco <[email protected]>
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.
No description provided.