From 0861d055226a989cf5ec4afd122deab821adf6da Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Tue, 5 Jul 2016 14:47:51 +0100 Subject: [PATCH] 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 | 58 +++++++++++++++++------------------------ prog/weaveutil/addrs.go | 48 ++++++++++++++++++---------------- prog/weaveutil/main.go | 1 - 3 files changed, 49 insertions(+), 58 deletions(-) diff --git a/common/utils.go b/common/utils.go index a5a07f82..51129500 100644 --- a/common/utils.go +++ b/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 + link, err := netlink.LinkByName(bridgeName) + if err != nil { + return NetDev{}, err + } - err := weavenet.WithNetNSLinkByPidUnsafe(1, bridgeName, func(link netlink.Link) error { - var err error - netdev, err = linkToNetDev(link) - if err != nil { - return err - } - return nil - }) - - return netdev, err + return linkToNetDev(link) } // Do post-attach configuration of all veths we have created diff --git a/prog/weaveutil/addrs.go b/prog/weaveutil/addrs.go index c35213a1..efdf5357 100644 --- a/prog/weaveutil/addrs.go +++ b/prog/weaveutil/addrs.go @@ -13,7 +13,6 @@ func containerAddrs(args []string) error { cmdUsage("container-addrs", " [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) -} diff --git a/prog/weaveutil/main.go b/prog/weaveutil/main.go index da338994..2550d56d 100644 --- a/prog/weaveutil/main.go +++ b/prog/weaveutil/main.go @@ -30,7 +30,6 @@ func main() { usage() os.Exit(1) } - cmd, found := commands[os.Args[1]] if !found { usage()