Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add UT for pkg/agent/ipassigner/ip_assigner_linux #5402

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add UT for pkg/agent/ipassigner/ip_assigner_linux #5402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1aea270
IPAssigner Vlan Unit Test
rajnkamr Aug 17, 2023
149e206
IpAssigner vlan & nonvlan UT
rajnkamr Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hack/update-codegen-dockerized.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ MOCKGEN_TARGETS=(
"pkg/agent/querier AgentQuerier testing"
"pkg/agent/route Interface testing"
"pkg/agent/ipassigner IPAssigner testing"
"pkg/agent/ipassigner/responder Responder testing"
"pkg/agent/secondarynetwork/podwatch InterfaceConfigurator,IPAMAllocator testing"
"pkg/agent/servicecidr Interface testing"
"pkg/agent/util/ipset Interface testing"
Expand Down
35 changes: 25 additions & 10 deletions pkg/agent/ipassigner/ip_assigner_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ import (
crdv1b1 "antrea.io/antrea/pkg/apis/crd/v1beta1"
)

var (
netlinkAdd = netlink.LinkAdd
netlinkDel = netlink.LinkDel
ensureRPF = util.EnsureRPFilterOnInterface
netlinkSetUp = netlink.LinkSetUp
netInterfaceByName = net.InterfaceByName
netlinkAddrAdd = netlink.AddrAdd
netlinkAddrDel = netlink.AddrDel
netlinkAddrList = netlink.AddrList
Comment on lines +39 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assignee heavily relies on netlink and there is an interface defined for it: pkg/agent/util/netlink.Interface. Instead of replacing each call repeatedly, assignee should hold the interface, and we just replace it with a fake implementation in unit test. pkg/agent/route/route_linux.go can be an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have following 4 types supported in netlink interface, we can use mocked apis for these
netlink.AddrList
netlink.AddrDel
netlink.AddrAdd
netlink.LinkSetUp
However others like netlink.LinkAdd, netlink.LinkDel, net.InterfaceByName are not supported

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netlink.LinkAdd, netlink.LinkDel can be added to the interface

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment hasn't been resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had Tried earlier replacing while holding the interface for assignee, it might not work unlike pkg/agent/route/route_linux.go as in route_linux.go, it is using netlink of type utilnetlink.Interface which matches type of interface pkg/agent/util/netlink/netlink_linux.go while replacing

// Client takes care of routing container packets in host network, coordinating ip route, ip rule, iptables and ipset.
type Client struct {
        netlink       utilnetlink.Interface

In assign function, it is directly using external netlink pkg

func (as *assignee) assign(ip net.IP, subnetInfo *crdv1b1.SubnetInfo) error {
        // If there is a real link, add the IP to its address list.
        if as.link != nil {
                addr := getIPNet(ip, subnetInfo)
                if err := netlink.AddrAdd(as.link, &netlink.Addr{IPNet: addr}); err != nil {

mockNetlink.EXPECT().AddrAdd(dummyDevice.Attrs().Name, &netlink.Addr{IPNet: expectedIPNet}).Return(nil)
For example above expectation are never met wrt interface defined in pkg/agent/util/netlink/netlink_linux.go ! similarly for all other netlink apis in ipassigner is used directly from netlink.

)

type advertiseArpNdp func(a *assignee, ip net.IP)

var advertiseResponder advertiseArpNdp = (*assignee).advertise

// VLAN interfaces created by antrea-agent will be named with the prefix.
// For example, when VLAN ID is 10, the name will be antrea-ext.10.
// It can be used to determine whether it's safe to delete an interface when it's no longer used.
Expand Down Expand Up @@ -79,7 +94,7 @@ func (as *assignee) deletable() bool {
}

func (as *assignee) destroy() error {
if err := netlink.LinkDel(as.link); err != nil {
if err := netlinkDel(as.link); err != nil {
return fmt.Errorf("error deleting interface %v: %w", as.link, err)
}
return nil
Expand All @@ -89,7 +104,7 @@ func (as *assignee) assign(ip net.IP, subnetInfo *crdv1b1.SubnetInfo) error {
// If there is a real link, add the IP to its address list.
if as.link != nil {
addr := getIPNet(ip, subnetInfo)
if err := netlink.AddrAdd(as.link, &netlink.Addr{IPNet: addr}); err != nil {
if err := netlinkAddrAdd(as.link, &netlink.Addr{IPNet: addr}); err != nil {
if !errors.Is(err, unix.EEXIST) {
return fmt.Errorf("failed to add IP %v to interface %s: %v", addr, as.link.Attrs().Name, err)
} else {
Expand All @@ -111,7 +126,7 @@ func (as *assignee) assign(ip net.IP, subnetInfo *crdv1b1.SubnetInfo) error {
}
}
// Always advertise the IP when the IP is newly assigned to this Node.
as.advertise(ip)
advertiseResponder(as, ip)
as.ips.Insert(ip.String())
return nil
}
Expand All @@ -134,7 +149,7 @@ func (as *assignee) unassign(ip net.IP, subnetInfo *crdv1b1.SubnetInfo) error {
// If there is a real link, delete the IP from its address list.
if as.link != nil {
addr := getIPNet(ip, subnetInfo)
if err := netlink.AddrDel(as.link, &netlink.Addr{IPNet: addr}); err != nil {
if err := netlinkAddrDel(as.link, &netlink.Addr{IPNet: addr}); err != nil {
if !errors.Is(err, unix.EADDRNOTAVAIL) {
return fmt.Errorf("failed to delete IP %v from interface %s: %v", ip, as.link.Attrs().Name, err)
} else {
Expand Down Expand Up @@ -171,7 +186,7 @@ func (as *assignee) getVLANID() (int, bool) {

func (as *assignee) loadIPAddresses() (map[string]*crdv1b1.SubnetInfo, error) {
assignedIPs := map[string]*crdv1b1.SubnetInfo{}
addresses, err := netlink.AddrList(as.link, netlink.FAMILY_ALL)
addresses, err := netlinkAddrList(as.link, netlink.FAMILY_ALL)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -362,7 +377,7 @@ func (a *ipAssigner) AssignIP(ip string, subnetInfo *crdv1b1.SubnetInfo, forceAd
if crdv1b1.CompareSubnetInfo(subnetInfo, oldSubnetInfo, true) {
klog.V(2).InfoS("The IP is already assigned", "ip", ip)
if forceAdvertise {
as.advertise(parsedIP)
advertiseResponder(as, parsedIP)
}
return false, nil
}
Expand Down Expand Up @@ -497,15 +512,15 @@ func (a *ipAssigner) getAssignee(subnetInfo *crdv1b1.SubnetInfo, createIfNotExis
},
VlanId: int(subnetInfo.VLAN),
}
if err := netlink.LinkAdd(vlan); err != nil {
if err := netlinkAdd(vlan); err != nil {
if !errors.Is(err, unix.EEXIST) {
return nil, fmt.Errorf("error creating VLAN sub-interface for VLAN %d", subnetInfo.VLAN)
}
}
// Loose mode is needed because incoming traffic received on the interface is expected to be received on the parent
// external interface when looking up the main table. To make it look up the custom table, we will need to restore
// the mark on the reply traffic and turn on src_valid_mark on this interface, which is more complicated.
if err := util.EnsureRPFilterOnInterface(name, 2); err != nil {
if err := ensureRPF(name, 2); err != nil {
return nil, err
}
as, err := a.addVLANAssignee(vlan, subnetInfo.VLAN)
Expand All @@ -516,10 +531,10 @@ func (a *ipAssigner) getAssignee(subnetInfo *crdv1b1.SubnetInfo, createIfNotExis
}

func (a *ipAssigner) addVLANAssignee(link netlink.Link, vlan int32) (*assignee, error) {
if err := netlink.LinkSetUp(link); err != nil {
if err := netlinkSetUp(link); err != nil {
return nil, fmt.Errorf("error setting up interface %v", link)
}
iface, err := net.InterfaceByName(link.Attrs().Name)
iface, err := netInterfaceByName(link.Attrs().Name)
if err != nil {
return nil, err
}
Expand Down
Loading
Loading