Fix "weave ps"

Make sure that any network namespace changes happens at the end of
the cmd invocation. Otherwise, we are risking to end up in a situation
in which OS scheduling threads are in a "wrong" namespace.

See WithNetNS for more details.

common/utils.go

@@ -90,29 +90,24 @@ func linkToNetDev(link netlink.Link) (NetDev, error) {
 	return netDev, nil
 }
 
-// Lookup the weave interface of a container
-func GetWeaveNetDevs(processID int) ([]NetDev, error) {
-	// Bail out if this container is running in the root namespace
-	nsToplevel, err := netns.GetFromPid(1)
-	if err != nil {
-		return nil, fmt.Errorf("unable to open root namespace: %s", err)
-	}
-	nsContainr, err := netns.GetFromPid(processID)
-	if err != nil {
-		return nil, fmt.Errorf("unable to open process %d namespace: %s", processID, err)
-	}
-	if nsToplevel.Equal(nsContainr) {
-		return nil, nil
-	}
+// ConnectedToBridgePredicate returns a function which is used to query whether
+// a given link is a veth interface which one end is connected to a bridge.
+// 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
 
-	weaveBridge, err := netlink.LinkByName("weave")
+	err := weavenet.WithNetNSLinkByPid(1, bridgeName, func(link netlink.Link) error {
+		br = link
+		return nil
+	})
 	if err != nil {
-		return nil, fmt.Errorf("Cannot find weave bridge: %s", err)
+		return nil, err
 	}
-	// Scan devices in root namespace to find those attached to weave bridge
+
 	indexes := make(map[int]struct{})
 	err = forEachLink(func(link netlink.Link) error {
-		if link.Attrs().MasterIndex == weaveBridge.Attrs().Index {
+		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.
@@ -126,11 +121,39 @@ func GetWeaveNetDevs(processID int) ([]NetDev, error) {
 	if err != nil {
 		return nil, err
 	}
-	return FindNetDevs(processID, func(link netlink.Link) bool {
+
+	return func(link netlink.Link) bool {
 		_, isveth := link.(*netlink.Veth)
 		_, found := indexes[link.Attrs().Index]
 		return isveth && found
-	})
+	}, nil
+
+}
+
+func GetNetDevsWithPredicate(processID int, predicate func(link netlink.Link) bool) ([]NetDev, error) {
+	// Bail out if this container is running in the root namespace
+	nsToplevel, err := netns.GetFromPid(1)
+	if err != nil {
+		return nil, fmt.Errorf("unable to open root namespace: %s", err)
+	}
+	nsContainr, err := netns.GetFromPid(processID)
+	if err != nil {
+		return nil, fmt.Errorf("unable to open process %d namespace: %s", processID, err)
+	}
+	if nsToplevel.Equal(nsContainr) {
+		return nil, nil
+	}
+
+	return FindNetDevs(processID, predicate)
+}
+
+// Lookup the weave interface of a container
+func GetWeaveNetDevs(processID int) ([]NetDev, error) {
+	p, err := ConnectedToBridgePredicate("weave")
+	if err != nil {
+		return nil, err
+	}
+	return GetNetDevsWithPredicate(processID, p)
 }
 
 // Get the weave bridge interface.

prog/weaveutil/addrs.go

@@ -2,7 +2,9 @@ package main
 
 import (
 	"fmt"
+
 	"github.com/fsouza/go-dockerclient"
+	"github.com/vishvananda/netlink"
 	"github.com/weaveworks/weave/common"
 )
 
@@ -11,51 +13,65 @@ func containerAddrs(args []string) error {
 		cmdUsage("container-addrs", "<bridgeName> [containerID ...]")
 	}
 	bridgeName := args[0]
+	containerIDs := args[1:]
 
-	c, err := docker.NewVersionedClientFromEnv("1.18")
+	client, err := docker.NewVersionedClientFromEnv("1.18")
 	if err != nil {
 		return err
 	}
 
-	for _, containerID := range args[1:] {
-		netDevs, err := getNetDevs(bridgeName, c, containerID)
-		if err != nil {
-			if _, ok := err.(*docker.NoSuchContainer); ok {
-				continue
+	pred, err := common.ConnectedToBridgePredicate(bridgeName)
+	if err != nil {
+		return err
+	}
+
+	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 {
+				return err
 			}
-			return err
 		}
+	}
 
-		for _, netDev := range netDevs {
-			fmt.Printf("%12s %s %s", containerID, netDev.Name, netDev.MAC.String())
-			for _, cidr := range netDev.CIDRs {
-				prefixLength, _ := cidr.Mask.Size()
-				fmt.Printf(" %v/%v", cidr.IP, prefixLength)
-			}
-			fmt.Println()
+	// 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
+	//     wrapping with WithNetNS*.
+	for _, cid := range containerIDs {
+		netDevs, err := getNetDevs(bridgeName, client, cid, containers[cid], pred)
+		if err != nil {
+			return err
 		}
+		printNetDevs(cid, netDevs)
 	}
 
 	return nil
 }
 
-func getNetDevs(bridgeName string, c *docker.Client, containerID string) ([]common.NetDev, error) {
-	if containerID == "weave:expose" {
-		if netDev, err := common.GetBridgeNetDev(bridgeName); err != nil {
-			return nil, err
-		} else {
-			return []common.NetDev{netDev}, nil
+func printNetDevs(cid string, netDevs []common.NetDev) {
+	for _, netDev := range netDevs {
+		fmt.Printf("%12s %s %s", cid, netDev.Name, netDev.MAC.String())
+		for _, cidr := range netDev.CIDRs {
+			prefixLength, _ := cidr.Mask.Size()
+			fmt.Printf(" %v/%v", cidr.IP, prefixLength)
 		}
+		fmt.Println()
 	}
+}
 
-	container, err := c.InspectContainer(containerID)
-	if err != nil {
-		return nil, err
+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.State.Pid == 0 {
+	if container == nil || container.State.Pid == 0 {
 		return nil, nil
 	}
 
-	return common.GetWeaveNetDevs(container.State.Pid)
+	return common.GetNetDevsWithPredicate(container.State.Pid, pred)
 }