From 22304806c83eebe0eb9200942c8275144ab69fe0 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Thu, 18 May 2023 00:41:48 +0900 Subject: [PATCH] Fix multus to support CNI plugin which does not create interface This change fixes multus to support CNI plugin which does not create any interface and return empty result. Some CNI plugin may do network configuration change only and does not create any interface and return empty CNI result. Current multus assumes that CNI config always creates some interfaces hence above CNI plugin is out of assumption and multus may not work with such plugins. --- pkg/multus/multus.go | 130 +++++++++++++++++-------------- pkg/multus/multus_cni020_test.go | 61 +++++++++++++++ pkg/multus/multus_cni040_test.go | 61 +++++++++++++++ pkg/multus/multus_cni100_test.go | 65 ++++++++++++++++ 4 files changed, 258 insertions(+), 59 deletions(-) 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",