Merge pull request #2951 from weaveworks/one-pass-unconnected

filter out unconnected pseudo nodes just once, at the end
This commit is contained in:
Matthias Radestock
2017-11-28 08:06:12 +00:00
committed by GitHub
11 changed files with 136 additions and 111 deletions

View File

@@ -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) {

View File

@@ -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{}

View File

@@ -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)})
}

View File

@@ -33,7 +33,7 @@ var ContainerRenderer = Memoise(MakeFilter(
MakeReduce(
MakeMap(
MapProcess2Container,
ColorConnectedProcessRenderer,
ProcessRenderer,
),
ConnectionJoin(MapContainer2IP, SelectContainer),
),

View File

@@ -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))

View File

@@ -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 }

View File

@@ -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")
}

View File

@@ -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},

View File

@@ -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

View File

@@ -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

View File

@@ -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