pkg/ip: Fix ipmasq teardown on v6-only interfaces

diff --git a/pkg/ip/ipmasq.go b/pkg/ip/ipmasq.go
index 7a549d1..ba00f13 100644 (file)
--- a/pkg/ip/ipmasq.go
+++ b/pkg/ip/ipmasq.go
@@ -75,7 +75,16 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
 
 // TeardownIPMasq undoes the effects of SetupIPMasq
 func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error {
-       ipt, err := iptables.New()
+       isV6 := ipn.IP.To4() == nil
+
+       var ipt *iptables.IPTables
+       var err error
+
+       if isV6 {
+               ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6)
+       } else {
+               ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
+       }
        if err != nil {
                return fmt.Errorf("failed to locate iptables: %v", err)
        }

diff --git a/pkg/ip/link.go b/pkg/ip/link.go
index fb8d3d5..3cb8486 100644 (file)
--- a/pkg/ip/link.go
+++ b/pkg/ip/link.go
@@ -158,6 +158,9 @@ func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, ne
 func DelLinkByName(ifName string) error {
        iface, err := netlink.LinkByName(ifName)
        if err != nil {
+               if err.Error() == "Link not found" {
+                       return ErrLinkNotFound
+               }
                return fmt.Errorf("failed to lookup %q: %v", ifName, err)
        }
 
@@ -168,9 +171,8 @@ func DelLinkByName(ifName string) error {
        return nil
 }
 
-// DelLinkByNameAddr remove an interface returns its IP address
-// of the specified family
-func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
+// DelLinkByNameAddr remove an interface and returns its addresses
+func DelLinkByNameAddr(ifName string) ([]*net.IPNet, error) {
        iface, err := netlink.LinkByName(ifName)
        if err != nil {
                if err != nil && err.Error() == "Link not found" {
@@ -179,7 +181,7 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
                return nil, fmt.Errorf("failed to lookup %q: %v", ifName, err)
        }
 
-       addrs, err := netlink.AddrList(iface, family)
+       addrs, err := netlink.AddrList(iface, netlink.FAMILY_ALL)
        if err != nil || len(addrs) == 0 {
                return nil, fmt.Errorf("failed to get IP addresses for %q: %v", ifName, err)
        }
@@ -188,7 +190,14 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
                return nil, fmt.Errorf("failed to delete %q: %v", ifName, err)
        }
 
-       return addrs[0].IPNet, nil
+       out := []*net.IPNet{}
+       for _, addr := range addrs {
+               if addr.IP.IsGlobalUnicast() {
+                       out = append(out, addr.IPNet)
+               }
+       }
+
+       return out, nil
 }
 
 func SetHWAddrByIP(ifName string, ip4 net.IP, ip6 net.IP) error {

diff --git a/pkg/ip/link_test.go b/pkg/ip/link_test.go
index a20a158..1049c86 100644 (file)
--- a/pkg/ip/link_test.go
+++ b/pkg/ip/link_test.go
@@ -27,7 +27,6 @@ import (
        "github.com/containernetworking/plugins/pkg/ns"
 
        "github.com/vishvananda/netlink"
-       "github.com/vishvananda/netlink/nl"
 )
 
 func getHwAddr(linkname string) string {
@@ -133,7 +132,7 @@ var _ = Describe("Link", func() {
                                defer GinkgoRecover()
 
                                // This string should match the expected error codes in the cmdDel functions of some of the plugins
-                               _, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST", netlink.FAMILY_V4)
+                               _, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST")
                                Expect(err).To(Equal(ip.ErrLinkNotFound))
 
                                return nil
@@ -220,16 +219,14 @@ var _ = Describe("Link", func() {
                })
        })
 
-       It("DelLinkByNameAddr must throw an error for configured interfaces", func() {
+       It("DelLinkByNameAddr should return no IPs when no IPs are configured", func() {
                _ = containerNetNS.Do(func(ns.NetNS) error {
                        defer GinkgoRecover()
 
                        // this will delete the host endpoint too
-                       addr, err := ip.DelLinkByNameAddr(containerVethName, nl.FAMILY_V4)
-                       Expect(err).To(HaveOccurred())
-
-                       var ipNetNil *net.IPNet
-                       Expect(addr).To(Equal(ipNetNil))
+                       addr, err := ip.DelLinkByNameAddr(containerVethName)
+                       Expect(err).NotTo(HaveOccurred())
+                       Expect(addr).To(HaveLen(0))
                        return nil
                })
        })

diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go
index b8a573e..63e0d89 100644 (file)
--- a/plugins/main/bridge/bridge.go
+++ b/plugins/main/bridge/bridge.go
@@ -474,10 +474,10 @@ func cmdDel(args *skel.CmdArgs) error {
        // There is a netns so try to clean up. Delete can be called multiple times
        // so don't return an error if the device is already removed.
        // If the device isn't there then don't try to clean up IP masq either.
-       var ipn *net.IPNet
+       var ipnets []*net.IPNet
        err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
                var err error
-               ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_ALL)
+               ipnets, err = ip.DelLinkByNameAddr(args.IfName)
                if err != nil && err == ip.ErrLinkNotFound {
                        return nil
                }
@@ -488,10 +488,14 @@ func cmdDel(args *skel.CmdArgs) error {
                return err
        }
 
-       if ipn != nil && n.IPMasq {
+       if n.IPMasq {
                chain := utils.FormatChainName(n.Name, args.ContainerID)
                comment := utils.FormatComment(n.Name, args.ContainerID)
-               err = ip.TeardownIPMasq(ipn, chain, comment)
+               for _, ipn := range ipnets {
+                       if err := ip.TeardownIPMasq(ipn, chain, comment); err != nil {
+                               return err
+                       }
+               }
        }
 
        return err

diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go
index 09924ef..74a9886 100644 (file)
--- a/plugins/main/ipvlan/ipvlan.go
+++ b/plugins/main/ipvlan/ipvlan.go
@@ -194,7 +194,7 @@ func cmdDel(args *skel.CmdArgs) error {
        // There is a netns so try to clean up. Delete can be called multiple times
        // so don't return an error if the device is already removed.
        err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-               if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil {
+               if err := ip.DelLinkByName(args.IfName); err != nil {
                        if err != ip.ErrLinkNotFound {
                                return err
                        }

diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go
index 3b69194..0589831 100644 (file)
--- a/plugins/main/macvlan/macvlan.go
+++ b/plugins/main/macvlan/macvlan.go
@@ -226,7 +226,7 @@ func cmdDel(args *skel.CmdArgs) error {
        // There is a netns so try to clean up. Delete can be called multiple times
        // so don't return an error if the device is already removed.
        err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-               if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil {
+               if err := ip.DelLinkByName(args.IfName); err != nil {
                        if err != ip.ErrLinkNotFound {
                                return err
                        }

diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go
index 226c0ef..70fc102 100644 (file)
--- a/plugins/main/ptp/ptp.go
+++ b/plugins/main/ptp/ptp.go
@@ -259,10 +259,10 @@ func cmdDel(args *skel.CmdArgs) error {
        // There is a netns so try to clean up. Delete can be called multiple times
        // so don't return an error if the device is already removed.
        // If the device isn't there then don't try to clean up IP masq either.
-       var ipn *net.IPNet
+       var ipnets []*net.IPNet
        err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
                var err error
-               ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
+               ipnets, err = ip.DelLinkByNameAddr(args.IfName)
                if err != nil && err == ip.ErrLinkNotFound {
                        return nil
                }
@@ -273,10 +273,12 @@ func cmdDel(args *skel.CmdArgs) error {
                return err
        }
 
-       if ipn != nil && conf.IPMasq {
+       if len(ipnets) != 0 && conf.IPMasq {
                chain := utils.FormatChainName(conf.Name, args.ContainerID)
                comment := utils.FormatComment(conf.Name, args.ContainerID)
-               err = ip.TeardownIPMasq(ipn, chain, comment)
+               for _, ipn := range ipnets {
+                       err = ip.TeardownIPMasq(ipn, chain, comment)
+               }
        }
 
        return err

diff --git a/plugins/main/vlan/vlan.go b/plugins/main/vlan/vlan.go
index 7f527b5..694d85a 100644 (file)
--- a/plugins/main/vlan/vlan.go
+++ b/plugins/main/vlan/vlan.go
@@ -181,9 +181,8 @@ func cmdDel(args *skel.CmdArgs) error {
        }
 
        err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-               _, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
-               // FIXME: use ip.ErrLinkNotFound when cni is revendored
-               if err != nil && err.Error() == "Link not found" {
+               err = ip.DelLinkByName(args.IfName)
+               if err != nil && err != ip.ErrLinkNotFound {
                        return nil
                }
                return err