Merge pull request #106 from sunya-ch/v1.0.4

Make sure execPlugin execute regardless of ipam return

Author: sunya-ch

cni/plugins/main/multi-nic/multi-nic.go

@@ -181,7 +181,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	n, deviceType, err := loadConf(args)
-	if err != nil {
+	if err != nil && n == nil {
 		utils.Logger.Debug(fmt.Sprintf("fail to load conf: %v", err))
 		return nil
 	}
@@ -192,7 +192,6 @@ func cmdDel(args *skel.CmdArgs) error {
 		err = ipam.ExecDel(n.IPAM.Type, injectedStdIn)
 		if err != nil {
 			utils.Logger.Debug(fmt.Sprintf("Failed ipam.ExecDel %s: %v", err, string(injectedStdIn)))
-			return nil
 		}
 	}
 
@@ -201,14 +200,14 @@ func cmdDel(args *skel.CmdArgs) error {
 	if n.NetConf.RawPrevResult != nil {
 		if err = version.ParsePrevResult(&n.NetConf); err != nil {
 			utils.Logger.Debug(fmt.Sprintf("could not parse prevResult: %v", err))
-			return nil
-		}
-		result, err = current.NewResultFromResult(n.NetConf.PrevResult)
-		if err != nil {
-			utils.Logger.Debug(fmt.Sprintf("could not convert result to current version: %v", err))
-			return nil
+		} else {
+			result, err = current.NewResultFromResult(n.NetConf.PrevResult)
+			if err != nil {
+				utils.Logger.Debug(fmt.Sprintf("could not convert result to current version: %v", err))
+			}
 		}
-	} else {
+	}
+	if result == nil {
 		result = &current.Result{CNIVersion: current.ImplementedSpecVersion}
 	}
 
@@ -224,7 +223,6 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 	if err != nil {
 		utils.Logger.Debug(fmt.Sprintf("Fail loading %v: %v", string(args.StdinData), err))
-		return nil
 	}
 	if len(confBytesArray) == 0 {
 		utils.Logger.Debug(fmt.Sprintf("zero config on cmdDel: %v (%d)", string(args.StdinData), len(n.Masters)))
@@ -237,7 +235,6 @@ func cmdDel(args *skel.CmdArgs) error {
 		_, err := execPlugin(deviceType, command, confBytes, args, ifName, false)
 		if err != nil {
 			utils.Logger.Debug(fmt.Sprintf("Fail execPlugin %v: %v", string(confBytes), err))
-			return nil
 		}
 	}
 	return nil
@@ -249,7 +246,7 @@ func cmdCheck(args *skel.CmdArgs) error {
 	}
 
 	n, deviceType, err := loadConf(args)
-	if err != nil {
+	if err != nil && n == nil {
 		utils.Logger.Debug(fmt.Sprintf("fail to load conf: %v", err))
 		return nil
 	}
@@ -259,14 +256,14 @@ func cmdCheck(args *skel.CmdArgs) error {
 	if n.NetConf.RawPrevResult != nil {
 		if err = version.ParsePrevResult(&n.NetConf); err != nil {
 			utils.Logger.Debug(fmt.Sprintf("could not parse prevResult: %v", err))
-			return nil
-		}
-		result, err = current.NewResultFromResult(n.NetConf.PrevResult)
-		if err != nil {
-			utils.Logger.Debug(fmt.Sprintf("could not convert result to current version: %v", err))
-			return nil
+		} else {
+			result, err = current.NewResultFromResult(n.NetConf.PrevResult)
+			if err != nil {
+				utils.Logger.Debug(fmt.Sprintf("could not convert result to current version: %v", err))
+			}
 		}
-	} else {
+	}
+	if result == nil {
 		result = &current.Result{CNIVersion: current.ImplementedSpecVersion}
 	}
 
@@ -282,7 +279,6 @@ func cmdCheck(args *skel.CmdArgs) error {
 	}
 	if err != nil {
 		utils.Logger.Debug(fmt.Sprintf("Fail loading %v: %v", string(args.StdinData), err))
-		return nil
 	}
 	if len(confBytesArray) == 0 {
 		utils.Logger.Debug(fmt.Sprintf("zero config on cmdCheck: %v (%d)", string(args.StdinData), len(n.Masters)))
@@ -295,7 +291,6 @@ func cmdCheck(args *skel.CmdArgs) error {
 		_, err := execPlugin(deviceType, command, confBytes, args, ifName, false)
 		if err != nil {
 			utils.Logger.Debug(fmt.Sprintf("Fail execPlugin %v: %v", string(confBytes), err))
-			return nil
 		}
 	}
 	return nil

cni/plugins/main/multi-nic/multi-nic_test.go

@@ -125,9 +125,8 @@ func buildOneConfig(cniVersion string, orig *NetConf, prevResult types.Result) (
 
 }
 
-func multinicAddCheckDelTest(conf, masterName string, originalNS, targetNS ns.NetNS) {
-	log.Printf("multinicAddCheckDelTest")
-	log.Printf("%s", conf)
+func multinicAddTest(conf, masterName string, originalNS, targetNS ns.NetNS) types.Result {
+	log.Printf("Add %s", conf)
 	// Unmarshal to pull out CNI spec version
 	rawConfig := make(map[string]interface{})
 	err := json.Unmarshal([]byte(conf), &rawConfig)
@@ -176,6 +175,23 @@ func multinicAddCheckDelTest(conf, masterName string, originalNS, targetNS ns.Ne
 		return nil
 	})
 	Expect(err).NotTo(HaveOccurred())
+	return result
+}
+
+func multinicCheckDelTest(conf, masterName string, originalNS, targetNS ns.NetNS, result types.Result) {
+	log.Printf("CheckDel %s", conf)
+	// Unmarshal to pull out CNI spec version
+	rawConfig := make(map[string]interface{})
+	err := json.Unmarshal([]byte(conf), &rawConfig)
+	Expect(err).NotTo(HaveOccurred())
+	cniVersion := rawConfig["cniVersion"].(string)
+
+	args := &skel.CmdArgs{
+		ContainerID: "dummy",
+		Netns:       targetNS.Path(),
+		IfName:      "net1",
+		StdinData:   []byte(conf),
+	}
 
 	n := &NetConf{}
 	err = json.Unmarshal([]byte(conf), &n)
@@ -228,6 +244,19 @@ func multinicAddCheckDelTest(conf, masterName string, originalNS, targetNS ns.Ne
 	Expect(err).NotTo(HaveOccurred())
 }
 
+func multinicAddCheckDelTest(conf, masterName string, originalNS, targetNS ns.NetNS) {
+	log.Printf("multinicAddCheckDelTest")
+	result := multinicAddTest(conf, masterName, originalNS, targetNS)
+	multinicCheckDelTest(conf, masterName, originalNS, targetNS, result)
+}
+
+func multinicDelWithoutDaemonTest(conf, masterName string, originalNS, targetNS ns.NetNS) {
+	log.Printf("multinicDelWithoutDaemonTest")
+	result := multinicAddTest(conf, masterName, originalNS, targetNS)
+	confWithoutDaemon := strings.ReplaceAll(conf, BRIDGE_HOST_IP, "")
+	multinicCheckDelTest(confWithoutDaemon, masterName, originalNS, targetNS, result)
+}
+
 type tester interface {
 	// verifyResult minimally verifies the Result and returns the interface's MAC address
 	verifyResult(result types.Result, name string) string
@@ -405,6 +434,7 @@ var _ = Describe("Operations", func() {
 			`, BRIDGE_HOST_IP, daemonPort)
 			conf := getConfig(ver, multiNICIPAM, masterNets)
 			multinicAddCheckDelTest(conf, "", originalNS, targetNS)
+			multinicDelWithoutDaemonTest(conf, "", originalNS, targetNS)
 		})
 		It(fmt.Sprintf("[%s] configures and deconfigures link with ADD/DEL (single-nic IPAM)", ver), func() {
 			conf := getConfig(ver, singleNICIPAM, masterNets)