Merge pull request #165 from ritusood/master

Merge Results from delegates

.goreleaser.yml

@@ -18,7 +18,7 @@ checksum:
   name_template: 'checksums.txt'
 snapshot:
   name_template: "{{ .Tag }}-snapshot"
-#release:
-#  draft: true
+release:
+  draft: true
 changelog:
   skip: true

multus/multus.go

@@ -33,6 +33,7 @@ import (
 	"github.com/containernetworking/cni/pkg/invoke"
 	"github.com/containernetworking/cni/pkg/skel"
 	cnitypes "github.com/containernetworking/cni/pkg/types"
+	"github.com/containernetworking/cni/pkg/types/current"
 	cniversion "github.com/containernetworking/cni/pkg/version"
 	"github.com/containernetworking/plugins/pkg/ns"
 	k8s "github.com/intel/multus-cni/k8sclient"
@@ -313,6 +314,107 @@ func delPlugins(exec invoke.Exec, argIfname string, delegates []*types.DelegateN
 	return nil
 }
 
+// Merge code based on CNI-Genie's code
+// https://github.com/Huawei-PaaS/CNI-Genie/blob/master/genie/genie-controller.go#L882
+func mergeWithResult(srcObj, dstObj cnitypes.Result) (cnitypes.Result, error) {
+	srcObj, err := updateRoutes(srcObj)
+	if err != nil {
+		return nil, logging.Errorf("Routes update failed: %v", err)
+	}
+
+	srcObj, err = fixInterfaces(srcObj)
+	if err != nil {
+		return nil, logging.Errorf("Failed to fix interfaces: %v", err)
+	}
+
+	if dstObj == nil {
+		return srcObj, nil
+	}
+	src, err := current.NewResultFromResult(srcObj)
+	if err != nil {
+		return nil, logging.Errorf("Couldn't convert old result to current version: %v", err)
+	}
+	dst, err := current.NewResultFromResult(dstObj)
+	if err != nil {
+		return nil, logging.Errorf("Couldn't convert old result to current version: %v", err)
+	}
+
+	ifacesLength := len(dst.Interfaces)
+
+	for _, iface := range src.Interfaces {
+		dst.Interfaces = append(dst.Interfaces, iface)
+	}
+	for _, ip := range src.IPs {
+		if ip.Interface != nil && *(ip.Interface) != -1 {
+			ip.Interface = current.Int(*(ip.Interface) + ifacesLength)
+		}
+		dst.IPs = append(dst.IPs, ip)
+	}
+	for _, route := range src.Routes {
+		dst.Routes = append(dst.Routes, route)
+	}
+
+	for _, ns := range src.DNS.Nameservers {
+		dst.DNS.Nameservers = append(dst.DNS.Nameservers, ns)
+	}
+	for _, s := range src.DNS.Search {
+		dst.DNS.Search = append(dst.DNS.Search, s)
+	}
+	for _, opt := range src.DNS.Options {
+		dst.DNS.Options = append(dst.DNS.Options, opt)
+	}
+	// TODO: what about DNS.domain?
+	return dst, nil
+}
+
+// updateRoutes changes nil gateway set in a route to a gateway from IPConfig
+// nil gw in route means default gw from result. When merging results from
+// many results default gw may be set from another CNI network. This may lead to
+// wrong routes.
+func updateRoutes(rObj cnitypes.Result) (cnitypes.Result, error) {
+	result, err := current.NewResultFromResult(rObj)
+	if err != nil {
+		return nil, logging.Errorf("Couldn't convert old result to current version: %v", err)
+	}
+	if len(result.Routes) == 0 {
+		return result, nil
+	}
+
+	var gw net.IP
+	for _, ip := range result.IPs {
+		if ip.Gateway != nil {
+			gw = ip.Gateway
+			break
+		}
+	}
+
+	for _, route := range result.Routes {
+		if route.GW == nil {
+			if gw == nil {
+				return nil, logging.Errorf("Couldn't find gw in result %v", result)
+			}
+			route.GW = gw
+		}
+	}
+	return result, nil
+}
+
+// fixInterfaces fixes bad result returned by CNI plugin
+// some plugins(for example calico) return empty Interfaces list but
+// in IPConfig sets Interface index to 0. In such case it should be nil
+func fixInterfaces(rObj cnitypes.Result) (cnitypes.Result, error) {
+	result, err := current.NewResultFromResult(rObj)
+	if err != nil {
+		return nil, logging.Errorf("Couldn't convert old result to current version: %v", err)
+	}
+	if len(result.Interfaces) == 0 {
+		for _, ip := range result.IPs {
+			ip.Interface = nil
+		}
+	}
+	return result, nil
+}
+
 func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cnitypes.Result, error) {
 	n, err := types.LoadNetConf(args.StdinData)
 	logging.Debugf("cmdAdd: %v, %v, %v", args, exec, kubeClient)
@@ -371,10 +473,10 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn
 			_ = delPlugins(exec, args.IfName, n.Delegates, idx, rt, n.BinDir)
 			return nil, logging.Errorf("Multus: Err adding pod to network %q: %v", netName, err)
 		}
-
-		// Master plugin result is always used if present
-		if delegate.MasterPlugin || result == nil {
-			result = tmpResult
+		// Merge results from delegates
+		result, err = mergeWithResult(tmpResult, result)
+		if err != nil {
+			return nil, logging.Errorf("Multus: Failed to merge results: %v", err)
 		}
 
 		//create the network status, only in case Multus as kubeconfig

multus/multus_test.go

@@ -27,7 +27,7 @@ import (
 
 	"github.com/containernetworking/cni/pkg/skel"
 	cnitypes "github.com/containernetworking/cni/pkg/types"
-	"github.com/containernetworking/cni/pkg/types/020"
+	types020 "github.com/containernetworking/cni/pkg/types/020"
 	cniversion "github.com/containernetworking/cni/pkg/version"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
@@ -227,7 +227,7 @@ var _ = Describe("multus operations", func() {
 		result, err := cmdAdd(args, fExec, nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 
@@ -375,7 +375,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(2))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 	})
@@ -454,7 +454,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(2))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 	})
@@ -514,7 +514,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(1))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 
@@ -614,7 +614,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(2))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 
 		os.Setenv("CNI_COMMAND", "DEL")
@@ -683,7 +683,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(1))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
 
@@ -761,7 +761,7 @@ var _ = Describe("multus operations", func() {
 		Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
 		Expect(fKubeClient.PodCount).To(Equal(2))
 		Expect(fKubeClient.NetCount).To(Equal(1))
-		r := result.(*types020.Result)
+		r, _ := result.GetAsVersion("0.2.0")
 		// plugin 1 is the masterplugin
 		Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())