Merge pull request spidernet-io#1871 from cyclinder/opt_coordinator

improve coordiantor

Makefile.defs

@@ -100,7 +100,7 @@ endif
 GO_BUILD_WITH_CGO = CGO_ENABLED=1 $(CGO_CC) $(GO) build
 
 #data race and lock debug
-ifneq ($(RACE),)
+ifeq ($(RACE),"1")
     GO_BUILD_FLAGS += -race
     GO_TEST_FLAGS += -race
     GOTEST_COVER_OPTS += -covermode=atomic

charts/spiderpool/README.md

@@ -131,15 +131,15 @@ helm install spiderpool spiderpool/spiderpool --wait --namespace kube-system \
 
 ### coordinator parameters
 
-| Name                           | Description                                                               | Value                 |
-| ------------------------------ | ------------------------------------------------------------------------- | --------------------- |
-| `coordinator.enabled`          | enable SpiderCoordinator                                                  | `true`                |
-| `coordinator.name`             | the name of the default SpiderCoordinator CR                              | `default-coordinator` |
-| `coordinator.tuneMode`         | optional network mode, ["underlay", "overlay", "disabled"]                | `underlay`            |
-| `coordinator.podCIDRType`      | Pod CIDR type that should be collected, [ "cluster", "calico", "cilium" ] | `cluster`             |
-| `coordinator.detectGateway`    | detect the reachability of the gateway                                    | `true`                |
-| `coordinator.detectIPConflict` | detect IP address conflicts                                               | `true`                |
-| `coordinator.tunePodRoutes`    | tune Pod routes                                                           | `true`                |
+| Name                           | Description                                                               | Value      |
+| ------------------------------ | ------------------------------------------------------------------------- | ---------- |
+| `coordinator.enabled`          | enable SpiderCoordinator                                                  | `true`     |
+| `coordinator.name`             | the name of the default SpiderCoordinator CR                              | `default`  |
+| `coordinator.tuneMode`         | optional network mode, ["underlay", "overlay", "disabled"]                | `underlay` |
+| `coordinator.podCIDRType`      | Pod CIDR type that should be collected, [ "cluster", "calico", "cilium" ] | `cluster`  |
+| `coordinator.detectGateway`    | detect the reachability of the gateway                                    | `true`     |
+| `coordinator.detectIPConflict` | detect IP address conflicts                                               | `true`     |
+| `coordinator.tunePodRoutes`    | tune Pod routes                                                           | `true`     |
 
 
 ### multus parameters

charts/spiderpool/templates/_helpers.tpl

@@ -210,8 +210,8 @@ return the spiderpoolInit image
 {{- end -}}
 {{- if .Values.spiderpoolInit.image.digest }}
     {{- print "@" .Values.spiderpoolInit.image.digest -}}
-{{- else if .Values.spiderpoolAgent.image.tag -}}
-    {{- printf ":%s" .Values.spiderpoolAgent.image.tag -}}
+{{- else if .Values.spiderpoolInit.image.tag -}}
+    {{- printf ":%s" .Values.spiderpoolInit.image.tag -}}
 {{- else -}}
     {{- printf ":v%s" .Chart.AppVersion -}}
 {{- end -}}

cmd/coordinator/Makefile

@@ -10,13 +10,15 @@ GO_BUILD_LDFLGAS= -ldflags "-X github.com/spidernet-io/spiderpool/internal/versi
 -X github.com/spidernet-io/spiderpool/internal/version.coordinatorBuildDate=$(GO_BUILD_TIME) \
 -X github.com/spidernet-io/spiderpool/internal/version.coordinatorVersion=$(GIT_TAG)"
 
+GO_BUILD_FLAG= CGO_ENABLED=0 $(GO) build
+
 all: $(TARGET)
 
 .PHONY: all $(TARGET)
 
 $(TARGET): ../../Makefile ../../Makefile.defs Makefile
 	@$(ECHO_GO)
-	$(QUIET)$(GO_BUILD) $(GO_BUILD_LDFLGAS) -o $(TARGET)
+	$(QUIET)$(GO_BUILD_FLAG) $(GO_BUILD_LDFLGAS) -o $(TARGET)
 
 clean:
 	@$(ECHO_CLEAN)

cmd/coordinator/cmd/cni_types.go

@@ -25,10 +25,11 @@ import (
 var (
 	defaultLogPath          = "/var/log/spidernet/coordinator.log"
 	defaultUnderlayVethName = "veth0"
-	defaultOverlayVethName  = "eth0"
-	defaultPodRuleTable     = 100
-	defaultNICPrefix        = "net"
-	BinNamePlugin           = filepath.Base(os.Args[0])
+	// by default, k8s pod's first NIC is eth0
+	defaultOverlayVethName = "eth0"
+	defaultPodRuleTable    = 100
+	defaultNICPrefix       = "net"
+	BinNamePlugin          = filepath.Base(os.Args[0])
 )
 
 type Mode string
@@ -131,10 +132,6 @@ func ParseConfig(stdin []byte, coordinatorConfig *models.CoordinatorConfig) (*Co
 		return nil, err
 	}
 
-	if conf.OnlyHardware {
-		return &conf, nil
-	}
-
 	if err = ValidateRoutes(&conf, coordinatorConfig); err != nil {
 		return nil, err
 	}
@@ -174,6 +171,10 @@ func ParseConfig(stdin []byte, coordinatorConfig *models.CoordinatorConfig) (*Co
 		conf.TunePodRoutes = pointer.Bool(*coordinatorConfig.TunePodRoutes)
 	}
 
+	if conf.TuneMode == "" {
+		conf.TuneMode = Mode(*coordinatorConfig.TuneMode)
+	}
+
 	if conf.PodDefaultRouteNIC == "" && coordinatorConfig.PodDefaultRouteNIC != "" {
 		conf.PodDefaultRouteNIC = coordinatorConfig.PodDefaultRouteNIC
 	}

cmd/coordinator/cmd/command_add.go

@@ -173,6 +173,7 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 				return err
 			}
 		}
+
 		if err != nil {
 			return err
 		}

cmd/coordinator/cmd/utils.go

@@ -29,19 +29,49 @@ type coordinator struct {
 	hostAddress, currentAddress                                  []netlink.Addr
 }
 
-// firstInvoke check if coordinator is first called.
+// firstInvoke check if coordinator is first called and do some checks:
+// underlay mode only works with underlay mode, which can't work with overlay
+// mode, and which can't be called in first cni invoked by using multus's
+// annotations: v1.multus-cni.io/default-network
 func (c *coordinator) coordinatorFirstInvoke(podFirstInterface string) error {
 	var err error
 	switch c.tuneMode {
 	case ModeUnderlay:
 		c.firstInvoke = c.currentInterface == podFirstInterface
-		return err
+		// underlay mode can't work with calico/cilium(overlay)
+		if !c.firstInvoke {
+			var exist bool
+			exist, err = networking.CheckInterfaceExist(c.netns, defaultUnderlayVethName)
+			if err != nil {
+				return fmt.Errorf("failed to CheckInterfaceExist: %v", err)
+			}
+
+			if !exist {
+				return fmt.Errorf("in multi-NIC mode, underlay mode can only work with underlay mode. please check pod's multus annotations")
+			}
+		}
+		return nil
 	case ModeOverlay:
+		// in overlay mode, it should no veth0 and currentInterface isn't eth0
+		if c.currentInterface == podFirstInterface {
+			return fmt.Errorf("in overlay mode, underlay mode can only work with underlay mode. please check pod's multus annotations")
+		}
+
+		exist, err := networking.CheckInterfaceExist(c.netns, defaultUnderlayVethName)
+		if err != nil {
+			return fmt.Errorf("failed to CheckInterfaceExist: %v", err)
+		}
+
+		if exist {
+			return fmt.Errorf("in multi-NIC mode, overlay mode can't work with underlay mode. please check pod's multus annotations")
+		}
+
 		c.firstInvoke, err = networking.IsFirstModeOverlayInvoke(c.netns, c.interfacePrefix)
 		return err
 	case ModeDisable:
 		return nil
 	}
+
 	return fmt.Errorf("unknown tuneMode: %s", c.tuneMode)
 }
 
@@ -88,11 +118,7 @@ func (c *coordinator) setupVeth(containerID string) error {
 		return nil
 	})
 
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return err
 }
 
 // setupNeighborhood setup neighborhood tables for pod and host.
@@ -147,6 +173,8 @@ func (c *coordinator) setupHijackRoutes(logger *zap.Logger, ruleTable int) error
 		return err
 	}
 
+	logger.Debug("Debug setupHijackRoutes", zap.String("v4Gw", v4Gw.String()), zap.String("v6Gw", v6Gw.String()))
+
 	err = c.netns.Do(func(_ ns.NetNS) error {
 		// make sure that veth0/eth0 forwards traffic within the cluster
 		// eq: ip route add <cluster/service cidr> dev veth0/eth0
@@ -164,7 +192,7 @@ func (c *coordinator) setupHijackRoutes(logger *zap.Logger, ruleTable int) error
 
 			if c.tuneMode == ModeOverlay && c.firstInvoke {
 				if err := networking.AddRoute(logger, unix.RT_TABLE_MAIN, netlink.SCOPE_UNIVERSE, c.podVethName, ipNet, v4Gw, v6Gw); err != nil {
-					logger.Error("failed to AddRoute for hijackCIDR", zap.Error(err))
+					logger.Error("failed to AddRoute for hijackCIDR", zap.String("Dst", ipNet.String()), zap.Error(err))
 					return fmt.Errorf("failed to AddRoute for hijackCIDR: %v", err)
 				}
 				logger.Debug("Add Route for hijackSubnet in pod successfully", zap.String("Dst", ipNet.String()))
@@ -236,19 +264,20 @@ func (c *coordinator) tunePodRoutes(logger *zap.Logger, configDefaultRouteNIC st
 		configDefaultRouteNIC = c.currentInterface
 	}
 
-	miss, err := networking.IsInterfaceMiss(c.netns, configDefaultRouteNIC)
+	exist, err := networking.CheckInterfaceExist(c.netns, configDefaultRouteNIC)
 	if err != nil {
-		logger.Error("failed to IsInterfaceMiss", zap.String("interface", configDefaultRouteNIC), zap.Error(err))
-		return fmt.Errorf("failed to IsInterfaceMiss: %v", err)
+		logger.Error("failed to CheckInterfaceExist", zap.String("interface", configDefaultRouteNIC), zap.Error(err))
+		return fmt.Errorf("failed to CheckInterfaceExist: %v", err)
 	}
 
-	if miss {
+	if !exist {
 		return fmt.Errorf("podDefaultRouteNIC: %s don't exist in pod", configDefaultRouteNIC)
 	}
 
-	podDefaultRouteNIC, err := networking.GetDefaultRouteInterface(c.netns, c.currentInterface, c.ipFamily)
+	podDefaultRouteNIC, err := networking.GetDefaultRouteInterface(c.ipFamily, c.currentInterface, c.netns)
 	if err != nil {
 		logger.Error("failed to GetDefaultRouteInterface", zap.Error(err))
+		return fmt.Errorf("failed to GetDefaultRouteInterface: %v", err)
 	}
 
 	if podDefaultRouteNIC == "" {

go.mod

@@ -47,13 +47,13 @@ require (
 require (
 	github.com/agiledragon/gomonkey/v2 v2.9.0
 	github.com/containernetworking/plugins v1.3.0
-	github.com/go-ping/ping v1.1.0
 	github.com/golang/mock v1.6.0
 	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
 	github.com/mdlayher/arp v0.0.0-20220512170110-6706a2966875
 	github.com/mdlayher/ethernet v0.0.0-20220221185849-529eae5b6118
 	github.com/mdlayher/ndp v1.0.1
 	github.com/openkruise/kruise-api v1.3.0
+	github.com/prometheus-community/pro-bing v0.2.0
 	github.com/prometheus/client_golang v1.14.0
 	github.com/spidernet-io/e2eframework v0.0.0-20230403061847-445757b963b3
 	github.com/spidernet-io/spiderdoctor v0.3.0

go.sum

@@ -250,8 +250,6 @@ github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/
 github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14=
 github.com/go-openapi/validate v0.22.1 h1:G+c2ub6q47kfX1sOBLwIQwzBVt8qmOAARyo/9Fqs9NU=
 github.com/go-openapi/validate v0.22.1/go.mod h1:rjnrwK57VJ7A8xqfpAOEKRH8yQSGUriMu5/zuPSQ1hg=
-github.com/go-ping/ping v1.1.0 h1:3MCGhVX4fyEUuhsfwPrsEdQw6xspHkv5zHsiSoDFZYw=
-github.com/go-ping/ping v1.1.0/go.mod h1:xIFjORFzTxqIV/tDVGO4eDy/bLuSyawEeojSm3GfRGk=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
 github.com/go-swagger/go-swagger v0.30.3 h1:HuzvdMRed/9Q8vmzVcfNBQByZVtT79DNZxZ18OprdoI=
 github.com/go-swagger/go-swagger v0.30.3/go.mod h1:neDPes8r8PCz2JPvHRDj8BTULLh4VJUt7n6MpQqxhHM=
@@ -384,7 +382,6 @@ github.com/google/pprof v0.0.0-20230323073829-e72429f035bd/go.mod h1:79YE0hCXdHa
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
@@ -608,6 +605,8 @@ github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndr
 github.com/pquerna/cachecontrol v0.1.0/go.mod h1:NrUG3Z7Rdu85UNR3vm7SOsl1nFIeSiQnrHV5K9mBcUI=
 github.com/projectcalico/api v0.0.0-20220722155641-439a754a988b h1:dW+UhJMzusDO6hqVGuCYeDxXWAzc7HnA9CsPN+uHPnA=
 github.com/projectcalico/api v0.0.0-20220722155641-439a754a988b/go.mod h1:Avoy1rTN1GfeisnHGf3WhQNqR+BuGOcwfNFsdWX6OHE=
+github.com/prometheus-community/pro-bing v0.2.0 h1:hyK7yPFndU3LCDwEQJwPQUCjNkp1DGP/VxyzrWfXZUU=
+github.com/prometheus-community/pro-bing v0.2.0/go.mod h1:20arNb2S8rNG3EtmjHyZZU92cfbhQx7oCHZ9sulAV+I=
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=

images/spiderpool-agent/Dockerfile

@@ -31,7 +31,7 @@ RUN  make GOARCH=${TARGETARCH}   \
 
 WORKDIR /src/cmd/coordinator
 RUN make GOARCH=${TARGETARCH}   \
-        RACE=${RACE} NOSTRIP=${NOSTRIP} NOOPT=${NOOPT} QUIET_MAKE=${QUIET_MAKE} \
+        NOSTRIP=${NOSTRIP} NOOPT=${NOOPT} QUIET_MAKE=${QUIET_MAKE} \
         DESTDIR_BIN=/tmp/install/${TARGETOS}/${TARGETARCH}/bin \
         all install
 

pkg/networking/gwconnection/connection.go

@@ -7,21 +7,21 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-ping/ping"
+	"github.com/prometheus-community/pro-bing"
 )
 
 func DetectGatewayConnection(gw string) error {
-	pingCtl := ping.New(gw)
+	pingCtl := probing.New(gw)
 	pingCtl.Interval = 100 * time.Millisecond
 	pingCtl.Count = 3
 	pingCtl.Timeout = 2 * time.Second
+
 	if err := pingCtl.Run(); err != nil {
 		return fmt.Errorf("failed to DetectGatewayConnection: %v", err)
 	}
 
-	stats := pingCtl.Statistics()
-	if stats.PacketLoss > 0 {
-		return fmt.Errorf("gateway: %s is unreachable", gw)
+	if pingCtl.Statistics().PacketLoss > 0 {
+		return fmt.Errorf("gateway %s is unreachable", gw)
 	}
 	return nil
 }

pkg/networking/networking/ip.go

@@ -5,14 +5,14 @@ package networking
 
 import (
 	"fmt"
+	"net"
+	"regexp"
+	"strings"
+
 	current "github.com/containernetworking/cni/pkg/types/100"
-	"github.com/containernetworking/plugins/pkg/ip"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/vishvananda/netlink"
 	"go.uber.org/zap"
-	"net"
-	"regexp"
-	"strings"
 )
 
 var DefaultInterfacesToExclude = []string{
@@ -155,19 +155,28 @@ func getAdders(link netlink.Link, ipfamily int) ([]netlink.Addr, error) {
 	return ipAddress, nil
 }
 
-func IsInterfaceMiss(netns ns.NetNS, iface string) (bool, error) {
-	err := netns.Do(func(_ ns.NetNS) error {
-		_, err := netlink.LinkByName(iface)
-		return err
-	})
-
-	if err == nil {
-		return false, nil
+func CheckInterfaceExist(netns ns.NetNS, iface string) (bool, error) {
+	var exist bool
+	var err error
+	if netns != nil {
+		err = netns.Do(func(_ ns.NetNS) error {
+			exist, err = isInterfaceExist(iface)
+			return err
+		})
+		return exist, err
 	}
+	return isInterfaceExist(iface)
+}
 
-	if strings.EqualFold(err.Error(), ip.ErrLinkNotFound.Error()) {
+func isInterfaceExist(iface string) (bool, error) {
+	_, err := netlink.LinkByName(iface)
+	if err == nil {
 		return true, nil
 	}
-	return false, err
 
+	if _, ok := err.(netlink.LinkNotFoundError); ok {
+		return false, nil
+	} else {
+		return false, err
+	}
 }

pkg/networking/networking/mac.go

@@ -7,14 +7,15 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
-	"github.com/containernetworking/plugins/pkg/ns"
-	"github.com/vishvananda/netlink"
-	"go.uber.org/zap"
 	"math/big"
 	"net"
 	"net/netip"
 	"os"
 	"regexp"
+
+	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/vishvananda/netlink"
+	"go.uber.org/zap"
 )
 
 // OverwriteHwAddress override the hardware address of the specified interface.
@@ -26,13 +27,13 @@ func OverwriteHwAddress(logger *zap.Logger, netns ns.NetNS, macPrefix, iface str
 	}
 
 	// we only focus on first element
-	nAddr, err := netip.ParsePrefix(ips[0].IP.String())
+	nAddr, err := netip.ParseAddr(ips[0].IP.String())
 	if err != nil {
 		logger.Error("failed to ParsePrefix", zap.Error(err))
 		return "", err
 	}
 
-	suffix, err := inetAton(nAddr.Addr())
+	suffix, err := inetAton(nAddr)
 	if err != nil {
 		logger.Error("failed to inetAton", zap.Error(err))
 		return "", err