feat: convert execs to ip to netlink calls #1697
Draft
Conversation
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
aauren
force-pushed
the
migrate_to_netlink_library
branch
from
38733d4 to
7a58331
Compare
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
aauren
added
override-stale
aauren
force-pushed
the
migrate_to_netlink_library
branch
2 times, most recently
from
4657546 to
cc734e5
Compare
aauren
force-pushed
the
migrate_to_netlink_library
branch
from
3a4526c to
08f07c7
Compare
Not making direct exec calls to user binary interfaces has long been a principle of kube-router. When kube-router was first coded, the netlink library was missing significant features that forced us to exec out. However, now netlink seems to have most of the functionality that we need. This converts all of the places where we can use netlink to use the netlink functionality.
The rt_tables list is already in an ordered form in terms of priority. Once one is found, it should be considered the optimal one and stop looking for additional tables.
Previously we were accidentally deleting all routes that were found, this mimics the previous functionality better by only deleting external IPs that were found in the externalIPRouteTable that are no longer in the activeExternalIPs map. Also improves logging around any routes that are deleted as this is likely of interest to all kube-router administrators.
In order for a local route to be valid it needs to have the scope set to host. When we were executing ip commands iproute2 just did this for us to make the command accurate. Now that we're communicating with the netlink socket, we need to do this conversion for ourselves. Without this we get an error that says "invalid argument" from the netlink subsystem. But if the route isn't local, then most of the routing logic for services doesn't work correctly because it acts upon external traffic as well as local traffic which isn't correct.
It has proven to be tricky to insert new rules without calling the designated NewRule() function from the netlink library. Usually attempts will fail with an operation not supported message. This improves the reliability of rule insertion.
Consolidate IP utility functions into a new file and add proper unit testing. Additionally consolidate logic and references to default route subnets.
When ip rules are evaluated in the netlink library, default routes for src and dst are equated to nil. This makes it difficult to evaluate them and requires additional handling in order for them. I filed an issue upstream so that this could potentially get fixed: vishvananda/netlink#1080 however if it doesn't get resolved, this should allow us to move forward.
Removes repeated logic of calculating IP address subnets for single subnet hosts and consolidates it in one place.
It used to be when we were using iproute2's CLI we needed to have the fwmark as a hex number so we were passing it as a string in that format. However, now that we use the netlink library directly, we already have the fwmark in the condition that we need it. So instead of doing all of these string <-> int conversions, lets just keep this simpler.
Instead of deleting and just hoping for the best, this change makes it so that we check first whether or not a route exists. This helps to reduce needless warnings that the user receives and is just all around more accurate.
aauren
force-pushed
the
migrate_to_netlink_library
branch
from
08f07c7 to
b0246ef
Compare
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.
Not making direct exec calls to user binary interfaces has long been a principle of kube-router. When kube-router was first coded, the netlink library was missing significant features that forced us to exec out. However, now netlink seems to have most of the functionality that we need.
This converts all of the places where we can use netlink to use the netlink functionality.
The current state of this PR is untested and still needs to undergo significant testing: