Merge pull request #577 from weaveworks/520-edge-bug

Include host scope for container joins based on IP.

circle.yml

@@ -24,7 +24,7 @@ dependencies:
     - git clone https://github.com/weaveworks/tools.git $TOOLS
     - sudo apt-get update
     - sudo apt-get --only-upgrade install tar libpcap0.8-dev
-    - sudo apt-get install jq
+    - sudo apt-get install jq pv
     - curl https://sdk.cloud.google.com | bash
     - test -z "$SECRET_PASSWORD" || bin/setup-circleci-secrets "$SECRET_PASSWORD"
     - go get $WEAVE_REPO/...

integration/setup.sh

@@ -6,7 +6,8 @@ set -e
 
 echo Copying scope images and scripts to hosts
 for HOST in $HOSTS; do
-	docker_on $HOST load -i ../scope.tar
+	SIZE=$(stat --printf="%s" ../scope.tar)
+	cat ../scope.tar | pv -N "scope.tar" -s $SIZE | $SSH -C $HOST sudo docker load
 	upload_executable $HOST ../scope
 	upload_executable $HOST ../scope /usr/local/scope/bin/scope
 done

probe/docker/container.go

@@ -22,12 +22,13 @@ import (
 
 // These constants are keys used in node metadata
 const (
-	ContainerName     = "docker_container_name"
-	ContainerCommand  = "docker_container_command"
-	ContainerPorts    = "docker_container_ports"
-	ContainerCreated  = "docker_container_created"
-	ContainerIPs      = "docker_container_ips"
-	ContainerHostname = "docker_container_hostname"
+	ContainerName          = "docker_container_name"
+	ContainerCommand       = "docker_container_command"
+	ContainerPorts         = "docker_container_ports"
+	ContainerCreated       = "docker_container_created"
+	ContainerIPs           = "docker_container_ips"
+	ContainerHostname      = "docker_container_hostname"
+	ContainerIPsWithScopes = "docker_container_ips_with_scopes"
 
 	NetworkRxDropped = "network_rx_dropped"
 	NetworkRxBytes   = "network_rx_bytes"
@@ -72,7 +73,7 @@ type Container interface {
 	Image() string
 	PID() int
 	Hostname() string
-	GetNode([]net.IP) report.Node
+	GetNode(string, []net.IP) report.Node
 
 	StartGatheringStats() error
 	StopGatheringStats()
@@ -219,20 +220,27 @@ func (c *container) ports(localAddrs []net.IP) string {
 	return strings.Join(ports, ", ")
 }
 
-func (c *container) GetNode(localAddrs []net.IP) report.Node {
+func (c *container) GetNode(hostID string, localAddrs []net.IP) report.Node {
 	c.RLock()
 	defer c.RUnlock()
 
+	ips := append(c.container.NetworkSettings.SecondaryIPAddresses, c.container.NetworkSettings.IPAddress)
+	// Treat all Docker IPs as local scoped.
+	ipsWithScopes := []string{}
+	for _, ip := range ips {
+		ipsWithScopes = append(ipsWithScopes, report.MakeScopedAddressNodeID(hostID, ip))
+	}
+
 	result := report.MakeNodeWith(map[string]string{
-		ContainerID:      c.ID(),
-		ContainerName:    strings.TrimPrefix(c.container.Name, "/"),
-		ContainerPorts:   c.ports(localAddrs),
-		ContainerCreated: c.container.Created.Format(time.RFC822),
-		ContainerCommand: c.container.Path + " " + strings.Join(c.container.Args, " "),
-		ImageID:          c.container.Image,
-		ContainerIPs: strings.Join(append(c.container.NetworkSettings.SecondaryIPAddresses,
-			c.container.NetworkSettings.IPAddress), " "),
-		ContainerHostname: c.Hostname(),
+		ContainerID:            c.ID(),
+		ContainerName:          strings.TrimPrefix(c.container.Name, "/"),
+		ContainerPorts:         c.ports(localAddrs),
+		ContainerCreated:       c.container.Created.Format(time.RFC822),
+		ContainerCommand:       c.container.Path + " " + strings.Join(c.container.Args, " "),
+		ImageID:                c.container.Image,
+		ContainerIPs:           strings.Join(ips, " "),
+		ContainerIPsWithScopes: strings.Join(ipsWithScopes, " "),
+		ContainerHostname:      c.Hostname(),
 	})
 	AddLabels(result, c.container.Config.Labels)
 
@@ -268,3 +276,9 @@ func (c *container) GetNode(localAddrs []net.IP) report.Node {
 func ExtractContainerIPs(nmd report.Node) []string {
 	return strings.Fields(nmd.Metadata[ContainerIPs])
 }
+
+// ExtractContainerIPsWithScopes returns the list of container IPs, prepended
+// with scopes, given a Node from the Container topology.
+func ExtractContainerIPsWithScopes(nmd report.Node) []string {
+	return strings.Fields(nmd.Metadata[ContainerIPsWithScopes])
+}

probe/docker/container_linux_test.go

@@ -66,19 +66,20 @@ func TestContainer(t *testing.T) {
 
 	// Now see if we go them
 	want := report.MakeNode().WithMetadata(map[string]string{
-		"docker_container_command": " ",
-		"docker_container_created": "01 Jan 01 00:00 UTC",
-		"docker_container_id":      "ping",
-		"docker_container_ips":     "1.2.3.4",
-		"docker_container_name":    "pong",
-		"docker_container_ports":   "1.2.3.4:80->80/tcp, 81/tcp",
-		"docker_image_id":          "baz",
-		"docker_label_foo1":        "bar1",
-		"docker_label_foo2":        "bar2",
-		"memory_usage":             "12345",
+		"docker_container_command":         " ",
+		"docker_container_created":         "01 Jan 01 00:00 UTC",
+		"docker_container_id":              "ping",
+		"docker_container_ips":             "1.2.3.4",
+		"docker_container_ips_with_scopes": "scope;1.2.3.4",
+		"docker_container_name":            "pong",
+		"docker_container_ports":           "1.2.3.4:80->80/tcp, 81/tcp",
+		"docker_image_id":                  "baz",
+		"docker_label_foo1":                "bar1",
+		"docker_label_foo2":                "bar2",
+		"memory_usage":                     "12345",
 	})
 	test.Poll(t, 100*time.Millisecond, want, func() interface{} {
-		node := c.GetNode([]net.IP{})
+		node := c.GetNode("scope", []net.IP{})
 		for k, v := range node.Metadata {
 			if v == "0" || v == "" {
 				delete(node.Metadata, k)
@@ -93,7 +94,7 @@ func TestContainer(t *testing.T) {
 	if c.PID() != 1 {
 		t.Errorf("%s != 1", c.PID())
 	}
-	if !reflect.DeepEqual(docker.ExtractContainerIPs(c.GetNode([]net.IP{})), []string{"1.2.3.4"}) {
-		t.Errorf("%v != %v", docker.ExtractContainerIPs(c.GetNode([]net.IP{})), []string{"1.2.3.4"})
+	if have := docker.ExtractContainerIPs(c.GetNode("", []net.IP{})); !reflect.DeepEqual(have, []string{"1.2.3.4"}) {
+		t.Errorf("%v != %v", have, []string{"1.2.3.4"})
 	}
 }

probe/docker/registry_test.go

@@ -41,7 +41,7 @@ func (c *mockContainer) StartGatheringStats() error {
 
 func (c *mockContainer) StopGatheringStats() {}
 
-func (c *mockContainer) GetNode(_ []net.IP) report.Node {
+func (c *mockContainer) GetNode(_ string, _ []net.IP) report.Node {
 	return report.MakeNodeWith(map[string]string{
 		docker.ContainerID:   c.c.ID,
 		docker.ContainerName: c.c.Name,

probe/docker/reporter.go

@@ -46,7 +46,7 @@ func (r *Reporter) containerTopology(localAddrs []net.IP) report.Topology {
 
 	r.registry.WalkContainers(func(c Container) {
 		nodeID := report.MakeContainerNodeID(r.hostID, c.ID())
-		result.AddNode(nodeID, c.GetNode(localAddrs))
+		result.AddNode(nodeID, c.GetNode(r.hostID, localAddrs))
 	})
 
 	return result

probe/overlay/weave.go

@@ -178,6 +178,13 @@ func (w *Weave) Tag(r report.Report) (report.Report, error) {
 		existingIPs := report.MakeIDList(docker.ExtractContainerIPs(node)...)
 		existingIPs = existingIPs.Add(e.ips...)
 		node.Metadata[docker.ContainerIPs] = strings.Join(existingIPs, " ")
+
+		existingIPsWithScopes := report.MakeIDList(docker.ExtractContainerIPsWithScopes(node)...)
+		for _, ip := range e.ips {
+			existingIPsWithScopes = existingIPsWithScopes.Add(report.MakeAddressNodeID("", ip))
+		}
+		node.Metadata[docker.ContainerIPsWithScopes] = strings.Join(existingIPsWithScopes, " ")
+
 		node.Metadata[WeaveMACAddress] = e.macAddress
 	}
 	return r, nil

probe/overlay/weave_test.go

@@ -51,10 +51,11 @@ func TestWeaveTaggerOverlayTopology(t *testing.T) {
 			Container: report.Topology{
 				Nodes: report.Nodes{
 					nodeID: report.MakeNodeWith(map[string]string{
-						docker.ContainerID:       mockContainerID,
-						overlay.WeaveDNSHostname: mockHostname,
-						overlay.WeaveMACAddress:  mockContainerMAC,
-						docker.ContainerIPs:      mockContainerIP,
+						docker.ContainerID:            mockContainerID,
+						overlay.WeaveDNSHostname:      mockHostname,
+						overlay.WeaveMACAddress:       mockContainerMAC,
+						docker.ContainerIPs:           mockContainerIP,
+						docker.ContainerIPsWithScopes: mockContainerIPWithScope,
 					}),
 				},
 			},
@@ -78,13 +79,14 @@ func TestWeaveTaggerOverlayTopology(t *testing.T) {
 }
 
 const (
-	mockHostID            = "host1"
-	mockWeavePeerName     = "winnebago"
-	mockWeavePeerNickName = "winny"
-	mockContainerID       = "83183a667c01"
-	mockContainerMAC      = "d6:f2:5a:12:36:a8"
-	mockContainerIP       = "10.0.0.123"
-	mockHostname          = "hostname.weave.local"
+	mockHostID               = "host1"
+	mockWeavePeerName        = "winnebago"
+	mockWeavePeerNickName    = "winny"
+	mockContainerID          = "83183a667c01"
+	mockContainerMAC         = "d6:f2:5a:12:36:a8"
+	mockContainerIP          = "10.0.0.123"
+	mockContainerIPWithScope = ";10.0.0.123"
+	mockHostname             = "hostname.weave.local"
 )
 
 var (

render/mapping.go

@@ -283,23 +283,25 @@ func MapEndpoint2IP(m RenderableNode, local report.Networks) RenderableNodes {
 	if ok {
 		return RenderableNodes{}
 	}
-	addr, ok := m.Metadata[endpoint.Addr]
+	scope, addr, port, ok := report.ParseEndpointNodeID(m.ID)
 	if !ok {
 		return RenderableNodes{}
 	}
-	if !local.Contains(net.ParseIP(addr)) {
+	if ip := net.ParseIP(addr); ip != nil && !local.Contains(ip) {
 		return RenderableNodes{TheInternetID: newDerivedPseudoNode(TheInternetID, TheInternetMajor, m)}
 	}
 
-	result := RenderableNodes{addr: NewRenderableNodeWith(addr, "", "", "", m)}
-	// Emit addr:port nodes as well, so connections from the internet to containers
-	// via port mapping also works.
-	port, ok := m.Metadata[endpoint.Port]
-	if ok {
-		id := fmt.Sprintf("%s:%s", addr, port)
-		result[id] = NewRenderableNodeWith(id, "", "", "", m)
+	// We don't always know what port a container is listening on, and
+	// container-to-container communications can be unambiguously identified
+	// without ports. OTOH, connections to the host IPs which have been port
+	// mapped to a container can only be unambiguously identified with the port.
+	// So we need to emit two nodes, for two different cases.
+	id := report.MakeScopedEndpointNodeID(scope, addr, "")
+	idWithPort := report.MakeScopedEndpointNodeID(scope, addr, port)
+	return RenderableNodes{
+		id:         NewRenderableNodeWith(id, "", "", "", m),
+		idWithPort: NewRenderableNodeWith(idWithPort, "", "", "", m),
 	}
-	return result
 }
 
 var portMappingMatch = regexp.MustCompile(`([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}):([0-9]+)->([0-9]+)/tcp`)
@@ -309,20 +311,24 @@ var portMappingMatch = regexp.MustCompile(`([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.
 // the endpoint topology.
 func MapContainer2IP(m RenderableNode, _ report.Networks) RenderableNodes {
 	result := RenderableNodes{}
-	addrs, ok := m.Metadata[docker.ContainerIPs]
-	if !ok {
-		return result
-	}
-	for _, addr := range strings.Fields(addrs) {
-		node := NewRenderableNodeWith(addr, "", "", "", m)
-		node.Counters[containersKey] = 1
-		result[addr] = node
+	if addrs, ok := m.Metadata[docker.ContainerIPsWithScopes]; ok {
+		for _, addr := range strings.Fields(addrs) {
+			scope, addr, ok := report.ParseAddressNodeID(addr)
+			if !ok {
+				continue
+			}
+			id := report.MakeScopedEndpointNodeID(scope, addr, "")
+			node := NewRenderableNodeWith(id, "", "", "", m)
+			node.Counters[containersKey] = 1
+			result[id] = node
+		}
 	}
 
-	// also output all the host:port port mappings
+	// Also output all the host:port port mappings (see above comment).
+	// In this case we assume this doesn't need a scope, as they are for host IPs.
 	for _, mapping := range portMappingMatch.FindAllStringSubmatch(m.Metadata[docker.ContainerPorts], -1) {
 		ip, port := mapping[1], mapping[2]
-		id := fmt.Sprintf("%s:%s", ip, port)
+		id := report.MakeScopedEndpointNodeID("", ip, port)
 		node := NewRenderableNodeWith(id, "", "", "", m)
 		node.Counters[containersKey] = 1
 		result[id] = node

report/id.go

@@ -76,6 +76,18 @@ func MakeAddressNodeID(hostID, address string) string {
 	return scope + ScopeDelim + address
 }
 
+// MakeScopedEndpointNodeID is like MakeEndpointNodeID, but it always
+// prefixes the ID witha scope.
+func MakeScopedEndpointNodeID(hostID, address, port string) string {
+	return hostID + ScopeDelim + address + ScopeDelim + port
+}
+
+// MakeScopedAddressNodeID is like MakeAddressNodeID, but it always
+// prefixes the ID witha scope.
+func MakeScopedAddressNodeID(hostID, address string) string {
+	return hostID + ScopeDelim + address
+}
+
 // MakeProcessNodeID produces a process node ID from its composite parts.
 func MakeProcessNodeID(hostID, pid string) string {
 	return hostID + ScopeDelim + pid