From 665c43a2cd589cd2b8f146766afa97247eebda7c Mon Sep 17 00:00:00 2001 From: Billy McFall <22157057+Billy99@users.noreply.github.com> Date: Tue, 27 Oct 2020 17:21:28 -0400 Subject: [PATCH] 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 | 24 ++++++++++++------------ multus/multus_test.go | 4 ++-- types/conf.go | 42 ++++++++++++++++++++++++------------------ types/conf_test.go | 10 +++++----- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/multus/multus.go b/multus/multus.go index 0d4bbb345..2a5326a07 100644 --- a/multus/multus.go +++ b/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 { - logging.Debugf("delPlugins: %v, %v, %s, %v, %d, %v, %s", exec, pod, argIfname, delegates, lastIdx, rt, binDir) +// delPlugins deletes plugins in reverse order from lastdIdx +// 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) - rt.IfName = ifName + ifName := getIfname(delegates[idx], args.IfName, idx) + 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, runtimeConfig) + rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, n.RuntimeConfig, delegate) 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, runtimeConfig) + rt := types.CreateCNIRuntimeConf(args, k8sArgs, ifName, in.RuntimeConfig, delegate) 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.IfName, in.Delegates, len(in.Delegates)-1, rt, in.BinDir) + return delPlugins(exec, pod, args, k8sArgs, in.Delegates, len(in.Delegates)-1, in.RuntimeConfig, in.BinDir) } func main() { diff --git a/multus/multus_test.go b/multus/multus_test.go index 2934415fa..773b929d0 100644 --- a/multus/multus_test.go +++ b/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()) diff --git a/types/conf.go b/types/conf.go index 0a20df118..2ab83b999 100644 --- a/types/conf.go +++ b/types/conf.go @@ -129,16 +129,16 @@ func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID st return delegateConf, nil } -// MergeCNIRuntimeConfig creates CNI runtimeconfig from delegate -func MergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetConf) *RuntimeConfig { - logging.Debugf("MergeCNIRuntimeConfig: %v %v", runtimeConfig, delegate) +// mergeCNIRuntimeConfig creates CNI runtimeconfig from delegate +func mergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetConf) *RuntimeConfig { + 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 -func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig) *libcni.RuntimeConf { - logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v", args, k8sArgs, ifName, rc) +// CreateCNIRuntimeConf create CNI RuntimeConf for a delegate. If delegate configuration +// exists, merge data with the runtime config. +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 { - capabilityArgs["portMappings"] = rc.PortMaps + if len(delegateRc.PortMaps) != 0 { + capabilityArgs["portMappings"] = delegateRc.PortMaps } - if rc.Bandwidth != nil { - capabilityArgs["bandwidth"] = rc.Bandwidth + if delegateRc.Bandwidth != nil { + capabilityArgs["bandwidth"] = delegateRc.Bandwidth } - if len(rc.IPs) != 0 { - capabilityArgs["ips"] = rc.IPs + if len(delegateRc.IPs) != 0 { + capabilityArgs["ips"] = delegateRc.IPs } - if len(rc.Mac) != 0 { - capabilityArgs["mac"] = rc.Mac + if len(delegateRc.Mac) != 0 { + capabilityArgs["mac"] = delegateRc.Mac } - if len(rc.InfinibandGUID) != 0 { - capabilityArgs["infinibandGUID"] = rc.InfinibandGUID + if len(delegateRc.InfinibandGUID) != 0 { + capabilityArgs["infinibandGUID"] = delegateRc.InfinibandGUID } rt.CapabilityArgs = capabilityArgs } diff --git a/types/conf_test.go b/types/conf_test.go index f5b56dae5..b1848b704 100644 --- a/types/conf_test.go +++ b/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))