From c68516ec221d67ce32af2126e02be3dec79fb254 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 28 Sep 2015 11:41:57 +0200 Subject: [PATCH 1/7] report: add StringSet (port of IDList, effectively) --- report/topology.go | 100 +++++++++++++++++++++++++++++++++++++++- report/topology_test.go | 66 ++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 report/topology_test.go diff --git a/report/topology.go b/report/topology.go index 0fb7b9765..dc8ffbd3c 100644 --- a/report/topology.go +++ b/report/topology.go @@ -2,6 +2,7 @@ package report import ( "fmt" + "sort" "strings" ) @@ -78,8 +79,9 @@ func (n Nodes) Merge(other Nodes) Nodes { // given node in a given topology, along with the edges emanating from the // node and metadata about those edges. type Node struct { - Metadata `json:"metadata,omitempty"` - Counters `json:"counters,omitempty"` + Metadata Metadata `json:"metadata,omitempty"` + Counters Counters `json:"counters,omitempty"` + Sets Sets `json:"sets,omitempty"` Adjacency IDList `json:"adjacency"` Edges EdgeMetadatas `json:"edges,omitempty"` } @@ -195,6 +197,100 @@ func (c Counters) Copy() Counters { return result } +// Sets is a string->set-of-strings map. +type Sets map[string]StringSet + +// Merge merges two sets maps into a fresh set, performing set-union merges as +// appropriate. +func (s Sets) Merge(other Sets) Sets { + result := s.Copy() + for k, v := range other { + result[k].Merge(v) + } + return result +} + +// Copy returns a value copy of the sets map. +func (s Sets) Copy() Sets { + result := Sets{} + for k, v := range s { + result[k] = v.Copy() + } + return result +} + +// StringSet is a sorted set of unique strings. Clients must use the Add +// method to add strings. +type StringSet []string + +// MakeStringSet makes a new StringSet with the given strings. +func MakeStringSet(strs ...string) StringSet { + if len(strs) <= 0 { + return StringSet{} + } + sort.Strings(strs) + for i := 1; i < len(strs); { // shuffle down any duplicates + if strs[i-1] == strs[i] { + strs = append(strs[:i-1], strs[i:]...) + continue + } + i++ + } + return StringSet(strs) +} + +// Add adds the strings to the StringSet. Add is the only valid way to grow a +// StringSet. Add returns the StringSet to enable chaining. +func (s StringSet) Add(strs ...string) StringSet { + for _, str := range strs { + i := sort.Search(len(s), func(i int) bool { return s[i] >= str }) + if i < len(s) && s[i] == str { + // The list already has the element. + continue + } + // It a new element, insert it in order. + s = append(s, "") + copy(s[i+1:], s[i:]) + s[i] = str + } + return s +} + +// Merge combines the two StringSets and returns a new result. +func (s StringSet) Merge(other StringSet) StringSet { + if len(other) == 0 { // Optimise special case, to avoid allocating + return s // (note unit test DeepEquals breaks if we don't do this) + } + result := make(StringSet, len(s)+len(other)) + for i, j, k := 0, 0, 0; ; k++ { + switch { + case i >= len(s): + copy(result[k:], other[j:]) + return result[:k+len(other)-j] + case j >= len(other): + copy(result[k:], s[i:]) + return result[:k+len(s)-i] + case s[i] < other[j]: + result[k] = s[i] + i++ + case s[i] > other[j]: + result[k] = other[j] + j++ + default: // equal + result[k] = s[i] + i++ + j++ + } + } +} + +// Copy returns a value copy of the StringSet. +func (s StringSet) Copy() StringSet { + result := make(StringSet, len(s)) + copy(result, s) + return result +} + // EdgeMetadatas collect metadata about each edge in a topology. Keys are the // remote node IDs, as in Adjacency. type EdgeMetadatas map[string]EdgeMetadata diff --git a/report/topology_test.go b/report/topology_test.go new file mode 100644 index 000000000..bd0a3a978 --- /dev/null +++ b/report/topology_test.go @@ -0,0 +1,66 @@ +package report_test + +import ( + "reflect" + "testing" + + "github.com/weaveworks/scope/report" +) + +func TestMakeStringSet(t *testing.T) { + for _, testcase := range []struct { + input []string + want report.StringSet + }{ + {input: []string{}, want: report.MakeStringSet()}, + {input: []string{"a"}, want: report.MakeStringSet("a")}, + {input: []string{"a", "a"}, want: report.MakeStringSet("a")}, + {input: []string{"b", "c", "a"}, want: report.MakeStringSet("a", "b", "c")}, + } { + if want, have := testcase.want, report.MakeStringSet(testcase.input...); !reflect.DeepEqual(want, have) { + t.Errorf("%v: want %v, have %v", testcase.input, want, have) + } + } +} + +func TestStringSetAdd(t *testing.T) { + for _, testcase := range []struct { + input report.StringSet + strs []string + want report.StringSet + }{ + {input: report.MakeStringSet(), strs: []string{}, want: report.MakeStringSet()}, + {input: report.MakeStringSet("a"), strs: []string{}, want: report.MakeStringSet("a")}, + {input: report.MakeStringSet(), strs: []string{"a"}, want: report.MakeStringSet("a")}, + {input: report.MakeStringSet("a"), strs: []string{"a"}, want: report.MakeStringSet("a")}, + {input: report.MakeStringSet("b"), strs: []string{"a", "b"}, want: report.MakeStringSet("a", "b")}, + {input: report.MakeStringSet("a"), strs: []string{"c", "b"}, want: report.MakeStringSet("a", "b", "c")}, + {input: report.MakeStringSet("a", "c"), strs: []string{"b", "b", "b"}, want: report.MakeStringSet("a", "b", "c")}, + } { + if want, have := testcase.want, testcase.input.Add(testcase.strs...); !reflect.DeepEqual(want, have) { + t.Errorf("%v + %v: want %v, have %v", testcase.input, testcase.strs, want, have) + } + } +} + +func TestStringSetMerge(t *testing.T) { + for _, testcase := range []struct { + input report.StringSet + other report.StringSet + want report.StringSet + }{ + {input: report.MakeStringSet(), other: report.MakeStringSet(), want: report.MakeStringSet()}, + {input: report.MakeStringSet("a"), other: report.MakeStringSet(), want: report.MakeStringSet("a")}, + {input: report.MakeStringSet(), other: report.MakeStringSet("a"), want: report.MakeStringSet("a")}, + {input: report.MakeStringSet("a"), other: report.MakeStringSet("b"), want: report.MakeStringSet("a", "b")}, + {input: report.MakeStringSet("b"), other: report.MakeStringSet("a"), want: report.MakeStringSet("a", "b")}, + {input: report.MakeStringSet("a"), other: report.MakeStringSet("a"), want: report.MakeStringSet("a")}, + {input: report.MakeStringSet("a", "c"), other: report.MakeStringSet("a", "b"), want: report.MakeStringSet("a", "b", "c")}, + {input: report.MakeStringSet("b"), other: report.MakeStringSet("a"), want: report.MakeStringSet("a", "b")}, + } { + if want, have := testcase.want, testcase.input.Merge(testcase.other); !reflect.DeepEqual(want, have) { + t.Errorf("%v + %v: want %v, have %v", testcase.input, testcase.other, want, have) + } + } + +} From 5a0e23b3eade3afdfc80e92ad190414d26eec570 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 27 Oct 2015 11:19:23 +0000 Subject: [PATCH 2/7] Some additional methods for using Sets. --- report/topology.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/report/topology.go b/report/topology.go index dc8ffbd3c..5faf1cc48 100644 --- a/report/topology.go +++ b/report/topology.go @@ -91,6 +91,7 @@ func MakeNode() Node { return Node{ Metadata: Metadata{}, Counters: Counters{}, + Sets: Sets{}, Adjacency: MakeIDList(), Edges: EdgeMetadatas{}, } @@ -115,6 +116,13 @@ func (n Node) WithCounters(c map[string]int) Node { return result } +func (n Node) WithSet(key string, set StringSet) Node { + result := n.Copy() + existing := n.Sets[key] + result.Sets[key] = existing.Merge(set) + return result +} + // WithAdjacent returns a fresh copy of n, with 'a' added to Adjacency func (n Node) WithAdjacent(a string) Node { result := n.Copy() @@ -136,6 +144,7 @@ func (n Node) Copy() Node { cp := MakeNode() cp.Metadata = n.Metadata.Copy() cp.Counters = n.Counters.Copy() + cp.Sets = n.Sets.Copy() cp.Adjacency = n.Adjacency.Copy() cp.Edges = n.Edges.Copy() return cp @@ -147,6 +156,7 @@ func (n Node) Merge(other Node) Node { cp := n.Copy() cp.Metadata = cp.Metadata.Merge(other.Metadata) cp.Counters = cp.Counters.Merge(other.Counters) + cp.Sets = cp.Sets.Merge(other.Sets) cp.Adjacency = cp.Adjacency.Merge(other.Adjacency) cp.Edges = cp.Edges.Merge(other.Edges) return cp From 69134311380f70cd95d072fbf0832bc7f0cc54e6 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 27 Oct 2015 13:12:37 +0000 Subject: [PATCH 3/7] Make IDList an alias for StringSet --- report/id_list.go | 57 ++++------------------------------------------ report/topology.go | 1 + 2 files changed, 6 insertions(+), 52 deletions(-) diff --git a/report/id_list.go b/report/id_list.go index c69ed6e5f..a530d3c92 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -3,73 +3,26 @@ package report import "sort" // IDList is a list of string IDs, which are always sorted and unique. -type IDList []string +type IDList StringSet // MakeIDList makes a new IDList. func MakeIDList(ids ...string) IDList { - if len(ids) <= 0 { - return IDList{} - } - sort.Strings(ids) - for i := 1; i < len(ids); { // shuffle down any duplicates - if ids[i-1] == ids[i] { - ids = append(ids[:i-1], ids[i:]...) - continue - } - i++ - } - return IDList(ids) + return IDList(MakeStringSet(ids...)) } // Add is the only correct way to add ids to an IDList. func (a IDList) Add(ids ...string) IDList { - for _, s := range ids { - i := sort.Search(len(a), func(i int) bool { return a[i] >= s }) - if i < len(a) && a[i] == s { - // The list already has the element. - continue - } - // It a new element, insert it in order. - a = append(a, "") - copy(a[i+1:], a[i:]) - a[i] = s - } - return a + return IDList(StringSet(a).Add(ids...)) } // Copy returns a copy of the IDList. func (a IDList) Copy() IDList { - result := make(IDList, len(a)) - copy(result, a) - return result + return IDList(StringSet(a).Copy()) } // Merge all elements from a and b into a new list func (a IDList) Merge(b IDList) IDList { - if len(b) == 0 { // Optimise special case, to avoid allocating - return a // (note unit test DeepEquals breaks if we don't do this) - } - d := make(IDList, len(a)+len(b)) - for i, j, k := 0, 0, 0; ; k++ { - switch { - case i >= len(a): - copy(d[k:], b[j:]) - return d[:k+len(b)-j] - case j >= len(b): - copy(d[k:], a[i:]) - return d[:k+len(a)-i] - case a[i] < b[j]: - d[k] = a[i] - i++ - case a[i] > b[j]: - d[k] = b[j] - j++ - default: // equal - d[k] = a[i] - i++ - j++ - } - } + return IDList(StringSet(a).Merge(StringSet(b))) } // Contains returns true if id is in the list. diff --git a/report/topology.go b/report/topology.go index 5faf1cc48..679337267 100644 --- a/report/topology.go +++ b/report/topology.go @@ -116,6 +116,7 @@ func (n Node) WithCounters(c map[string]int) Node { return result } +// WithSet returns a fresh copy of n, with set merged in at key. func (n Node) WithSet(key string, set StringSet) Node { result := n.Copy() existing := n.Sets[key] From 6e9ad995b02b6ef7abd2a3642a7a33c1e7624b6a Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 27 Oct 2015 14:08:56 +0000 Subject: [PATCH 4/7] Use Node.Sets for Docker/Weave IPs --- probe/docker/container.go | 27 ++++++++++++------------ probe/docker/container_linux_test.go | 7 ++++--- probe/overlay/weave.go | 29 ++++++++++++-------------- probe/overlay/weave_test.go | 5 +++-- render/mapping.go | 18 +++++++++------- render/renderable_node.go | 1 + render/short_lived_connections_test.go | 5 +++-- report/topology.go | 9 +++++++- 8 files changed, 56 insertions(+), 45 deletions(-) diff --git a/probe/docker/container.go b/probe/docker/container.go index 8f78d34bb..af721b8cd 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -195,9 +195,9 @@ func (c *container) StopGatheringStats() { return } -func (c *container) ports(localAddrs []net.IP) string { +func (c *container) ports(localAddrs []net.IP) report.StringSet { if c.container.NetworkSettings == nil { - return "" + return report.MakeStringSet() } ports := []string{} @@ -217,7 +217,7 @@ func (c *container) ports(localAddrs []net.IP) string { } } - return strings.Join(ports, ", ") + return report.MakeStringSet(ports...) } func (c *container) GetNode(hostID string, localAddrs []net.IP) report.Node { @@ -232,15 +232,16 @@ func (c *container) GetNode(hostID string, localAddrs []net.IP) report.Node { } result := report.MakeNodeWith(map[string]string{ - ContainerID: c.ID(), - ContainerName: strings.TrimPrefix(c.container.Name, "/"), + ContainerID: c.ID(), + ContainerName: strings.TrimPrefix(c.container.Name, "/"), + ContainerCreated: c.container.Created.Format(time.RFC822), + ContainerCommand: c.container.Path + " " + strings.Join(c.container.Args, " "), + ImageID: c.container.Image, + ContainerHostname: c.Hostname(), + }).WithSets(report.Sets{ 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(), + ContainerIPs: report.MakeStringSet(ips...), + ContainerIPsWithScopes: report.MakeStringSet(ipsWithScopes...), }) AddLabels(result, c.container.Config.Labels) @@ -274,11 +275,11 @@ func (c *container) GetNode(hostID string, localAddrs []net.IP) report.Node { // ExtractContainerIPs returns the list of container IPs given a Node from the Container topology. func ExtractContainerIPs(nmd report.Node) []string { - return strings.Fields(nmd.Metadata[ContainerIPs]) + return []string(nmd.Sets[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]) + return []string(nmd.Sets[ContainerIPsWithScopes]) } diff --git a/probe/docker/container_linux_test.go b/probe/docker/container_linux_test.go index 39f11b994..5e4e76da8 100644 --- a/probe/docker/container_linux_test.go +++ b/probe/docker/container_linux_test.go @@ -69,14 +69,15 @@ func TestContainer(t *testing.T) { "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", + }).WithSets(report.Sets{ + "docker_container_ports": report.MakeStringSet("1.2.3.4:80->80/tcp", "81/tcp"), + "docker_container_ips": report.MakeStringSet("1.2.3.4"), + "docker_container_ips_with_scopes": report.MakeStringSet("scope;1.2.3.4"), }) test.Poll(t, 100*time.Millisecond, want, func() interface{} { node := c.GetNode("scope", []net.IP{}) diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index 6c4c87ae4..05ff51b92 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -164,28 +164,25 @@ func (w *Weave) Tag(r report.Report) (report.Report, error) { if err != nil { return r, nil } - containersByPrefix := map[string]report.Node{} - for _, node := range r.Container.Nodes { - prefix := node.Metadata[docker.ContainerID][:12] - containersByPrefix[prefix] = node + psEntriesByPrefix := map[string]psEntry{} + for _, entry := range psEntries { + psEntriesByPrefix[entry.containerIDPrefix] = entry } - for _, e := range psEntries { - node, ok := containersByPrefix[e.containerIDPrefix] + for id, node := range r.Container.Nodes { + prefix := node.Metadata[docker.ContainerID][:12] + entry, ok := psEntriesByPrefix[prefix] if !ok { continue } - 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)) + ipsWithScope := report.MakeStringSet() + for _, ip := range entry.ips { + ipsWithScope = ipsWithScope.Add(report.MakeAddressNodeID("", ip)) } - node.Metadata[docker.ContainerIPsWithScopes] = strings.Join(existingIPsWithScopes, " ") - - node.Metadata[WeaveMACAddress] = e.macAddress + node = node.WithSet(docker.ContainerIPs, report.MakeStringSet(entry.ips...)) + node = node.WithSet(docker.ContainerIPsWithScopes, ipsWithScope) + node.Metadata[WeaveMACAddress] = entry.macAddress + r.Container.Nodes[id] = node } return r, nil } diff --git a/probe/overlay/weave_test.go b/probe/overlay/weave_test.go index 3950653d8..02914d48d 100644 --- a/probe/overlay/weave_test.go +++ b/probe/overlay/weave_test.go @@ -54,8 +54,9 @@ func TestWeaveTaggerOverlayTopology(t *testing.T) { docker.ContainerID: mockContainerID, overlay.WeaveDNSHostname: mockHostname, overlay.WeaveMACAddress: mockContainerMAC, - docker.ContainerIPs: mockContainerIP, - docker.ContainerIPsWithScopes: mockContainerIPWithScope, + }).WithSets(report.Sets{ + docker.ContainerIPs: report.MakeStringSet(mockContainerIP), + docker.ContainerIPsWithScopes: report.MakeStringSet(mockContainerIPWithScope), }), }, }, diff --git a/render/mapping.go b/render/mapping.go index 3fead832d..b381a3ca9 100644 --- a/render/mapping.go +++ b/render/mapping.go @@ -311,8 +311,8 @@ 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{} - if addrs, ok := m.Metadata[docker.ContainerIPsWithScopes]; ok { - for _, addr := range strings.Fields(addrs) { + if addrs, ok := m.Sets[docker.ContainerIPsWithScopes]; ok { + for _, addr := range addrs { scope, addr, ok := report.ParseAddressNodeID(addr) if !ok { continue @@ -326,12 +326,14 @@ func MapContainer2IP(m RenderableNode, _ report.Networks) RenderableNodes { // 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 := report.MakeScopedEndpointNodeID("", ip, port) - node := NewRenderableNodeWith(id, "", "", "", m) - node.Counters[containersKey] = 1 - result[id] = node + for _, portMapping := range m.Sets[docker.ContainerPorts] { + if mapping := portMappingMatch.FindStringSubmatch(portMapping); mapping != nil { + ip, port := mapping[1], mapping[2] + id := report.MakeScopedEndpointNodeID("", ip, port) + node := NewRenderableNodeWith(id, "", "", "", m) + node.Counters[containersKey] = 1 + result[id] = node + } } return result diff --git a/render/renderable_node.go b/render/renderable_node.go index 591ede43d..7af08e81b 100644 --- a/render/renderable_node.go +++ b/render/renderable_node.go @@ -130,6 +130,7 @@ func (rn RenderableNode) Prune() RenderableNode { cp.Node.Metadata = report.Metadata{} // snip cp.Node.Counters = report.Counters{} // snip cp.Node.Edges = report.EdgeMetadatas{} // snip + cp.Node.Sets = report.Sets{} // snip return cp } diff --git a/render/short_lived_connections_test.go b/render/short_lived_connections_test.go index b5d6b1a93..7461a5bd5 100644 --- a/render/short_lived_connections_test.go +++ b/render/short_lived_connections_test.go @@ -50,9 +50,10 @@ var ( containerNodeID: report.MakeNode().WithMetadata(map[string]string{ docker.ContainerID: containerID, docker.ContainerName: containerName, - docker.ContainerIPs: containerIP, - docker.ContainerPorts: fmt.Sprintf("%s:%s->%s/tcp", serverIP, serverPort, serverPort), report.HostNodeID: serverHostNodeID, + }).WithSets(report.Sets{ + docker.ContainerIPs: report.MakeStringSet(containerIP), + docker.ContainerPorts: report.MakeStringSet(fmt.Sprintf("%s:%s->%s/tcp", serverIP, serverPort, serverPort)), }), }, }, diff --git a/report/topology.go b/report/topology.go index 679337267..7b5c5e0c7 100644 --- a/report/topology.go +++ b/report/topology.go @@ -124,6 +124,13 @@ func (n Node) WithSet(key string, set StringSet) Node { return result } +// WithSets returns a fresh copy of n, with sets merged in. +func (n Node) WithSets(sets Sets) Node { + result := n.Copy() + result.Sets = result.Sets.Merge(sets) + return result +} + // WithAdjacent returns a fresh copy of n, with 'a' added to Adjacency func (n Node) WithAdjacent(a string) Node { result := n.Copy() @@ -216,7 +223,7 @@ type Sets map[string]StringSet func (s Sets) Merge(other Sets) Sets { result := s.Copy() for k, v := range other { - result[k].Merge(v) + result[k] = result[k].Merge(v) } return result } From ec4425fb807bafaafb1dbf35db1ae40b40aec3a0 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 27 Oct 2015 14:35:07 +0000 Subject: [PATCH 5/7] Use Node.Sets for Host Local Networks. --- app/origin_host.go | 2 +- probe/host/reporter.go | 4 ++-- probe/host/reporter_test.go | 3 ++- render/short_lived_connections_test.go | 10 ++++++---- render/theinternet.go | 7 +------ render/theinternet_test.go | 5 +++-- test/fixture/report_fixture.go | 7 +++++-- 7 files changed, 20 insertions(+), 18 deletions(-) diff --git a/app/origin_host.go b/app/origin_host.go index 3a5cdfd3b..03e7921aa 100644 --- a/app/origin_host.go +++ b/app/origin_host.go @@ -30,7 +30,7 @@ func getOriginHost(t report.Topology, nodeID string) (OriginHost, bool) { return OriginHost{ Hostname: h.Metadata[host.HostName], OS: h.Metadata[host.OS], - Networks: strings.Split(h.Metadata[host.LocalNetworks], " "), + Networks: h.Sets[host.LocalNetworks], Load: h.Metadata[host.Load], }, true } diff --git a/probe/host/reporter.go b/probe/host/reporter.go index c0226f011..ab4ca9249 100644 --- a/probe/host/reporter.go +++ b/probe/host/reporter.go @@ -2,7 +2,6 @@ package host import ( "runtime" - "strings" "time" "github.com/weaveworks/scope/report" @@ -71,11 +70,12 @@ func (r *Reporter) Report() (report.Report, error) { rep.Host.AddNode(report.MakeHostNodeID(r.hostID), report.MakeNodeWith(map[string]string{ Timestamp: Now(), HostName: r.hostName, - LocalNetworks: strings.Join(localCIDRs, " "), OS: runtime.GOOS, Load: GetLoad(), KernelVersion: kernel, Uptime: uptime.String(), + }).WithSets(report.Sets{ + LocalNetworks: report.MakeStringSet(localCIDRs...), })) return rep, nil diff --git a/probe/host/reporter_test.go b/probe/host/reporter_test.go index d41d47e47..48529294b 100644 --- a/probe/host/reporter_test.go +++ b/probe/host/reporter_test.go @@ -48,11 +48,12 @@ func TestReporter(t *testing.T) { want.Host.AddNode(report.MakeHostNodeID(hostID), report.MakeNodeWith(map[string]string{ host.Timestamp: now, host.HostName: hostname, - host.LocalNetworks: network, host.OS: runtime.GOOS, host.Load: load, host.Uptime: uptime, host.KernelVersion: kernel, + }).WithSets(report.Sets{ + host.LocalNetworks: report.MakeStringSet(network), })) have, _ := host.NewReporter(hostID, hostname, localNets).Report() if !reflect.DeepEqual(want, have) { diff --git a/render/short_lived_connections_test.go b/render/short_lived_connections_test.go index 7461a5bd5..8f36c9507 100644 --- a/render/short_lived_connections_test.go +++ b/render/short_lived_connections_test.go @@ -7,6 +7,7 @@ import ( "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/probe/endpoint" + "github.com/weaveworks/scope/probe/host" "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" @@ -48,9 +49,9 @@ var ( Container: report.Topology{ Nodes: report.Nodes{ containerNodeID: report.MakeNode().WithMetadata(map[string]string{ - docker.ContainerID: containerID, - docker.ContainerName: containerName, - report.HostNodeID: serverHostNodeID, + docker.ContainerID: containerID, + docker.ContainerName: containerName, + report.HostNodeID: serverHostNodeID, }).WithSets(report.Sets{ docker.ContainerIPs: report.MakeStringSet(containerIP), docker.ContainerPorts: report.MakeStringSet(fmt.Sprintf("%s:%s->%s/tcp", serverIP, serverPort, serverPort)), @@ -60,8 +61,9 @@ var ( Host: report.Topology{ Nodes: report.Nodes{ serverHostNodeID: report.MakeNodeWith(map[string]string{ - "local_networks": "192.168.0.0/16", report.HostNodeID: serverHostNodeID, + }).WithSets(report.Sets{ + host.LocalNetworks: report.MakeStringSet("192.168.0.0/16"), }), }, }, diff --git a/render/theinternet.go b/render/theinternet.go index 4dc291381..4e67bb820 100644 --- a/render/theinternet.go +++ b/render/theinternet.go @@ -2,7 +2,6 @@ package render import ( "net" - "strings" "github.com/weaveworks/scope/probe/host" "github.com/weaveworks/scope/report" @@ -19,11 +18,7 @@ func LocalNetworks(r report.Report) report.Networks { ) for _, md := range r.Host.Nodes { - val, ok := md.Metadata[host.LocalNetworks] - if !ok { - continue - } - for _, s := range strings.Fields(val) { + for _, s := range md.Sets[host.LocalNetworks] { _, ipNet, err := net.ParseCIDR(s) if err != nil { continue diff --git a/render/theinternet_test.go b/render/theinternet_test.go index 6d5744d0d..9abfdcb1f 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -16,8 +16,9 @@ func TestReportLocalNetworks(t *testing.T) { Host: report.Topology{ Nodes: report.Nodes{ "nonets": report.MakeNode(), - "foo": report.MakeNodeWith(map[string]string{ - host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33", + "foo": report.MakeNode().WithSets(report.Sets{ + host.LocalNetworks: report.MakeStringSet( + "10.0.0.1/8", "192.168.1.1/24", "10.0.0.1/8", "badnet/33"), }), }, }, diff --git a/test/fixture/report_fixture.go b/test/fixture/report_fixture.go index 7e09d5a88..15b2aab6a 100644 --- a/test/fixture/report_fixture.go +++ b/test/fixture/report_fixture.go @@ -5,6 +5,7 @@ import ( "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/probe/endpoint" + "github.com/weaveworks/scope/probe/host" "github.com/weaveworks/scope/probe/kubernetes" "github.com/weaveworks/scope/probe/process" "github.com/weaveworks/scope/render" @@ -282,17 +283,19 @@ var ( Nodes: report.Nodes{ ClientHostNodeID: report.MakeNodeWith(map[string]string{ "host_name": ClientHostName, - "local_networks": "10.10.10.0/24", "os": "Linux", "load": "0.01 0.01 0.01", report.HostNodeID: ClientHostNodeID, + }).WithSets(report.Sets{ + host.LocalNetworks: report.MakeStringSet("10.10.10.0/24"), }), ServerHostNodeID: report.MakeNodeWith(map[string]string{ "host_name": ServerHostName, - "local_networks": "10.10.10.0/24", "os": "Linux", "load": "0.01 0.01 0.01", report.HostNodeID: ServerHostNodeID, + }).WithSets(report.Sets{ + host.LocalNetworks: report.MakeStringSet("10.10.10.0/24"), }), }, }, From 244e2f4b4a0bde896b24d6e6d393e3fbfdf15cee Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 27 Oct 2015 14:37:21 +0000 Subject: [PATCH 6/7] gofmt + build --- app/origin_host.go | 1 - probe/docker/container_linux_test.go | 16 ++++++++-------- probe/overlay/weave_test.go | 6 +++--- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/origin_host.go b/app/origin_host.go index 03e7921aa..d51676b6a 100644 --- a/app/origin_host.go +++ b/app/origin_host.go @@ -2,7 +2,6 @@ package main import ( "net/http" - "strings" "github.com/gorilla/mux" diff --git a/probe/docker/container_linux_test.go b/probe/docker/container_linux_test.go index 5e4e76da8..3a73cceaa 100644 --- a/probe/docker/container_linux_test.go +++ b/probe/docker/container_linux_test.go @@ -66,14 +66,14 @@ 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_name": "pong", - "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_name": "pong", + "docker_image_id": "baz", + "docker_label_foo1": "bar1", + "docker_label_foo2": "bar2", + "memory_usage": "12345", }).WithSets(report.Sets{ "docker_container_ports": report.MakeStringSet("1.2.3.4:80->80/tcp", "81/tcp"), "docker_container_ips": report.MakeStringSet("1.2.3.4"), diff --git a/probe/overlay/weave_test.go b/probe/overlay/weave_test.go index 02914d48d..ac9a489d7 100644 --- a/probe/overlay/weave_test.go +++ b/probe/overlay/weave_test.go @@ -51,9 +51,9 @@ 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.ContainerID: mockContainerID, + overlay.WeaveDNSHostname: mockHostname, + overlay.WeaveMACAddress: mockContainerMAC, }).WithSets(report.Sets{ docker.ContainerIPs: report.MakeStringSet(mockContainerIP), docker.ContainerIPsWithScopes: report.MakeStringSet(mockContainerIPWithScope), From bdf39aeaabb20d1addab7dd14f01404cca9aae97 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Thu, 29 Oct 2015 12:23:38 +0000 Subject: [PATCH 7/7] Don't mutate the input array for MakeStringSet --- report/topology.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/report/topology.go b/report/topology.go index 7b5c5e0c7..617536f81 100644 --- a/report/topology.go +++ b/report/topology.go @@ -246,15 +246,17 @@ func MakeStringSet(strs ...string) StringSet { if len(strs) <= 0 { return StringSet{} } - sort.Strings(strs) - for i := 1; i < len(strs); { // shuffle down any duplicates - if strs[i-1] == strs[i] { - strs = append(strs[:i-1], strs[i:]...) + result := make([]string, len(strs)) + copy(result, strs) + sort.Strings(result) + for i := 1; i < len(result); { // shuffle down any duplicates + if result[i-1] == result[i] { + result = append(result[:i-1], result[i:]...) continue } i++ } - return StringSet(strs) + return StringSet(result) } // Add adds the strings to the StringSet. Add is the only valid way to grow a