From 4779f1d2bfaa2feeda1378a597b79f48a0f6c236 Mon Sep 17 00:00:00 2001 From: Paul Fisher Date: Wed, 1 Nov 2017 10:14:04 -0700 Subject: [PATCH 1/2] ipvlan: support enslaving an interface returned by ipam For IP allocation schemes that cannot be interface agnostic, master can be set to "ipam". In this configuration, the IPAM plugin is required to return a single interface name for the ipvlan plugin to enslave. --- plugins/main/ipvlan/README.md | 3 ++- plugins/main/ipvlan/ipvlan.go | 20 ++++++++++++----- plugins/main/ipvlan/ipvlan_test.go | 36 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/plugins/main/ipvlan/README.md b/plugins/main/ipvlan/README.md index 988555ef..f0abc4f5 100644 --- a/plugins/main/ipvlan/README.md +++ b/plugins/main/ipvlan/README.md @@ -27,7 +27,7 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC * `name` (string, required): the name of the network. * `type` (string, required): "ipvlan". -* `master` (string, required): name of the host interface to enslave. +* `master` (string, required): name of the host interface to enslave or "ipam" to enslave an interface returned by ipam. * `mode` (string, optional): one of "l2", "l3". Defaults to "l2". * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel. * `ipam` (dictionary, required): IPAM configuration to be used for this network. @@ -38,3 +38,4 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC Therefore the container will not be able to reach the host via `ipvlan` interface. Be sure to also have container join a network that provides connectivity to the host (e.g. `ptp`). * A single master interface can not be enslaved by both `macvlan` and `ipvlan`. +* For IP allocation schemes that cannot be interface agnostic, master can be set to `ipam`. In this configuration, the ipam plugin is required to return a single interface name for the ipvlan plugin to enslave. diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go index 74a98863..73cd5a54 100644 --- a/plugins/main/ipvlan/ipvlan.go +++ b/plugins/main/ipvlan/ipvlan.go @@ -138,11 +138,6 @@ func cmdAdd(args *skel.CmdArgs) error { } defer netns.Close() - ipvlanInterface, err := createIpvlan(n, args.IfName, netns) - if err != nil { - return err - } - // run the IPAM plugin and get back the config to apply r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) if err != nil { @@ -157,6 +152,21 @@ func cmdAdd(args *skel.CmdArgs) error { if len(result.IPs) == 0 { return errors.New("IPAM plugin returned missing IP config") } + + if n.Master == "ipam" { + // Use an IPAM supplied master interface + if len(result.Interfaces) == 1 && result.Interfaces[0].Name != "" { + n.Master = result.Interfaces[0].Name + } else { + return errors.New("IPAM plugin returned missing master interface") + } + } + + ipvlanInterface, err := createIpvlan(n, args.IfName, netns) + if err != nil { + return err + } + for _, ipc := range result.IPs { // All addresses belong to the ipvlan interface ipc.Interface = current.Int(0) diff --git a/plugins/main/ipvlan/ipvlan_test.go b/plugins/main/ipvlan/ipvlan_test.go index a2b92179..94ac3bf9 100644 --- a/plugins/main/ipvlan/ipvlan_test.go +++ b/plugins/main/ipvlan/ipvlan_test.go @@ -224,4 +224,40 @@ var _ = Describe("ipvlan Operations", func() { }) Expect(err).NotTo(HaveOccurred()) }) + + It("errors if master should originate from ipam but an ipam interface is not returned", func() { + const IFNAME = "ipvl0" + + conf := `{ + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "ipam", + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24" + } +}` + targetNs, err := ns.NewNS() + Expect(err).NotTo(HaveOccurred()) + defer targetNs.Close() + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNs.Path(), + IfName: IFNAME, + StdinData: []byte(conf), + } + + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + _, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { + return cmdAdd(args) + }) + Expect(err.Error()).To(Equal("IPAM plugin returned missing master interface")) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) }) From 5c7e7c09133832afa48b37b1465186f521daf7e1 Mon Sep 17 00:00:00 2001 From: Paul Fisher Date: Sat, 23 Dec 2017 10:59:48 -0800 Subject: [PATCH 2/2] ipvlan: support chaining for master interface and IP configuration For IP allocation schemes that cannot be interface agnostic, the ipvlan plugin can be chained with an earlier plugin that handles this logic. If "master" is omitted from the ipvlan configuration, then the previous Result must contain a single interface name for the ipvlan plugin to enslave. If "ipam" is omitted, then the previous Result is used to configure the ipvlan interface. --- plugins/main/ipvlan/README.md | 10 +- plugins/main/ipvlan/ipvlan.go | 85 ++++++++---- plugins/main/ipvlan/ipvlan_test.go | 202 ++++++++++++++--------------- 3 files changed, 163 insertions(+), 134 deletions(-) diff --git a/plugins/main/ipvlan/README.md b/plugins/main/ipvlan/README.md index f0abc4f5..089ef144 100644 --- a/plugins/main/ipvlan/README.md +++ b/plugins/main/ipvlan/README.md @@ -27,10 +27,10 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC * `name` (string, required): the name of the network. * `type` (string, required): "ipvlan". -* `master` (string, required): name of the host interface to enslave or "ipam" to enslave an interface returned by ipam. +* `master` (string, required unless chained): name of the host interface to enslave. * `mode` (string, optional): one of "l2", "l3". Defaults to "l2". * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel. -* `ipam` (dictionary, required): IPAM configuration to be used for this network. +* `ipam` (dictionary, required unless chained): IPAM configuration to be used for this network. ## Notes @@ -38,4 +38,8 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC Therefore the container will not be able to reach the host via `ipvlan` interface. Be sure to also have container join a network that provides connectivity to the host (e.g. `ptp`). * A single master interface can not be enslaved by both `macvlan` and `ipvlan`. -* For IP allocation schemes that cannot be interface agnostic, master can be set to `ipam`. In this configuration, the ipam plugin is required to return a single interface name for the ipvlan plugin to enslave. +* For IP allocation schemes that cannot be interface agnostic, the ipvlan plugin +can be chained with an earlier plugin that handles this logic. If `master` is +omitted, then the previous Result must contain a single interface name for the +ipvlan plugin to enslave. If `ipam` is omitted, then the previous Result is used +to configure the ipvlan interface. diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go index 73cd5a54..269fa8e8 100644 --- a/plugins/main/ipvlan/ipvlan.go +++ b/plugins/main/ipvlan/ipvlan.go @@ -32,6 +32,12 @@ import ( type NetConf struct { types.NetConf + + // support chaining for master interface and IP decisions + // occurring prior to running ipvlan plugin + RawPrevResult *map[string]interface{} `json:"prevResult"` + PrevResult *current.Result `json:"-"` + Master string `json:"master"` Mode string `json:"mode"` MTU int `json:"mtu"` @@ -49,8 +55,31 @@ func loadConf(bytes []byte) (*NetConf, string, error) { if err := json.Unmarshal(bytes, n); err != nil { return nil, "", fmt.Errorf("failed to load netconf: %v", err) } + // Parse previous result + if n.RawPrevResult != nil { + resultBytes, err := json.Marshal(n.RawPrevResult) + if err != nil { + return nil, "", fmt.Errorf("could not serialize prevResult: %v", err) + } + res, err := version.NewResult(n.CNIVersion, resultBytes) + if err != nil { + return nil, "", fmt.Errorf("could not parse prevResult: %v", err) + } + n.RawPrevResult = nil + n.PrevResult, err = current.NewResultFromResult(res) + if err != nil { + return nil, "", fmt.Errorf("could not convert result to current version: %v", err) + } + } if n.Master == "" { - return nil, "", fmt.Errorf(`"master" field is required. It specifies the host interface name to virtualize`) + if n.PrevResult == nil { + return nil, "", fmt.Errorf(`"master" field is required. It specifies the host interface name to virtualize`) + } + if len(n.PrevResult.Interfaces) == 1 && n.PrevResult.Interfaces[0].Name != "" { + n.Master = n.PrevResult.Interfaces[0].Name + } else { + return nil, "", fmt.Errorf("chained master failure. PrevResult lacks a single named interface") + } } return n, n.CNIVersion, nil } @@ -138,35 +167,32 @@ func cmdAdd(args *skel.CmdArgs) error { } defer netns.Close() - // run the IPAM plugin and get back the config to apply - r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) - if err != nil { - return err - } - // Convert whatever the IPAM result was into the current Result type - result, err := current.NewResultFromResult(r) - if err != nil { - return err - } - - if len(result.IPs) == 0 { - return errors.New("IPAM plugin returned missing IP config") - } - - if n.Master == "ipam" { - // Use an IPAM supplied master interface - if len(result.Interfaces) == 1 && result.Interfaces[0].Name != "" { - n.Master = result.Interfaces[0].Name - } else { - return errors.New("IPAM plugin returned missing master interface") - } - } - ipvlanInterface, err := createIpvlan(n, args.IfName, netns) if err != nil { return err } + var result *current.Result + // Configure iface from PrevResult if we have IPs and an IPAM + // block has not been configured + if n.IPAM.Type == "" && n.PrevResult != nil && len(n.PrevResult.IPs) > 0 { + result = n.PrevResult + } else { + // run the IPAM plugin and get back the config to apply + r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) + if err != nil { + return err + } + // Convert whatever the IPAM result was into the current Result type + result, err = current.NewResultFromResult(r) + if err != nil { + return err + } + + if len(result.IPs) == 0 { + return errors.New("IPAM plugin returned missing IP config") + } + } for _, ipc := range result.IPs { // All addresses belong to the ipvlan interface ipc.Interface = current.Int(0) @@ -192,9 +218,12 @@ func cmdDel(args *skel.CmdArgs) error { return err } - err = ipam.ExecDel(n.IPAM.Type, args.StdinData) - if err != nil { - return err + // On chained invocation, IPAM block can be empty + if n.IPAM.Type != "" { + err = ipam.ExecDel(n.IPAM.Type, args.StdinData) + if err != nil { + return err + } } if args.Netns == "" { diff --git a/plugins/main/ipvlan/ipvlan_test.go b/plugins/main/ipvlan/ipvlan_test.go index 94ac3bf9..bed6ddb5 100644 --- a/plugins/main/ipvlan/ipvlan_test.go +++ b/plugins/main/ipvlan/ipvlan_test.go @@ -33,6 +33,79 @@ import ( const MASTER_NAME = "eth0" +func ipvlanAddDelTest(conf, IFNAME string, originalNS ns.NetNS) { + targetNs, err := ns.NewNS() + Expect(err).NotTo(HaveOccurred()) + defer targetNs.Close() + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNs.Path(), + IfName: IFNAME, + StdinData: []byte(conf), + } + + var result *current.Result + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + r, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + + result, err = current.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + + Expect(len(result.Interfaces)).To(Equal(1)) + Expect(result.Interfaces[0].Name).To(Equal(IFNAME)) + Expect(len(result.IPs)).To(Equal(1)) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + // Make sure ipvlan link exists in the target namespace + err = targetNs.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + link, err := netlink.LinkByName(IFNAME) + Expect(err).NotTo(HaveOccurred()) + Expect(link.Attrs().Name).To(Equal(IFNAME)) + + hwaddr, err := net.ParseMAC(result.Interfaces[0].Mac) + Expect(err).NotTo(HaveOccurred()) + Expect(link.Attrs().HardwareAddr).To(Equal(hwaddr)) + + addrs, err := netlink.AddrList(link, syscall.AF_INET) + Expect(err).NotTo(HaveOccurred()) + Expect(len(addrs)).To(Equal(1)) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + err = testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error { + return cmdDel(args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + // Make sure ipvlan link has been deleted + err = targetNs.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + link, err := netlink.LinkByName(IFNAME) + Expect(err).To(HaveOccurred()) + Expect(link).To(BeNil()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + var _ = Describe("ipvlan Operations", func() { var originalNS ns.NetNS @@ -116,76 +189,35 @@ var _ = Describe("ipvlan Operations", func() { } }`, MASTER_NAME) - targetNs, err := ns.NewNS() - Expect(err).NotTo(HaveOccurred()) - defer targetNs.Close() + ipvlanAddDelTest(conf, IFNAME, originalNS) + }) - args := &skel.CmdArgs{ - ContainerID: "dummy", - Netns: targetNs.Path(), - IfName: IFNAME, - StdinData: []byte(conf), - } + It("configures and deconfigures an iplvan link with ADD/DEL when chained", func() { + const IFNAME = "ipvl0" - var result *current.Result - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() + conf := fmt.Sprintf(`{ + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "prevResult": { + "interfaces": [ + { + "name": "%s" + } + ], + "ips": [ + { + "version": "4", + "address": "10.1.2.2/24", + "gateway": "10.1.2.1", + "interface": 0 + } + ], + "routes": [] + } +}`, MASTER_NAME) - r, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { - return cmdAdd(args) - }) - Expect(err).NotTo(HaveOccurred()) - - result, err = current.GetResult(r) - Expect(err).NotTo(HaveOccurred()) - - Expect(len(result.Interfaces)).To(Equal(1)) - Expect(result.Interfaces[0].Name).To(Equal(IFNAME)) - Expect(len(result.IPs)).To(Equal(1)) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - // Make sure ipvlan link exists in the target namespace - err = targetNs.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - link, err := netlink.LinkByName(IFNAME) - Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(IFNAME)) - - hwaddr, err := net.ParseMAC(result.Interfaces[0].Mac) - Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().HardwareAddr).To(Equal(hwaddr)) - - addrs, err := netlink.AddrList(link, syscall.AF_INET) - Expect(err).NotTo(HaveOccurred()) - Expect(len(addrs)).To(Equal(1)) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - err = testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error { - return cmdDel(args) - }) - Expect(err).NotTo(HaveOccurred()) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - // Make sure ipvlan link has been deleted - err = targetNs.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - link, err := netlink.LinkByName(IFNAME) - Expect(err).To(HaveOccurred()) - Expect(link).To(BeNil()) - return nil - }) - Expect(err).NotTo(HaveOccurred()) + ipvlanAddDelTest(conf, IFNAME, originalNS) }) It("deconfigures an unconfigured ipvlan link with DEL", func() { @@ -224,40 +256,4 @@ var _ = Describe("ipvlan Operations", func() { }) Expect(err).NotTo(HaveOccurred()) }) - - It("errors if master should originate from ipam but an ipam interface is not returned", func() { - const IFNAME = "ipvl0" - - conf := `{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "ipam", - "ipam": { - "type": "host-local", - "subnet": "10.1.2.0/24" - } -}` - targetNs, err := ns.NewNS() - Expect(err).NotTo(HaveOccurred()) - defer targetNs.Close() - - args := &skel.CmdArgs{ - ContainerID: "dummy", - Netns: targetNs.Path(), - IfName: IFNAME, - StdinData: []byte(conf), - } - - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - _, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { - return cmdAdd(args) - }) - Expect(err.Error()).To(Equal("IPAM plugin returned missing master interface")) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - }) })