diff --git a/app/api_topologies.go b/app/api_topologies.go index 53405196a3..5aaff9d180 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -200,7 +200,8 @@ func MakeRegistry() *Registry { registry.Add( APITopologyDesc{ id: processesID, - renderer: render.FilterUnconnected(render.ProcessWithContainerNameRenderer), + renderer: render.ProcessWithContainerNameRenderer, + filter: render.FilterUnconnected, Name: "Processes", Rank: 1, Options: unconnectedFilter, @@ -209,14 +210,16 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: processesByNameID, parent: processesID, - renderer: render.FilterUnconnected(render.ProcessNameRenderer), + renderer: render.ProcessNameRenderer, + filter: render.FilterUnconnected, Name: "by name", Options: unconnectedFilter, HideIfEmpty: true, }, APITopologyDesc{ id: containersID, - renderer: render.FilterUnconnectedPseudo(render.ContainerWithImageNameRenderer), + renderer: render.ContainerWithImageNameRenderer, + filter: render.FilterUnconnectedPseudo, Name: "Containers", Rank: 2, Options: containerFilters, @@ -224,20 +227,23 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: containersByHostnameID, parent: containersID, - renderer: render.FilterUnconnectedPseudo(render.ContainerHostnameRenderer), + renderer: render.ContainerHostnameRenderer, + filter: render.FilterUnconnectedPseudo, Name: "by DNS name", Options: containerFilters, }, APITopologyDesc{ id: containersByImageID, parent: containersID, - renderer: render.FilterUnconnectedPseudo(render.ContainerImageRenderer), + renderer: render.ContainerImageRenderer, + filter: render.FilterUnconnectedPseudo, Name: "by image", Options: containerFilters, }, APITopologyDesc{ id: podsID, - renderer: render.FilterUnconnectedPseudo(render.PodRenderer), + renderer: render.PodRenderer, + filter: render.FilterUnconnectedPseudo, Name: "Pods", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -246,7 +252,8 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: kubeControllersID, parent: podsID, - renderer: render.FilterUnconnectedPseudo(render.KubeControllerRenderer), + renderer: render.KubeControllerRenderer, + filter: render.FilterUnconnectedPseudo, Name: "controllers", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, @@ -254,14 +261,16 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: servicesID, parent: podsID, - renderer: render.FilterUnconnectedPseudo(render.PodServiceRenderer), + renderer: render.PodServiceRenderer, + filter: render.FilterUnconnectedPseudo, Name: "services", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ id: ecsTasksID, - renderer: render.FilterUnconnectedPseudo(render.ECSTaskRenderer), + renderer: render.ECSTaskRenderer, + filter: render.FilterUnconnectedPseudo, Name: "Tasks", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -270,14 +279,16 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: ecsServicesID, parent: ecsTasksID, - renderer: render.FilterUnconnectedPseudo(render.ECSServiceRenderer), + renderer: render.ECSServiceRenderer, + filter: render.FilterUnconnectedPseudo, Name: "services", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ id: swarmServicesID, - renderer: render.FilterUnconnectedPseudo(render.SwarmServiceRenderer), + renderer: render.SwarmServiceRenderer, + filter: render.FilterUnconnectedPseudo, Name: "services", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -285,14 +296,16 @@ func MakeRegistry() *Registry { }, APITopologyDesc{ id: hostsID, - renderer: render.FilterUnconnectedPseudo(render.HostRenderer), + renderer: render.HostRenderer, + filter: render.FilterUnconnectedPseudo, Name: "Hosts", Rank: 4, }, APITopologyDesc{ id: weaveID, parent: hostsID, - renderer: render.FilterUnconnectedPseudo(render.WeaveRenderer), + renderer: render.WeaveRenderer, + filter: render.FilterUnconnectedPseudo, Name: "Weave Net", }, ) @@ -305,6 +318,7 @@ type APITopologyDesc struct { id string parent string renderer render.Renderer + filter render.Transformer Name string `json:"name"` Rank int `json:"rank"` @@ -496,13 +510,13 @@ func (r *Registry) renderTopologies(rpt report.Report, req *http.Request) []APIT return updateFilters(rpt, topologies) } -func computeStats(rpt report.Report, renderer render.Renderer, filter render.FilterFunc) topologyStats { +func computeStats(rpt report.Report, renderer render.Renderer, transformer render.Transformer) topologyStats { var ( nodes int realNodes int edges int ) - r := render.Render(rpt, renderer, filter) + r := render.Render(rpt, renderer, transformer) for _, n := range r.Nodes { nodes++ if n.Topology != render.Pseudo { @@ -519,7 +533,7 @@ func computeStats(rpt report.Report, renderer render.Renderer, filter render.Fil } // RendererForTopology .. -func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.FilterFunc, error) { +func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Transformer, error) { topology, ok := r.get(topologyID) if !ok { return nil, nil, fmt.Errorf("topology not found: %s", topologyID) @@ -527,8 +541,8 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt topology = updateFilters(rpt, []APITopologyDesc{topology})[0] if len(values) == 0 { - // Do not apply filtering if no options where provided - return topology.renderer, nil, nil + // if no options where provided, only apply base filter + return topology.renderer, topology.filter, nil } var filters []render.FilterFunc @@ -539,9 +553,9 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt } } if len(filters) > 0 { - return topology.renderer, render.ComposeFilterFuncs(filters...), nil + return topology.renderer, render.Transformers([]render.Transformer{render.ComposeFilterFuncs(filters...), topology.filter}), nil } - return topology.renderer, nil, nil + return topology.renderer, topology.filter, nil } type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) @@ -552,7 +566,7 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { } } -type rendererHandler func(context.Context, render.Renderer, render.FilterFunc, report.RenderContext, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Transformer, report.RenderContext, http.ResponseWriter, *http.Request) func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index a238adcf41..2dbfeb6a5e 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -164,14 +164,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det var ( topologyRegistry = app.MakeRegistry() - filter render.FilterFunc + filterFunc render.FilterFunc ) if exclude == true { - filter = render.DoesNotHaveLabel(fixture.TestLabelKey2, fixture.ApplicationLabelValue2) + filterFunc = render.DoesNotHaveLabel(fixture.TestLabelKey2, fixture.ApplicationLabelValue2) } else { - filter = render.HasLabel(fixture.TestLabelKey1, fixture.ApplicationLabelValue1) + filterFunc = render.HasLabel(fixture.TestLabelKey1, fixture.ApplicationLabelValue1) } - option := app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", filter, false) + option := app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", filterFunc, false) topologyRegistry.AddContainerFilters(option) urlvalues := url.Values{} diff --git a/app/api_topology.go b/app/api_topology.go index e0a3ded161..145b5aa85b 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,14 +29,14 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, filter).Nodes), + Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, transformer).Nodes), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] @@ -53,14 +53,12 @@ func handleNode(ctx context.Context, renderer render.Renderer, filter render.Fil http.NotFound(w, r) return } - if filter != nil { - nodes = filter.Apply(nodes) - if filteredNode, ok := nodes.Nodes[nodeID]; ok { - node = filteredNode - } else { // we've lost the node during filtering; put it back - nodes.Nodes[nodeID] = node - nodes.Filtered-- - } + nodes = transformer.Transform(nodes) + if filteredNode, ok := nodes.Nodes[nodeID]; ok { + node = filteredNode + } else { // we've lost the node during filtering; put it back + nodes.Nodes[nodeID] = node + nodes.Filtered-- } respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) } diff --git a/render/container.go b/render/container.go index 0dda442866..1c3eb044e3 100644 --- a/render/container.go +++ b/render/container.go @@ -33,7 +33,7 @@ var ContainerRenderer = Memoise(MakeFilter( MakeReduce( MakeMap( MapProcess2Container, - ColorConnectedProcessRenderer, + ProcessRenderer, ), ConnectionJoin(MapContainer2IP, SelectContainer), ), diff --git a/render/container_test.go b/render/container_test.go index 9ec27887ca..ca9f51dfad 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -16,8 +16,14 @@ import ( ) var ( - filterApplication = render.AnyFilterFunc(render.IsPseudoTopology, render.IsApplication) - filterSystem = render.AnyFilterFunc(render.IsPseudoTopology, render.IsSystem) + filterApplication = render.Transformers([]render.Transformer{ + render.AnyFilterFunc(render.IsPseudoTopology, render.IsApplication), + render.FilterUnconnectedPseudo, + }) + filterSystem = render.Transformers([]render.Transformer{ + render.AnyFilterFunc(render.IsPseudoTopology, render.IsSystem), + render.FilterUnconnectedPseudo, + }) ) func TestMapProcess2Container(t *testing.T) { @@ -74,7 +80,7 @@ func TestContainerFilterRenderer(t *testing.T) { } func TestContainerHostnameRenderer(t *testing.T) { - have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, nil).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, render.Transformers(nil)).Nodes) want := utils.Prune(expected.RenderedContainerHostnames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -93,7 +99,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) { } func TestContainerImageRenderer(t *testing.T) { - have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, nil).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, render.Transformers(nil)).Nodes) want := utils.Prune(expected.RenderedContainerImages) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/render/filters.go b/render/filters.go index 150a6098eb..6620344c75 100644 --- a/render/filters.go +++ b/render/filters.go @@ -60,15 +60,13 @@ func Complement(f FilterFunc) FilterFunc { return func(node report.Node) bool { return !f(node) } } -// Apply applies the filter to all nodes -func (f FilterFunc) Apply(nodes Nodes) Nodes { +// Transform applies the filter to all nodes +func (f FilterFunc) Transform(nodes Nodes) Nodes { output := report.Nodes{} - inDegrees := map[string]int{} filtered := nodes.Filtered for id, node := range nodes.Nodes { if f(node) { output[id] = node - inDegrees[id] = 0 } else { filtered++ } @@ -80,25 +78,12 @@ func (f FilterFunc) Apply(nodes Nodes) Nodes { for _, dstID := range node.Adjacency { if _, ok := output[dstID]; ok { newAdjacency = newAdjacency.Add(dstID) - inDegrees[dstID]++ } } node.Adjacency = newAdjacency output[id] = node } - // Remove unconnected pseudo nodes, see #483. - for id, inDegree := range inDegrees { - if inDegree > 0 { - continue - } - node := output[id] - if node.Topology != Pseudo || len(node.Adjacency) > 0 { - continue - } - delete(output, id) - filtered++ - } return Nodes{Nodes: output, Filtered: filtered} } @@ -128,7 +113,7 @@ func MakeFilterPseudo(f FilterFunc, r Renderer) Renderer { // Render implements Renderer func (f Filter) Render(rpt report.Report) Nodes { - return f.FilterFunc.Apply(f.Renderer.Render(rpt)) + return f.FilterFunc.Transform(f.Renderer.Render(rpt)) } // IsConnectedMark is the key added to Node.Metadata by @@ -143,6 +128,22 @@ func IsConnected(node report.Node) bool { return ok } +// connected returns the node ids of nodes which have edges to/from +// them, excluding edges to/from themselves. +func connected(nodes report.Nodes) map[string]struct{} { + res := map[string]struct{}{} + void := struct{}{} + for id, node := range nodes { + for _, adj := range node.Adjacency { + if adj != id { + res[id] = void + res[adj] = void + } + } + } + return res +} + // ColorConnected colors nodes with the IsConnectedMark key if they // have edges to or from them. Edges to/from yourself are not counted // here (see #656). @@ -150,20 +151,8 @@ func ColorConnected(r Renderer) Renderer { return CustomRenderer{ Renderer: r, RenderFunc: func(input Nodes) Nodes { - connected := map[string]struct{}{} - void := struct{}{} - - for id, node := range input.Nodes { - for _, adj := range node.Adjacency { - if adj != id { - connected[id] = void - connected[adj] = void - } - } - } - output := input.Copy() - for id := range connected { + for id := range connected(input.Nodes) { output[id] = output[id].WithLatest(IsConnectedMark, mtime.Now(), "true") } return Nodes{Nodes: output, Filtered: input.Filtered} @@ -171,26 +160,28 @@ func ColorConnected(r Renderer) Renderer { } } -// FilterUnconnected produces a renderer that filters unconnected nodes -// from the given renderer -func FilterUnconnected(r Renderer) Renderer { - return MakeFilterPseudo(IsConnected, ColorConnected(r)) +type filterUnconnected struct { + onlyPseudo bool } -// FilterUnconnectedPseudo produces a renderer that filters -// unconnected pseudo nodes from the given renderer -func FilterUnconnectedPseudo(r Renderer) Renderer { - return MakeFilterPseudo( - func(node report.Node) bool { - if !IsPseudoTopology(node) { - return true - } - return IsConnected(node) - }, - ColorConnected(r), - ) +// Transform implements Transformer +func (f filterUnconnected) Transform(input Nodes) Nodes { + connected := connected(input.Nodes) + return FilterFunc(func(node report.Node) bool { + if _, ok := connected[node.ID]; ok || (f.onlyPseudo && !IsPseudoTopology(node)) { + return true + } + return false + }).Transform(input) } +// FilterUnconnected is a transformer that filters unconnected nodes +var FilterUnconnected = filterUnconnected{onlyPseudo: false} + +// FilterUnconnectedPseudo is a transformer that filters unconnected +// pseudo nodes +var FilterUnconnectedPseudo = filterUnconnected{onlyPseudo: true} + // Noop allows all nodes through func Noop(_ report.Node) bool { return true } diff --git a/render/filters_test.go b/render/filters_test.go index 5854641fd7..b31dfc1fb2 100644 --- a/render/filters_test.go +++ b/render/filters_test.go @@ -9,9 +9,12 @@ import ( "github.com/weaveworks/scope/test/reflect" ) -func isNotBar(node report.Node) bool { - return node.ID != "bar" -} +var filterBar = render.Transformers([]render.Transformer{ + render.FilterFunc(func(node report.Node) bool { + return node.ID != "bar" + }), + render.FilterUnconnectedPseudo, +}) func TestFilterRender(t *testing.T) { renderer := mockRenderer{Nodes: report.Nodes{ @@ -20,7 +23,7 @@ func TestFilterRender(t *testing.T) { "baz": report.MakeNode("baz"), }} have := report.MakeIDList() - for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes { + for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.FilterFunc(render.IsConnected)).Nodes { have = have.Add(id) } want := report.MakeIDList("foo", "bar") @@ -36,7 +39,7 @@ func TestFilterRender2(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz"), }} - have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, filterBar).Nodes if have["foo"].Adjacency.Contains("bar") { t.Error("adjacencies for removed nodes should have been removed") } @@ -53,7 +56,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { } renderer := mockRenderer{Nodes: nodes} want := nodes - have := render.Render(report.MakeReport(), renderer, nil).Nodes + have := render.Render(report.MakeReport(), renderer, render.Transformers(nil)).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } @@ -64,7 +67,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("baz"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo), }} - have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, filterBar).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -75,7 +78,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo).WithAdjacent("bar"), }} - have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, filterBar).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -89,7 +92,7 @@ func TestFilterUnconnectedSelf(t *testing.T) { "foo": report.MakeNode("foo").WithAdjacent("foo"), } renderer := mockRenderer{Nodes: nodes} - have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes + have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.FilterFunc(render.IsConnected)).Nodes if len(have) > 0 { t.Error("expected node only connected to self to be removed") } diff --git a/render/host.go b/render/host.go index 4f00356f32..3d7ceccbcc 100644 --- a/render/host.go +++ b/render/host.go @@ -10,7 +10,7 @@ import ( // not memoised var HostRenderer = MakeReduce( endpoints2Hosts{}, - CustomRenderer{RenderFunc: nodes2Hosts, Renderer: ColorConnectedProcessRenderer}, + CustomRenderer{RenderFunc: nodes2Hosts, Renderer: ProcessRenderer}, CustomRenderer{RenderFunc: nodes2Hosts, Renderer: ContainerRenderer}, CustomRenderer{RenderFunc: nodes2Hosts, Renderer: ContainerImageRenderer}, CustomRenderer{RenderFunc: nodes2Hosts, Renderer: PodRenderer}, diff --git a/render/pod_test.go b/render/pod_test.go index 01c5983410..d707110d39 100644 --- a/render/pod_test.go +++ b/render/pod_test.go @@ -20,7 +20,10 @@ func TestPodRenderer(t *testing.T) { } } -var filterNonKubeSystem = render.Complement(render.IsNamespace("kube-system")) +var filterNonKubeSystem = render.Transformers([]render.Transformer{ + render.Complement(render.IsNamespace("kube-system")), + render.FilterUnconnectedPseudo, +}) func TestPodFilterRenderer(t *testing.T) { // tag on containers or pod namespace in the topology and ensure diff --git a/render/process.go b/render/process.go index b69586e211..9dfde7d130 100644 --- a/render/process.go +++ b/render/process.go @@ -25,14 +25,11 @@ func renderProcesses(rpt report.Report) bool { var EndpointRenderer = SelectEndpoint // ProcessRenderer is a Renderer which produces a renderable process -// graph by merging the endpoint graph and the process topology. -var ProcessRenderer = Memoise(endpoints2Processes{}) - -// ColorConnectedProcessRenderer colors connected nodes from -// ProcessRenderer. Since the process topology views only show -// connected processes, we need this info to determine whether +// graph by merging the endpoint graph and the process topology. It +// also colors connected nodes. Since the process topology views only +// show connected processes, we need this info to determine whether // processes appearing in a details panel are linkable. -var ColorConnectedProcessRenderer = Memoise(ColorConnected(ProcessRenderer)) +var ProcessRenderer = Memoise(ColorConnected(endpoints2Processes{})) // processWithContainerNameRenderer is a Renderer which produces a process // graph enriched with container names where appropriate diff --git a/render/render.go b/render/render.go index bcf3e24953..05901aa529 100644 --- a/render/render.go +++ b/render/render.go @@ -29,15 +29,28 @@ func (r Nodes) Merge(o Nodes) Nodes { } } -// Render renders the report and then applies the filter -func Render(rpt report.Report, renderer Renderer, filter FilterFunc) Nodes { - nodes := renderer.Render(rpt) - if filter != nil { - nodes = filter.Apply(nodes) +// Transformer is something that transforms one set of Nodes to +// another set of Nodes. +type Transformer interface { + Transform(nodes Nodes) Nodes +} + +// Transformers is a composition of Transformers +type Transformers []Transformer + +// Transform implements Transformer +func (ts Transformers) Transform(nodes Nodes) Nodes { + for _, t := range ts { + nodes = t.Transform(nodes) } return nodes } +// Render renders the report and then transforms it +func Render(rpt report.Report, renderer Renderer, transformer Transformer) Nodes { + return transformer.Transform(renderer.Render(rpt)) +} + // Reduce renderer is a Renderer which merges together the output of several // other renderers. type Reduce []Renderer