Address PR comments

- Do not re-enter the root netns when querying weave bridge.
- Enter the root netns when constructing the predicate.

common/utils.go

@@ -95,29 +95,24 @@ func linkToNetDev(link netlink.Link) (NetDev, error) {
 // The returned function should be called from a container network namespace which
 // the bridge does NOT belong to.
 func ConnectedToBridgePredicate(bridgeName string) (func(link netlink.Link) bool, error) {
-	var br netlink.Link
-
-	err := weavenet.WithNetNSLinkByPidUnsafe(1, bridgeName, func(link netlink.Link) error {
-		br = link
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	indexes := make(map[int]struct{})
-	err = forEachLink(func(link netlink.Link) error {
-		if link.Attrs().MasterIndex == br.Attrs().Index {
-			peerIndex := link.Attrs().ParentIndex
-			if peerIndex == 0 {
-				// perhaps running on an older kernel where ParentIndex doesn't work.
-				// as fall-back, assume the indexes are consecutive
-				peerIndex = link.Attrs().Index - 1
-			}
-			indexes[peerIndex] = struct{}{}
-		}
-		return nil
-	})
+
+	// Scan devices in root namespace to find those attached to weave bridge
+	err := weavenet.WithNetNSLinkByPidUnsafe(1, bridgeName,
+		func(br netlink.Link) error {
+			return forEachLink(func(link netlink.Link) error {
+				if link.Attrs().MasterIndex == br.Attrs().Index {
+					peerIndex := link.Attrs().ParentIndex
+					if peerIndex == 0 {
+						// perhaps running on an older kernel where ParentIndex doesn't work.
+						// as fall-back, assume the indexes are consecutive
+						peerIndex = link.Attrs().Index - 1
+					}
+					indexes[peerIndex] = struct{}{}
+				}
+				return nil
+			})
+		})
 	if err != nil {
 		return nil, err
 	}
@@ -127,7 +122,6 @@ func ConnectedToBridgePredicate(bridgeName string) (func(link netlink.Link) bool
 		_, found := indexes[link.Attrs().Index]
 		return isveth && found
 	}, nil
-
 }
 
 func GetNetDevsWithPredicate(processID int, predicate func(link netlink.Link) bool) ([]NetDev, error) {
@@ -153,23 +147,19 @@ func GetWeaveNetDevs(processID int) ([]NetDev, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	return GetNetDevsWithPredicate(processID, p)
 }
 
 // Get the weave bridge interface.
+// NB: Should be called from the root network namespace.
 func GetBridgeNetDev(bridgeName string) (NetDev, error) {
-	var netdev NetDev
-
-	err := weavenet.WithNetNSLinkByPidUnsafe(1, bridgeName, func(link netlink.Link) error {
-		var err error
-		netdev, err = linkToNetDev(link)
-		if err != nil {
-			return err
-		}
-		return nil
-	})
+	link, err := netlink.LinkByName(bridgeName)
+	if err != nil {
+		return NetDev{}, err
+	}
 
-	return netdev, err
+	return linkToNetDev(link)
 }
 
 // Do post-attach configuration of all veths we have created

prog/weaveutil/addrs.go

@@ -13,7 +13,6 @@ func containerAddrs(args []string) error {
 		cmdUsage("container-addrs", "<bridgeName> [containerID ...]")
 	}
 	bridgeName := args[0]
-	containerIDs := args[1:]
 
 	client, err := docker.NewVersionedClientFromEnv("1.18")
 	if err != nil {
@@ -25,21 +24,33 @@ func containerAddrs(args []string) error {
 		return err
 	}
 
+	var containerIDs []string
 	containers := make(map[string]*docker.Container)
-	for _, cid := range containerIDs {
-		if containers[cid], err = client.InspectContainer(cid); err != nil {
-			containers[cid] = nil
-			if _, ok := err.(*docker.NoSuchContainer); !ok {
+
+	for _, cid := range args[1:] {
+		if cid == "weave:expose" {
+			netDev, err := common.GetBridgeNetDev(bridgeName)
+			if err != nil {
 				return err
 			}
+			printNetDevs(cid, []common.NetDev{netDev})
+			continue
 		}
+		if containers[cid], err = client.InspectContainer(cid); err != nil {
+			if _, ok := err.(*docker.NoSuchContainer); ok {
+				continue
+			}
+			return err
+		}
+		// To output in the right order, we keep the list of container IDs
+		containerIDs = append(containerIDs, cid)
 	}
 
 	// NB: Because network namespaces (netns) are changed many times inside the loop,
-	//     it's NOT safe to exec any code depending on the host netns without
+	//     it's NOT safe to exec any code depending on the root netns without
 	//     wrapping with WithNetNS*.
 	for _, cid := range containerIDs {
-		netDevs, err := getNetDevs(bridgeName, client, cid, containers[cid], pred)
+		netDevs, err := getNetDevs(client, containers[cid], pred)
 		if err != nil {
 			return err
 		}
@@ -49,6 +60,13 @@ func containerAddrs(args []string) error {
 	return nil
 }
 
+func getNetDevs(c *docker.Client, container *docker.Container, pred func(netlink.Link) bool) ([]common.NetDev, error) {
+	if container.State.Pid == 0 {
+		return nil, nil
+	}
+	return common.GetNetDevsWithPredicate(container.State.Pid, pred)
+}
+
 func printNetDevs(cid string, netDevs []common.NetDev) {
 	for _, netDev := range netDevs {
 		fmt.Printf("%12s %s %s", cid, netDev.Name, netDev.MAC.String())
@@ -59,19 +77,3 @@ func printNetDevs(cid string, netDevs []common.NetDev) {
 		fmt.Println()
 	}
 }
-
-func getNetDevs(bridgeName string, c *docker.Client, cid string, container *docker.Container, pred func(netlink.Link) bool) ([]common.NetDev, error) {
-	if cid == "weave:expose" {
-		netDev, err := common.GetBridgeNetDev(bridgeName)
-		if err != nil {
-			return nil, err
-		}
-		return []common.NetDev{netDev}, nil
-	}
-
-	if container == nil || container.State.Pid == 0 {
-		return nil, nil
-	}
-
-	return common.GetNetDevsWithPredicate(container.State.Pid, pred)
-}

prog/weaveutil/main.go

@@ -30,7 +30,6 @@ func main() {
 		usage()
 		os.Exit(1)
 	}
-
 	cmd, found := commands[os.Args[1]]
 	if !found {
 		usage()