From 7f5c37ef1e8365ccc64e5b61fe1e4c9e35d7475b Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Fri, 1 Jul 2016 21:32:03 +0100 Subject: [PATCH] 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 | 63 ++++++++++++++++++++++++++------------- prog/weaveutil/addrs.go | 66 +++++++++++++++++++++++++---------------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/common/utils.go b/common/utils.go index 4189271452..f79ad9e053 100644 --- a/common/utils.go +++ b/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. diff --git a/prog/weaveutil/addrs.go b/prog/weaveutil/addrs.go index 3708fc327e..c35213a12a 100644 --- a/prog/weaveutil/addrs.go +++ b/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", " [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) }