Skip to content

Commit

Permalink
Add unit tests for pkg/agent/route/route_windows.go
Browse files Browse the repository at this point in the history
Signed-off-by: Hongliang Liu <[email protected]>
  • Loading branch information
hongliangl committed Jul 31, 2023
1 parent 652c721 commit fe1a900
Show file tree
Hide file tree
Showing 10 changed files with 756 additions and 193 deletions.
1 change: 1 addition & 0 deletions hack/update-codegen-dockerized.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ MOCKGEN_TARGETS=(
"pkg/agent/util/ipset Interface testing"
"pkg/agent/util/iptables Interface testing mock_iptables_linux.go" # Must specify linux.go suffix, otherwise compilation would fail on windows platform as source file has linux build tag.
"pkg/agent/util/netlink Interface testing mock_netlink_linux.go"
"pkg/agent/util/winnetutil Interface testing mock_net_windows.go"
"pkg/antctl AntctlClient ."
"pkg/controller/networkpolicy EndpointQuerier testing"
"pkg/controller/querier ControllerQuerier testing"
Expand Down
18 changes: 7 additions & 11 deletions pkg/agent/agent_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"antrea.io/antrea/pkg/agent/externalnode"
"antrea.io/antrea/pkg/agent/interfacestore"
"antrea.io/antrea/pkg/agent/util"
antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
"antrea.io/antrea/pkg/apis/crd/v1alpha1"
"antrea.io/antrea/pkg/ovs/ovsctl"
utilip "antrea.io/antrea/pkg/util/ip"
Expand Down Expand Up @@ -369,21 +370,16 @@ func (i *Initializer) getTunnelPortLocalIP() net.IP {
// The routes will be restored on OVS bridge interface after the IP configuration
// is moved to the OVS bridge.
func (i *Initializer) saveHostRoutes() error {
routes, err := util.GetNetRoutesAll()
family := antreasyscall.AF_INET
filter := &util.Route{
LinkIndex: i.nodeConfig.UplinkNetConfig.Index,
GatewayAddress: net.ParseIP(i.nodeConfig.UplinkNetConfig.Gateway),
}
routes, err := util.RouteListFiltered(family, filter)
if err != nil {
return err
}
for _, route := range routes {
if route.LinkIndex != i.nodeConfig.UplinkNetConfig.Index {
continue
}
if route.GatewayAddress.String() != i.nodeConfig.UplinkNetConfig.Gateway {
continue
}
// Skip IPv6 routes before we support IPv6 stack.
if route.DestinationSubnet.IP.To4() == nil {
continue
}
// Skip default route. The default route will be added automatically when
// configuring IP address on OVS bridge interface.
if route.DestinationSubnet.IP.IsUnspecified() {
Expand Down
17 changes: 2 additions & 15 deletions pkg/agent/route/route_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -920,8 +920,6 @@ func TestAddRoutes(t *testing.T) {
func TestDeleteRoutes(t *testing.T) {
tests := []struct {
name string
networkConfig *config.NetworkConfig
nodeConfig *config.NodeConfig
podCIDR *net.IPNet
existingNodeRoutes map[string][]*netlink.Route
existingNodeNeighbors map[string]*netlink.Neigh
Expand Down Expand Up @@ -966,8 +964,6 @@ func TestDeleteRoutes(t *testing.T) {
mockIPSet := ipsettest.NewMockInterface(ctrl)
c := &Client{netlink: mockNetlink,
ipset: mockIPSet,
networkConfig: tt.networkConfig,
nodeConfig: tt.nodeConfig,
nodeRoutes: sync.Map{},
nodeNeighbors: sync.Map{},
}
Expand Down Expand Up @@ -1456,27 +1452,18 @@ func TestAddExternalIPRoute(t *testing.T) {
tests := []struct {
name string
externalIPs []string
serviceRoutes map[string]*netlink.Route
expectedCalls func(mockNetlink *netlinktest.MockInterfaceMockRecorder)
}{
{
name: "IPv4",
serviceRoutes: map[string]*netlink.Route{
externalIPv4Addr1: ipv4Route1,
externalIPv4Addr2: ipv4Route2,
},
name: "IPv4",
externalIPs: []string{externalIPv4Addr1, externalIPv4Addr2},
expectedCalls: func(mockNetlink *netlinktest.MockInterfaceMockRecorder) {
mockNetlink.RouteReplace(ipv4Route1)
mockNetlink.RouteReplace(ipv4Route2)
},
},
{
name: "IPv6",
serviceRoutes: map[string]*netlink.Route{
externalIPv6Addr1: ipv6Route1,
externalIPv6Addr2: ipv6Route2,
},
name: "IPv6",
externalIPs: []string{externalIPv6Addr1, externalIPv6Addr2},
expectedCalls: func(mockNetlink *netlinktest.MockInterfaceMockRecorder) {
mockNetlink.RouteReplace(ipv6Route1)
Expand Down
57 changes: 28 additions & 29 deletions pkg/agent/route/route_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ import (
"antrea.io/antrea/pkg/agent/openflow"
"antrea.io/antrea/pkg/agent/servicecidr"
"antrea.io/antrea/pkg/agent/util"
antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
"antrea.io/antrea/pkg/agent/util/winfirewall"
"antrea.io/antrea/pkg/agent/util/winnetutil"
binding "antrea.io/antrea/pkg/ovs/openflow"
iputil "antrea.io/antrea/pkg/util/ip"
)
Expand All @@ -56,12 +58,13 @@ var (
type Client struct {
nodeConfig *config.NodeConfig
networkConfig *config.NetworkConfig
netUtil winnetutil.Interface
// nodeRoutes caches ip routes to remote Pods. It's a map of podCIDR to routes.
nodeRoutes *sync.Map
nodeRoutes sync.Map
// serviceRoutes caches ip routes about Services.
serviceRoutes *sync.Map
serviceRoutes sync.Map
// netNatStaticMappings caches Windows NetNat for NodePort.
netNatStaticMappings *sync.Map
netNatStaticMappings sync.Map
fwClient *winfirewall.Client
bridgeInfIndex int
noSNAT bool
Expand All @@ -74,9 +77,10 @@ type Client struct {
func NewClient(networkConfig *config.NetworkConfig, noSNAT, proxyAll, connectUplinkToBridge, multicastEnabled bool, serviceCIDRProvider servicecidr.Interface) (*Client, error) {
return &Client{
networkConfig: networkConfig,
nodeRoutes: &sync.Map{},
serviceRoutes: &sync.Map{},
netNatStaticMappings: &sync.Map{},
netUtil: &util.Handle{},
nodeRoutes: sync.Map{},
serviceRoutes: sync.Map{},
netNatStaticMappings: sync.Map{},
fwClient: winfirewall.NewClient(),
noSNAT: noSNAT,
proxyAll: proxyAll,
Expand Down Expand Up @@ -168,7 +172,7 @@ func (c *Client) Reconcile(podCIDRs []string) error {
if c.proxyAll && c.isServiceRoute(rt) {
continue
}
err := util.RemoveNetRoute(rt)
err := c.netUtil.RemoveNetRoute(rt)
if err != nil {
return err
}
Expand Down Expand Up @@ -203,7 +207,7 @@ func (c *Client) AddRoutes(podCIDR *net.IPNet, nodeName string, peerNodeIP, peer
return nil
}
// Remove the existing route entry if the gateway address is not as expected.
if err := util.RemoveNetRoute(existingRoute); err != nil {
if err := c.netUtil.RemoveNetRoute(existingRoute); err != nil {
klog.Errorf("Failed to delete existing route entry with destination %s gateway %s on %s (%s)", podCIDR.String(), peerGwIP.String(), nodeName, peerNodeIP)
return err
}
Expand All @@ -213,7 +217,7 @@ func (c *Client) AddRoutes(podCIDR *net.IPNet, nodeName string, peerNodeIP, peer
return nil
}

if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
return err
}

Expand All @@ -232,7 +236,7 @@ func (c *Client) DeleteRoutes(podCIDR *net.IPNet) error {
}

rt := obj.(*util.Route)
if err := util.RemoveNetRoute(rt); err != nil {
if err := c.netUtil.RemoveNetRoute(rt); err != nil {
return err
}
c.nodeRoutes.Delete(podCIDR.String())
Expand All @@ -247,13 +251,13 @@ func (c *Client) addVirtualServiceIPRoute(isIPv6 bool) error {
svcIP := config.VirtualServiceIPv4

neigh := generateNeigh(svcIP, linkIndex)
if err := util.ReplaceNetNeighbor(neigh); err != nil {
if err := c.netUtil.ReplaceNetNeighbor(neigh); err != nil {
return fmt.Errorf("failed to add new IP neighbour for %s: %w", svcIP, err)
}
klog.InfoS("Added virtual Service IP neighbor", "neighbor", neigh)

route := generateRoute(virtualServiceIPv4Net, net.IPv4zero, linkIndex, util.MetricHigh)
if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
return fmt.Errorf("failed to install route for virtual Service IP %s: %w", svcIP.String(), err)
}
c.serviceRoutes.Store(svcIP.String(), route)
Expand All @@ -270,7 +274,7 @@ func (c *Client) addServiceCIDRRoute(serviceCIDR *net.IPNet) error {
oldServiceCIDRRoute, serviceCIDRRouteExists := c.serviceRoutes.Load(serviceIPv4CIDRKey)
// Generate a route with the new ClusterIP CIDR and install it.
route := generateRoute(serviceCIDR, gw, linkIndex, metric)
if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
return fmt.Errorf("failed to install a new Service CIDR route: %w", err)
}

Expand Down Expand Up @@ -310,7 +314,7 @@ func (c *Client) addServiceCIDRRoute(serviceCIDR *net.IPNet) error {

// Remove stale routes.
for _, rt := range staleRoutes {
if err := util.RemoveNetRoute(rt); err != nil {
if err := c.netUtil.RemoveNetRoute(rt); err != nil {
return fmt.Errorf("failed to delete stale Service CIDR route %s: %w", rt.String(), err)
} else {
klog.V(4).InfoS("Deleted stale Service CIDR route successfully", "route", rt)
Expand All @@ -327,7 +331,7 @@ func (c *Client) addVirtualNodePortDNATIPRoute(isIPv6 bool) error {
gw := config.VirtualServiceIPv4

route := generateRoute(virtualNodePortDNATIPv4Net, gw, linkIndex, util.MetricHigh)
if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
return fmt.Errorf("failed to install route for NodePort DNAT IP %s: %w", vIP.String(), err)
}
c.serviceRoutes.Store(vIP.String(), route)
Expand Down Expand Up @@ -368,7 +372,7 @@ func (c *Client) syncIPInfra() {

func (c *Client) syncRoute() error {
restoreRoute := func(route *util.Route) bool {
if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
klog.ErrorS(err, "Failed to sync route", "Route", route)
return false
}
Expand Down Expand Up @@ -404,7 +408,7 @@ func (c *Client) syncNetNatStaticMapping() error {

c.netNatStaticMappings.Range(func(_, v interface{}) bool {
mapping := v.(*util.NetNatStaticMapping)
if err := util.ReplaceNetNatStaticMapping(mapping); err != nil {
if err := c.netUtil.ReplaceNetNatStaticMapping(mapping); err != nil {
klog.ErrorS(err, "Failed to add netNatStaticMapping", "netNatStaticMapping", mapping)
return false
}
Expand All @@ -424,20 +428,15 @@ func (c *Client) isServiceRoute(route *util.Route) bool {
}

func (c *Client) listIPRoutesOnGW() (map[string]*util.Route, error) {
routes, err := util.GetNetRoutesAll()
family := antreasyscall.AF_INET
filter := &util.Route{LinkIndex: c.nodeConfig.GatewayConfig.LinkIndex}
routes, err := c.netUtil.RouteListFiltered(family, filter)
if err != nil {
return nil, err
}
rtMap := make(map[string]*util.Route)
for idx := range routes {
rt := routes[idx]
if rt.LinkIndex != c.nodeConfig.GatewayConfig.LinkIndex {
continue
}
// Only process IPv4 route entries in the loop.
if rt.DestinationSubnet.IP.To4() == nil {
continue
}
// Retrieve the route entries that use global unicast IP address as the destination. "GetNetRoutesAll" also
// returns the entries of loopback, broadcast, and multicast, which are added by the system when adding a new IP
// on the interface. Since removing those route entries might introduce the host networking issues, ignore them
Expand Down Expand Up @@ -492,7 +491,7 @@ func (c *Client) AddNodePort(nodePortAddresses []net.IP, port uint16, protocol b
InternalPort: port,
Protocol: protocol,
}
if err := util.ReplaceNetNatStaticMapping(netNatStaticMapping); err != nil {
if err := c.netUtil.ReplaceNetNatStaticMapping(netNatStaticMapping); err != nil {
return err
}
c.netNatStaticMappings.Store(fmt.Sprintf("%d-%s", port, protocol), netNatStaticMapping)
Expand All @@ -508,7 +507,7 @@ func (c *Client) DeleteNodePort(nodePortAddresses []net.IP, port uint16, protoco
return nil
}
netNatStaticMapping := obj.(*util.NetNatStaticMapping)
if err := util.RemoveNetNatStaticMapping(netNatStaticMapping); err != nil {
if err := c.netUtil.RemoveNetNatStaticMapping(netNatStaticMapping); err != nil {
return err
}
c.netNatStaticMappings.Delete(key)
Expand All @@ -525,7 +524,7 @@ func (c *Client) AddExternalIPRoute(externalIP net.IP) error {
svcIPNet := util.NewIPNet(externalIP)

route := generateRoute(svcIPNet, gw, linkIndex, metric)
if err := util.ReplaceNetRoute(route); err != nil {
if err := c.netUtil.ReplaceNetRoute(route); err != nil {
return fmt.Errorf("failed to install route for external IP %s: %w", externalIPStr, err)
}
c.serviceRoutes.Store(externalIPStr, route)
Expand All @@ -541,7 +540,7 @@ func (c *Client) DeleteExternalIPRoute(externalIP net.IP) error {
klog.V(2).InfoS("Didn't find route for external IP", "IP", externalIPStr)
return nil
}
if err := util.RemoveNetRoute(route.(*util.Route)); err != nil {
if err := c.netUtil.RemoveNetRoute(route.(*util.Route)); err != nil {
return fmt.Errorf("failed to delete route for external IP %s: %w", externalIPStr, err)
}
c.serviceRoutes.Delete(externalIPStr)
Expand Down
Loading

0 comments on commit fe1a900

Please sign in to comment.