Fix CNI driver to generate SPEC compliant response (#660)

* Fix CNI driver to generate SPEC compliant response * Add error checking for ParseCIDR * Fix unit tests for new implementation * Fix test
CentaurusInfra · Sep 2, 2022 · 00b2579 · 00b2579
1 parent 06b7cbf
commit 00b2579
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 28 deletions.
diff --git a/cmd/mizarcni/app/worker.go b/cmd/mizarcni/app/worker.go
@@ -18,31 +18,31 @@ package app
 
 import (
 	"fmt"
+	"net"
 	"strings"
 
 	"centaurusinfra.io/mizar/pkg/object"
 	"centaurusinfra.io/mizar/pkg/util/grpcclientutil"
 	"centaurusinfra.io/mizar/pkg/util/netutil"
 	"centaurusinfra.io/mizar/pkg/util/netvariablesutil"
 
-	cniTypesVer "github.com/containernetworking/cni/pkg/types/current"
+	cni "github.com/containernetworking/cni/pkg/types/current"
 )
 
 func DoInit(netVariables *object.NetVariables) (string, error) {
 	netvariablesutil.LoadEnvVariables(netVariables)
 	return netvariablesutil.MountNetNSIfNeeded(netVariables)
 }
 
-func DoCmdAdd(netVariables *object.NetVariables, stdinData []byte) (cniTypesVer.Result, string, error) {
+func DoCmdAdd(netVariables *object.NetVariables, stdinData []byte) (cni.Result, string, error) {
 	tracelog := strings.Builder{}
-	result := cniTypesVer.Result{
-		CNIVersion: netVariables.CniVersion,
-	}
+	result := cni.Result{}
 
 	if err := netvariablesutil.LoadCniConfig(netVariables, stdinData); err != nil {
 		return result, tracelog.String(), err
 	}
 	tracelog.WriteString(fmt.Sprintf("CNI_ADD: Args: '%s'\n", netVariables))
+	result.CNIVersion = netVariables.CniVersion
 
 	interfaces, err := grpcclientutil.ConsumeInterfaces(*netVariables)
 	if err != nil {
@@ -51,8 +51,11 @@ func DoCmdAdd(netVariables *object.NetVariables, stdinData []byte) (cniTypesVer.
 	if len(interfaces) == 0 {
 		return result, tracelog.String(), fmt.Errorf("No interfaces found for Pod '%s/%s'", netVariables.K8sPodNamespace, netVariables.K8sPodName)
 	}
+	if len(interfaces) > 1 {
+		return result, tracelog.String(), fmt.Errorf("Unsupported - multiple interfaces found for Pod '%s/%s'", netVariables.K8sPodNamespace, netVariables.K8sPodName)
+	}
 
-	for index, intf := range interfaces {
+	for idx, intf := range interfaces {
 		tracelog.WriteString(fmt.Sprintf("CNI_ADD: Activating interface: '%s'\n", intf))
 		activateIfLog, err := netutil.ActivateInterface(
 			netVariables.IfName,
@@ -62,42 +65,42 @@ func DoCmdAdd(netVariables *object.NetVariables, stdinData []byte) (cniTypesVer.
 			intf.Address.IpAddress,
 			intf.Address.GatewayIp)
 		if activateIfLog != "" {
-			tracelog.WriteString(fmt.Sprintf("CNI_ADD: Activate interface result: '%s'\n", activateIfLog))
+			tracelog.WriteString(fmt.Sprintf("CNI_ADD: Activate interface log: '%s'\n", activateIfLog))
 		}
 		if err != nil {
 			return result, tracelog.String(), err
 		}
 
-		result.Interfaces = append(result.Interfaces, &cniTypesVer.Interface{
-			Name:    intf.InterfaceId.Interface,
-			Mac:     intf.Address.Mac,
-			Sandbox: netVariables.NetNS,
-		})
-
-		_, ipnet, err := netutil.ParseCIDR(intf.Address.IpAddress)
+		result.Interfaces = append(result.Interfaces,
+			&cni.Interface{
+				Name:    netVariables.IfName,
+				Mac:     intf.Address.Mac,
+				Sandbox: netVariables.NetNS,
+			})
+		ipAddr, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%s", intf.Address.IpAddress, intf.Address.IpPrefix))
 		if err != nil {
 			return result, tracelog.String(), err
 		}
-		result.IPs = append(result.IPs, &cniTypesVer.IPConfig{
-			Version:   intf.Address.Version,
-			Address:   *ipnet,
-			Gateway:   netutil.ParseIP(intf.Address.GatewayIp),
-			Interface: cniTypesVer.Int(index),
-		})
+		result.IPs = append(result.IPs,
+			&cni.IPConfig{
+				Version:   intf.Address.Version,
+				Interface: &idx,
+				Address:   net.IPNet{IP: ipAddr, Mask: ipNet.Mask},
+				Gateway:   net.ParseIP(intf.Address.GatewayIp),
+			})
 	}
 
 	return result, tracelog.String(), nil
 }
 
-func DoCmdDel(netVariables *object.NetVariables, stdinData []byte) (cniTypesVer.Result, string, error) {
+func DoCmdDel(netVariables *object.NetVariables, stdinData []byte) (cni.Result, string, error) {
 	tracelog := strings.Builder{}
-	result := cniTypesVer.Result{
-		CNIVersion: netVariables.CniVersion,
-	}
+	result := cni.Result{}
 
 	if err := netvariablesutil.LoadCniConfig(netVariables, stdinData); err != nil {
 		return result, tracelog.String(), err
 	}
+	result.CNIVersion = netVariables.CniVersion
 	tracelog.WriteString(fmt.Sprintf("CNI_DEL: Deleting NetNS: '%s'\n", netVariables.NetNS))
 	netutil.DeleteNetNS(netVariables.NetNS)
 

diff --git a/cmd/mizarcni/app/worker_test.go b/cmd/mizarcni/app/worker_test.go
@@ -134,7 +134,8 @@ func Test_DoCmdAdd(t *testing.T) {
 
 		Convey("Given ParseCIDR error, got expected result", func() {
 			expectedInfo := "Expected Info"
-			expectedError := errors.New("Expected Error")
+			expectedErrorStr := "invalid CIDR address"
+			expectedError := errors.New(expectedErrorStr)
 			patches := ApplyFunc(netvariablesutil.LoadCniConfig, func(_ *object.NetVariables, _ []byte) error {
 				return nil
 			})
@@ -165,11 +166,12 @@ func Test_DoCmdAdd(t *testing.T) {
 			So(tracelog, ShouldContainSubstring, "CNI_ADD: Args: ")
 			So(tracelog, ShouldContainSubstring, "Activating interface")
 			So(tracelog, ShouldContainSubstring, expectedInfo)
-			So(err, ShouldEqual, expectedError)
+			So(err.Error(), ShouldContainSubstring, expectedErrorStr)
 		})
 
 		Convey("Given no error, got expected result", func() {
 			expectedInfo := "Expected Info"
+			expectedErrorStr := "invalid CIDR address"
 			patches := ApplyFunc(netvariablesutil.LoadCniConfig, func(_ *object.NetVariables, _ []byte) error {
 				return nil
 			})
@@ -201,7 +203,7 @@ func Test_DoCmdAdd(t *testing.T) {
 			So(tracelog, ShouldContainSubstring, "CNI_ADD: Args: ")
 			So(tracelog, ShouldContainSubstring, "Activating interface")
 			So(tracelog, ShouldContainSubstring, expectedInfo)
-			So(err, ShouldBeNil)
+			So(err.Error(), ShouldContainSubstring, expectedErrorStr)
 		})
 	})
 }

diff --git a/cmd/mizarcni/mizarcni.go b/cmd/mizarcni/mizarcni.go
@@ -65,6 +65,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		klog.Infof("CNI_ADD: Success - interface added for pod '%s/%s'\n",
 				netVariables.K8sPodNamespace, netVariables.K8sPodName)
 	}
+	klog.Infof("CNI_ADD: RESULT= '%+v'\n", result)
 	klog.Infof("CNI_ADD: <<<<\n--------------------------------\n")
 	result.Print()
 	return err
@@ -84,6 +85,7 @@ func cmdDel(args *skel.CmdArgs) error {
 		klog.Infof("CNI_DEL: Success - interface deleted for pod '%s/%s'\n",
 				netVariables.K8sPodNamespace, netVariables.K8sPodName)
 	}
+	klog.Infof("CNI_DEL: RESULT= '%+v'\n", result)
 	klog.Infof("CNI_DEL: <<<<\n--------------------------------\n")
 	result.Print()
 	return err

diff --git a/go.mod b/go.mod
@@ -11,7 +11,6 @@ require (
 	github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852
 	golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c
 	google.golang.org/grpc v1.39.0
-	google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 // indirect
 	google.golang.org/protobuf v1.26.0
 	k8s.io/klog/v2 v2.10.0
 )