From 366f2120cb88c85deab6343b7062fd38fdb0ece9 Mon Sep 17 00:00:00 2001 From: Ritu Sood Date: Wed, 10 Oct 2018 12:30:37 +0000 Subject: [PATCH 1/2] Merge Results from delegates Currently Master Plugin result is returned by Multus. This patch merges the results from all delegates as per the CNI spec. This fixes issues of Multus and Virtlet. Signed-off-by: Ritu Sood --- multus/multus.go | 110 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 6 deletions(-) diff --git a/multus/multus.go b/multus/multus.go index c4ae13aca..b0419fc09 100644 --- a/multus/multus.go +++ b/multus/multus.go @@ -22,6 +22,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "os" "path/filepath" @@ -29,6 +30,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" "github.com/containernetworking/cni/pkg/version" "github.com/containernetworking/plugins/pkg/ns" k8s "github.com/intel/multus-cni/k8sclient" @@ -215,6 +217,104 @@ func delPlugins(exec invoke.Exec, argIfname string, delegates []*types.DelegateN return nil } +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 -1 +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 = current.Int(-1) + } + } + return result, nil +} func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cnitypes.Result, error) { logging.Debugf("cmdAdd: %v, %v, %v", args, exec, kubeClient) @@ -252,10 +352,10 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn if err != nil { break } - - // 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 @@ -270,7 +370,6 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn } } } - if err != nil { // Ignore errors; DEL must be idempotent anyway _ = delPlugins(exec, args.IfName, n.Delegates, lastIdx, rt, n.BinDir) @@ -286,7 +385,6 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn } } } - return result, nil } From b90f0e2ed68df60571ac49d8322d086d5804934f Mon Sep 17 00:00:00 2001 From: Ritu Sood Date: Sun, 20 Jan 2019 02:18:36 -0800 Subject: [PATCH 2/2] Set interface to nil Signed-off-by: Ritu Sood --- multus/multus.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/multus/multus.go b/multus/multus.go index b0419fc09..0e2fe512b 100644 --- a/multus/multus.go +++ b/multus/multus.go @@ -302,7 +302,7 @@ func updateRoutes(rObj cnitypes.Result) (cnitypes.Result, error) { // 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 -1 +// 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 { @@ -310,7 +310,7 @@ func fixInterfaces(rObj cnitypes.Result) (cnitypes.Result, error) { } if len(result.Interfaces) == 0 { for _, ip := range result.IPs { - ip.Interface = current.Int(-1) + ip.Interface = nil } } return result, nil