Merge RuntimeConf on delPlugins

RuntimeConfig depends on the delegate configuration. The Netconf runtime
information should be merged with the delegate config and the result
added to each command that is sent to the delegates. That was being done
for all commands except for delPlugins.

Do not export MergeCNIRuntimeConfig and call it from
CreateCNIRuntimeConf that now accepts a delegate ptr.

Call it from delPlugins for each delegate

Signed-off-by: Billy McFall <22157057+Billy99@users.noreply.github.com>

multus/multus.go

@@ -443,16 +443,19 @@ func delegateDel(exec invoke.Exec, pod *v1.Pod, ifName string, delegateConf *typ
 	return err
 }
 
-func delPlugins(exec invoke.Exec, pod *v1.Pod, argIfname string, delegates []*types.DelegateNetConf, lastIdx int, rt *libcni.RuntimeConf, binDir string) error {
+// delPlugins deletes plugins in reverse order from lastdIdx
-	logging.Debugf("delPlugins: %v, %v, %s, %v, %d, %v, %s", exec, pod, argIfname, delegates, lastIdx, rt, binDir)
+// Uses netRt as base RuntimeConf (coming from NetConf) but merges it
+// with each of the delegates' configuration
+func delPlugins(exec invoke.Exec, pod *v1.Pod, args *skel.CmdArgs, k8sArgs *types.K8sArgs, delegates []*types.DelegateNetConf, lastIdx int, netRt *types.RuntimeConfig, binDir string) error {
+	logging.Debugf("delPlugins: %v, %v, %v, %v, %v, %d, %v, %s", exec, pod, args, k8sArgs, delegates, lastIdx, netRt, binDir)
 	if os.Setenv("CNI_COMMAND", "DEL") != nil {
-		return logging.Errorf("delPlugins: error setting envionment variable CNI_COMMAND to a value of DEL")
+		return logging.Errorf("delPlugins: error setting environment variable CNI_COMMAND to a value of DEL")
 	}
 
 	var errorstrings []string
 	for idx := lastIdx; idx >= 0; idx-- {
-		ifName := getIfname(delegates[idx], argIfname, idx)
+		ifName := getIfname(delegates[idx], args.IfName, idx)
-		rt.IfName = ifName
+		rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, netRt, delegates[idx])
 		// Attempt to delete all but do not error out, instead, collect all errors.
 		if err := delegateDel(exec, pod, ifName, delegates[idx], rt, binDir); err != nil {
 			errorstrings = append(errorstrings, err.Error())
@@ -563,8 +566,7 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c
 	for idx, delegate := range n.Delegates {
 		ifName := getIfname(delegate, args.IfName, idx)
 
-		runtimeConfig := types.MergeCNIRuntimeConfig(n.RuntimeConfig, delegate)
+		rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, n.RuntimeConfig, delegate)
-		rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, runtimeConfig)
 		tmpResult, err = delegateAdd(exec, kubeClient, pod, ifName, delegate, rt, n.BinDir, cniArgs)
 		if err != nil {
 			// If the add failed, tear down all networks we already added
@@ -573,7 +575,7 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c
 				netName = delegate.ConfList.Name
 			}
 			// Ignore errors; DEL must be idempotent anyway
-			_ = delPlugins(exec, nil, args.IfName, n.Delegates, idx, rt, n.BinDir)
+			_ = delPlugins(exec, nil, args, k8sArgs, n.Delegates, idx, n.RuntimeConfig, n.BinDir)
 			return nil, cmdPluginErr(k8sArgs, netName, "error adding container to network %q: %v", netName, err)
 		}
 
@@ -656,8 +658,7 @@ func cmdCheck(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo)
 	for idx, delegate := range in.Delegates {
 		ifName := getIfname(delegate, args.IfName, idx)
 
-		runtimeConfig := types.MergeCNIRuntimeConfig(in.RuntimeConfig, delegate)
+		rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, in.RuntimeConfig, delegate)
-		rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, runtimeConfig)
 		err = delegateCheck(exec, ifName, delegate, rt, in.BinDir)
 		if err != nil {
 			return err
@@ -805,8 +806,7 @@ func cmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er
 		}
 	}
 
-	rt := types.CreateCNIRuntimeConf(args, k8sArgs, "", in.RuntimeConfig)
+	return delPlugins(exec, pod, args, k8sArgs, in.Delegates, len(in.Delegates)-1, in.RuntimeConfig, in.BinDir)
-	return delPlugins(exec, pod, args.IfName, in.Delegates, len(in.Delegates)-1, rt, in.BinDir)
 }
 
 func main() {

multus/multus_test.go

@@ -2031,7 +2031,7 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() {
 		rawnetconflist := []byte(`{"cniVersion":"0.2.0","name":"weave1","type":"weave-net"}`)
 		k8sargs, err := k8sclient.GetK8sArgs(args)
 		n, err := types.LoadNetConf(args.StdinData)
-		rt := types.CreateCNIRuntimeConf(args, k8sargs, args.IfName, n.RuntimeConfig)
+		rt := types.CreateCNIRuntimeConf(args, k8sargs, args.IfName, n.RuntimeConfig, nil)
 
 		err = conflistDel(rt, rawnetconflist, binDir, fExec)
 		Expect(err).To(HaveOccurred())
@@ -3277,7 +3277,7 @@ var _ = Describe("multus operations cniVersion 0.4.0 config", func() {
 		rawnetconflist := []byte(`{"cniVersion":"0.4.0","name":"weave1","type":"weave-net"}`)
 		k8sargs, err := k8sclient.GetK8sArgs(args)
 		n, err := types.LoadNetConf(args.StdinData)
-		rt := types.CreateCNIRuntimeConf(args, k8sargs, args.IfName, n.RuntimeConfig)
+		rt := types.CreateCNIRuntimeConf(args, k8sargs, args.IfName, n.RuntimeConfig, nil)
 
 		err = conflistDel(rt, rawnetconflist, binDir, fExec)
 		Expect(err).To(HaveOccurred())

types/conf.go

@@ -129,16 +129,16 @@ func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID st
 	return delegateConf, nil
 }
 
-// MergeCNIRuntimeConfig creates CNI runtimeconfig from delegate
+// mergeCNIRuntimeConfig creates CNI runtimeconfig from delegate
-func MergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetConf) *RuntimeConfig {
+func mergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetConf) *RuntimeConfig {
-	logging.Debugf("MergeCNIRuntimeConfig: %v %v", runtimeConfig, delegate)
+	logging.Debugf("mergeCNIRuntimeConfig: %v %v", runtimeConfig, delegate)
 	if runtimeConfig == nil {
 		runtimeConfig = &RuntimeConfig{}
 	}
 
 	// multus inject RuntimeConfig only in case of non MasterPlugin.
 	if delegate.MasterPlugin != true {
-		logging.Debugf("MergeCNIRuntimeConfig: add runtimeConfig for net-attach-def: %v", runtimeConfig)
+		logging.Debugf("mergeCNIRuntimeConfig: add runtimeConfig for net-attach-def: %v", runtimeConfig)
 		if delegate.PortMappingsRequest != nil {
 			runtimeConfig.PortMaps = delegate.PortMappingsRequest
 		}
@@ -159,9 +159,15 @@ func MergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetCo
 	return runtimeConfig
 }
 
-// CreateCNIRuntimeConf create CNI RuntimeConf
+// CreateCNIRuntimeConf create CNI RuntimeConf for a delegate. If delegate configuration
-func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig) *libcni.RuntimeConf {
+// exists, merge data with the runtime config.
-	logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v", args, k8sArgs, ifName, rc)
+func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig, delegate *DelegateNetConf) *libcni.RuntimeConf {
+	logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v %v", args, k8sArgs, ifName, rc, delegate)
+
+	delegateRc := rc
+	if delegate != nil {
+		delegateRc = mergeCNIRuntimeConfig(delegateRc, delegate)
+	}
 
 	// In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go#buildCNIRuntimeConf
 	// Todo
@@ -179,22 +185,22 @@ func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, r
 		},
 	}
 
-	if rc != nil {
+	if delegateRc != nil {
 		capabilityArgs := map[string]interface{}{}
-		if len(rc.PortMaps) != 0 {
+		if len(delegateRc.PortMaps) != 0 {
-			capabilityArgs["portMappings"] = rc.PortMaps
+			capabilityArgs["portMappings"] = delegateRc.PortMaps
 		}
-		if rc.Bandwidth != nil {
+		if delegateRc.Bandwidth != nil {
-			capabilityArgs["bandwidth"] = rc.Bandwidth
+			capabilityArgs["bandwidth"] = delegateRc.Bandwidth
 		}
-		if len(rc.IPs) != 0 {
+		if len(delegateRc.IPs) != 0 {
-			capabilityArgs["ips"] = rc.IPs
+			capabilityArgs["ips"] = delegateRc.IPs
 		}
-		if len(rc.Mac) != 0 {
+		if len(delegateRc.Mac) != 0 {
-			capabilityArgs["mac"] = rc.Mac
+			capabilityArgs["mac"] = delegateRc.Mac
 		}
-		if len(rc.InfinibandGUID) != 0 {
+		if len(delegateRc.InfinibandGUID) != 0 {
-			capabilityArgs["infinibandGUID"] = rc.InfinibandGUID
+			capabilityArgs["infinibandGUID"] = delegateRc.InfinibandGUID
 		}
 		rt.CapabilityArgs = capabilityArgs
 	}

types/conf_test.go

@@ -565,7 +565,7 @@ var _ = Describe("config operations", func() {
 			HostIP:        "anotherSampleHostIP",
 		}
 
-		rt := CreateCNIRuntimeConf(args, k8sArgs, "", rc)
+		rt := CreateCNIRuntimeConf(args, k8sArgs, "", rc, nil)
 		fmt.Println("rt.ContainerID: ", rt.ContainerID)
 		Expect(rt.ContainerID).To(Equal("123456789"))
 		Expect(rt.NetNS).To(Equal(args.Netns))
@@ -678,7 +678,7 @@ var _ = Describe("config operations", func() {
 		Expect(delegateConf.PortMappingsRequest).To(Equal(networkSelection.PortMappingsRequest))
 	})
 
-	It("test MergeCNIRuntimeConfig with masterPlugin", func() {
+	It("test mergeCNIRuntimeConfig with masterPlugin", func() {
 		conf := `{
 			"name": "node-cni-network",
 			"type": "multus",
@@ -717,13 +717,13 @@ var _ = Describe("config operations", func() {
 		delegate, err := LoadDelegateNetConf([]byte(conf), networkSelection, "")
 		delegate.MasterPlugin = true
 		Expect(err).NotTo(HaveOccurred())
-		runtimeConf := MergeCNIRuntimeConfig(&RuntimeConfig{}, delegate)
+		runtimeConf := mergeCNIRuntimeConfig(&RuntimeConfig{}, delegate)
 		Expect(runtimeConf.PortMaps).To(BeNil())
 		Expect(runtimeConf.Bandwidth).To(BeNil())
 		Expect(runtimeConf.InfinibandGUID).To(Equal(""))
 	})
 
-	It("test MergeCNIRuntimeConfig with delegate plugin", func() {
+	It("test mergeCNIRuntimeConfig with delegate plugin", func() {
 		conf := `{
 			"name": "weave1",
 			"cniVersion": "0.2.0",
@@ -753,7 +753,7 @@ var _ = Describe("config operations", func() {
 		}
 		delegate, err := LoadDelegateNetConf([]byte(conf), networkSelection, "")
 		Expect(err).NotTo(HaveOccurred())
-		runtimeConf := MergeCNIRuntimeConfig(&RuntimeConfig{}, delegate)
+		runtimeConf := mergeCNIRuntimeConfig(&RuntimeConfig{}, delegate)
 		Expect(runtimeConf.PortMaps).NotTo(BeNil())
 		Expect(len(runtimeConf.PortMaps)).To(BeEquivalentTo(1))
 		Expect(runtimeConf.PortMaps[0]).To(Equal(portMapEntry1))