multus/types: handle delegates as Go structs and other cleanups

k8sclient/k8sclient.go

@@ -25,10 +25,10 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
 
-	"github.com/intel/multus-cni/types"
 	"github.com/containernetworking/cni/libcni"
 	"github.com/containernetworking/cni/pkg/skel"
 	cnitypes "github.com/containernetworking/cni/pkg/types"
+	"github.com/intel/multus-cni/types"
 )
 
 // NoK8sNetworkError indicates error, no network in kubernetes
@@ -335,62 +335,54 @@ func getPodNetworkObj(client *kubernetes.Clientset, netObjs []map[string]interfa
 	return netconf, nil
 }
 
-func getMultusDelegates(delegate string) ([]map[string]interface{}, error) {
-	tmpNetconf := &types.NetConf{}
-	tmpDelegate := "{\"delegates\": " + delegate + "}"
-
+func getMultusDelegates(delegate string) ([]*types.DelegateNetConf, error) {
 	if delegate == "" {
 		return nil, fmt.Errorf("getMultusDelegates: TPR network obj data can't be empty")
 	}
 
-	if err := json.Unmarshal([]byte(tmpDelegate), tmpNetconf); err != nil {
+	n, err := types.LoadNetConf([]byte("{\"delegates\": " + delegate + "}"))
+	if err != nil {
 		return nil, fmt.Errorf("getMultusDelegates: failed to load netconf for delegate %v: %v", delegate, err)
 	}
 
-	if tmpNetconf.Delegates == nil {
+	if len(n.Delegates) == 0 {
 		return nil, fmt.Errorf(`getMultusDelegates: "delegates" is must, refer Multus README.md for the usage guide`)
 	}
 
-	return tmpNetconf.Delegates, nil
+	return n.Delegates, nil
 }
 
-func GetK8sNetwork(args *skel.CmdArgs, kubeconfig string, confdir string) ([]map[string]interface{}, error) {
+func GetK8sNetwork(args *skel.CmdArgs, kubeconfig, confdir string) ([]*types.DelegateNetConf, error) {
 	k8sArgs := types.K8sArgs{}
-	var podNet []map[string]interface{}
 
 	err := cnitypes.LoadArgs(args.Args, &k8sArgs)
 	if err != nil {
-		return podNet, err
+		return nil, err
 	}
 
 	k8sclient, err := createK8sClient(kubeconfig)
 	if err != nil {
-		return podNet, err
+		return nil, err
 	}
 
 	netAnnot, err := getPodNetworkAnnotation(k8sclient, k8sArgs)
 	if err != nil {
-		return podNet, err
+		return nil, err
 	}
 
 	if len(netAnnot) == 0 {
-		return podNet, &NoK8sNetworkError{"no kubernetes network found"}
+		return nil, &NoK8sNetworkError{"no kubernetes network found"}
 	}
 
 	netObjs, err := parsePodNetworkObject(netAnnot)
 	if err != nil {
-		return podNet, err
+		return nil, err
 	}
 
 	multusDelegates, err := getPodNetworkObj(k8sclient, netObjs, confdir)
 	if err != nil {
-		return podNet, err
+		return nil, err
 	}
 
-	podNet, err = getMultusDelegates(multusDelegates)
-	if err != nil {
-		return podNet, err
-	}
-
-	return podNet, nil
+	return getMultusDelegates(multusDelegates)
 }

multus/multus.go

@@ -25,66 +25,16 @@ import (
 	"os"
 	"path/filepath"
 
-	k8s "github.com/intel/multus-cni/k8sclient"
-	"github.com/intel/multus-cni/types"
 	"github.com/containernetworking/cni/pkg/invoke"
 	"github.com/containernetworking/cni/pkg/ns"
 	"github.com/containernetworking/cni/pkg/skel"
+	cnitypes "github.com/containernetworking/cni/pkg/types"
 	"github.com/containernetworking/cni/pkg/version"
+	k8s "github.com/intel/multus-cni/k8sclient"
+	"github.com/intel/multus-cni/types"
 	"github.com/vishvananda/netlink"
 )
 
-const (
-	defaultCNIDir  = "/var/lib/cni/multus"
-	defaultConfDir = "/etc/cni/multus/net.d"
-)
-
-var masterpluginEnabled bool
-var defaultcninetwork bool
-
-//taken from cni/plugins/meta/flannel/flannel.go
-func isString(i interface{}) bool {
-	_, ok := i.(string)
-	return ok
-}
-
-func isBool(i interface{}) bool {
-	_, ok := i.(bool)
-	return ok
-}
-
-func loadNetConf(bytes []byte) (*types.NetConf, error) {
-	netconf := &types.NetConf{}
-	if err := json.Unmarshal(bytes, netconf); err != nil {
-		return nil, fmt.Errorf("failed to load netconf: %v", err)
-	}
-
-	if netconf.Kubeconfig != "" && netconf.Delegates != nil {
-		defaultcninetwork = true
-	}
-
-	if netconf.CNIDir == "" {
-		netconf.CNIDir = defaultCNIDir
-	}
-
-	if netconf.ConfDir == "" {
-		netconf.ConfDir = defaultConfDir
-	}
-
-	if netconf.Kubeconfig == "" || !defaultcninetwork {
-		return nil, fmt.Errorf(`You must also set the delegates & the kubeconfig, refer to the README`)
-	}
-
-	if len(netconf.Delegates) == 0 && !defaultcninetwork {
-		return nil, fmt.Errorf(`delegates or kubeconfig option is must, refer README.md`)
-	}
-
-	// default network in multus conf as master plugin
-	netconf.Delegates[0]["masterplugin"] = true
-
-	return netconf, nil
-}
-
 func saveScratchNetConf(containerID, dataDir string, netconf []byte) error {
 	if err := os.MkdirAll(dataDir, 0700); err != nil {
 		return fmt.Errorf("failed to create the multus data directory(%q): %v", dataDir, err)
@@ -107,18 +57,21 @@ func consumeScratchNetConf(containerID, dataDir string) ([]byte, error) {
 	return ioutil.ReadFile(path)
 }
 
-func getifname() (f func() string) {
-	var interfaceIndex int
-	f = func() string {
-		ifname := fmt.Sprintf("net%d", interfaceIndex)
-		interfaceIndex++
-		return ifname
+func getIfname(delegate *types.DelegateNetConf, argif string, idx int) string {
+	if delegate.IfnameRequest != "" {
+		return delegate.IfnameRequest
+	}
+	if delegate.MasterPlugin {
+		// master plugin always uses the CNI-provided interface name
+		return argif
 	}
 
-	return
+	// Otherwise construct a unique interface name from the delegate's
+	// position in the delegate list
+	return fmt.Sprintf("net%d", idx)
 }
 
-func saveDelegates(containerID, dataDir string, delegates []map[string]interface{}) error {
+func saveDelegates(containerID, dataDir string, delegates []*types.DelegateNetConf) error {
 	delegatesBytes, err := json.Marshal(delegates)
 	if err != nil {
 		return fmt.Errorf("error serializing delegate netconf: %v", err)
@@ -131,43 +84,6 @@ func saveDelegates(containerID, dataDir string, delegates []map[string]interface
 	return err
 }
 
-func checkDelegate(netconf map[string]interface{}) error {
-	if netconf["type"] == nil {
-		return fmt.Errorf("delegate must have the field 'type'")
-	}
-
-	if !isString(netconf["type"]) {
-		return fmt.Errorf("delegate field 'type' must be a string")
-	}
-
-	if netconf["masterplugin"] != nil {
-		if !isBool(netconf["masterplugin"]) {
-			return fmt.Errorf("delegate field 'masterplugin' must be a bool")
-		}
-	}
-
-	if netconf["masterplugin"] != nil {
-		if netconf["masterplugin"].(bool) != false && masterpluginEnabled != true {
-			masterpluginEnabled = true
-		} else if netconf["masterplugin"].(bool) != false && masterpluginEnabled == true {
-			return fmt.Errorf("only one delegate can have 'masterplugin'")
-		}
-	}
-	return nil
-}
-
-func isMasterplugin(netconf map[string]interface{}) bool {
-	if netconf["masterplugin"] == nil {
-		return false
-	}
-
-	if netconf["masterplugin"].(bool) == true {
-		return true
-	}
-
-	return false
-}
-
 func validateIfName(nsname string, ifname string) error {
 	podNs, err := ns.GetNS(nsname)
 	if err != nil {
@@ -188,89 +104,44 @@ func validateIfName(nsname string, ifname string) error {
 	return err
 }
 
-func delegateAdd(podif func() string, argif string, netconf map[string]interface{}, onlyMaster bool) (bool, error) {
-	netconfBytes, err := json.Marshal(netconf)
+func delegateAdd(ifName string, delegate *types.DelegateNetConf) (cnitypes.Result, error) {
+	if os.Setenv("CNI_IFNAME", ifName) != nil {
+		return nil, fmt.Errorf("Multus: error in setting CNI_IFNAME")
+	}
+
+	if err := validateIfName(os.Getenv("CNI_NETNS"), ifName); err != nil {
+		return nil, fmt.Errorf("cannot set %q ifname to %q: %v", delegate.Type, ifName, err)
+	}
+
+	result, err := invoke.DelegateAdd(delegate.Type, delegate.Bytes)
 	if err != nil {
-		return true, fmt.Errorf("Multus: error serializing multus delegate netconf: %v", err)
+		return nil, fmt.Errorf("Multus: error in invoke Delegate add - %q: %v", delegate.Type, err)
 	}
 
-	if isMasterplugin(netconf) != onlyMaster {
-		return true, nil
-	}
-
-	if !isMasterplugin(netconf) {
-		if os.Setenv("CNI_IFNAME", podif()) != nil {
-			return true, fmt.Errorf("Multus: error in setting CNI_IFNAME")
-		}
-	} else {
-		if os.Setenv("CNI_IFNAME", argif) != nil {
-			return true, fmt.Errorf("Multus: error in setting CNI_IFNAME")
-		}
-	}
-
-	if netconf["ifnameRequest"] != nil {
-		if os.Setenv("CNI_IFNAME", netconf["ifnameRequest"].(string)) != nil {
-			return true, fmt.Errorf("Multus: error in setting CNI_IFNAME")
-		}
-	}
-
-	err = validateIfName(os.Getenv("CNI_NETNS"), os.Getenv("CNI_IFNAME"))
-	if err != nil {
-		return true, fmt.Errorf("cannot set %q ifname: %v", netconf["type"].(string), err)
-	}
-
-	result, err := invoke.DelegateAdd(netconf["type"].(string), netconfBytes)
-	if err != nil {
-		return true, fmt.Errorf("Multus: error in invoke Delegate add - %q: %v", netconf["type"].(string), err)
-	}
-
-	if !isMasterplugin(netconf) {
-		return true, nil
-	}
-
-	return false, result.Print()
+	return result, nil
 }
 
-func delegateDel(podif func() string, argif string, netconf map[string]interface{}) error {
-	netconfBytes, err := json.Marshal(netconf)
-	if err != nil {
-		return fmt.Errorf("Multus: error serializing multus delegate netconf: %v", err)
+func delegateDel(ifName string, delegateConf *types.DelegateNetConf) error {
+	if os.Setenv("CNI_IFNAME", ifName) != nil {
+		return fmt.Errorf("Multus: error in setting CNI_IFNAME")
 	}
 
-	if !isMasterplugin(netconf) {
-		if os.Setenv("CNI_IFNAME", podif()) != nil {
-			return fmt.Errorf("Multus: error in setting CNI_IFNAME")
-		}
-	} else {
-		if os.Setenv("CNI_IFNAME", argif) != nil {
-			return fmt.Errorf("Multus: error in setting CNI_IFNAME")
-		}
+	if err := invoke.DelegateDel(delegateConf.Type, delegateConf.Bytes); err != nil {
+		return fmt.Errorf("Multus: error in invoke Delegate del - %q: %v", delegateConf.Type, err)
 	}
 
-	err = invoke.DelegateDel(netconf["type"].(string), netconfBytes)
-	if err != nil {
-		return fmt.Errorf("Multus: error in invoke Delegate del - %q: %v", netconf["type"].(string), err)
-	}
-
-	return err
+	return nil
 }
 
-func clearPlugins(mIdx int, pIdx int, argIfname string, delegates []map[string]interface{}) error {
-
+func delPlugins(argIfname string, delegates []*types.DelegateNetConf, lastIdx int) error {
 	if os.Setenv("CNI_COMMAND", "DEL") != nil {
 		return fmt.Errorf("Multus: error in setting CNI_COMMAND to DEL")
 	}
 
-	podifName := getifname()
-	r := delegateDel(podifName, argIfname, delegates[mIdx])
-	if r != nil {
-		return r
-	}
-
-	for idx := 0; idx < pIdx && idx != mIdx; idx++ {
-		r := delegateDel(podifName, argIfname, delegates[idx])
-		if r != nil {
-			return r
+	for idx := lastIdx; idx >= 0; idx-- {
+		ifName := getIfname(delegates[idx], argIfname, idx)
+		if err := delegateDel(ifName, delegates[idx]); err != nil {
+			return err
 		}
 	}
 
@@ -278,40 +149,24 @@ func clearPlugins(mIdx int, pIdx int, argIfname string, delegates []map[string]i
 }
 
 func cmdAdd(args *skel.CmdArgs) error {
-	var result error
 	var nopodnet bool
-	n, err := loadNetConf(args.StdinData)
+	n, err := types.LoadNetConf(args.StdinData)
 	if err != nil {
 		return fmt.Errorf("err in loading netconf: %v", err)
 	}
 
 	if n.Kubeconfig != "" {
-		podDelegate, err := k8s.GetK8sNetwork(args, n.Kubeconfig, n.ConfDir)
+		delegates, err := k8s.GetK8sNetwork(args, n.Kubeconfig, n.ConfDir)
 		if err != nil {
 			if _, ok := err.(*k8s.NoK8sNetworkError); ok {
 				nopodnet = true
-				if !defaultcninetwork {
-					return fmt.Errorf("Multus: Err in getting k8s network from the pod spec annotation, check the pod spec or set delegate for the default network, Refer the README.md: %v", err)
-				}
-			} else if !defaultcninetwork {
+			} else {
 				return fmt.Errorf("Multus: Err in getting k8s network from pod: %v", err)
 			}
 		}
 
-		// If it's empty just leave it as the netconfig states (e.g. just default)
-		if len(podDelegate) != 0 {
-			// In the case that we force the default
-			// We add the found configs from CRD
-			for _, eachDelegate := range podDelegate {
-				eachDelegate["masterplugin"] = false
-				n.Delegates = append(n.Delegates, eachDelegate)
-			}
-		}
-	}
-
-	for _, delegate := range n.Delegates {
-		if err := checkDelegate(delegate); err != nil {
-			return fmt.Errorf("Multus: Err in delegate conf: %v", err)
+		if err = n.AddDelegates(delegates); err != nil {
+			return err
 		}
 	}
 
@@ -321,65 +176,50 @@ func cmdAdd(args *skel.CmdArgs) error {
 		}
 	}
 
-	podifName := getifname()
-	var mIndex int
-	for index, delegate := range n.Delegates {
-		err, r := delegateAdd(podifName, args.IfName, delegate, true)
-		if err != true {
-			result = r
-			mIndex = index
-		} else if (err != false) && r != nil {
-			return r
+	var result, tmpResult cnitypes.Result
+	lastIdx := 0
+	for idx, delegate := range n.Delegates {
+		lastIdx = idx
+		ifName := getIfname(delegate, args.IfName, idx)
+		tmpResult, err = delegateAdd(ifName, delegate)
+		if err != nil {
+			break
+		}
+
+		// Master plugin result is always used if present
+		if delegate.MasterPlugin || result == nil {
+			result = tmpResult
 		}
 	}
-
-	for index, delegate := range n.Delegates {
-		err, r := delegateAdd(podifName, args.IfName, delegate, false)
-		if err != true {
-			result = r
-		} else if (err != false) && r != nil {
-			perr := clearPlugins(mIndex, index, args.IfName, n.Delegates)
-			if perr != nil {
-				return perr
-			}
-			return r
-		}
+	if err != nil {
+		// Ignore errors; DEL must be idempotent anyway
+		_ = delPlugins(args.IfName, n.Delegates, lastIdx)
+		return err
 	}
 
-	return result
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {
-	var result error
 	var nopodnet bool
 
-	in, err := loadNetConf(args.StdinData)
+	in, err := types.LoadNetConf(args.StdinData)
 	if err != nil {
 		return err
 	}
 
 	if in.Kubeconfig != "" {
-		podDelegate, r := k8s.GetK8sNetwork(args, in.Kubeconfig, in.ConfDir)
+		delegates, r := k8s.GetK8sNetwork(args, in.Kubeconfig, in.ConfDir)
 		if r != nil {
 			if _, ok := r.(*k8s.NoK8sNetworkError); ok {
 				nopodnet = true
-				// no network found from default and annotaed network,
-				// we do nothing to remove network for the pod!
-				if !defaultcninetwork {
-					return fmt.Errorf("Multus: Err in getting k8s network from the poc spec, check the pod spec or set delegate for the default network, Refer the README.md: %v", r)
-				}
 			} else {
 				return fmt.Errorf("Multus: Err in getting k8s network from pod: %v", r)
 			}
 		}
 
-		if len(podDelegate) != 0 {
-			// In the case that we force the default
-			// We add the found configs from CRD (in reverse order)
-			for i := len(podDelegate) - 1; i >= 0; i-- {
-				podDelegate[i]["masterplugin"] = false
-				in.Delegates = append(in.Delegates, podDelegate[i])
-			}
+		if err = in.AddDelegates(delegates); err != nil {
+			return err
 		}
 	}
 
@@ -398,16 +238,7 @@ func cmdDel(args *skel.CmdArgs) error {
 		}
 	}
 
-	podifName := getifname()
-	for _, delegate := range in.Delegates {
-		r := delegateDel(podifName, args.IfName, delegate)
-		if r != nil {
-			return r
-		}
-		result = r
-	}
-
-	return result
+	return delPlugins(args.IfName, in.Delegates, len(in.Delegates)-1)
 }
 
 func main() {

types/conf.go

@@ -0,0 +1,106 @@
+// Copyright (c) 2017 Intel Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package types
+
+import (
+	"encoding/json"
+	"fmt"
+)
+
+const (
+	defaultCNIDir  = "/var/lib/cni/multus"
+	defaultConfDir = "/etc/cni/multus/net.d"
+)
+
+// Convert a raw delegate config map into a DelegateNetConf structure
+func loadDelegateNetConf(rawConf map[string]interface{}) (*DelegateNetConf, error) {
+	bytes, err := json.Marshal(rawConf)
+	if err != nil {
+		return nil, fmt.Errorf("error marshalling delegate config: %v", err)
+	}
+	delegateConf := &DelegateNetConf{}
+	if err = json.Unmarshal(bytes, delegateConf); err != nil {
+		return nil, fmt.Errorf("error unmarshalling delegate config: %v", err)
+	}
+	delegateConf.RawConfig = rawConf
+	delegateConf.Bytes = bytes
+
+	// Do some minimal validation
+	if delegateConf.Type == "" {
+		return nil, fmt.Errorf("delegate must have the 'type' field")
+	}
+
+	return delegateConf, nil
+}
+
+func LoadNetConf(bytes []byte) (*NetConf, error) {
+	netconf := &NetConf{}
+	if err := json.Unmarshal(bytes, netconf); err != nil {
+		return nil, fmt.Errorf("failed to load netconf: %v", err)
+	}
+
+	// Delegates must always be set. If no kubeconfig is present, the
+	// delegates are executed in-order.  If a kubeconfig is present,
+	// at least one delegate must be present and the first delegate is
+	// the master plugin. Kubernetes CRD delegates are then appended to
+	// the existing delegate list and all delegates executed in-order.
+
+	if len(netconf.RawDelegates) == 0 {
+		return nil, fmt.Errorf("at least one delegate must be specified")
+	}
+
+	if netconf.CNIDir == "" {
+		netconf.CNIDir = defaultCNIDir
+	}
+	if netconf.ConfDir == "" {
+		netconf.ConfDir = defaultConfDir
+	}
+
+	for idx, rawConf := range netconf.RawDelegates {
+		delegateConf, err := loadDelegateNetConf(rawConf)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load delegate %d config: %v", idx, err)
+		}
+		netconf.Delegates = append(netconf.Delegates, delegateConf)
+	}
+	netconf.RawDelegates = nil
+
+	// First delegate is always the master plugin
+	netconf.Delegates[0].MasterPlugin = true
+
+	return netconf, nil
+}
+
+func (d *DelegateNetConf) updateRawConfig() error {
+	if d.IfnameRequest != "" {
+		d.RawConfig["ifnameRequest"] = d.IfnameRequest
+	} else {
+		delete(d.RawConfig, "ifnameRequest")
+	}
+
+	bytes, err := json.Marshal(d.RawConfig)
+	if err != nil {
+		return err
+	}
+	d.Bytes = bytes
+	return nil
+}
+
+// AddDelegates appends the new delegates to the delegates list
+func (n *NetConf) AddDelegates(newDelegates []*DelegateNetConf) error {
+	n.Delegates = append(n.Delegates, newDelegates...)
+	return nil
+}

types/types.go

@@ -25,10 +25,24 @@ import (
 // NetConf for cni config file written in json
 type NetConf struct {
 	types.NetConf
-	ConfDir    string                   `json:"confDir"`
-	CNIDir     string                   `json:"cniDir"`
-	Delegates  []map[string]interface{} `json:"delegates"`
-	Kubeconfig string                   `json:"kubeconfig"`
+	ConfDir string `json:"confDir"`
+	CNIDir  string `json:"cniDir"`
+	// RawDelegates is private to the NetConf class; use Delegates instead
+	RawDelegates []map[string]interface{} `json:"delegates"`
+	Delegates    []*DelegateNetConf       `json:"-"`
+	Kubeconfig   string                   `json:"kubeconfig"`
+}
+
+type DelegateNetConf struct {
+	types.NetConf
+	IfnameRequest string `json:"ifnameRequest,omitempty"`
+	// MasterPlugin is only used internal housekeeping
+	MasterPlugin bool `json:"-"`
+
+	// Raw unmarshalled JSON
+	RawConfig map[string]interface{}
+	// Raw bytes
+	Bytes []byte
 }
 
 type Network struct {