diff --git a/multus/multus.go b/multus/multus.go index a9ceb1051..6703be617 100644 --- a/multus/multus.go +++ b/multus/multus.go @@ -316,16 +316,20 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn var result, tmpResult cnitypes.Result var netStatus []*types.NetworkStatus - var rt *libcni.RuntimeConf - lastIdx := 0 cniArgs := os.Getenv("CNI_ARGS") for idx, delegate := range n.Delegates { - lastIdx = idx ifName := getIfname(delegate, args.IfName, idx) - rt, _ = types.LoadCNIRuntimeConf(args, k8sArgs, ifName, n.RuntimeConfig) + rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, n.RuntimeConfig) tmpResult, err = delegateAdd(exec, ifName, delegate, rt, n.BinDir, cniArgs) if err != nil { - break + // If the add failed, tear down all networks we already added + netName := delegate.Conf.Name + if netName == "" { + netName = delegate.ConfList.Name + } + // Ignore errors; DEL must be idempotent anyway + _ = delPlugins(exec, args.IfName, n.Delegates, idx, rt, n.BinDir) + return nil, logging.Errorf("Multus: Err adding pod to network %q: %v", netName, err) } // Master plugin result is always used if present @@ -338,7 +342,7 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn if !types.CheckSystemNamespaces(kc.Podnamespace, n.SystemNamespaces) { delegateNetStatus, err := types.LoadNetworkStatus(tmpResult, delegate.Conf.Name, delegate.MasterPlugin) if err != nil { - return nil, logging.Errorf("Multus: Err in setting networks status: %v", err) + return nil, logging.Errorf("Multus: Err in setting network status: %v", err) } netStatus = append(netStatus, delegateNetStatus) @@ -346,12 +350,6 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn } } - if err != nil { - // Ignore errors; DEL must be idempotent anyway - _ = delPlugins(exec, args.IfName, n.Delegates, lastIdx, rt, n.BinDir) - return nil, logging.Errorf("Multus: Err in tearing down failed plugins: %v", err) - } - //set the network status annotation in apiserver, only in case Multus as kubeconfig if n.Kubeconfig != "" && kc != nil { if !types.CheckSystemNamespaces(kc.Podnamespace, n.SystemNamespaces) { @@ -436,7 +434,7 @@ func cmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) err } } - rt, _ := types.LoadCNIRuntimeConf(args, k8sArgs, "", in.RuntimeConfig) + rt := types.CreateCNIRuntimeConf(args, k8sArgs, "", in.RuntimeConfig) return delPlugins(exec, args.IfName, in.Delegates, len(in.Delegates)-1, rt, in.BinDir) } diff --git a/multus/multus_test.go b/multus/multus_test.go index 2547247c5..d419951ae 100644 --- a/multus/multus_test.go +++ b/multus/multus_test.go @@ -245,6 +245,62 @@ var _ = Describe("multus operations", func() { }) + It("executes delegates and cleans up on failure", func() { + expectedConf1 := `{ + "name": "weave1", + "cniVersion": "0.2.0", + "type": "weave-net" +}` + expectedConf2 := `{ + "name": "other1", + "cniVersion": "0.2.0", + "type": "other-plugin" +}` + args := &skel.CmdArgs{ + ContainerID: "123456789", + Netns: testNS.Path(), + IfName: "eth0", + StdinData: []byte(fmt.Sprintf(`{ + "name": "node-cni-network", + "type": "multus", + "defaultnetworkfile": "/tmp/foo.multus.conf", + "defaultnetworkwaitseconds": 3, + "delegates": [%s,%s] +}`, expectedConf1, expectedConf2)), + } + + // Touch the default network file. + configPath := "/tmp/foo.multus.conf" + os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) + + fExec := &fakeExec{} + expectedResult1 := &types020.Result{ + CNIVersion: "0.2.0", + IP4: &types020.IPConfig{ + IP: *testhelpers.EnsureCIDR("1.1.1.2/24"), + }, + } + fExec.addPlugin(nil, "eth0", expectedConf1, expectedResult1, nil) + + // This plugin invocation should fail + err := fmt.Errorf("expected plugin failure") + fExec.addPlugin(nil, "net1", expectedConf2, nil, err) + + os.Setenv("CNI_COMMAND", "ADD") + os.Setenv("CNI_IFNAME", "eth0") + _, err = cmdAdd(args, fExec, nil) + Expect(fExec.addIndex).To(Equal(2)) + Expect(fExec.delIndex).To(Equal(2)) + Expect(err).To(MatchError("Multus: Err adding pod to network \"other1\": Multus: error in invoke Delegate add - \"other-plugin\": expected plugin failure")) + + // Cleanup default network file. + if _, errStat := os.Stat(configPath); errStat == nil { + errRemove := os.Remove(configPath) + Expect(errRemove).NotTo(HaveOccurred()) + } + + }) + It("executes delegates with interface name and MAC and IP addr", func() { podNet := `[{"name":"net1", "interface": "test1", diff --git a/types/conf.go b/types/conf.go index aa37f41ee..115784575 100644 --- a/types/conf.go +++ b/types/conf.go @@ -94,7 +94,7 @@ func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID st return delegateConf, nil } -func LoadCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig) (*libcni.RuntimeConf, error) { +func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig) *libcni.RuntimeConf { logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v", args, k8sArgs, ifName, rc) // In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go#buildCNIRuntimeConf @@ -117,7 +117,7 @@ func LoadCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc "portMappings": rc.PortMaps, } } - return rt, nil + return rt } func LoadNetworkStatus(r types.Result, netName string, defaultNet bool) (*NetworkStatus, error) {