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.
2025-09-17 06:55:48 +00:00 · 2023-05-18 00:41:48 +09:00
parent 0c37bb043c
commit 22304806c8
4 changed files with 264 additions and 65 deletions
--- 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,71 +639,80 @@ 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)
-			}
-		}
-
-		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)
-			}
-		}
-
-		// 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)
-			}
-		}
-
-		// 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)
-			}
-		}
-
 		// Master plugin result is always used if present
 		if delegate.MasterPlugin || result == nil {
 			result = tmpResult
 		}

+		res, err := cni100.NewResultFromResult(tmpResult)
+		if err != nil {
+			logging.Errorf("CmdAdd: failed to read result: %v, but proceed", 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)
+				}
+			}
+
+			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)
+				}
+			}
+
+			// 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)
+				}
+			}
+
+			// 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
 		// it can be copied to the NetworkStatus.
 		devinfo, err := getDelegateDeviceInfo(delegate, rt)
--- 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",
--- 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 := `{
--- 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",