From 6dd45f38f94b379f9eb05e1c4f0e09f6944e67ed Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Mon, 21 Feb 2022 23:55:33 +0900 Subject: [PATCH 1/6] Replace setenv with runtimeConfig set (#785) setenv refers environment variables, which is unique in process, not unique to go routine. Hence it may causes some issue in multi threaded case, hence it is replaced with libcni's runtimeConfig value set to set these variables at libcni side, after process fork. --- pkg/multus/multus.go | 48 +- pkg/multus/multus_test.go | 1019 ++++--------------------------------- pkg/types/conf.go | 31 +- pkg/types/conf_test.go | 2 - 4 files changed, 133 insertions(+), 967 deletions(-) diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index 9b6276211..acd68e35e 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -206,7 +206,7 @@ func confCheck(rt *libcni.RuntimeConf, rawNetconf []byte, multusNetconf *types.N } func confDel(rt *libcni.RuntimeConf, rawNetconf []byte, multusNetconf *types.NetConf, exec invoke.Exec) error { - logging.Debugf("conflistDel: %v, %s", rt, string(rawNetconf)) + logging.Debugf("confDel: %v, %s", rt, string(rawNetconf)) // In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go binDirs := filepath.SplitList(os.Getenv("CNI_PATH")) binDirs = append([]string{multusNetconf.BinDir}, binDirs...) @@ -285,23 +285,15 @@ func conflistDel(rt *libcni.RuntimeConf, rawnetconflist []byte, multusNetconf *t return err } -func delegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, ifName string, delegate *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf, cniArgs string) (cnitypes.Result, error) { - logging.Debugf("delegateAdd: %v, %s, %v, %v", exec, ifName, delegate, rt) - if os.Setenv("CNI_IFNAME", ifName) != nil { - return nil, logging.Errorf("delegateAdd: error setting environment variable CNI_IFNAME") - } +func delegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, delegate *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) (cnitypes.Result, error) { + logging.Debugf("delegateAdd: %v, %v, %v", exec, delegate, rt) - if err := validateIfName(os.Getenv("CNI_NETNS"), ifName); err != nil { - return nil, logging.Errorf("delegateAdd: cannot set %q interface name to %q: %v", delegate.Conf.Type, ifName, err) + if err := validateIfName(rt.NetNS, rt.IfName); err != nil { + return nil, logging.Errorf("delegateAdd: cannot set %q interface name to %q: %v", delegate.Conf.Type, rt.IfName, err) } // Deprecated in ver 3.5. if delegate.MacRequest != "" || delegate.IPRequest != nil { - if cniArgs != "" { - cniArgs = fmt.Sprintf("%s;IgnoreUnknown=true", cniArgs) - } else { - cniArgs = "IgnoreUnknown=true" - } if delegate.MacRequest != "" { // validate Mac address _, err := net.ParseMAC(delegate.MacRequest) @@ -309,8 +301,7 @@ func delegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, ifNa return nil, logging.Errorf("delegateAdd: failed to parse mac address %q", delegate.MacRequest) } - cniArgs = fmt.Sprintf("%s;MAC=%s", cniArgs, delegate.MacRequest) - logging.Debugf("delegateAdd: set MAC address %q to %q", delegate.MacRequest, ifName) + logging.Debugf("delegateAdd: set MAC address %q to %q", delegate.MacRequest, rt.IfName) rt.Args = append(rt.Args, [2]string{"MAC", delegate.MacRequest}) } @@ -328,8 +319,7 @@ func delegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, ifNa } ips := strings.Join(delegate.IPRequest, ",") - cniArgs = fmt.Sprintf("%s;IP=%s", cniArgs, ips) - logging.Debugf("delegateAdd: set IP address %q to %q", ips, ifName) + logging.Debugf("delegateAdd: set IP address %q to %q", ips, rt.IfName) rt.Args = append(rt.Args, [2]string{"IP", ips}) } } @@ -390,11 +380,8 @@ func delegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, ifNa return result, nil } -func delegateCheck(exec invoke.Exec, ifName string, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { - logging.Debugf("delegateCheck: %v, %s, %v, %v", exec, ifName, delegateConf, rt) - if os.Setenv("CNI_IFNAME", ifName) != nil { - return logging.Errorf("delegateCheck: error setting environment variable CNI_IFNAME") - } +func delegateCheck(exec invoke.Exec, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { + logging.Debugf("delegateCheck: %v, %v, %v", exec, delegateConf, rt) if logging.GetLoggingLevel() >= logging.VerboseLevel { var cniConfName string @@ -422,11 +409,8 @@ func delegateCheck(exec invoke.Exec, ifName string, delegateConf *types.Delegate return err } -func delegateDel(exec invoke.Exec, pod *v1.Pod, ifName string, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { - logging.Debugf("delegateDel: %v, %v, %s, %v, %v", exec, pod, ifName, delegateConf, rt) - if os.Setenv("CNI_IFNAME", ifName) != nil { - return logging.Errorf("delegateDel: error setting environment variable CNI_IFNAME") - } +func delegateDel(exec invoke.Exec, pod *v1.Pod, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { + logging.Debugf("delegateDel: %v, %v, %v, %v", exec, pod, delegateConf, rt) if logging.GetLoggingLevel() >= logging.VerboseLevel { var confName string @@ -463,16 +447,13 @@ func delegateDel(exec invoke.Exec, pod *v1.Pod, ifName string, delegateConf *typ // with each of the delegates' configuration func delPlugins(exec invoke.Exec, pod *v1.Pod, args *skel.CmdArgs, k8sArgs *types.K8sArgs, delegates []*types.DelegateNetConf, lastIdx int, netRt *types.RuntimeConfig, multusNetconf *types.NetConf) error { logging.Debugf("delPlugins: %v, %v, %v, %v, %v, %d, %v", exec, pod, args, k8sArgs, delegates, lastIdx, netRt) - if os.Setenv("CNI_COMMAND", "DEL") != nil { - return logging.Errorf("delPlugins: error setting environment variable CNI_COMMAND to a value of DEL") - } var errorstrings []string for idx := lastIdx; idx >= 0; idx-- { ifName := getIfname(delegates[idx], args.IfName, idx) rt, cniDeviceInfoPath := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, netRt, delegates[idx]) // Attempt to delete all but do not error out, instead, collect all errors. - if err := delegateDel(exec, pod, ifName, delegates[idx], rt, multusNetconf); err != nil { + if err := delegateDel(exec, pod, delegates[idx], rt, multusNetconf); err != nil { errorstrings = append(errorstrings, err.Error()) } if cniDeviceInfoPath != "" { @@ -624,7 +605,6 @@ func CmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c var result, tmpResult cnitypes.Result var netStatus []nettypes.NetworkStatus - cniArgs := os.Getenv("CNI_ARGS") for idx, delegate := range n.Delegates { ifName := getIfname(delegate, args.IfName, idx) rt, cniDeviceInfoPath := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, n.RuntimeConfig, delegate) @@ -638,7 +618,7 @@ func CmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c } netName := "" - tmpResult, err = delegateAdd(exec, kubeClient, pod, ifName, delegate, rt, n, cniArgs) + tmpResult, err = delegateAdd(exec, kubeClient, pod, delegate, rt, n) if err != nil { // If the add failed, tear down all networks we already added netName = delegate.Conf.Name @@ -777,7 +757,7 @@ func CmdCheck(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) ifName := getIfname(delegate, args.IfName, idx) rt, _ := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, in.RuntimeConfig, delegate) - err = delegateCheck(exec, ifName, delegate, rt, in) + err = delegateCheck(exec, delegate, rt, in) if err != nil { return err } diff --git a/pkg/multus/multus_test.go b/pkg/multus/multus_test.go index e20cf1d35..2e4880a2a 100644 --- a/pkg/multus/multus_test.go +++ b/pkg/multus/multus_test.go @@ -68,30 +68,36 @@ type fakeExec struct { delIndex int chkIndex int expectedDelSkip int - plugins []*fakePlugin + plugins map[string]*fakePlugin +} + +func newFakeExec() *fakeExec { + return &fakeExec{ + plugins: map[string]*fakePlugin{}, + } } func (f *fakeExec) addPlugin(expectedEnv []string, expectedIfname, expectedConf string, result *current.Result, err error) { - f.plugins = append(f.plugins, &fakePlugin{ + f.plugins[expectedIfname] = &fakePlugin{ expectedEnv: expectedEnv, expectedConf: expectedConf, expectedIfname: expectedIfname, result: result, err: err, - }) + } if err != nil && err.Error() == "missing network name" { f.expectedDelSkip++ } } func (f *fakeExec) addPlugin020(expectedEnv []string, expectedIfname, expectedConf string, result *types020.Result, err error) { - f.plugins = append(f.plugins, &fakePlugin{ + f.plugins[expectedIfname] = &fakePlugin{ expectedEnv: expectedEnv, expectedConf: expectedConf, expectedIfname: expectedIfname, result: result, err: err, - }) + } if err != nil && err.Error() == "missing network name" { f.expectedDelSkip++ } @@ -125,8 +131,22 @@ func gatherCNIEnv(environ []string) []string { return filtered } +func ParseEnvironment(environ []string) map[string]string { + m := map[string]string{} + + for _, e := range environ { + if e != "" { + parts := strings.SplitN(e, "=", 2) + ExpectWithOffset(2, len(parts)).To(Equal(2)) + m[parts[0]] = parts[1] + } + } + return m +} + func (f *fakeExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData []byte, environ []string) ([]byte, error) { - cmd := os.Getenv("CNI_COMMAND") + envMap := ParseEnvironment(environ) + cmd := envMap["CNI_COMMAND"] var index int var err error var resultJSON []byte @@ -148,7 +168,7 @@ func (f *fakeExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData // Should never be reached Expect(false).To(BeTrue()) } - plugin := f.plugins[index] + plugin := f.plugins[envMap["CNI_IFNAME"]] //GinkgoT().Logf("[%s %d] exec plugin %q found %+v\n", cmd, index, pluginPath, plugin) fmt.Printf("[%s %d] exec plugin %q found %+v\n", cmd, index, pluginPath, plugin) @@ -174,7 +194,7 @@ func (f *fakeExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData Expect(writer).To(MatchJSON(plugin.expectedConf)) } if plugin.expectedIfname != "" { - Expect(os.Getenv("CNI_IFNAME")).To(Equal(plugin.expectedIfname)) + Expect(envMap["CNI_IFNAME"]).To(Equal(plugin.expectedIfname)) } if len(plugin.expectedEnv) > 0 { @@ -225,6 +245,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { var testNS ns.NetNS var tmpDir string resultCNIVersion := "0.4.0" + configPath := "/tmp/foo.multus.conf" BeforeEach(func() { // Create a new NetNS so we don't modify the host @@ -236,9 +257,18 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { tmpDir, err = ioutil.TempDir("", "multus_tmp") Expect(err).NotTo(HaveOccurred()) + + // Touch the default network file. + os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) }) AfterEach(func() { + // Cleanup default network file. + if _, errStat := os.Stat(configPath); errStat == nil { + errRemove := os.Remove(configPath) + Expect(errRemove).NotTo(HaveOccurred()) + } + Expect(testNS.Close()).To(Succeed()) os.Unsetenv("CNI_PATH") os.Unsetenv("CNI_ARGS") @@ -270,11 +300,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { logging.SetLogLevel("verbose") - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -301,8 +327,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }` fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -310,17 +334,9 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } }) It("executes delegates given faulty namespace", func() { @@ -346,11 +362,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { } // Netns is given garbage value - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -377,26 +389,8 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }` fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } + _, err := CmdAdd(args, fExec, nil) + Expect(err).To(MatchError("[//:weave1]: error adding container to network \"weave1\": delegateAdd: cannot set \"weave-net\" interface name to \"eth0\": validateIfName: no net namespace fsdadfad found: failed to Statfs \"fsdadfad\": no such file or directory")) }) It("returns the previous result using CmdCheck", func() { @@ -423,11 +417,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { logging.SetLogLevel("verbose") - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -454,8 +444,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }` fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -467,644 +455,9 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { err = CmdCheck(args, fExec, nil) Expect(err).To(HaveOccurred()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("executes delegates given faulty namespace", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: "fsdadfad", - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - // Netns is given garbage value - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("returns the previous result using CmdCheck", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: testNS.Path(), - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - - logging.SetLogLevel("verbose") - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - // Check is not supported until v 0.4.0 - err = CmdCheck(args, fExec, nil) - Expect(err).To(HaveOccurred()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("executes delegates given faulty namespace", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: "fsdadfad", - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - // Netns is given garbage value - fmt.Println("args.Netns: ", args.Netns) - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("returns the previous result using CmdCheck", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: testNS.Path(), - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - - logging.SetLogLevel("verbose") - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - // Check is not supported until v 0.4.0 - err = CmdCheck(args, fExec, nil) - Expect(err).To(HaveOccurred()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("executes delegates given faulty namespace", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: "fsdadfad", - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - // Netns is given garbage value - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("returns the previous result using CmdCheck", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: testNS.Path(), - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - - logging.SetLogLevel("verbose") - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - // Check is not supported until v 0.4.0 - err = CmdCheck(args, fExec, nil) - Expect(err).To(HaveOccurred()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("executes delegates given faulty namespace", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: "fsdadfad", - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - // Netns is given garbage value - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) - - It("returns the previous result using CmdCheck", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: testNS.Path(), - IfName: "eth0", - StdinData: []byte(`{ - "name": "node-cni-network", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - },{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }] - }`), - } - - // 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"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }` - fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil) - - expectedResult2 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.5/24"), - }, - } - expectedConf2 := `{ - "name": "other1", - "cniVersion": "0.2.0", - "type": "other-plugin" - }` - fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - r := result.(*types020.Result) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - - // Check is not supported until v 0.4.0 - err = CmdCheck(args, fExec, nil) - Expect(err).To(HaveOccurred()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } }) It("fails to load NetConf with bad json in CmdAdd/Del", func() { @@ -1130,11 +483,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { } // Missing close bracket in StdinData - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1161,8 +510,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }` fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") _, err := CmdAdd(args, fExec, nil) Expect(err).To(HaveOccurred()) @@ -1214,11 +561,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`, expectedConf1, expectedConf2)), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1231,19 +574,11 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { err := fmt.Errorf("expected plugin failure") fExec.addPlugin020(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("[//:other1]: error adding container to network \"other1\": 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 and cleans up on failure with missing name field", func() { @@ -1272,11 +607,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`, expectedConf1, expectedConf2)), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1289,19 +620,10 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { err := fmt.Errorf("expected plugin failure") fExec.addPlugin020(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(1)) Expect(fExec.delIndex).To(Equal(2)) Expect(err).To(HaveOccurred()) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) It("executes delegates with runtimeConfigs", func() { @@ -1347,7 +669,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: resultCNIVersion, IPs: []*current.IPConfig{{ @@ -1410,8 +732,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1454,9 +774,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - _, err = CmdAdd(args, &fakeExec{}, clientInfo) + _, err = CmdAdd(args, newFakeExec(), clientInfo) Expect(err.Error()).To(ContainSubstring("expected pod UID \"foobar\" but got %q from Kube API", fakePod.UID)) }) @@ -1484,7 +802,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: resultCNIVersion, IPs: []*current.IPConfig{{ @@ -1520,8 +838,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1549,7 +865,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: resultCNIVersion, IPs: []*current.IPConfig{{ @@ -1574,8 +890,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { context.TODO(), fakePod, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1603,7 +917,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: resultCNIVersion, IPs: []*current.IPConfig{{ @@ -1628,8 +942,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { context.TODO(), fakePod, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1672,7 +984,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1714,8 +1026,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net3", net3)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1755,7 +1065,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1784,8 +1094,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1793,9 +1101,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - // delete pod to emulate no pod info clientInfo.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) nilPod, err := clientInfo.Client.CoreV1().Pods(fakePod.ObjectMeta.Namespace).Get( @@ -1832,7 +1137,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1858,8 +1163,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1867,8 +1170,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") // set fKubeClient to nil to emulate no pod info fKubeClient.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) err = CmdDel(args, fExec, fKubeClient) @@ -1900,7 +1201,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -1926,8 +1227,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -1935,8 +1234,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") // set fKubeClient to nil to emulate no pod info fKubeClient.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) err = CmdDel(args, fExec, fKubeClient) @@ -1970,7 +1267,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedConf1 := `{ "capabilities": {"portMappings": true}, "name": "mynet-confList", @@ -1983,8 +1280,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { } }` fExec.addPlugin020(nil, "eth0", expectedConf1, nil, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") _, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) }) @@ -2017,7 +1312,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() fExec.addPlugin020(nil, "eth0", net1, expectedResult1, nil) fKubeClient := NewFakeClientInfo() @@ -2025,16 +1320,12 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { _, err := fKubeClient.AddNetAttachDef(testhelpers.NewFakeNetAttachDef("kube-system", "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) r := result.(*types020.Result) Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -2069,7 +1360,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`, tmpCNIDir)), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -2094,8 +1385,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { _, err = fKubeClient.AddNetAttachDef( testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -2109,8 +1398,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { Expect(err).NotTo(HaveOccurred()) By("Delete and check net count is not incremented") - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -2145,7 +1432,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`, tmpCNIDir)), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -2170,8 +1457,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { _, err = fKubeClient.AddNetAttachDef( testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -2188,8 +1473,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { Expect(err).NotTo(HaveOccurred()) By("Delete and check pod/net count is incremented") - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -2217,11 +1500,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := &types020.Result{ CNIVersion: "0.2.0", IP4: &types020.IPConfig{ @@ -2248,9 +1527,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }` fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - fakeMultusNetConf := types.NetConf{ BinDir: "/opt/cni/bin", } @@ -2290,11 +1566,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { }`), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedConf1 := `{ "capabilities": {"portMappings": true}, "name": "mynet-confList", @@ -2307,8 +1579,6 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { } }` fExec.addPlugin020(nil, "eth0", expectedConf1, nil, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") _, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) err = CmdDel(args, fExec, nil) @@ -2320,6 +1590,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { var testNS ns.NetNS var tmpDir string resultCNIVersion := "0.4.0" + configPath := "/tmp/foo.multus.conf" BeforeEach(func() { // Create a new NetNS so we don't modify the host @@ -2331,9 +1602,19 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { tmpDir, err = ioutil.TempDir("", "multus_tmp") Expect(err).NotTo(HaveOccurred()) + + // Touch the default network file. + os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) + }) AfterEach(func() { + // Cleanup default network file. + if _, errStat := os.Stat(configPath); errStat == nil { + errRemove := os.Remove(configPath) + Expect(errRemove).NotTo(HaveOccurred()) + } + Expect(testNS.Close()).To(Succeed()) os.Unsetenv("CNI_PATH") os.Unsetenv("CNI_ARGS") @@ -2365,11 +1646,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { logging.SetLogLevel("verbose") - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2398,8 +1675,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }` fExec.addPlugin(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) @@ -2407,21 +1682,12 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "CHECK") err = CmdCheck(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } }) It("executes delegates given faulty namespace", func() { @@ -2447,11 +1713,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { } // Netns is given garbage value - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2480,25 +1742,8 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }` fExec.addPlugin(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - result, err := CmdAdd(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) - // plugin 1 is the masterplugin - Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") - err = CmdDel(args, fExec, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } + _, err := CmdAdd(args, fExec, nil) + Expect(err).To(MatchError("[//:weave1]: error adding container to network \"weave1\": delegateAdd: cannot set \"weave-net\" interface name to \"eth0\": validateIfName: no net namespace fsdadfad found: failed to Statfs \"fsdadfad\": no such file or directory")) }) It("returns the previous result using CmdCheck", func() { @@ -2533,11 +1778,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { logging.SetLogLevel("verbose") - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2566,8 +1807,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }` fExec.addPlugin(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) @@ -2575,21 +1814,12 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { // plugin 1 is the masterplugin Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "CHECK") err = CmdCheck(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } }) It("fails to load NetConf with bad json in CmdAdd/Del", func() { @@ -2615,11 +1845,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { } // Missing close bracket in StdinData - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2648,8 +1874,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }` fExec.addPlugin(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") _, err := CmdAdd(args, fExec, nil) Expect(err).To(HaveOccurred()) @@ -2681,11 +1905,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`, expectedConf1, expectedConf2)), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2699,19 +1919,10 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { 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("[//:other1]: error adding container to network \"other1\": 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 and cleans up on failure with missing name field", func() { @@ -2740,11 +1951,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`, expectedConf1, expectedConf2)), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2758,19 +1965,10 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { err := fmt.Errorf("missing network name") 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(1)) Expect(fExec.delIndex).To(Equal(1)) Expect(err).To(HaveOccurred()) - - // Cleanup default network file. - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - }) It("executes delegates with runtimeConfigs", func() { @@ -2816,7 +2014,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: resultCNIVersion, IPs: []*current.IPConfig{{ @@ -2879,8 +2077,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -2924,7 +2120,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -2969,8 +2165,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net3", net3)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -3002,7 +2196,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3033,16 +2227,12 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) // plugin 1 is the masterplugin Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") // set fKubeClient to nil to emulate no pod info clientInfo.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) err = CmdDel(args, fExec, clientInfo) @@ -3074,7 +2264,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3102,16 +2292,12 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) // plugin 1 is the masterplugin Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") // set fKubeClient to nil to emulate no pod info fKubeClient.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) err = CmdDel(args, fExec, fKubeClient) @@ -3143,7 +2329,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3171,16 +2357,12 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) // plugin 1 is the masterplugin Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") // set fKubeClient to nil to emulate no pod info fKubeClient.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) err = CmdDel(args, fExec, fKubeClient) @@ -3214,7 +2396,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedConf1 := `{ "capabilities": {"portMappings": true}, "name": "mynet-confList", @@ -3227,8 +2409,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { } }` fExec.addPlugin(nil, "eth0", expectedConf1, nil, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") _, err := CmdAdd(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) }) @@ -3262,7 +2442,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - fExec := &fakeExec{} + fExec := newFakeExec() fExec.addPlugin(nil, "eth0", net1, expectedResult1, nil) fKubeClient := NewFakeClientInfo() @@ -3270,15 +2450,11 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { _, err := fKubeClient.AddNetAttachDef(testhelpers.NewFakeNetAttachDef("kube-system", "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue()) - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -3313,7 +2489,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`, tmpCNIDir)), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3340,8 +2516,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { _, err = fKubeClient.AddNetAttachDef( testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -3354,8 +2528,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { Expect(err).NotTo(HaveOccurred()) By("Delete and check net count is not incremented") - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -3390,7 +2562,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`, tmpCNIDir)), } - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3417,8 +2589,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { _, err = fKubeClient.AddNetAttachDef( testhelpers.NewFakeNetAttachDef(fakePod.ObjectMeta.Namespace, "net1", net1)) Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") result, err := CmdAdd(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) @@ -3434,8 +2604,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { Expect(err).NotTo(HaveOccurred()) By("Delete and check pod/net count is incremented") - os.Setenv("CNI_COMMAND", "DEL") - os.Setenv("CNI_IFNAME", "eth0") err = CmdDel(args, fExec, fKubeClient) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -3463,11 +2631,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }`), } - // Touch the default network file. - configPath := "/tmp/foo.multus.conf" - os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0755) - - fExec := &fakeExec{} + fExec := newFakeExec() expectedResult1 := ¤t.Result{ CNIVersion: "0.4.0", IPs: []*current.IPConfig{{ @@ -3496,9 +2660,6 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() { }` fExec.addPlugin(nil, "net1", expectedConf2, expectedResult2, nil) - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - fakeMultusNetConf := types.NetConf{ BinDir: "/opt/cni/bin", } diff --git a/pkg/types/conf.go b/pkg/types/conf.go index a4f624883..82bbeddbd 100644 --- a/pkg/types/conf.go +++ b/pkg/types/conf.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "net" + "os" "strings" "github.com/containernetworking/cni/libcni" @@ -184,7 +185,7 @@ func mergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetCo // CreateCNIRuntimeConf create CNI RuntimeConf for a delegate. If delegate configuration // exists, merge data with the runtime config. func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig, delegate *DelegateNetConf) (*libcni.RuntimeConf, string) { - logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v %v", args, k8sArgs, ifName, rc, delegate) + logging.Debugf("CreateCNIRuntimeConf: %v, %v, %s, %v %v", args, k8sArgs, ifName, rc, delegate) var cniDeviceInfoFile string var delegateRc *RuntimeConfig @@ -208,7 +209,7 @@ func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, r ContainerID: args.ContainerID, NetNS: args.Netns, IfName: ifName, - // NOTE: Verbose logging depends on this order, so please keep Args order. + // NOTE: Verbose logging (pod namespace/pod name)depends on this order, so please keep Args order. Args: [][2]string{ {"IgnoreUnknown", string("true")}, {"K8S_POD_NAMESPACE", string(k8sArgs.K8S_POD_NAMESPACE)}, @@ -218,6 +219,32 @@ func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, r }, } + // get CNI_ARGS and set it if it does not exist in rt.Args + cniArgs := os.Getenv("CNI_ARGS") + if cniArgs != "" { + for _, arg := range strings.Split(cniArgs, ";") { + for _, keyval := range strings.Split(arg, "=") { + if len(keyval) != 2 { + logging.Errorf("CreateCNIRuntimeConf: CNI_ARGS %s %s %d is not recognized as CNI arg, skipped", arg, keyval, len(keyval)) + continue + } + + envKey := string(keyval[0]) + envVal := string(keyval[1]) + isExists := false + for _, rtArg := range rt.Args { + if rtArg[0] == envKey { + isExists = true + } + } + if isExists != false { + logging.Debugf("CreateCNIRuntimeConf: add new val: %s", arg) + rt.Args = append(rt.Args, [2]string{envKey, envVal}) + } + } + } + } + if delegateRc != nil { capabilityArgs := map[string]interface{}{} if len(delegateRc.PortMaps) != 0 { diff --git a/pkg/types/conf_test.go b/pkg/types/conf_test.go index 94c4bab87..591e422cc 100644 --- a/pkg/types/conf_test.go +++ b/pkg/types/conf_test.go @@ -47,7 +47,6 @@ var _ = Describe("config operations", func() { var err error testNS, err = testutils.NewNS() Expect(err).NotTo(HaveOccurred()) - os.Setenv("CNI_NETNS", testNS.Path()) os.Setenv("CNI_PATH", "/some/path") tmpDir, err = ioutil.TempDir("", "multus_tmp") @@ -57,7 +56,6 @@ var _ = Describe("config operations", func() { AfterEach(func() { Expect(testNS.Close()).To(Succeed()) os.Unsetenv("CNI_PATH") - os.Unsetenv("CNI_ARGS") err := os.RemoveAll(tmpDir) Expect(err).NotTo(HaveOccurred()) }) From 450e1d341435691dbb9aafbbf430dc3d4345f8a5 Mon Sep 17 00:00:00 2001 From: Balazs Nemeth Date: Mon, 28 Feb 2022 13:50:39 +0100 Subject: [PATCH 2/6] check version incompatibility (#762) * multus: entrypoint: disallow incompatible cni versions When top level CNI version is 0.4.0 or more, nested CNI version can't be less than 0.4.0 since these are incompatible. This closes issue #737. Signed-off-by: Balazs Nemeth * multus: thick: disallow incompatible cni versions Similarly to disallowing incompatible versions in entrypoint.sh, add the same logic in go for the thick plugin. Signed-off-by: Balazs Nemeth * multus: add unit test for incompatible cni versions Signed-off-by: Balazs Nemeth --- cmd/controller/main.go | 8 +++- go.mod | 1 + images/entrypoint.sh | 31 ++++++++++++-- pkg/config/generator.go | 50 +++++++++++++++++++--- pkg/config/generator_test.go | 82 ++++++++++++++++++++++++++++-------- pkg/config/manager.go | 13 +++--- pkg/config/manager_test.go | 2 +- vendor/modules.txt | 1 + 8 files changed, 153 insertions(+), 35 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 8a50cce58..a02578c09 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -139,10 +139,14 @@ func main() { configurationOptions = append( configurationOptions, config.WithReadinessFileIndicator(*readinessIndicator)) } - multusConfig := config.NewMultusConfig(multusPluginName, *cniVersion, *multusKubeconfig, configurationOptions...) + + multusConfig, err := config.NewMultusConfig(multusPluginName, *cniVersion, *multusKubeconfig, configurationOptions...) + if err != nil { + _ = logging.Errorf("Failed to create multus config: %v", err) + os.Exit(3) + } var configManager *config.Manager - var err error if *multusMasterCni == "" { configManager, err = config.NewManager(*multusConfig, *multusAutoconfigDir) } else { diff --git a/go.mod b/go.mod index b8208d6ac..5cb11793a 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module gopkg.in/k8snetworkplumbingwg/multus-cni.v3 go 1.16 require ( + github.com/blang/semver v3.5.1+incompatible github.com/containernetworking/cni v0.8.1 github.com/containernetworking/plugins v0.9.1 github.com/fsnotify/fsnotify v1.4.9 diff --git a/images/entrypoint.sh b/images/entrypoint.sh index fdd0d3c78..92e8cc426 100755 --- a/images/entrypoint.sh +++ b/images/entrypoint.sh @@ -87,6 +87,25 @@ if ! type python3 &> /dev/null; then alias python=python3 fi +function checkCniVersion { + cniversion_python_tmpfile=$(mktemp) + cat << EOF > $cniversion_python_tmpfile +import json, sys + +def version(v): + return [int(x) for x in v.split(".")] + +v_040 = version("0.4.0") +v_top_level = sys.argv[2] +with open(sys.argv[1], "r") as f: + v_nested = json.load(f)["cniVersion"] +if version(v_top_level) >= v_040 and version(v_nested) < v_040: + msg = "Multus cni version is %s while master plugin cni version is %s" + print(msg % (v_top_level, v_nested)) +EOF + python $cniversion_python_tmpfile $1 $2 +} + # Parse parameters given as arguments to this script. while [ "$1" != "" ]; do PARAM=`echo $1 | awk -F= '{print $1}'` @@ -317,7 +336,7 @@ if [ "$MULTUS_CONF_FILE" == "auto" ]; then *) error "Log levels should be one of: debug/verbose/error/panic, did not understand $MULTUS_LOG_LEVEL" usage - exit 1 + exit 1 esac LOG_LEVEL_STRING="\"logLevel\": \"$MULTUS_LOG_LEVEL\"," fi @@ -369,11 +388,17 @@ EOF NESTED_CAPABILITIES_STRING="$(cat $MULTUS_AUTOCONF_DIR/$MASTER_PLUGIN | \ python $capabilities_python_filter_tmpfile)" rm $capabilities_python_filter_tmpfile - log "Nested capabilities string: $NESTED_CAPABILITIES_STRING" + log "Nested capabilities string: $NESTED_CAPABILITIES_STRING" MASTER_PLUGIN_LOCATION=$MULTUS_AUTOCONF_DIR/$MASTER_PLUGIN MASTER_PLUGIN_JSON="$(cat $MASTER_PLUGIN_LOCATION)" log "Using $MASTER_PLUGIN_LOCATION as a source to generate the Multus configuration" + CHECK_CNI_VERSION=$(checkCniVersion $MASTER_PLUGIN_LOCATION $CNI_VERSION) + if [ "$CHECK_CNI_VERSION" != "" ] ; then + error "$CHECK_CNI_VERSION" + exit 1 + fi + CONF=$(cat <<-EOF { $CNI_VERSION_STRING @@ -399,7 +424,7 @@ EOF mv $tmpfile $CNI_CONF_DIR/00-multus.conf log "Config file created @ $CNI_CONF_DIR/00-multus.conf" echo $CONF - + # If we're not performing the cleanup on exit, we can safely rename the config file. if [ "$RENAME_SOURCE_CONFIG_FILE" == true ]; then mv ${MULTUS_AUTOCONF_DIR}/${MASTER_PLUGIN} ${MULTUS_AUTOCONF_DIR}/${MASTER_PLUGIN}.old diff --git a/pkg/config/generator.go b/pkg/config/generator.go index 5ec4530d5..32f022d5b 100644 --- a/pkg/config/generator.go +++ b/pkg/config/generator.go @@ -17,12 +17,15 @@ package config import ( "encoding/json" + "errors" "fmt" "io/ioutil" "path/filepath" "sort" "strings" "time" + + "github.com/blang/semver" ) const ( @@ -52,7 +55,7 @@ type MultusConf struct { // NewMultusConfig creates a basic configuration generator. It can be mutated // via the `With...` methods. -func NewMultusConfig(pluginName string, cniVersion string, kubeconfig string, configurationOptions ...Option) *MultusConf { +func NewMultusConfig(pluginName string, cniVersion string, kubeconfig string, configurationOptions ...Option) (*MultusConf, error) { multusConfig := &MultusConf{ Name: MultusDefaultNetworkName, CNIVersion: cniVersion, @@ -61,10 +64,45 @@ func NewMultusConfig(pluginName string, cniVersion string, kubeconfig string, co Kubeconfig: kubeconfig, Delegates: []interface{}{}, } - for _, configOption := range configurationOptions { - configOption(multusConfig) + + err := multusConfig.Mutate(configurationOptions...) + return multusConfig, err +} + +// CheckVersionCompatibility checks compatibilty of the +// top level cni version with the delegate cni version. +// Since version 0.4.0, CHECK was introduced, which +// causes incompatibility. +func CheckVersionCompatibility(mc *MultusConf) error { + const versionFmt = "delegate cni version is %s while top level cni version is %s" + v040, _ := semver.Make("0.4.0") + multusCNIVersion, err := semver.Make(mc.CNIVersion) + + if err != nil { + return errors.New("couldn't get top level cni version") } - return multusConfig + + if multusCNIVersion.GTE(v040) { + for _, delegate := range mc.Delegates { + delegatesMap, ok := delegate.(map[string]interface{}) + if !ok { + return errors.New("couldn't get cni version of delegate") + } + delegateVersion, ok := delegatesMap["cniVersion"].(string) + if !ok { + return errors.New("couldn't get cni version of delegate") + } + v, err := semver.Make(delegateVersion) + if err != nil { + return err + } + if v.LT(v040) { + return fmt.Errorf(versionFmt, delegateVersion, mc.CNIVersion) + } + } + } + + return nil } // Generate generates the multus configuration from whatever state is currently @@ -76,10 +114,12 @@ func (mc *MultusConf) Generate() (string, error) { // Mutate updates the MultusConf attributes according to the provided // configuration `Option`s -func (mc *MultusConf) Mutate(configurationOptions ...Option) { +func (mc *MultusConf) Mutate(configurationOptions ...Option) error { for _, configOption := range configurationOptions { configOption(mc) } + + return CheckVersionCompatibility(mc) } // WithNamespaceIsolation mutates the inner state to enable the diff --git a/pkg/config/generator_test.go b/pkg/config/generator_test.go index 0d1f82649..cc6c40d5c 100644 --- a/pkg/config/generator_test.go +++ b/pkg/config/generator_test.go @@ -45,46 +45,51 @@ var primaryCNIConfig = map[string]interface{}{ "logfile-maxage": 5, } -func newMultusConfigWithDelegates(pluginName string, cniVersion string, kubeconfig string, primaryCNIPluginConfig interface{}, configOptions ...Option) *MultusConf { - multusConfig := NewMultusConfig(pluginName, cniVersion, kubeconfig, configOptions...) - multusConfig.Delegates = []interface{}{primaryCNIPluginConfig} - return multusConfig +func newMultusConfigWithDelegates(pluginName string, cniVersion string, kubeconfig string, primaryCNIPluginConfig interface{}, configOptions ...Option) (*MultusConf, error) { + multusConfig, err := NewMultusConfig(pluginName, cniVersion, kubeconfig, configOptions...) + if err != nil { + return multusConfig, err + } + return multusConfig, multusConfig.Mutate(withDelegates(primaryCNIPluginConfig.(map[string]interface{}))) } func TestBasicMultusConfig(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig) + assertError(t, err, nil) expectedResult := "{\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithNamespaceIsolation(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, WithNamespaceIsolation()) + assertError(t, err, nil) expectedResult := "{\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"namespaceIsolation\":true,\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithReadinessIndicator(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, WithReadinessFileIndicator("/a/b/u/it-lives")) + assertError(t, err, nil) expectedResult := "{\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"readinessindicatorfile\":\"/a/b/u/it-lives\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithLoggingConfiguration(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, @@ -92,96 +97,104 @@ func TestMultusConfigWithLoggingConfiguration(t *testing.T) { WithLogLevel("notice"), WithLogToStdErr(), WithLogFile("/u/y/w/log.1")) + assertError(t, err, nil) expectedResult := "{\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"logFile\":\"/u/y/w/log.1\",\"logLevel\":\"notice\",\"logToStderr\":true,\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithGlobalNamespace(t *testing.T) { const globalNamespace = "come-along-ns" - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, WithGlobalNamespaces(globalNamespace)) + assertError(t, err, nil) expectedResult := "{\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"globalNamespaces\":\"come-along-ns\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithAdditionalBinDir(t *testing.T) { const anotherCNIBinDir = "a-dir-somewhere" - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, WithAdditionalBinaryFileDir(anotherCNIBinDir)) + assertError(t, err, nil) expectedResult := "{\"binDir\":\"a-dir-somewhere\",\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithCapabilities(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, withCapabilities( documentHelper(`{"capabilities": {"portMappings": true}}`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithMultipleCapabilities(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, withCapabilities( documentHelper(`{"capabilities": {"portMappings": true, "tuning": true}}`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true,\"tuning\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithMultipleCapabilitiesFilterOnlyEnabled(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, withCapabilities( documentHelper(`{"capabilities": {"portMappings": true, "tuning": false}}`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithMultipleCapabilitiesDefinedOnAPlugin(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, withCapabilities( documentHelper(`{"plugins": [ {"capabilities": {"portMappings": true, "tuning": true}} ] }`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true,\"tuning\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithCapabilitiesDefinedOnMultiplePlugins(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, primaryCNIConfig, withCapabilities( documentHelper(`{"plugins": [ {"capabilities": { "portMappings": true }}, {"capabilities": { "tuning": true }} ]}`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true,\"tuning\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } func TestMultusConfigWithCapabilitiesDefinedOnMultiplePluginsFilterOnlyEnabled(t *testing.T) { - multusConfig := newMultusConfigWithDelegates( + multusConfig, err := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, @@ -202,13 +215,48 @@ func TestMultusConfigWithCapabilitiesDefinedOnMultiplePluginsFilterOnlyEnabled(t } ] }`))) + assertError(t, err, nil) expectedResult := "{\"capabilities\":{\"portMappings\":true},\"cniVersion\":\"0.4.0\",\"delegates\":[{\"cniVersion\":\"1.0.0\",\"dns\":\"{}\",\"ipam\":\"{}\",\"logFile\":\"/var/log/ovn-kubernetes/ovn-k8s-cni-overlay.log\",\"logLevel\":\"5\",\"logfile-maxage\":5,\"logfile-maxbackups\":5,\"logfile-maxsize\":100,\"name\":\"ovn-kubernetes\",\"type\":\"ovn-k8s-cni-overlay\"}],\"kubeconfig\":\"/a/b/c/kubeconfig.kubeconfig\",\"name\":\"multus-cni-network\",\"type\":\"myCNI\"}" newTestCase(t, multusConfig.Generate).assertResult(expectedResult) } +func assertError(t *testing.T, actual error, expected error) { + if actual != nil && expected != nil { + if actual.Error() != expected.Error() { + t.Fatalf("multus config generation failed.\nExpected:\n%v\nbut GOT:\n%v", expected.Error(), actual.Error()) + } + } + + if actual == nil && expected != nil { + t.Fatalf("multus config generation failed.\nExpected:\n%v\nbut didn't get error", expected.Error()) + } else if actual != nil && expected == nil { + t.Fatalf("multus config generation failed.\nDidn't expect error\nbut GOT: %v\n", actual.Error()) + } +} + +func invalidDelegateCNIVersion(delegateCNIVersion, multusCNIVersion string) error { + return fmt.Errorf("delegate cni version is %s while top level cni version is %s", delegateCNIVersion, multusCNIVersion) +} + +func TestVersionIncompatibility(t *testing.T) { + const delegateCNIVersion = "0.3.0" + + primaryCNIConfigOld := primaryCNIConfig + tmpVer := primaryCNIConfig["cniVersion"] + primaryCNIConfig["cniVersion"] = delegateCNIVersion + _, err := newMultusConfigWithDelegates( + primaryCNIName, + cniVersion, + kubeconfig, + primaryCNIConfigOld) + primaryCNIConfig["cniVersion"] = tmpVer + + assertError(t, invalidDelegateCNIVersion(delegateCNIVersion, cniVersion), err) +} + func TestMultusConfigWithOverriddenName(t *testing.T) { newNetworkName := "mega-net-2000" - multusConfig := newMultusConfigWithDelegates( + multusConfig, _ := newMultusConfigWithDelegates( primaryCNIName, cniVersion, kubeconfig, diff --git a/pkg/config/manager.go b/pkg/config/manager.go index 4120fef2a..8652f84b5 100644 --- a/pkg/config/manager.go +++ b/pkg/config/manager.go @@ -78,8 +78,9 @@ func newManager(config MultusConf, multusConfigDir string, defaultCNIPluginName } if err := configManager.loadPrimaryCNIConfigFromFile(); err != nil { - return nil, fmt.Errorf("failed to load the primary CNI configuration as a multus delegate") + return nil, fmt.Errorf("failed to load the primary CNI configuration as a multus delegate with error '%v'", err) } + return configManager, nil } @@ -88,8 +89,7 @@ func (m *Manager) loadPrimaryCNIConfigFromFile() error { if err != nil { return logging.Errorf("failed to access the primary CNI configuration from %s: %v", m.primaryCNIConfigPath, err) } - m.loadPrimaryCNIConfigurationData(primaryCNIConfigData) - return nil + return m.loadPrimaryCNIConfigurationData(primaryCNIConfigData) } // OverrideNetworkName overrides the name of the multus configuration with the @@ -104,15 +104,14 @@ func (m *Manager) OverrideNetworkName() error { if networkName == "" { return fmt.Errorf("the primary CNI Configuration does not feature the network name: %v", m.cniConfigData) } - m.multusConfig.Mutate(WithOverriddenName(networkName)) - return nil + return m.multusConfig.Mutate(WithOverriddenName(networkName)) } -func (m *Manager) loadPrimaryCNIConfigurationData(primaryCNIConfigData interface{}) { +func (m *Manager) loadPrimaryCNIConfigurationData(primaryCNIConfigData interface{}) error { cniConfigData := primaryCNIConfigData.(map[string]interface{}) m.cniConfigData = cniConfigData - m.multusConfig.Mutate( + return m.multusConfig.Mutate( withDelegates(cniConfigData), withCapabilities(cniConfigData)) } diff --git a/pkg/config/manager_test.go b/pkg/config/manager_test.go index 6df78f85f..389a529a7 100644 --- a/pkg/config/manager_test.go +++ b/pkg/config/manager_test.go @@ -61,7 +61,7 @@ var _ = Describe(suiteName, func() { defaultCniConfig = fmt.Sprintf("%s/%s", multusConfigDir, primaryCNIPluginName) Expect(ioutil.WriteFile(defaultCniConfig, []byte(primaryCNIPluginTemplate), userRWPermission)).To(Succeed()) - multusConf := NewMultusConfig( + multusConf, _ := NewMultusConfig( primaryCNIName, cniVersion, kubeconfig) diff --git a/vendor/modules.txt b/vendor/modules.txt index aedc372f7..a4c7c2761 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -4,6 +4,7 @@ github.com/Microsoft/go-winio/pkg/guid # github.com/beorn7/perks v1.0.1 github.com/beorn7/perks/quantile # github.com/blang/semver v3.5.1+incompatible +## explicit github.com/blang/semver # github.com/cespare/xxhash/v2 v2.1.1 github.com/cespare/xxhash/v2 From 7559625a388a78b6362be422a70c9bd8e6b72cef Mon Sep 17 00:00:00 2001 From: Doug Smith Date: Thu, 3 Mar 2022 12:02:24 -0500 Subject: [PATCH 3/6] only warn when netns can't be opened (#803) --- pkg/multus/multus.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index acd68e35e..83ba8349f 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -781,11 +781,11 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er // so don't return an error if the device is already removed. // https://github.com/kubernetes/kubernetes/issues/43014#issuecomment-287164444 _, ok := err.(ns.NSPathNotExistErr) + netnsfound = false if ok { - netnsfound = false logging.Debugf("CmdDel: WARNING netns may not exist, netns: %s, err: %s", args.Netns, err) } else { - return cmdErr(nil, "failed to open netns %q: %v", netns, err) + logging.Debugf("CmdDel: WARNING failed to open netns %q: %v", netns, err) } } From 6c12dc8c4f44f1b410593223461feb0a3f0997d0 Mon Sep 17 00:00:00 2001 From: Doug Smith Date: Thu, 3 Mar 2022 12:02:37 -0500 Subject: [PATCH 4/6] crio: mount /run rslave (#802) to prevent "unknown FS magic on "/var/run/netns/*": 1021994" errors Signed-off-by: Peter Hunt Co-authored-by: Peter Hunt --- deployments/multus-daemonset-crio.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/deployments/multus-daemonset-crio.yml b/deployments/multus-daemonset-crio.yml index eb110397c..bed862baa 100644 --- a/deployments/multus-daemonset-crio.yml +++ b/deployments/multus-daemonset-crio.yml @@ -193,6 +193,7 @@ spec: volumeMounts: - name: run mountPath: /run + mountPropagation: HostToContainer - name: cni mountPath: /host/etc/cni/net.d - name: cnibin From 45428a53ceedcf6359a7c2f75e8b84d64a10148b Mon Sep 17 00:00:00 2001 From: Cyclinder Date: Fri, 4 Mar 2022 15:01:21 +0800 Subject: [PATCH 5/6] fix the usage of flag "overrideNetworkName" (#805) --- cmd/controller/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index a02578c09..bd777165e 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -80,7 +80,7 @@ func main() { additionalBinDir := flag.String(multusAdditionalBinDirVarName, defaultMultusAdditionalBinDir, "Additional binary directory to specify in the configurations. Used only with --multus-conf-file=auto.") readinessIndicator := flag.String(multusReadinessIndicatorFile, defaultMultusReadinessIndicatorFile, "Which file should be used as the readiness indicator. Used only with --multus-conf-file=auto.") multusKubeconfig := flag.String(multusKubeconfigPath, defaultMultusKubeconfigPath, "The path to the kubeconfig") - overrideNetworkName := flag.Bool("override-network-name", false, "Used when ") + overrideNetworkName := flag.Bool("override-network-name", false, "Used when we need overrides the name of the multus configuration with the name of the delegated primary CNI") flag.BoolVar(&versionOpt, "version", false, "Show application version") flag.BoolVar(&versionOpt, "v", false, "Show application version") flag.Parse() From 2d533342119dc7265f67888efd39a24711f8d9ab Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Tue, 5 Apr 2022 02:03:33 +0900 Subject: [PATCH 6/6] Remove error handling for getPod to force to proceed cmdDel. In cmdDel, CNI Spec mentioned that plugin should proceed cmdDel without any error, hence the change removes error returning at cmdDel. fix #822 --- pkg/multus/multus.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index 83ba8349f..b60bc69c0 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -815,7 +815,8 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er pod, err := getPod(kubeClient, k8sArgs, true) if err != nil { - return err + // getPod may be failed but just do print error in its log and continue to delete + logging.Errorf("Multus: getPod failed: %v, but continue to delete", err) } // Read the cache to get delegates json for the pod