diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index 24cff874a..e24784d0e 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -369,11 +369,14 @@ func DelegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, dele } if pod != nil { - // send kubernetes events - if delegate.Name != "" { - kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v from %s", rt.IfName, ips, delegate.Name) - } else { - kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v", rt.IfName, ips) + // check Interfaces and IPs because some CNI plugin just return empty result + if res.Interfaces != nil || res.IPs != nil { + // send kubernetes events + if delegate.Name != "" { + kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v from %s", rt.IfName, ips, delegate.Name) + } else { + kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v", rt.IfName, ips) + } } } else { // for further debug https://github.com/k8snetworkplumbingwg/multus-cni/issues/481 @@ -636,69 +639,78 @@ func CmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c return nil, cmdPluginErr(k8sArgs, netName, "error adding container to network %q: %v", netName, err) } - // Remove gateway from routing table if the gateway is not used - deleteV4gateway := false - deleteV6gateway := false - adddefaultgateway := false - if delegate.IsFilterV4Gateway { - deleteV4gateway = true - logging.Debugf("Marked interface %v for v4 gateway deletion", ifName) - } else { - // Otherwise, determine if this interface now gets our default route. - // According to - // https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9) - // the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds, - // else we'll update the defaultgateway to the one specified. - if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 { - deleteV4gateway = true - adddefaultgateway = true - logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest) - } + // Master plugin result is always used if present + if delegate.MasterPlugin || result == nil { + result = tmpResult } - if delegate.IsFilterV6Gateway { - deleteV6gateway = true - logging.Debugf("Marked interface %v for v6 gateway deletion", ifName) - } else { - // Otherwise, determine if this interface now gets our default route. - // According to - // https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9) - // the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds, - // else we'll update the defaultgateway to the one specified. - if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 { - deleteV6gateway = true - adddefaultgateway = true - logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest) - } + res, err := cni100.NewResultFromResult(tmpResult) + if err != nil { + logging.Errorf("CmdAdd: failed to read result: %v, but proceed", err) } - // Remove gateway if `default-route` network selection is specified - if deleteV4gateway || deleteV6gateway { - err = netutils.DeleteDefaultGW(args.Netns, ifName) - if err != nil { - return nil, cmdErr(k8sArgs, "error deleting default gateway: %v", err) - } - err = netutils.DeleteDefaultGWCache(n.CNIDir, rt, netName, ifName, deleteV4gateway, deleteV6gateway) - if err != nil { - return nil, cmdErr(k8sArgs, "error deleting default gateway in cache: %v", err) + // check Interfaces and IPs because some CNI plugin does not create any interface + // and just returns empty result + if res != nil && (res.Interfaces != nil || res.IPs != nil) { + // Remove gateway from routing table if the gateway is not used + deleteV4gateway := false + deleteV6gateway := false + adddefaultgateway := false + if delegate.IsFilterV4Gateway { + deleteV4gateway = true + logging.Debugf("Marked interface %v for v4 gateway deletion", ifName) + } else { + // Otherwise, determine if this interface now gets our default route. + // According to + // https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9) + // the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds, + // else we'll update the defaultgateway to the one specified. + if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 { + deleteV4gateway = true + adddefaultgateway = true + logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest) + } } - } - // Here we'll set the default gateway which specified in `default-route` network selection - if adddefaultgateway { - err = netutils.SetDefaultGW(args.Netns, ifName, *delegate.GatewayRequest) - if err != nil { - return nil, cmdErr(k8sArgs, "error setting default gateway: %v", err) + if delegate.IsFilterV6Gateway { + deleteV6gateway = true + logging.Debugf("Marked interface %v for v6 gateway deletion", ifName) + } else { + // Otherwise, determine if this interface now gets our default route. + // According to + // https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9) + // the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds, + // else we'll update the defaultgateway to the one specified. + if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 { + deleteV6gateway = true + adddefaultgateway = true + logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest) + } } - err = netutils.AddDefaultGWCache(n.CNIDir, rt, netName, ifName, *delegate.GatewayRequest) - if err != nil { - return nil, cmdErr(k8sArgs, "error setting default gateway in cache: %v", err) + + // Remove gateway if `default-route` network selection is specified + if deleteV4gateway || deleteV6gateway { + err = netutils.DeleteDefaultGW(args.Netns, ifName) + if err != nil { + return nil, cmdErr(k8sArgs, "error deleting default gateway: %v", err) + } + err = netutils.DeleteDefaultGWCache(n.CNIDir, rt, netName, ifName, deleteV4gateway, deleteV6gateway) + if err != nil { + return nil, cmdErr(k8sArgs, "error deleting default gateway in cache: %v", err) + } } - } - // Master plugin result is always used if present - if delegate.MasterPlugin || result == nil { - result = tmpResult + // Here we'll set the default gateway which specified in `default-route` network selection + if adddefaultgateway { + err = netutils.SetDefaultGW(args.Netns, ifName, *delegate.GatewayRequest) + if err != nil { + return nil, cmdErr(k8sArgs, "error setting default gateway: %v", err) + } + err = netutils.AddDefaultGWCache(n.CNIDir, rt, netName, ifName, *delegate.GatewayRequest) + if err != nil { + return nil, cmdErr(k8sArgs, "error setting default gateway in cache: %v", err) + } + } } // Read devInfo from CNIDeviceInfoFile if it exists so diff --git a/pkg/multus/multus_cni020_test.go b/pkg/multus/multus_cni020_test.go index 23558deb6..8868677d3 100644 --- a/pkg/multus/multus_cni020_test.go +++ b/pkg/multus/multus_cni020_test.go @@ -154,6 +154,67 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) }) + It("executes delegates (plugin without interface)", 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") + + fExec := newFakeExec() + 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) + + // other1 just returns empty result + expectedResult2 := &types020.Result{ + CNIVersion: "0.2.0", + } + expectedConf2 := `{ + "name": "other1", + "cniVersion": "0.2.0", + "type": "other-plugin" + }` + fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil) + + 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()) + + err = CmdDel(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) + }) + It("executes delegates given faulty namespace", func() { args := &skel.CmdArgs{ ContainerID: "123456789", diff --git a/pkg/multus/multus_cni040_test.go b/pkg/multus/multus_cni040_test.go index f7d7803df..86ff62689 100644 --- a/pkg/multus/multus_cni040_test.go +++ b/pkg/multus/multus_cni040_test.go @@ -247,6 +247,67 @@ var _ = Describe("multus operations cniVersion 0.3.1 config", func() { Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) }) + It("executes delegates (plugin without interface)", 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.3.1", + "type": "weave-net" + },{ + "name": "other1", + "cniVersion": "0.3.1", + "type": "other-plugin" + }] + }`), + } + + logging.SetLogLevel("verbose") + + fExec := newFakeExec() + expectedResult1 := &cni040.Result{ + CNIVersion: "0.3.1", + IPs: []*cni040.IPConfig{{ + Address: *testhelpers.EnsureCIDR("1.1.1.2/24"), + }}, + } + expectedConf1 := `{ + "name": "weave1", + "cniVersion": "0.3.1", + "type": "weave-net" + }` + fExec.addPlugin040(nil, "eth0", expectedConf1, expectedResult1, nil) + + // other1 just returns empty result + expectedResult2 := &cni040.Result{ + CNIVersion: "0.3.1", + } + expectedConf2 := `{ + "name": "other1", + "cniVersion": "0.3.1", + "type": "other-plugin" + }` + fExec.addPlugin040(nil, "net1", expectedConf2, expectedResult2, nil) + + result, err := CmdAdd(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(fExec.addIndex).To(Equal(len(fExec.plugins))) + r := result.(*cni040.Result) + // plugin 1 is the masterplugin + Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) + + err = CmdDel(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) + }) + It("fails when pod UID is provided and does not match Kube API pod UID", func() { fakePod := testhelpers.NewFakePod("testpod", "net1", "") net1 := `{ diff --git a/pkg/multus/multus_cni100_test.go b/pkg/multus/multus_cni100_test.go index 54d36a4dd..e193f4a56 100644 --- a/pkg/multus/multus_cni100_test.go +++ b/pkg/multus/multus_cni100_test.go @@ -192,6 +192,71 @@ var _ = Describe("multus operations cniVersion 1.0.0 config", func() { 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("executes delegates (plugin without interface)", 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": "1.0.0", + "type": "weave-net" + },{ + "name": "other1", + "cniVersion": "1.0.0", + "type": "other-plugin" + }] + }`), + } + + logging.SetLogLevel("verbose") + + fExec := newFakeExec() + expectedResult1 := &cni100.Result{ + CNIVersion: "1.0.0", + IPs: []*cni100.IPConfig{{ + Address: *testhelpers.EnsureCIDR("1.1.1.2/24"), + }, + }, + } + expectedConf1 := `{ + "name": "weave1", + "cniVersion": "1.0.0", + "type": "weave-net" + }` + fExec.addPlugin100(nil, "eth0", expectedConf1, expectedResult1, nil) + + // other1 just returns empty result + expectedResult2 := &cni100.Result{ + CNIVersion: "0.4.0", + } + expectedConf2 := `{ + "name": "other1", + "cniVersion": "1.0.0", + "type": "other-plugin" + }` + fExec.addPlugin100(nil, "net1", expectedConf2, expectedResult2, nil) + + 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()) + + err = CmdCheck(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + + err = CmdDel(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) + }) + It("returns the previous result using CmdCheck", func() { args := &skel.CmdArgs{ ContainerID: "123456789",