Detect IP conflicts before gateway detection to fix gateway detection…

… could potentially update the arp table causing communication fail Signed-off-by: Cyclinder Kuo <[email protected]>
spidernet-io · Dec 31, 2024 · 6ec9f62 · 6ec9f62
1 parent 415359e
commit 6ec9f62
Showing 1 changed file with 53 additions and 39 deletions.
diff --git a/cmd/coordinator/cmd/command_add.go b/cmd/coordinator/cmd/command_add.go
@@ -190,7 +190,55 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 	logger.Sugar().Infof("Get coordinator config: %+v", c)
 
 	errg := errgroup.Group{}
-	//  we do detect gateway connection firstly
+	//
+	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, &errg)
+	} else {
+		logger.Debug("disable detect ip conflict")
+	}
+
+	cleanIPsFn := func() error {
+		_, 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 nil
+	}
+
+	if err = errg.Wait(); err != nil {
+		logger.Error("failed to detect ip conflict", zap.Error(err))
+		if errors.Is(err, constant.ErrIPConflict) || errors.Is(err, constant.ErrGatewayUnreachable) {
+			if err = cleanIPsFn(); err != nil {
+				return err
+			}
+		}
+	}
+
+	// overwrite mac address
+	if len(conf.MacPrefix) != 0 {
+		hwAddr, err := networking.OverwriteHwAddress(logger, c.netns, conf.MacPrefix, args.IfName)
+		if err != nil {
+			return fmt.Errorf("failed to update hardware address for interface %s, maybe hardware_prefix(%s) is invalid: %v", args.IfName, conf.MacPrefix, err)
+		}
+		logger.Info("Fix mac address successfully", zap.String("interface", args.IfName), zap.String("macAddress", hwAddr))
+	}
+
+	//  we do detect gateway connection lastly
 	if conf.DetectGateway != nil && *conf.DetectGateway {
 		logger.Debug("Try to detect gateway")
 
@@ -202,7 +250,6 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 				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)
@@ -226,46 +273,13 @@ func CmdAdd(args *skel.CmdArgs) (err error) {
 		logger.Debug("disable detect gateway")
 	}
 
-	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, &errg)
-	} else {
-		logger.Debug("disable detect ip conflict")
-	}
-
 	if err = errg.Wait(); err != nil {
-		logger.Error("failed to detect gateway and ip checking", zap.Error(err))
-		if errors.Is(err, constant.ErrIPConflict) || errors.Is(err, constant.ErrGatewayUnreachable) {
-			_, 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)
+		logger.Error("failed to detect gateway reachable", zap.Error(err))
+		if errors.Is(err, constant.ErrGatewayUnreachable) {
+			if err = cleanIPsFn(); err != nil {
+				return err
 			}
 		}
-
-		return err
-	}
-
-	// overwrite mac address
-	if len(conf.MacPrefix) != 0 {
-		hwAddr, err := networking.OverwriteHwAddress(logger, c.netns, conf.MacPrefix, args.IfName)
-		if err != nil {
-			return fmt.Errorf("failed to update hardware address for interface %s, maybe hardware_prefix(%s) is invalid: %v", args.IfName, conf.MacPrefix, err)
-		}
-		logger.Info("Override hardware address successfully", zap.String("interface", args.IfName), zap.String("hardware address", hwAddr))
 	}
 
 	// set txqueuelen