Merge pull request #88 from lyft/ipvlan-master-intf-ipam

ipvlan: support enslaving an interface returned by ipam

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.
+* `master` (string, required unless chained): name of the host interface to enslave.
 * `mode` (string, optional): one of "l2", "l3", "l3s". 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,3 +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, 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.

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
 }
@@ -143,19 +172,26 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	// run the IPAM plugin and get back the config to apply
+	var result *current.Result
-	r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData)
+	// Configure iface from PrevResult if we have IPs and an IPAM
-	if err != nil {
+	// block has not been configured
-		return err
+	if n.IPAM.Type == "" && n.PrevResult != nil && len(n.PrevResult.IPs) > 0 {
-	}
+		result = n.PrevResult
-	// Convert whatever the IPAM result was into the current Result type
+	} else {
-	result, err := current.NewResultFromResult(r)
+		// run the IPAM plugin and get back the config to apply
-	if err != nil {
+		r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData)
-		return err
+		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 {
+		if len(result.IPs) == 0 {
-		return errors.New("IPAM plugin returned missing IP config")
+			return errors.New("IPAM plugin returned missing IP config")
+		}
 	}
 	for _, ipc := range result.IPs {
 		// All addresses belong to the ipvlan interface
@@ -182,9 +218,12 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	err = ipam.ExecDel(n.IPAM.Type, args.StdinData)
+	// On chained invocation, IPAM block can be empty
-	if err != nil {
+	if n.IPAM.Type != "" {
-		return err
+		err = ipam.ExecDel(n.IPAM.Type, args.StdinData)
+		if err != nil {
+			return err
+		}
 	}
 
 	if args.Netns == "" {

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()
+		ipvlanAddDelTest(conf, IFNAME, originalNS)
-		Expect(err).NotTo(HaveOccurred())
+	})
-		defer targetNs.Close()
 
-		args := &skel.CmdArgs{
+	It("configures and deconfigures an iplvan link with ADD/DEL when chained", func() {
-			ContainerID: "dummy",
+		const IFNAME = "ipvl0"
-			Netns:       targetNs.Path(),
-			IfName:      IFNAME,
-			StdinData:   []byte(conf),
-		}
 
-		var result *current.Result
+		conf := fmt.Sprintf(`{
-		err = originalNS.Do(func(ns.NetNS) error {
+    "cniVersion": "0.3.1",
-			defer GinkgoRecover()
+    "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 {
+		ipvlanAddDelTest(conf, IFNAME, originalNS)
-				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())
 	})
 
 	It("deconfigures an unconfigured ipvlan link with DEL", func() {