From c65aecd9e138097151f27b02fcf4b61737f72fbf Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 15 Jun 2015 14:36:48 +0200 Subject: [PATCH 1/3] Fix node scoping rules We only want to scope (i.e. prefix with hostID) those addresses that are deemed loopback, to disambiguate them. Otherwise, we want to leave addresses in unscoped form, so they can be matched, and links between communicating nodes properly made. So, we make the isLoopback check in MakeAddressID, and omit hostID if the address isn't loopback. So far so good. But this breaks topology rendering, as we were relying on extracting hostID from adjacency node IDs, to populate origin hosts in the rendered node output. So we need another way to get origin host from an arbitrary node. A survey revealed no reliable way to get that information from IDs in their new form. However, we have access to node metadata, so this changeset introduces the OriginHostTagger, which tags each node with its origin host, via the foreign-key semantics we'll use going forward. --- probe/main.go | 16 ++++++++----- probe/tag/origin_host_tagger.go | 29 ++++++++++++++++++++++++ probe/tag/origin_host_tagger_test.go | 25 ++++++++++++++++++++ probe/tag/topology_tagger.go | 7 ++++-- report/id.go | 16 ++++++++++--- report/report.go | 7 ++++++ report/topology.go | 31 ++++++++++++++----------- report/topology_test.go | 34 ++++++++++++++++++---------- 8 files changed, 129 insertions(+), 36 deletions(-) create mode 100644 probe/tag/origin_host_tagger.go create mode 100644 probe/tag/origin_host_tagger_test.go diff --git a/probe/main.go b/probe/main.go index c5c505f36f..9b04b9763e 100644 --- a/probe/main.go +++ b/probe/main.go @@ -66,7 +66,13 @@ func main() { } defer publisher.Close() - taggers := []tag.Tagger{tag.NewTopologyTagger()} + var ( + hostName = hostname() + hostID = hostName // TODO: we should sanitize the hostname + ) + + taggers := []tag.Tagger{tag.NewTopologyTagger(), tag.NewOriginHostTagger(hostID)} + var dockerTagger *tag.DockerTagger if *dockerEnabled && runtime.GOOS == linux { var err error @@ -84,11 +90,9 @@ func main() { defer close(quit) go func() { var ( - hostName = hostname() - hostID = hostName // TODO: we should sanitize the hostname - pubTick = time.Tick(*publishInterval) - spyTick = time.Tick(*spyInterval) - r = report.MakeReport() + pubTick = time.Tick(*publishInterval) + spyTick = time.Tick(*spyInterval) + r = report.MakeReport() ) for { diff --git a/probe/tag/origin_host_tagger.go b/probe/tag/origin_host_tagger.go new file mode 100644 index 0000000000..7e96be7d9a --- /dev/null +++ b/probe/tag/origin_host_tagger.go @@ -0,0 +1,29 @@ +package tag + +import ( + "github.com/weaveworks/scope/report" +) + +type originHostTagger struct{ hostNodeID string } + +// NewOriginHostTagger tags each node with a foreign key linking it to its +// origin host in the host topology. +func NewOriginHostTagger(hostID string) Tagger { + return &originHostTagger{hostNodeID: report.MakeHostNodeID(hostID)} +} + +func (t originHostTagger) Tag(r report.Report) report.Report { + for _, topology := range []*report.Topology{ + &(r.Endpoint), + &(r.Address), + &(r.Process), + &(r.Container), + &(r.Host), + } { + md := report.NodeMetadata{report.HostNodeID: t.hostNodeID} + for nodeID := range topology.NodeMetadatas { + (*topology).NodeMetadatas[nodeID].Merge(md) + } + } + return r +} diff --git a/probe/tag/origin_host_tagger_test.go b/probe/tag/origin_host_tagger_test.go new file mode 100644 index 0000000000..a53ec8a3cc --- /dev/null +++ b/probe/tag/origin_host_tagger_test.go @@ -0,0 +1,25 @@ +package tag_test + +import ( + "reflect" + "testing" + + "github.com/weaveworks/scope/probe/tag" + "github.com/weaveworks/scope/report" +) + +func TestOriginHostTagger(t *testing.T) { + var ( + hostID = "foo" + endpointNodeID = report.MakeEndpointNodeID(hostID, "1.2.3.4", "56789") // hostID ignored + nodeMetadata = report.NodeMetadata{"foo": "bar"} + ) + + r := report.MakeReport() + r.Endpoint.NodeMetadatas[endpointNodeID] = nodeMetadata + want := nodeMetadata.Merge(report.NodeMetadata{report.HostNodeID: report.MakeHostNodeID(hostID)}) + have := tag.NewOriginHostTagger(hostID).Tag(r).Endpoint.NodeMetadatas[endpointNodeID].Copy() + if !reflect.DeepEqual(want, have) { + t.Errorf("\nwant %+v\nhave %+v", want, have) + } +} diff --git a/probe/tag/topology_tagger.go b/probe/tag/topology_tagger.go index 734c022f3c..a860ca55f1 100644 --- a/probe/tag/topology_tagger.go +++ b/probe/tag/topology_tagger.go @@ -13,8 +13,11 @@ func NewTopologyTagger() Tagger { func (topologyTagger) Tag(r report.Report) report.Report { for val, topology := range map[string]*report.Topology{ - "endpoint": &(r.Endpoint), - "address": &(r.Address), + "endpoint": &(r.Endpoint), + "address": &(r.Address), + "process": &(r.Process), + "container": &(r.Container), + "host": &(r.Host), } { md := report.NodeMetadata{"topology": val} for nodeID := range topology.NodeMetadatas { diff --git a/report/id.go b/report/id.go index 249ed15fb3..5973a51c1a 100644 --- a/report/id.go +++ b/report/id.go @@ -27,7 +27,7 @@ func MakeAdjacencyID(srcNodeID string) string { return ">" + srcNodeID } -// ParseAdjacencyID produces a node id from an adjancency id +// ParseAdjacencyID produces a node ID from an adjancency ID. func ParseAdjacencyID(adjacencyID string) (string, bool) { if !strings.HasPrefix(adjacencyID, ">") { return "", false @@ -56,6 +56,10 @@ func MakeEndpointNodeID(hostID, address, port string) string { // MakeAddressNodeID produces an address node ID from its composite parts. func MakeAddressNodeID(hostID, address string) string { + if !isLoopback(address) { + // Only loopback addresses get scoped by hostID. + hostID = "" + } return hostID + ScopeDelim + address } @@ -77,8 +81,9 @@ func MakeContainerNodeID(hostID, containerID string) string { return hostID + ScopeDelim + containerID } -// ParseNodeID produces the scope and remainder from a node ID -func ParseNodeID(nodeID string) (string, string, bool) { +// ParseNodeID produces the host ID and remainder (typically an address) from +// a node ID. Note that hostID may be blank. +func ParseNodeID(nodeID string) (hostID string, remainder string, ok bool) { fields := strings.SplitN(nodeID, ScopeDelim, 2) if len(fields) != 2 { return "", "", false @@ -120,3 +125,8 @@ func AddressIDAddresser(id string) net.IP { func PanicIDAddresser(id string) net.IP { panic(fmt.Sprintf("PanicIDAddresser called on %q", id)) } + +func isLoopback(address string) bool { + ip := net.ParseIP(address) + return ip != nil && ip.IsLoopback() +} diff --git a/report/report.go b/report/report.go index 889c227766..3a6fdad1c3 100644 --- a/report/report.go +++ b/report/report.go @@ -33,6 +33,13 @@ type Report struct { Host Topology } +const ( + // HostNodeID is a metadata foreign key, linking a node in any topology to + // a node in the host topology. That host node is the origin host, where + // the node was originally detected. + HostNodeID = "host_node_id" +) + // RenderableNode is the data type that's yielded to the JavaScript layer as // an element of a topology. It should contain information that's relevant // to rendering a node when there are many nodes visible at once. diff --git a/report/topology.go b/report/topology.go index 64e64cf601..544102f741 100644 --- a/report/topology.go +++ b/report/topology.go @@ -88,16 +88,19 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod // Build a set of RenderableNodes for all non-pseudo probes, and an // addressID to nodeID lookup map. Multiple addressIDs can map to the same // RenderableNodes. - address2mapped := map[string]string{} + var ( + source2mapped = map[string]string{} // source node ID -> mapped node ID + source2host = map[string]string{} // source node ID -> origin host ID + ) for nodeID, metadata := range t.NodeMetadatas { mapped, ok := mapFunc(nodeID, metadata) if !ok { continue } - // mapped.ID needs not be unique over all addressIDs. If not, we just overwrite - // the existing data, on the assumption that the MapFunc returns the same - // data. + // mapped.ID needs not be unique over all addressIDs. If not, we just + // overwrite the existing data, on the assumption that the MapFunc + // returns the same data. nodes[mapped.ID] = RenderableNode{ ID: mapped.ID, LabelMajor: mapped.Major, @@ -107,24 +110,26 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod Origins: IDList{nodeID}, Metadata: AggregateMetadata{}, // later } - address2mapped[nodeID] = mapped.ID + source2mapped[nodeID] = mapped.ID + source2host[nodeID] = metadata[HostNodeID] } // Walk the graph and make connections. for src, dsts := range t.Adjacency { var ( - srcNodeID, ok1 = ParseAdjacencyID(src) - srcOriginHostID, _, ok2 = ParseNodeID(srcNodeID) - srcRenderableID = address2mapped[srcNodeID] // must exist - srcRenderableNode = nodes[srcRenderableID] // must exist + srcNodeID, ok = ParseAdjacencyID(src) + //srcOriginHostID, _, ok2 = ParseNodeID(srcNodeID) + srcHostNodeID = source2host[srcNodeID] + srcRenderableID = source2mapped[srcNodeID] // must exist + srcRenderableNode = nodes[srcRenderableID] // must exist ) - if !ok1 || !ok2 { + if !ok { log.Printf("bad adjacency ID %q", src) continue } for _, dstNodeID := range dsts { - dstRenderableID, ok := address2mapped[dstNodeID] + dstRenderableID, ok := source2mapped[dstNodeID] if !ok { pseudoNode, ok := pseudoFunc(srcNodeID, srcRenderableNode, dstNodeID) if !ok { @@ -138,11 +143,11 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod Pseudo: true, Metadata: AggregateMetadata{}, // populated below - or not? } - address2mapped[dstNodeID] = dstRenderableID + source2mapped[dstNodeID] = dstRenderableID } srcRenderableNode.Adjacency = srcRenderableNode.Adjacency.Add(dstRenderableID) - srcRenderableNode.Origins = srcRenderableNode.Origins.Add(MakeHostNodeID(srcOriginHostID)) + srcRenderableNode.Origins = srcRenderableNode.Origins.Add(srcHostNodeID) srcRenderableNode.Origins = srcRenderableNode.Origins.Add(srcNodeID) edgeID := MakeEdgeID(srcNodeID, dstNodeID) if md, ok := t.EdgeMetadatas[edgeID]; ok { diff --git a/report/topology_test.go b/report/topology_test.go index e9afc34bff..0a402b5c30 100644 --- a/report/topology_test.go +++ b/report/topology_test.go @@ -19,6 +19,10 @@ var ( randomHostID = "random.hostname.com" unknownHostID = "" + clientHostNodeID = MakeHostNodeID(clientHostID) + serverHostNodeID = MakeHostNodeID(serverHostID) + randomHostNodeID = MakeHostNodeID(randomHostID) + client54001 = MakeEndpointNodeID(clientHostID, "10.10.10.20", "54001") // curl (1) client54002 = MakeEndpointNodeID(clientHostID, "10.10.10.20", "54002") // curl (2) unknownClient1 = MakeEndpointNodeID(serverHostID, "10.10.10.10", "54010") // we want to ensure two unknown clients, connnected @@ -45,19 +49,22 @@ var ( // care to test into the fixture. Just be sure to include the bits // that the mapping funcs extract :) client54001: NodeMetadata{ - "name": "curl", - "domain": "client-54001-domain", - "pid": "10001", + "name": "curl", + "domain": "client-54001-domain", + "pid": "10001", + HostNodeID: clientHostNodeID, }, client54002: NodeMetadata{ - "name": "curl", // should be same as above! - "domain": "client-54002-domain", // may be different than above - "pid": "10001", // should be same as above! + "name": "curl", // should be same as above! + "domain": "client-54002-domain", // may be different than above + "pid": "10001", // should be same as above! + HostNodeID: clientHostNodeID, }, server80: NodeMetadata{ - "name": "apache", - "domain": "server-80-domain", - "pid": "215", + "name": "apache", + "domain": "server-80-domain", + "pid": "215", + HostNodeID: serverHostNodeID, }, }, EdgeMetadatas: EdgeMetadatas{ @@ -107,13 +114,16 @@ var ( }, NodeMetadatas: NodeMetadatas{ clientIP: NodeMetadata{ - "name": "client.hostname.com", // hostname + "name": "client.hostname.com", // hostname + HostNodeID: clientHostNodeID, }, randomIP: NodeMetadata{ - "name": "random.hostname.com", // hostname + "name": "random.hostname.com", // hostname + HostNodeID: randomHostNodeID, }, serverIP: NodeMetadata{ - "name": "server.hostname.com", // hostname + "name": "server.hostname.com", // hostname + HostNodeID: serverHostNodeID, }, }, EdgeMetadatas: EdgeMetadatas{ From c900c17ad52dd22c2ecbd558f15352fab4535c52 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 15 Jun 2015 14:51:19 +0200 Subject: [PATCH 2/3] app: fix tests --- app/api_topology_test.go | 3 +-- app/mock_reporter_test.go | 34 ++++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/api_topology_test.go b/app/api_topology_test.go index f8f0729d14..7a060d4621 100644 --- a/app/api_topology_test.go +++ b/app/api_topology_test.go @@ -32,8 +32,7 @@ func TestAPITopologyApplications(t *testing.T) { report.MakeEndpointNodeID("hostA", "192.168.1.1", "12345"), report.MakeEndpointNodeID("hostA", "192.168.1.1", "12346"), report.MakeHostNodeID("hostA"), - ), node.Origins, - ) + ), node.Origins) equals(t, "curl", node.LabelMajor) equals(t, "node-a.local (23128)", node.LabelMinor) equals(t, "23128", node.Rank) diff --git a/app/mock_reporter_test.go b/app/mock_reporter_test.go index c097b74f6e..5529b52da6 100644 --- a/app/mock_reporter_test.go +++ b/app/mock_reporter_test.go @@ -55,24 +55,28 @@ func (s StaticReport) Report() report.Report { }, NodeMetadatas: report.NodeMetadatas{ report.MakeEndpointNodeID("hostA", "192.168.1.1", "12345"): report.NodeMetadata{ - "pid": "23128", - "name": "curl", - "domain": "node-a.local", + "pid": "23128", + "name": "curl", + "domain": "node-a.local", + report.HostNodeID: report.MakeHostNodeID("hostA"), }, report.MakeEndpointNodeID("hostA", "192.168.1.1", "12346"): report.NodeMetadata{ // <-- same as :12345 - "pid": "23128", - "name": "curl", - "domain": "node-a.local", + "pid": "23128", + "name": "curl", + "domain": "node-a.local", + report.HostNodeID: report.MakeHostNodeID("hostA"), }, report.MakeEndpointNodeID("hostA", "192.168.1.1", "8888"): report.NodeMetadata{ - "pid": "55100", - "name": "ssh", - "domain": "node-a.local", + "pid": "55100", + "name": "ssh", + "domain": "node-a.local", + report.HostNodeID: report.MakeHostNodeID("hostA"), }, report.MakeEndpointNodeID("hostB", "192.168.1.2", "80"): report.NodeMetadata{ - "pid": "215", - "name": "apache", - "domain": "node-b.local", + "pid": "215", + "name": "apache", + "domain": "node-b.local", + report.HostNodeID: report.MakeHostNodeID("hostB"), }, }, }, @@ -107,10 +111,12 @@ func (s StaticReport) Report() report.Report { }, NodeMetadatas: report.NodeMetadatas{ report.MakeAddressNodeID("hostA", "192.168.1.1"): report.NodeMetadata{ - "name": "host-a", + "name": "host-a", + report.HostNodeID: report.MakeHostNodeID("hostA"), }, report.MakeAddressNodeID("hostB", "192.168.1.2"): report.NodeMetadata{ - "name": "host-b", + "name": "host-b", + report.HostNodeID: report.MakeHostNodeID("hostB"), }, }, }, From 20eb64968bdfe5a290904c8e851fefb7ef9d4472 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 15 Jun 2015 14:58:27 +0200 Subject: [PATCH 3/3] probe: origin host tagger: use report.Topologies method --- probe/tag/origin_host_tagger.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/probe/tag/origin_host_tagger.go b/probe/tag/origin_host_tagger.go index 7e96be7d9a..6d3f59b287 100644 --- a/probe/tag/origin_host_tagger.go +++ b/probe/tag/origin_host_tagger.go @@ -13,16 +13,10 @@ func NewOriginHostTagger(hostID string) Tagger { } func (t originHostTagger) Tag(r report.Report) report.Report { - for _, topology := range []*report.Topology{ - &(r.Endpoint), - &(r.Address), - &(r.Process), - &(r.Container), - &(r.Host), - } { + for _, topology := range r.Topologies() { md := report.NodeMetadata{report.HostNodeID: t.hostNodeID} for nodeID := range topology.NodeMetadatas { - (*topology).NodeMetadatas[nodeID].Merge(md) + topology.NodeMetadatas[nodeID].Merge(md) } } return r