feat: Add native linux endpoint client to prep removing OVS #1471
Conversation
rbtr
left a comment
rbtr
left a comment
There was a problem hiding this comment.
some initial comments - several things which apply globally, and I expect fixes to be applied to the whole PR even though I have only commented on a single instance.
pls also address the feedback from the linter
tamilmani1989
left a comment
tamilmani1989
left a comment
There was a problem hiding this comment.
Thanks for this PR.. Left some comments
rbtr
left a comment
rbtr
left a comment
There was a problem hiding this comment.
some nitpicks but generally lgtm
| if err := client.netlink.AddOrRemoveStaticArp(netlink.ADD, | ||
| interfaceName, | ||
| virtualGwNet.IP, | ||
| hardwareAddr, | ||
| false); err != nil { |
There was a problem hiding this comment.
unwrap
splitting method signatures or invocations across lines is worse for readability
| } else { | ||
| // Any other error | ||
| return errors.Wrap(deleteNSIfNotNilErr, "failed to create vlan vnet link after making new ns") | ||
| } |
There was a problem hiding this comment.
i think you could flip this conditional to eliminate the else branch entirely
| } | ||
| type NativeEndpointClient struct { | ||
| eth0VethName string // So like eth0 | ||
| vlanVethName string // So like eth0.1 |
| } | ||
| defer ns.Close() | ||
| // Enter the network namespace | ||
| log.Printf("[ExecuteInNS] Entering vnetns %s.", ns.file.Name()) |
There was a problem hiding this comment.
log looks confusing. why vnetns? i guess this is generic function
|
|
||
| // Exit network namespace | ||
| defer func() { | ||
| log.Printf("[ExecuteInNS] Exiting vnetns %s.", ns.file.Name()) |
| }) | ||
| } | ||
|
|
||
| func (client *NativeEndpointClient) DeleteEndpointsImpl(ep *endpoint, routesLeft int) error { |
There was a problem hiding this comment.
wanted to understand on routesLeft logic.. lets talk offline about this
| DeleteNamed(name string) (err error) | ||
| } | ||
| type NativeEndpointClient struct { | ||
| eth0VethName string // So like eth0 |
There was a problem hiding this comment.
this is not veth..rename as "primaryHostIfName"
nw.extIf.Name > eth0 (eth0) eth0.vlanid > eth0.X (eth0.1) %s%s hostIfName > vnet (A1veth0) %s%s-2 contIfName > container (B1veth0)
Directly instantiating struct because nothing special happens in NewNativeEndpointClient
Forgot to remove uintptr from mocknetns
Works on VMSS
If I use GetNetworkInterface, I need to be in the vnet NS, but that means I will need to call ExecuteInNS, which causes tests to fail.
Hopefully this fixes the windows lint error
Maybe this will fix the windows linter?
Maybe this fixes the linter error?
Tests ok, Native ok
Tests ok, ping ok, wget ok
|
rebased for CI fixes; lgtm |
3 participants
Reason for Change:
Creates a linux native endpoint client which is meant to replace the existing endpoint client that uses OVS. Uses the native linux kernel to replicate the functionality of OVS. Using the native client should improve the reliability of the multitenancy datapath because there will no longer be a dependency on OVS.
Issue Fixed:
Requirements:
Notes:
Creates a new netns package and uses vishvananda's netns and netlink library.