Merge pull request #110 from dcbw/host-device-honor-ifname

host-device: respect CNI_IFNAME/args.IfName

plugins/main/host-device/host-device.go

@@ -68,15 +68,20 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 	defer containerNs.Close()
 
-	dev, err := moveLinkIn(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs)
+	hostDev, err := getLink(cfg.Device, cfg.HWAddr, cfg.KernelPath)
+	if err != nil {
+		return fmt.Errorf("failed to find host device: %v", err)
+	}
+
+	contDev, err := moveLinkIn(hostDev, containerNs, args.IfName)
 	if err != nil {
 		return fmt.Errorf("failed to move link %v", err)
 	}
-	return printLink(dev, cfg.CNIVersion, containerNs)
+	return printLink(contDev, cfg.CNIVersion, containerNs)
 }
 
 func cmdDel(args *skel.CmdArgs) error {
-	cfg, err := loadConf(args.StdinData)
+	_, err := loadConf(args.StdinData)
 	if err != nil {
 		return err
 	}
@@ -85,44 +90,67 @@ func cmdDel(args *skel.CmdArgs) error {
 		return fmt.Errorf("failed to open netns %q: %v", args.Netns, err)
 	}
 	defer containerNs.Close()
-	defer fmt.Println(`{}`)
+
-	return moveLinkOut(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs)
+	if err := moveLinkOut(containerNs, args.IfName); err != nil {
+		return err
+	}
+
+	return nil
 }
 
-func moveLinkIn(device, hwAddr, kernelPath string, containerNs ns.NetNS) (netlink.Link, error) {
+func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, ifName string) (netlink.Link, error) {
-	dev, err := getLink(device, hwAddr, kernelPath)
+	if err := netlink.LinkSetNsFd(hostDev, int(containerNs.Fd())); err != nil {
-	if err != nil {
-		return nil, err
-	}
-	if err := netlink.LinkSetNsFd(dev, int(containerNs.Fd())); err != nil {
 		return nil, err
 	}
 
+	var contDev netlink.Link
 	if err := containerNs.Do(func(_ ns.NetNS) error {
-		dev, err = netlink.LinkByName(dev.Attrs().Name)
+		var err error
+		contDev, err = netlink.LinkByName(hostDev.Attrs().Name)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to find %q: %v", hostDev.Attrs().Name, err)
+		}
+		// Save host device name into the container device's alias property
+		if err := netlink.LinkSetAlias(contDev, hostDev.Attrs().Name); err != nil {
+			return fmt.Errorf("failed to set alias to %q: %v", hostDev.Attrs().Name, err)
+		}
+		// Rename container device to respect args.IfName
+		if err := netlink.LinkSetName(contDev, ifName); err != nil {
+			return fmt.Errorf("failed to rename device %q to %q: %v", hostDev.Attrs().Name, ifName, err)
+		}
+		// Retrieve link again to get up-to-date name and attributes
+		contDev, err = netlink.LinkByName(ifName)
+		if err != nil {
+			return fmt.Errorf("failed to find %q: %v", ifName, err)
 		}
 		return nil
 	}); err != nil {
 		return nil, err
 	}
 
-	return dev, nil
+	return contDev, nil
 }
 
-func moveLinkOut(device, hwAddr, kernelPath string, containerNs ns.NetNS) error {
+func moveLinkOut(containerNs ns.NetNS, ifName string) error {
 	defaultNs, err := ns.GetCurrentNS()
 	if err != nil {
 		return err
 	}
+	defer defaultNs.Close()
 
 	return containerNs.Do(func(_ ns.NetNS) error {
-		dev, err := getLink(device, hwAddr, kernelPath)
+		dev, err := netlink.LinkByName(ifName)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to find %q: %v", ifName, err)
 		}
-		return netlink.LinkSetNsFd(dev, int(defaultNs.Fd()))
+		// Rename device to it's original name
+		if err := netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil {
+			return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err)
+		}
+		if err := netlink.LinkSetNsFd(dev, int(defaultNs.Fd())); err != nil {
+			return fmt.Errorf("failed to move %q to host netns: %v", dev.Attrs().Alias, err)
+		}
+		return nil
 	})
 }
 

plugins/main/host-device/host-device_test.go

@@ -45,7 +45,6 @@ var _ = Describe("base functionality", func() {
 	})
 
 	It("Works with a valid config", func() {
-
 		var origLink netlink.Link
 
 		// prepare ifname in original namespace
@@ -69,6 +68,7 @@ var _ = Describe("base functionality", func() {
 		targetNS, err := ns.NewNS()
 		Expect(err).NotTo(HaveOccurred())
 
+		CNI_IFNAME := "eth0"
 		conf := fmt.Sprintf(`{
 			"cniVersion": "0.3.0",
 			"name": "cni-plugin-host-device-test",
@@ -78,14 +78,14 @@ var _ = Describe("base functionality", func() {
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
 			Netns:       targetNS.Path(),
-			IfName:      ifname,
+			IfName:      CNI_IFNAME,
 			StdinData:   []byte(conf),
 		}
 		var resI types.Result
 		err = originalNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
 			var err error
-			resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), ifname, []byte(conf), func() error { return cmdAdd(args) })
+			resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), CNI_IFNAME, []byte(conf), func() error { return cmdAdd(args) })
 			return err
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -95,7 +95,7 @@ var _ = Describe("base functionality", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(res.Interfaces).To(Equal([]*current.Interface{
 			{
-				Name:    ifname,
+				Name:    CNI_IFNAME,
 				Mac:     origLink.Attrs().HardwareAddr.String(),
 				Sandbox: targetNS.Path(),
 			},
@@ -104,9 +104,9 @@ var _ = Describe("base functionality", func() {
 		// assert that dummy0 is now in the target namespace
 		err = targetNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
-			link, err := netlink.LinkByName(ifname)
+			link, err := netlink.LinkByName(CNI_IFNAME)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(link.Attrs().Name).To(Equal(ifname))
+			Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr))
 			return nil
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -120,10 +120,10 @@ var _ = Describe("base functionality", func() {
 		})
 		Expect(err).NotTo(HaveOccurred())
 
-		// Check that deleting the device moves it back
+		// Check that deleting the device moves it back and restores the name
 		err = originalNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
-			err = testutils.CmdDelWithResult(targetNS.Path(), ifname, func() error { return cmdDel(args) })
+			err = testutils.CmdDelWithResult(targetNS.Path(), CNI_IFNAME, func() error { return cmdDel(args) })
 			Expect(err).NotTo(HaveOccurred())
 
 			_, err := netlink.LinkByName(ifname)