diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 1878e31207f..6b9a97897c7 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -860,8 +860,8 @@ func (c *Controller) initNodeChassis() error { } for _, node := range nodes { if err := c.UpdateChassisTag(node); err != nil { + klog.Error(err) if _, ok := err.(*ErrChassisNotFound); !ok { - klog.Error(err) return err } } diff --git a/pkg/controller/node.go b/pkg/controller/node.go index ce3f2cda145..e6bd78e29ea 100644 --- a/pkg/controller/node.go +++ b/pkg/controller/node.go @@ -810,13 +810,17 @@ func (c *Controller) UpdateChassisTag(node *v1.Node) error { klog.Errorf("failed to gc chassis: %v", err) return err } - return &ErrChassisNotFound{Chassis: annoChassisName, Node: node.Name} + err = &ErrChassisNotFound{Chassis: annoChassisName, Node: node.Name} + klog.Error(err) + return err } if chassis.ExternalIDs == nil || chassis.ExternalIDs["vendor"] != util.CniTypeName { klog.Infof("init tag %s for node %s chassis %s", util.CniTypeName, node.Name, chassis.Name) if err = c.OVNSbClient.UpdateChassisTag(chassis.Name, node.Name); err != nil { - return fmt.Errorf("failed to init chassis tag, %w", err) + err := fmt.Errorf("failed to init chassis tag, %w", err) + klog.Error(err) + return err } } return nil diff --git a/pkg/util/arp_test.go b/pkg/util/arp_test.go index 3460695cf51..a2e87589f75 100644 --- a/pkg/util/arp_test.go +++ b/pkg/util/arp_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "github.com/vishvananda/netlink" "golang.org/x/sys/unix" ) @@ -96,8 +97,8 @@ func TestArpResolve(t *testing.T) { if mac == nil { t.Errorf("ARP resolved MAC address is nil: try %d, link name %s, default gw %s", count, linkName, defaultGW) } - // should failed - defaultGW = "xx.xx.xx.xx" + // invalid link name + linkName = "invalid" mac, count, err = ArpResolve(linkName, defaultGW, time.Second, maxRetry, done) if err == nil { t.Errorf("Expect error, but got nil: try %d, link name %s, default gw %s", count, linkName, defaultGW) @@ -105,4 +106,166 @@ func TestArpResolve(t *testing.T) { if mac != nil { t.Errorf("Expect nil MAC address, but got %v: try %d, link name %s, default gw %s", mac, count, linkName, defaultGW) } + linkName = link.Attrs().Name + // invalid gw + defaultGW = "x.x.x.x" + mac, count, err = ArpResolve(linkName, defaultGW, time.Second, maxRetry, done) + if err == nil { + t.Errorf("Expect error, but got nil: try %d, link name %s, default gw %s", count, linkName, defaultGW) + } + if mac != nil { + t.Errorf("Expect nil MAC address, but got %v: try %d, link name %s, default gw %s", mac, count, linkName, defaultGW) + } + // unreachable gw + defaultGW = "123.45.67.8" + mac, count, err = ArpResolve(linkName, defaultGW, time.Second, maxRetry, done) + if err == nil { + t.Errorf("Expect error, but got nil: try %d, link name %s, default gw %s", count, linkName, defaultGW) + } + if mac != nil { + t.Errorf("Expect nil MAC address, but got %v: try %d, link name %s, default gw %s", mac, count, linkName, defaultGW) + } +} + +func TestDetectIPConflict(t *testing.T) { + // get the default route gw and nic + routes, err := netlink.RouteList(nil, unix.AF_UNSPEC) + if errors.Is(err, netlink.ErrNotImplemented) { + return // skip if not implemented + } + if err != nil { + t.Fatalf("failed to get routes: %v", err) + } + var validIP, invalidIP string + var nicIndex int + var inMac, outMac net.HardwareAddr + for _, r := range routes { + if r.Dst != nil && r.Dst.IP.String() == "0.0.0.0" { + nicIndex = r.LinkIndex + validIP = r.Src.String() + } + } + + if nicIndex == 0 { + t.Fatalf("failed to get nic") + } + + link, err := netlink.LinkByIndex(nicIndex) + if err != nil { + t.Fatalf("failed to get link: %v", err) + } + linkName := link.Attrs().Name + inMac = link.Attrs().HardwareAddr + outMac, err = ArpDetectIPConflict(linkName, validIP, inMac) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP request operation not permitted") + return + } + t.Errorf("Error resolving ARP: %v", err) + } + require.Nil(t, err) + require.Nil(t, outMac) + // invalid ip + invalidIP = "x.x.x.x" + outMac, err = ArpDetectIPConflict(linkName, invalidIP, inMac) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP request operation not permitted") + return + } + } + require.NotNil(t, err) + require.Nil(t, outMac) + // invalid nil nic + linkName = "" + outMac, err = ArpDetectIPConflict(linkName, validIP, inMac) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP request operation not permitted") + return + } + } + require.NotNil(t, err) + require.Nil(t, outMac) + + // invalid mac + outMac, err = ArpDetectIPConflict(linkName, validIP, nil) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP request operation not permitted") + return + } + } + require.NotNil(t, err) + require.Nil(t, outMac) +} + +func TestAnnounceArpAddress(t *testing.T) { + // get the default route gw and nic + routes, err := netlink.RouteList(nil, unix.AF_UNSPEC) + if errors.Is(err, netlink.ErrNotImplemented) { + return // skip if not implemented + } + if err != nil { + t.Fatalf("failed to get routes: %v", err) + } + var validIP, invalidIP string + var nicIndex int + var inMac net.HardwareAddr + for _, r := range routes { + if r.Dst != nil && r.Dst.IP.String() == "0.0.0.0" { + nicIndex = r.LinkIndex + validIP = r.Src.String() + } + } + if nicIndex == 0 { + t.Fatalf("failed to get nic") + } + + link, err := netlink.LinkByIndex(nicIndex) + if err != nil { + t.Fatalf("failed to get link: %v", err) + } + maxRetry := 1 + linkName := link.Attrs().Name + inMac = link.Attrs().HardwareAddr + err = AnnounceArpAddress(linkName, validIP, inMac, maxRetry, time.Second) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP announce operation not permitted") + return + } + } + require.Nil(t, err) + // invalid link name + linkName = "invalid" + err = AnnounceArpAddress(linkName, validIP, inMac, maxRetry, time.Second) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP announce operation not permitted") + return + } + } + require.NotNil(t, err) + linkName = link.Attrs().Name + // invalid ip + invalidIP = "x.x.x.x" + err = AnnounceArpAddress(linkName, invalidIP, inMac, maxRetry, time.Second) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP announce operation not permitted") + return + } + } + require.NotNil(t, err) + // invalid mc + err = AnnounceArpAddress(linkName, validIP, nil, maxRetry, time.Second) + if err != nil { + if strings.Contains(err.Error(), "not permitted") { + t.Skip("ARP announce operation not permitted") + return + } + } + require.NotNil(t, err) }