diff --git a/render/detailed_node.go b/render/detailed_node.go index 66e42d135..1dac743ed 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -109,9 +109,11 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { } } -func getRenderingContext(r report.Report, n RenderableNode) (multiContainer bool, multiHost bool) { - originHosts := make(map[string]struct{}) - originContainers := make(map[string]struct{}) +func getRenderingContext(r report.Report, n RenderableNode) (multiContainer, multiHost bool) { + var ( + originHosts = map[string]struct{}{} + originContainers = map[string]struct{}{} + ) for _, id := range n.Origins { for _, topology := range r.Topologies() { if nmd, ok := topology.NodeMetadatas[id]; ok { diff --git a/render/expected/expected.go b/render/expected/expected.go index a3f0248fa..7349fdb7a 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -186,6 +186,7 @@ var ( LabelMinor: "1 process", Rank: "bash", Pseudo: false, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, @@ -245,6 +246,7 @@ var ( LabelMinor: test.ServerHostName, Rank: "", Pseudo: true, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, @@ -303,6 +305,7 @@ var ( LabelMinor: test.ServerHostName, Rank: "", Pseudo: true, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, diff --git a/render/render.go b/render/render.go index 205d68b75..5990885d8 100644 --- a/render/render.go +++ b/render/render.go @@ -16,26 +16,6 @@ type Renderer interface { // other renderers. type Reduce []Renderer -// Map is a Renderer which produces a set of RenderableNodes from the set of -// RenderableNodes produced by another Renderer. -type Map struct { - MapFunc - Renderer -} - -// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology -// by using a map function and topology selector. -type LeafMap struct { - Selector report.TopologySelector - Mapper LeafMapFunc - Pseudo PseudoFunc -} - -// FilterUnconnected is a Renderer which filters out unconnected nodes. -type FilterUnconnected struct { - Renderer -} - // MakeReduce is the only sane way to produce a Reduce Renderer. func MakeReduce(renderers ...Renderer) Renderer { return Reduce(renderers) @@ -54,11 +34,18 @@ func (r Reduce) Render(rpt report.Report) RenderableNodes { func (r Reduce) EdgeMetadata(rpt report.Report, localID, remoteID string) report.EdgeMetadata { metadata := report.EdgeMetadata{} for _, renderer := range r { - metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID)) + metadata = metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID)) } return metadata } +// Map is a Renderer which produces a set of RenderableNodes from the set of +// RenderableNodes produced by another Renderer. +type Map struct { + MapFunc + Renderer +} + // Render transforms a set of RenderableNodes produces by another Renderer. // using a map function func (m Map) Render(rpt report.Report) RenderableNodes { @@ -133,11 +120,19 @@ func (m Map) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableID st output := report.EdgeMetadata{} for _, edge := range oldEdges { metadata := m.Renderer.EdgeMetadata(rpt, edge.src, edge.dst) - output.Merge(metadata) + output = output.Merge(metadata) } return output } +// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology +// by using a map function and topology selector. +type LeafMap struct { + Selector report.TopologySelector + Mapper LeafMapFunc + Pseudo PseudoFunc +} + // Render transforms a given Report into a set of RenderableNodes, which // the UI will render collectively as a graph. Note that a RenderableNode will // always be rendered with other nodes, and therefore contains limited detail. @@ -234,8 +229,8 @@ func (m LeafMap) Render(rpt report.Report) RenderableNodes { // We propagate edge metadata to nodes on both ends of the edges. // TODO we should 'reverse' one end of the edge meta data - ingress -> egress etc. if md, ok := t.EdgeMetadatas[report.MakeEdgeID(srcNodeID, dstNodeID)]; ok { - srcRenderableNode.EdgeMetadata.Merge(md) - dstRenderableNode.EdgeMetadata.Merge(md) + srcRenderableNode.EdgeMetadata = srcRenderableNode.EdgeMetadata.Merge(md) + dstRenderableNode.EdgeMetadata = dstRenderableNode.EdgeMetadata.Merge(md) nodes[dstRenderableID] = dstRenderableNode } } @@ -268,12 +263,17 @@ func (m LeafMap) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableI dst = mapped.ID } if src == srcRenderableID && dst == dstRenderableID { - metadata.Flatten(edgeMeta) + metadata = metadata.Flatten(edgeMeta) } } return metadata } +// FilterUnconnected is a Renderer which filters out unconnected nodes. +type FilterUnconnected struct { + Renderer +} + // Render produces a set of RenderableNodes given a Report func (f FilterUnconnected) Render(rpt report.Report) RenderableNodes { return OnlyConnected(f.Renderer.Render(rpt)) diff --git a/render/render_test.go b/render/render_test.go index 03a9ae232..8371edaa6 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -6,6 +6,7 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) type mockRenderer struct { @@ -76,12 +77,12 @@ func TestMapRender2(t *testing.T) { "baz": {ID: "baz"}, }}, } - want := render.RenderableNodes{ + want := sterilize(render.RenderableNodes{ "bar": render.RenderableNode{ID: "bar"}, - } + }, false) have := mapper.Render(report.MakeReport()) if !reflect.DeepEqual(want, have) { - t.Errorf("want %+v, have %+v", want, have) + t.Error(test.Diff(want, have)) } } @@ -129,8 +130,8 @@ func TestMapEdge(t *testing.T) { } mapper := render.Map{ - MapFunc: func(nodes render.RenderableNode) (render.RenderableNode, bool) { - return render.RenderableNode{ID: "_" + nodes.ID}, true + MapFunc: func(n render.RenderableNode) (render.RenderableNode, bool) { + return render.RenderableNode{ID: "_" + n.ID}, true }, Renderer: render.LeafMap{ Selector: selector, @@ -143,7 +144,7 @@ func TestMapEdge(t *testing.T) { EgressPacketCount: newu64(1), EgressByteCount: newu64(2), }), mapper.EdgeMetadata(report.MakeReport(), "_foo", "_bar"); !reflect.DeepEqual(want, have) { - t.Errorf("want %+v, have %+v", want, have) + t.Error(test.Diff(want, have)) } } diff --git a/render/renderable_node.go b/render/renderable_node.go index 074ca3774..48168edc0 100644 --- a/render/renderable_node.go +++ b/render/renderable_node.go @@ -16,8 +16,8 @@ type RenderableNode struct { Adjacency report.IDList `json:"adjacency,omitempty"` // Node IDs (in the same topology domain) Origins report.IDList `json:"origins,omitempty"` // Core node IDs that contributed information - report.EdgeMetadata `json:"metadata"` // Numeric sums - report.NodeMetadata `json:"-"` // merged NodeMetadata of the nodes used to build this + report.EdgeMetadata `json:"metadata"` // Numeric sums + report.NodeMetadata `json:"XXXNODEMETADATA"` // merged NodeMetadata of the nodes used to build this // TODO ### } // RenderableNodes is a set of RenderableNodes @@ -56,8 +56,8 @@ func (rn *RenderableNode) Merge(other RenderableNode) { rn.Adjacency = rn.Adjacency.Merge(other.Adjacency) rn.Origins = rn.Origins.Merge(other.Origins) - rn.EdgeMetadata.Merge(other.EdgeMetadata) - rn.NodeMetadata.Merge(other.NodeMetadata) + rn.EdgeMetadata = rn.EdgeMetadata.Merge(other.EdgeMetadata) + rn.NodeMetadata = rn.NodeMetadata.Merge(other.NodeMetadata) } // NewRenderableNode makes a new RenderableNode @@ -80,8 +80,8 @@ func newDerivedNode(id string, node RenderableNode) RenderableNode { LabelMinor: "", Rank: "", Pseudo: node.Pseudo, - EdgeMetadata: node.EdgeMetadata, - Origins: node.Origins, + Origins: node.Origins.Copy(), + EdgeMetadata: node.EdgeMetadata.Copy(), NodeMetadata: report.MakeNodeMetadata(), } } @@ -105,8 +105,8 @@ func newDerivedPseudoNode(id, major string, node RenderableNode) RenderableNode LabelMinor: "", Rank: "", Pseudo: true, - EdgeMetadata: node.EdgeMetadata, - Origins: node.Origins, + Origins: node.Origins.Copy(), + EdgeMetadata: node.EdgeMetadata.Copy(), NodeMetadata: report.MakeNodeMetadata(), } } diff --git a/render/renderable_node_test.go b/render/renderable_node_test.go index efa66b545..f54362296 100644 --- a/render/renderable_node_test.go +++ b/render/renderable_node_test.go @@ -6,6 +6,7 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) func TestMergeRenderableNodes(t *testing.T) { @@ -17,19 +18,45 @@ func TestMergeRenderableNodes(t *testing.T) { "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, } - - want := render.RenderableNodes{ + want := sterilize(render.RenderableNodes{ "foo": render.RenderableNode{ID: "foo"}, "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, - } + }, false) nodes1.Merge(nodes2) - - if !reflect.DeepEqual(want, nodes1) { - t.Errorf("want %+v, have %+v", want, nodes1) + if have := sterilize(nodes1, false); !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } +func sterilize(r render.RenderableNodes, destructive bool) render.RenderableNodes { + // Since introducing new map fields to the report.NodeMetadata type, its + // zero value is •not valid• -- every time you need one, you need to use + // the report.MakeNodeMetadata constructor. (Similarly, but not exactly + // the same, is that a zero-value Adjacency is not the same as a created + // but empty Adjacency.) + // + // But we're not doing this in tests. So this function sterilizes invalid + // RenderableNodes by fixing all nil Metadata fields. The proper fix + // involves lots of annoying changes to instantiation. + // + // The extra destructive parameter is to support a historical test use + // case where we explicitly don't compare node metadata. + for id, n := range r { + if n.Adjacency == nil { + n.Adjacency = report.IDList{} + } + if destructive || n.NodeMetadata.Metadata == nil { + n.NodeMetadata.Metadata = map[string]string{} + } + if destructive || n.NodeMetadata.Counters == nil { + n.NodeMetadata.Counters = map[string]int{} + } + r[id] = n + } + return r +} + func TestMergeRenderableNode(t *testing.T) { node1 := render.RenderableNode{ ID: "foo", @@ -49,19 +76,19 @@ func TestMergeRenderableNode(t *testing.T) { Adjacency: report.MakeIDList("a2"), Origins: report.MakeIDList("o2"), } - want := render.RenderableNode{ - ID: "foo", - LabelMajor: "major", - LabelMinor: "minor", - Rank: "rank", - Pseudo: false, - Adjacency: report.MakeIDList("a1", "a2"), - Origins: report.MakeIDList("o1", "o2"), + ID: "foo", + LabelMajor: "major", + LabelMinor: "minor", + Rank: "rank", + Pseudo: false, + Adjacency: report.MakeIDList("a1", "a2"), + Origins: report.MakeIDList("o1", "o2"), + NodeMetadata: report.MakeNodeMetadata(), + EdgeMetadata: report.EdgeMetadata{}, } node1.Merge(node2) - - if !reflect.DeepEqual(want, node1) { - t.Errorf("want %+v, have %+v", want, node1) + if have := node1; !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } diff --git a/render/theinternet_test.go b/render/theinternet_test.go index 283e3a963..fad04b5c3 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -12,11 +12,16 @@ import ( ) func TestReportLocalNetworks(t *testing.T) { - r := report.MakeReport() - r.Merge(report.Report{Host: report.Topology{NodeMetadatas: report.NodeMetadatas{ - "nonets": report.MakeNodeMetadata(), - "foo": report.MakeNodeMetadataWith(map[string]string{host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33"}), - }}}) + r := report.MakeReport().Merge(report.Report{ + Host: report.Topology{ + NodeMetadatas: report.NodeMetadatas{ + "nonets": report.MakeNodeMetadata(), + "foo": report.MakeNodeMetadataWith(map[string]string{ + host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33", + }), + }, + }, + }) want := report.Networks([]*net.IPNet{ mustParseCIDR("10.0.0.1/8"), mustParseCIDR("192.168.1.1/24"), diff --git a/render/topologies_test.go b/render/topologies_test.go index f8f0402f8..f46ec0061 100644 --- a/render/topologies_test.go +++ b/render/topologies_test.go @@ -6,55 +6,45 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/render/expected" - "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" ) -func trimNodeMetadata(rns render.RenderableNodes) render.RenderableNodes { - result := render.RenderableNodes{} - for id, rn := range rns { - rn.NodeMetadata = report.MakeNodeMetadata() - result[id] = rn - } - return result -} - func TestProcessRenderer(t *testing.T) { - have := render.ProcessRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedProcesses, have) { - t.Error(test.Diff(expected.RenderedProcesses, have)) + have := sterilize(render.ProcessRenderer.Render(test.Report), true) + want := expected.RenderedProcesses + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestProcessNameRenderer(t *testing.T) { - have := render.ProcessNameRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedProcessNames, have) { - t.Error(test.Diff(expected.RenderedProcessNames, have)) + have := sterilize(render.ProcessNameRenderer.Render(test.Report), true) + want := expected.RenderedProcessNames + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestContainerRenderer(t *testing.T) { - have := render.ContainerRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedContainers, have) { - t.Error(test.Diff(expected.RenderedContainers, have)) + have := sterilize(render.ContainerRenderer.Render(test.Report), true) + want := expected.RenderedContainers + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestContainerImageRenderer(t *testing.T) { - have := render.ContainerImageRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedContainerImages, have) { - t.Error(test.Diff(expected.RenderedContainerImages, have)) + have := sterilize(render.ContainerImageRenderer.Render(test.Report), true) + want := expected.RenderedContainerImages + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestHostRenderer(t *testing.T) { - have := render.HostRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedHosts, have) { - t.Error(test.Diff(expected.RenderedHosts, have)) + have := sterilize(render.HostRenderer.Render(test.Report), true) + want := expected.RenderedHosts + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } diff --git a/report/merge_test.go b/report/merge_test.go index 1dab3a92f..11674bd7c 100644 --- a/report/merge_test.go +++ b/report/merge_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) const ( @@ -191,11 +192,30 @@ func TestMergeEdgeMetadatas(t *testing.T) { }, } { if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { - t.Errorf("%s: want\n\t%#v, have\n\t%#v", name, c.want, have) + t.Errorf("%s:\n%s", name, test.Diff(c.want, have)) } } } +func TestFlattenEdgeMetadata(t *testing.T) { + have := (report.EdgeMetadata{ + EgressPacketCount: newu64(1), + MaxConnCountTCP: newu64(2), + }).Flatten(report.EdgeMetadata{ + EgressPacketCount: newu64(4), + EgressByteCount: newu64(8), + MaxConnCountTCP: newu64(16), + }) + want := report.EdgeMetadata{ + EgressPacketCount: newu64(1 + 4), + EgressByteCount: newu64(8), + MaxConnCountTCP: newu64(2 + 16), // flatten should sum MaxConnCountTCP + } + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + func TestMergeNodeMetadatas(t *testing.T) { for name, c := range map[string]struct { a, b, want report.NodeMetadatas