Detect IPConflicting and gatewayReachable in ipam without coordinator

Signed-off-by: Cyclinder Kuo <[email protected]>
spidernet-io · Jan 15, 2025 · 392a3f7 · 392a3f7
1 parent 068a5cc
commit 392a3f7
Show file tree

Hide file tree

Showing 21 changed files with 332 additions and 325 deletions.
diff --git a/api/v1/agent/models/coordinator_config.go b/api/v1/agent/models/coordinator_config.go
diff --git a/api/v1/agent/models/ip_config.go b/api/v1/agent/models/ip_config.go
diff --git a/api/v1/agent/openapi.yaml b/api/v1/agent/openapi.yaml
@@ -306,6 +306,10 @@ definitions:
         type: string
       vlan:
         type: integer
+      enableGatewayDetection:
+        type: boolean
+      enableIPConflictDetection:
+        type: boolean
     required:
       - version
       - address
@@ -340,10 +344,6 @@ definitions:
         type: integer
       txQueueLen:
         type: integer
-      detectIPConflict:
-        type: boolean
-      detectGateway:
-        type: boolean
       vethLinkAddress:
         type: string
     required:

diff --git a/api/v1/agent/server/embedded_spec.go b/api/v1/agent/server/embedded_spec.go
diff --git a/cmd/coordinator/cmd/cni_types.go b/cmd/coordinator/cmd/cni_types.go
@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
-	"time"
 
 	"github.com/containernetworking/cni/pkg/types"
 	"github.com/containernetworking/cni/pkg/version"
@@ -44,24 +43,21 @@ const (
 
 type Config struct {
 	types.NetConf
-	DetectGateway      *bool          `json:"detectGateway,omitempty"`
-	VethLinkAddress    string         `json:"vethLinkAddress,omitempty"`
-	MacPrefix          string         `json:"podMACPrefix,omitempty"`
-	MultusNicPrefix    string         `json:"multusNicPrefix,omitempty"`
-	PodDefaultCniNic   string         `json:"podDefaultCniNic,omitempty"`
-	OverlayPodCIDR     []string       `json:"overlayPodCIDR,omitempty"`
-	ServiceCIDR        []string       `json:"serviceCIDR,omitempty"`
-	HijackCIDR         []string       `json:"hijackCIDR,omitempty"`
-	TunePodRoutes      *bool          `json:"tunePodRoutes,omitempty"`
-	PodDefaultRouteNIC string         `json:"podDefaultRouteNic,omitempty"`
-	Mode               Mode           `json:"mode,omitempty"`
-	HostRuleTable      *int64         `json:"hostRuleTable,omitempty"`
-	HostRPFilter       *int32         `json:"hostRPFilter,omitempty" `
-	PodRPFilter        *int32         `json:"podRPFilter,omitempty" `
-	TxQueueLen         *int64         `json:"txQueueLen,omitempty"`
-	IPConflict         *bool          `json:"detectIPConflict,omitempty"`
-	DetectOptions      *DetectOptions `json:"detectOptions,omitempty"`
-	LogOptions         *LogOptions    `json:"logOptions,omitempty"`
+	VethLinkAddress    string      `json:"vethLinkAddress,omitempty"`
+	MacPrefix          string      `json:"podMACPrefix,omitempty"`
+	MultusNicPrefix    string      `json:"multusNicPrefix,omitempty"`
+	PodDefaultCniNic   string      `json:"podDefaultCniNic,omitempty"`
+	OverlayPodCIDR     []string    `json:"overlayPodCIDR,omitempty"`
+	ServiceCIDR        []string    `json:"serviceCIDR,omitempty"`
+	HijackCIDR         []string    `json:"hijackCIDR,omitempty"`
+	TunePodRoutes      *bool       `json:"tunePodRoutes,omitempty"`
+	PodDefaultRouteNIC string      `json:"podDefaultRouteNic,omitempty"`
+	Mode               Mode        `json:"mode,omitempty"`
+	HostRuleTable      *int64      `json:"hostRuleTable,omitempty"`
+	HostRPFilter       *int32      `json:"hostRPFilter,omitempty" `
+	PodRPFilter        *int32      `json:"podRPFilter,omitempty" `
+	TxQueueLen         *int64      `json:"txQueueLen,omitempty"`
+	LogOptions         *LogOptions `json:"logOptions,omitempty"`
 }
 
 // DetectOptions enable ip conflicting check for pod's ip
@@ -142,15 +138,6 @@ func ParseConfig(stdin []byte, coordinatorConfig *models.CoordinatorConfig) (*Co
 		return nil, err
 	}
 
-	if conf.IPConflict == nil && coordinatorConfig.DetectIPConflict {
-		conf.IPConflict = ptr.To(true)
-	}
-
-	conf.DetectOptions, err = ValidateDelectOptions(conf.DetectOptions)
-	if err != nil {
-		return nil, err
-	}
-
 	if conf.HostRuleTable == nil && coordinatorConfig.HostRuleTable > 0 {
 		conf.HostRuleTable = ptr.To(coordinatorConfig.HostRuleTable)
 	}
@@ -163,10 +150,6 @@ func ParseConfig(stdin []byte, coordinatorConfig *models.CoordinatorConfig) (*Co
 		conf.HostRuleTable = ptr.To(int64(500))
 	}
 
-	if conf.DetectGateway == nil {
-		conf.DetectGateway = ptr.To(coordinatorConfig.DetectGateway)
-	}
-
 	if conf.TunePodRoutes == nil {
 		conf.TunePodRoutes = coordinatorConfig.TunePodRoutes
 	}
@@ -270,37 +253,3 @@ func validateRPFilterConfig(rpfilter *int32, coordinatorConfig int64) (*int32, e
 	}
 	return rpfilter, nil
 }
-
-func ValidateDelectOptions(config *DetectOptions) (*DetectOptions, error) {
-	if config == nil {
-		return &DetectOptions{
-			Interval: "10ms",
-			TimeOut:  "100ms",
-			Retry:    3,
-		}, nil
-	}
-
-	if config.Retry == 0 {
-		config.Retry = 3
-	}
-
-	if config.Interval == "" {
-		config.Interval = "10ms"
-	}
-
-	if config.TimeOut == "" {
-		config.TimeOut = "500ms"
-	}
-
-	_, err := time.ParseDuration(config.Interval)
-	if err != nil {
-		return nil, fmt.Errorf("invalid detectOptions.interval %s: %v, input like: 1s or 1m", config.Interval, err)
-	}
-
-	_, err = time.ParseDuration(config.TimeOut)
-	if err != nil {
-		return nil, fmt.Errorf("invalid detectOptions.timeout %s: %v, input like: 1s or 1m", config.TimeOut, err)
-	}
-
-	return config, nil
-}
diff --git a/cmd/coordinator/cmd/command_add.go b/cmd/coordinator/cmd/command_add.go
@@ -4,28 +4,21 @@
 package cmd
 
 import (
-	"context"
-	"errors"
 	"fmt"
-	"net"
 	"time"
 
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
 	current "github.com/containernetworking/cni/pkg/types/100"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/vishvananda/netlink"
-	"go.uber.org/multierr"
 	"go.uber.org/zap"
 
 	"github.com/spidernet-io/spiderpool/api/v1/agent/client/daemonset"
 	"github.com/spidernet-io/spiderpool/api/v1/agent/models"
 	plugincmd "github.com/spidernet-io/spiderpool/cmd/spiderpool/cmd"
 	"github.com/spidernet-io/spiderpool/pkg/constant"
-	"github.com/spidernet-io/spiderpool/pkg/errgroup"
 	"github.com/spidernet-io/spiderpool/pkg/logutils"
-	"github.com/spidernet-io/spiderpool/pkg/networking/gwconnection"
-	"github.com/spidernet-io/spiderpool/pkg/networking/ipchecking"
 	"github.com/spidernet-io/spiderpool/pkg/networking/networking"
 	"github.com/spidernet-io/spiderpool/pkg/networking/sysctl"
 	"github.com/spidernet-io/spiderpool/pkg/openapi"
@@ -118,6 +111,7 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 	if err != nil {
 		return fmt.Errorf("failed to get current netns: %v", err)
 	}
+	defer c.hostNs.Close()
 	logger.Sugar().Debugf("Get current host netns: %v", c.hostNs.Path())
 
 	// checking if the nic is in up state
@@ -189,44 +183,6 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 
 	logger.Sugar().Infof("Get coordinator config: %+v", c)
 
-	errgConflict := errgroup.Group{}
-	// IP conflict detection must precede gateway detection, which avoids the
-	// possibility that gateway detection may update arp table entries first and cause
-	// communication problems when IP conflict detection fails
-	// see https://github.com/spidernet-io/spiderpool/issues/4475
-	var ipc *ipchecking.IPChecker
-	if conf.IPConflict != nil && *conf.IPConflict {
-		logger.Debug("Try to detect ip conflict")
-		ipc, err = ipchecking.NewIPChecker(conf.DetectOptions.Retry, conf.DetectOptions.Interval, conf.DetectOptions.TimeOut, c.hostNs, c.netns, logger)
-		if err != nil {
-			return fmt.Errorf("failed to run NewIPChecker: %w", err)
-		}
-		ipc.DoIPConflictChecking(prevResult.IPs, c.currentInterface, &errgConflict)
-	} else {
-		logger.Debug("disable detect ip conflict")
-	}
-
-	if err = errgConflict.Wait(); err != nil {
-		logger.Error("failed to detect ip conflict", zap.Error(err))
-		if errors.Is(err, constant.ErrIPConflict) {
-			logger.Info("ip conflict detected, clean up conflict IPs")
-			_, innerErr := client.Daemonset.DeleteIpamIps(daemonset.NewDeleteIpamIpsParams().WithContext(context.TODO()).WithIpamBatchDelArgs(
-				&models.IpamBatchDelArgs{
-					ContainerID:  &args.ContainerID,
-					NetNamespace: args.Netns,
-					PodName:      (*string)(&k8sArgs.K8S_POD_NAME),
-					PodNamespace: (*string)(&k8sArgs.K8S_POD_NAMESPACE),
-					PodUID:       (*string)(&k8sArgs.K8S_POD_UID),
-				},
-			))
-			if innerErr != nil {
-				logger.Sugar().Errorf("failed to clean up conflict IPs, error: %v", innerErr)
-				return multierr.Append(err, innerErr)
-			}
-		}
-		return err
-	}
-
 	// Fixed Mac addresses must come after IP conflict detection, otherwise the switch learns to communicate
 	// with the wrong Mac address when IP conflict detection fails
 	if len(conf.MacPrefix) != 0 {
@@ -237,65 +193,6 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 		logger.Info("Fix mac address successfully", zap.String("interface", args.IfName), zap.String("macAddress", hwAddr))
 	}
 
-	//  we do detect gateway connection lastly
-	// Finally, there is gateway detection, which updates the correct arp table entries
-	// once there are no IP address conflicts and fixed Mac addresses
-	errgGateway := errgroup.Group{}
-	if conf.DetectGateway != nil && *conf.DetectGateway {
-		logger.Debug("Try to detect gateway")
-
-		var gws []net.IP
-		err = c.netns.Do(func(netNS ns.NetNS) error {
-			gws, err = networking.GetDefaultGatewayByName(c.currentInterface, c.ipFamily)
-			if err != nil {
-				logger.Error("failed to GetDefaultGatewayByName", zap.Error(err))
-				return fmt.Errorf("failed to GetDefaultGatewayByName: %v", err)
-			}
-			logger.Debug("Get GetDefaultGatewayByName", zap.Any("Gws", gws))
-			p, err := gwconnection.New(conf.DetectOptions.Retry, conf.DetectOptions.Interval, conf.DetectOptions.TimeOut, c.currentInterface, logger)
-			if err != nil {
-				return fmt.Errorf("failed to init the gateway client: %v", err)
-			}
-			p.ParseAddrFromPreresult(prevResult.IPs)
-			for _, gw := range gws {
-				if gw.To4() != nil {
-					p.V4Gw = gw
-					errgGateway.Go(c.hostNs, c.netns, p.ArpingOverIface)
-				} else {
-					p.V6Gw = gw
-					errgGateway.Go(c.hostNs, c.netns, p.NDPingOverIface)
-				}
-			}
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-	} else {
-		logger.Debug("disable detect gateway")
-	}
-
-	if err = errgGateway.Wait(); err != nil {
-		logger.Error("failed to detect gateway reachable", zap.Error(err))
-		if errors.Is(err, constant.ErrGatewayUnreachable) {
-			logger.Info("gateway unreachable detected, clean up conflict IPs")
-			_, innerErr := client.Daemonset.DeleteIpamIps(daemonset.NewDeleteIpamIpsParams().WithContext(context.TODO()).WithIpamBatchDelArgs(
-				&models.IpamBatchDelArgs{
-					ContainerID:  &args.ContainerID,
-					NetNamespace: args.Netns,
-					PodName:      (*string)(&k8sArgs.K8S_POD_NAME),
-					PodNamespace: (*string)(&k8sArgs.K8S_POD_NAMESPACE),
-					PodUID:       (*string)(&k8sArgs.K8S_POD_UID),
-				},
-			))
-			if innerErr != nil {
-				logger.Sugar().Errorf("failed to clean up conflict IPs, error: %v", innerErr)
-				return multierr.Append(err, innerErr)
-			}
-		}
-		return err
-	}
-
 	// set txqueuelen
 	if conf.TxQueueLen != nil && *conf.TxQueueLen > 0 {
 		if err = networking.LinkSetTxqueueLen(args.IfName, int(*conf.TxQueueLen)); err != nil {

diff --git a/cmd/coordinator/cmd/utils.go b/cmd/coordinator/cmd/utils.go
@@ -703,3 +703,7 @@ OUTER2:
 
 	return finalNodeIpList, nil
 }
+
+func AnnouncePodIPs() error {
+	return nil
+}