diff --git a/go-controller/pkg/node/gateway_init.go b/go-controller/pkg/node/gateway_init.go index 0a22318c3b..546b0425fb 100644 --- a/go-controller/pkg/node/gateway_init.go +++ b/go-controller/pkg/node/gateway_init.go @@ -139,7 +139,7 @@ func getGatewayNextHops() ([]net.IP, string, error) { } gatewayIntf := config.Gateway.Interface if needIPv4NextHop || needIPv6NextHop || gatewayIntf == "" { - defaultGatewayIntf, defaultGatewayNextHops, err := getDefaultGatewayInterfaceDetails(gatewayIntf) + defaultGatewayIntf, defaultGatewayNextHops, err := getDefaultGatewayInterfaceDetails(gatewayIntf, config.IPv4Mode, config.IPv6Mode) if err != nil { return nil, "", err } diff --git a/go-controller/pkg/node/helper_linux.go b/go-controller/pkg/node/helper_linux.go index dc9497765e..ea86b10814 100644 --- a/go-controller/pkg/node/helper_linux.go +++ b/go-controller/pkg/node/helper_linux.go @@ -7,7 +7,6 @@ import ( "fmt" "net" - "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" "github.com/pkg/errors" "github.com/vishvananda/netlink" @@ -18,31 +17,41 @@ import ( // which the default gateway (for route to 0.0.0.0) is configured. // optionally pass the pre-determined gateway interface // It also returns the default gateways themselves. -func getDefaultGatewayInterfaceDetails(gwIface string) (string, []net.IP, error) { +func getDefaultGatewayInterfaceDetails(gwIface string, ipV4Mode, ipV6Mode bool) (string, []net.IP, error) { var intfName string var gatewayIPs []net.IP - if config.IPv4Mode { + if ipV4Mode { intfIPv4Name, gw, err := getDefaultGatewayInterfaceByFamily(netlink.FAMILY_V4, gwIface) if err != nil { return "", gatewayIPs, err } intfName = intfIPv4Name - gatewayIPs = append(gatewayIPs, gw) + + // only add the GW IP if it is specified + if len(gw) != 0 { + gatewayIPs = append(gatewayIPs, gw) + } } - if config.IPv6Mode { + if ipV6Mode { intfIPv6Name, gw, err := getDefaultGatewayInterfaceByFamily(netlink.FAMILY_V6, gwIface) if err != nil { return "", gatewayIPs, err } - // validate that both IP Families use the same interface for the gateway + + // if there is an interface specified for both IP families + // validate they use the same one if intfName == "" { intfName = intfIPv6Name - } else if intfName != intfIPv6Name { + } else if (len(intfName) > 0 && len(intfIPv6Name) > 0) && intfName != intfIPv6Name { return "", nil, fmt.Errorf("multiple gateway interfaces detected: %s %s", intfName, intfIPv6Name) } - gatewayIPs = append(gatewayIPs, gw) + + // only add the GW IP if it is specified + if len(gw) != 0 { + gatewayIPs = append(gatewayIPs, gw) + } } return intfName, gatewayIPs, nil @@ -58,7 +67,7 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net gwIfIdx := 0 // gw interface provided if len(gwIface) > 0 { - link, err := netlink.LinkByName(gwIface) + link, err := util.GetNetLinkOps().LinkByName(gwIface) if err != nil { return "", nil, fmt.Errorf("error looking up gw interface: %q, error: %w", gwIface, err) } @@ -69,7 +78,7 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net klog.Infof("Provided gateway interface %q, found as index: %d", gwIface, gwIfIdx) } - routeList, err := netlink.RouteListFiltered(family, filter, mask) + routeList, err := util.GetNetLinkOps().RouteListFiltered(family, filter, mask) if err != nil { return "", nil, errors.Wrapf(err, "failed to get routing table in node") } @@ -78,7 +87,7 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net for _, r := range routes { // no multipath if len(r.MultiPath) == 0 { - intfLink, err := netlink.LinkByIndex(r.LinkIndex) + intfLink, err := util.GetNetLinkOps().LinkByIndex(r.LinkIndex) if err != nil { klog.Warningf("Failed to get interface link for route %v : %v", r, err) continue @@ -104,7 +113,7 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net // TODO: revisit for full multipath support // xref: https://github.com/vishvananda/netlink/blob/6ffafa9fc19b848776f4fd608c4ad09509aaacb4/route.go#L137-L145 for _, nh := range r.MultiPath { - intfLink, err := netlink.LinkByIndex(nh.LinkIndex) + intfLink, err := util.GetNetLinkOps().LinkByIndex(nh.LinkIndex) if err != nil { klog.Warningf("Failed to get interface link for route %v : %v", nh, err) continue diff --git a/go-controller/pkg/node/helper_linux_test.go b/go-controller/pkg/node/helper_linux_test.go index ebf2088418..f5d2d2ae36 100644 --- a/go-controller/pkg/node/helper_linux_test.go +++ b/go-controller/pkg/node/helper_linux_test.go @@ -1,9 +1,16 @@ package node import ( + "fmt" + "net" "reflect" "testing" + ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing" + netlink_mocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/github.com/vishvananda/netlink" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" + util_mocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/mocks" + "github.com/stretchr/testify/assert" "github.com/vishvananda/netlink" ) @@ -174,3 +181,267 @@ func TestFilterRoutesByIfIndex(t *testing.T) { } } } + +func TestGetDefaultGatewayInterfaceByFamily(t *testing.T) { + mockNetLinkOps := new(util_mocks.NetLinkOps) + mockLink := new(netlink_mocks.Link) + // below sets the `netLinkOps` in util/net_linux.go to a mock instance for purpose of unit tests execution + util.SetNetLinkOpMockInst(mockNetLinkOps) + defer util.ResetNetLinkOpMockInst() + + defaultIf := "testInterface" + customIf := "customTestInterface" + defaultGWIP := ovntest.MustParseIP("1.1.1.1") + customGWIP := ovntest.MustParseIP("fd99::1") + + tests := []struct { + desc string + ipFamily int + gwIface string + expIntfName string + expGatewayIP net.IP + expErr bool + netLinkOpsMockHelper []ovntest.TestifyMockHelper + linkMockHelper []ovntest.TestifyMockHelper + }{ + { + desc: "no default routes returns empty values", + ipFamily: netlink.FAMILY_V4, + expGatewayIP: net.IP{}, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{}, nil}}, + }, + }, + { + desc: "first default route is used when no gw is specified", + gwIface: "", + ipFamily: netlink.FAMILY_V4, + expIntfName: defaultIf, + expGatewayIP: defaultGWIP, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 2, + Gw: defaultGWIP, + }, + { + LinkIndex: 9, + Gw: ovntest.MustParseIP("3.3.3.3"), + }, + }, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + }, + }, + { + desc: "only routes from the provided GW are considered", + gwIface: customIf, + ipFamily: netlink.FAMILY_V6, + expIntfName: customIf, + expGatewayIP: customGWIP, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "LinkByName", OnCallMethodArgType: []string{"string", "string"}, RetArgList: []interface{}{mockLink, nil}}, + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIP, + }, + { + LinkIndex: 2, + Gw: customGWIP, + }, + }, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Index: 2}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Index: 2}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: customIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: customIf}}}, + }, + }, + } + for i, tc := range tests { + t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) { + ovntest.ProcessMockFnList(&mockNetLinkOps.Mock, tc.netLinkOpsMockHelper) + ovntest.ProcessMockFnList(&mockLink.Mock, tc.linkMockHelper) + intfName, gwIP, err := getDefaultGatewayInterfaceByFamily(tc.ipFamily, tc.gwIface) + if intfName != tc.expIntfName { + t.Fatalf("TestGetDefaultGatewayInterfaceByFamily(%d): Default gateway interface should be '%v' but got '%v'", + i, tc.expIntfName, intfName) + } + if !reflect.DeepEqual(tc.expGatewayIP, gwIP) { + t.Fatalf("TestGetDefaultGatewayInterfaceByFamily(%d): Default gateway IP should be '%v' but got '%v'", + i, tc.expGatewayIP, gwIP) + } + + t.Log(err) + if tc.expErr { + assert.Error(t, err) + } else { + assert.Nil(t, err) + } + mockNetLinkOps.AssertExpectations(t) + mockLink.AssertExpectations(t) + }) + } +} + +func TestGetDefaultGatewayInterfaceDetails(t *testing.T) { + mockNetLinkOps := new(util_mocks.NetLinkOps) + mockLink := new(netlink_mocks.Link) + // below sets the `netLinkOps` in util/net_linux.go to a mock instance for purpose of unit tests execution + util.SetNetLinkOpMockInst(mockNetLinkOps) + defer util.ResetNetLinkOpMockInst() + + defaultIf := "testInterface" + defaultGWIPv4 := ovntest.MustParseIP("1.1.1.1") + defaultGWIPv6 := ovntest.MustParseIP("fd99::1") + + tests := []struct { + desc string + ipV4Mode bool + ipV6Mode bool + gwIface string + expIntfName string + expGatewayIPs []net.IP + expErr bool + netLinkOpsMockHelper []ovntest.TestifyMockHelper + linkMockHelper []ovntest.TestifyMockHelper + }{ + { + desc: "no default routes returns empty values", + ipV4Mode: true, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{}, nil}}, + }, + }, + { + desc: "only ipv4 GW set in dual-stack returns valid interface and one gw", + ipV4Mode: true, + ipV6Mode: true, + expGatewayIPs: []net.IP{defaultGWIPv4}, + expIntfName: defaultIf, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIPv4, + }, + }, nil}}, + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{}, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + }, + }, + { + desc: "only ipv6 GW set in dual-stack returns valid interface and one gw", + ipV4Mode: true, + ipV6Mode: true, + expGatewayIPs: []net.IP{defaultGWIPv6}, + expIntfName: defaultIf, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{}, nil}}, + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIPv6, + }, + }, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + }, + }, + { + desc: "in dual-stack the function fails if the default GWs are on different interfaces", + ipV4Mode: true, + ipV6Mode: true, + expErr: true, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIPv4, + }, + }, nil}}, + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 2, + Gw: defaultGWIPv6, + }, + }, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: "invalidInterface"}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: "invalidInterface"}}}, + }, + }, + { + desc: "in dual-stack the function returns both GW ips", + ipV4Mode: true, + ipV6Mode: true, + expGatewayIPs: []net.IP{defaultGWIPv4, defaultGWIPv6}, + expIntfName: defaultIf, + netLinkOpsMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIPv4, + }, + }, nil}}, + {OnCallMethodName: "RouteListFiltered", OnCallMethodArgType: []string{"int", "*netlink.Route", "uint64"}, RetArgList: []interface{}{[]netlink.Route{ + { + LinkIndex: 1, + Gw: defaultGWIPv6, + }, + }, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + {OnCallMethodName: "LinkByIndex", OnCallMethodArgType: []string{"int"}, RetArgList: []interface{}{mockLink, nil}}, + }, + linkMockHelper: []ovntest.TestifyMockHelper{ + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: defaultIf}}}, + }, + }, + } + for i, tc := range tests { + t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) { + ovntest.ProcessMockFnList(&mockNetLinkOps.Mock, tc.netLinkOpsMockHelper) + ovntest.ProcessMockFnList(&mockLink.Mock, tc.linkMockHelper) + intfName, gwIPs, err := getDefaultGatewayInterfaceDetails(tc.gwIface, tc.ipV4Mode, tc.ipV6Mode) + if intfName != tc.expIntfName { + t.Fatalf("TestGetDefaultGatewayInterfaceDetails(%d): Default gateway interface should be '%v' but got '%v'", + i, tc.expIntfName, intfName) + } + if !reflect.DeepEqual(tc.expGatewayIPs, gwIPs) { + t.Fatalf("TestGetDefaultGatewayInterfaceDetails(%d): Default gateway IPs should be '%v' but got '%v'", + i, tc.expGatewayIPs, gwIPs) + } + + t.Log(err) + if tc.expErr { + assert.Error(t, err) + } else { + assert.Nil(t, err) + } + mockNetLinkOps.AssertExpectations(t) + mockLink.AssertExpectations(t) + }) + } +}