Merge pull request #1095 from s1061123/fix/plugin-without-interface

Fix multus to support CNI plugin which does not create interface

.github/workflows/image-build.yml

@@ -64,11 +64,14 @@ jobs:
         uses: docker/setup-buildx-action@v2
 
       - name: Download OKD Builder Dockerfile
-        run: curl https://raw.githubusercontent.com/okd-project/images/main/okd-builder.Dockerfile -o images/okd-builder.Dockerfile
+        run: curl https://raw.githubusercontent.com/okd-project/images/main/builder/Dockerfile -o images/okd-builder.Dockerfile
 
       - name: Patch OKD Builder Dockerfile to workaround error
         run: sed -i -e "s/yum install -y yum-utils/rpm --import \/etc\/pki\/rpm-gpg\/*;yum install -y yum-utils/" images/okd-builder.Dockerfile
 
+      - name: Create root for builder
+        run: mkdir root
+
       - name: Organically build golang builder image
         run: docker build -t local/okdbuilder:latest -f images/okd-builder.Dockerfile .
 

README.md

@@ -62,3 +62,5 @@ In addition to the [quick-start guide](docs/quickstart.md), you may:
 ## Contact Us
 
 For any questions about Multus CNI, open up a GitHub issue or feel free to ask a question in #general in the [NPWG Slack](https://npwg-team.slack.com/).
+
+To be invited, use [this slack invite link](https://join.slack.com/t/npwg-team/shared_invite/zt-1u2vmsn2b-tKdOokdPY73zn9B32JoAOg).

pkg/multus/multus.go

@@ -369,12 +369,15 @@ func DelegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, dele
 	}
 
 	if pod != nil {
+		// 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
 		logging.Errorf("DelegateAdd: pod nil pointer: namespace: %s, name: %s, container id: %s, pod: %v", rt.Args[1][1], rt.Args[2][1], rt.Args[3][1], pod)
@@ -636,6 +639,19 @@ 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)
 		}
 
+		// 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
@@ -695,10 +711,6 @@ func CmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c
 					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
 		}
 
 		// Read devInfo from CNIDeviceInfoFile if it exists so

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",

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 := `{

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",

pkg/server/api/shim.go

@@ -65,7 +65,8 @@ func CmdCheck(args *skel.CmdArgs) error {
 func CmdDel(args *skel.CmdArgs) error {
 	_, _, err := postRequest(args)
 	if err != nil {
-		return logging.Errorf("CmdDel (shim): %v", err)
+		// No error in DEL (as of CNI spec)
+		logging.Errorf("CmdCheck (shim): %v", err)
 	}
 	return nil
 }