From ec7c468758cd90339ff0452bf7a68104c0411024 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 2 Jan 2018 09:40:18 +0000 Subject: [PATCH 1/5] make it possible for users to see unconnected processes We prevented this because rendering them was expensive. It still is, but less so and actually works well, especially in table mode. --- app/api_topologies.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 1d205ce60..6f5dd3fc6 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -188,9 +188,8 @@ func MakeRegistry() *Registry { ID: "unconnected", Default: "hide", Options: []APITopologyOption{ - // Show the user why there are filtered nodes in this view. - // Don't give them the option to show those nodes. - {Value: "hide", Label: "Unconnected nodes hidden", filter: nil, filterPseudo: false}, + {Value: "show", Label: "Show Unconnected", filter: nil, filterPseudo: false}, + {Value: "hide", Label: "Hide Unconnected", filter: render.IsConnected, filterPseudo: false}, }, }, } @@ -201,7 +200,7 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: processesID, renderer: render.ProcessWithContainerNameRenderer, - filter: render.FilterUnconnected, + filter: render.FilterUnconnectedPseudo, Name: "Processes", Rank: 1, Options: unconnectedFilter, @@ -211,7 +210,7 @@ func MakeRegistry() *Registry { id: processesByNameID, parent: processesID, renderer: render.ProcessNameRenderer, - filter: render.FilterUnconnected, + filter: render.FilterUnconnectedPseudo, Name: "by name", Options: unconnectedFilter, HideIfEmpty: true, From 3582c221fe3b68ca30e4329dfa91c026ed3b64f4 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 2 Jan 2018 09:47:15 +0000 Subject: [PATCH 2/5] processes are now always linkable --- render/detailed/summary.go | 1 - render/process.go | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/render/detailed/summary.go b/render/detailed/summary.go index 16705e370..b73d43da1 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -240,7 +240,6 @@ func processNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary { base.LabelMinor = hostID base.Rank = hostID } - base.Linkable = render.IsConnected(n) return base } diff --git a/render/process.go b/render/process.go index 1fdd08b63..0ef12832f 100644 --- a/render/process.go +++ b/render/process.go @@ -27,9 +27,8 @@ var EndpointRenderer = SelectEndpoint // ProcessRenderer is a Renderer which produces a renderable process // 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. +// also colors connected nodes, so we can apply a filter to show/hide +// unconnected nodes depending on user choice. var ProcessRenderer = Memoise(ColorConnected(endpoints2Processes{})) // processWithContainerNameRenderer is a Renderer which produces a process From 9754bf238571152d31c72680c1c97660babbef0b Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 2 Jan 2018 10:10:34 +0000 Subject: [PATCH 3/5] refactor: remove support for non-linkable nodes since they are now always linkable. --- .../node-details-table-node-link.js | 23 +++++++------------ render/detailed/node_test.go | 7 ------ render/detailed/summary.go | 8 +++---- render/detailed/summary_test.go | 4 ---- 4 files changed, 11 insertions(+), 31 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-table-node-link.js b/client/app/scripts/components/node-details/node-details-table-node-link.js index 0ecd15aeb..1c3303b9b 100644 --- a/client/app/scripts/components/node-details/node-details-table-node-link.js +++ b/client/app/scripts/components/node-details/node-details-table-node-link.js @@ -32,24 +32,17 @@ class NodeDetailsTableNodeLink extends React.Component { } render() { - const { label, labelMinor, linkable } = this.props; + const { label, labelMinor } = this.props; const title = !labelMinor ? label : `${label} (${labelMinor})`; - if (linkable) { - return ( - - {label} - - ); - } return ( - + {label} ); diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 11fafc6ed..aa6f86f39 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -37,9 +37,7 @@ func TestMakeDetailedHostNode(t *testing.T) { containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) process1NodeSummary := child(t, render.ProcessRenderer, fixture.ClientProcess1NodeID) - process1NodeSummary.Linkable = true process2NodeSummary := child(t, render.ProcessRenderer, fixture.ClientProcess2NodeID) - process2NodeSummary.Linkable = true podNodeSummary := child(t, render.PodRenderer, fixture.ClientPodNodeID) want := detailed.Node{ NodeSummary: detailed.NodeSummary{ @@ -50,7 +48,6 @@ func TestMakeDetailedHostNode(t *testing.T) { Rank: "hostname.com", Pseudo: false, Shape: "circle", - Linkable: true, }, Adjacency: report.MakeIDList(fixture.ServerHostNodeID), Metadata: []report.MetadataRow{ @@ -188,7 +185,6 @@ func TestMakeDetailedContainerNode(t *testing.T) { have := detailed.MakeNode("containers", detailed.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) - serverProcessNodeSummary.Linkable = true want := detailed.Node{ NodeSummary: detailed.NodeSummary{ BasicNodeSummary: detailed.BasicNodeSummary{ @@ -197,7 +193,6 @@ func TestMakeDetailedContainerNode(t *testing.T) { LabelMinor: "server.hostname.com", Rank: fixture.ServerContainerImageName, Shape: "hexagon", - Linkable: true, Pseudo: false, }, Metadata: []report.MetadataRow{ @@ -321,7 +316,6 @@ func TestMakeDetailedPodNode(t *testing.T) { containerNodeSummary := child(t, render.ContainerWithImageNameRenderer, fixture.ServerContainerNodeID) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) - serverProcessNodeSummary.Linkable = true // Temporary workaround for: https://github.com/weaveworks/scope/issues/1295 want := detailed.Node{ NodeSummary: detailed.NodeSummary{ BasicNodeSummary: detailed.BasicNodeSummary{ @@ -330,7 +324,6 @@ func TestMakeDetailedPodNode(t *testing.T) { LabelMinor: "1 container", Rank: "ping/pong-b", Shape: "heptagon", - Linkable: true, Pseudo: false, }, Metadata: []report.MetadataRow{ diff --git a/render/detailed/summary.go b/render/detailed/summary.go index b73d43da1..038217534 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -50,7 +50,6 @@ type BasicNodeSummary struct { Rank string `json:"rank"` Shape string `json:"shape,omitempty"` Stack bool `json:"stack,omitempty"` - Linkable bool `json:"linkable,omitempty"` // Whether this node can be linked-to Pseudo bool `json:"pseudo,omitempty"` } @@ -104,10 +103,9 @@ var primaryAPITopology = map[string]string{ // possible. This summary is sufficient for rendering links to the node. func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, bool) { summary := BasicNodeSummary{ // This is unlikely to look very good, but is a reasonable fallback - ID: n.ID, - Label: n.ID, - Shape: report.Triangle, - Linkable: true, + ID: n.ID, + Label: n.ID, + Shape: report.Triangle, } if t, ok := r.Topology(n.Topology); ok { summary.Shape = t.GetShape() diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index 2044723ca..057f61ad0 100644 --- a/render/detailed/summary_test.go +++ b/render/detailed/summary_test.go @@ -130,7 +130,6 @@ func TestMakeNodeSummary(t *testing.T) { LabelMinor: fixture.ClientHostName, Rank: fixture.ClientContainerImageName, Shape: "hexagon", - Linkable: true, }, Metadata: []report.MetadataRow{ {ID: docker.ImageName, Label: "Image", Value: fixture.ClientContainerImageName, Priority: 1}, @@ -150,7 +149,6 @@ func TestMakeNodeSummary(t *testing.T) { LabelMinor: "1 container", Rank: fixture.ClientContainerImageName, Shape: "hexagon", - Linkable: true, Stack: true, }, Metadata: []report.MetadataRow{ @@ -170,7 +168,6 @@ func TestMakeNodeSummary(t *testing.T) { LabelMinor: "hostname.com", Rank: "hostname.com", Shape: "circle", - Linkable: true, }, Metadata: []report.MetadataRow{ {ID: host.HostName, Label: "Hostname", Value: fixture.ClientHostName, Priority: 11}, @@ -190,7 +187,6 @@ func TestMakeNodeSummary(t *testing.T) { Rank: "apache", Shape: "square", Stack: true, - Linkable: true, }, }, }, From dd41956d60cacecba73ee5fb32d0bfe68bd69ecc Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 2 Jan 2018 10:17:36 +0000 Subject: [PATCH 4/5] refactor: remove support for non-linkable connections Connections have always been linkable, so this is dead code. --- render/detailed/connections.go | 2 -- render/detailed/node_test.go | 17 ++++++----------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/render/detailed/connections.go b/render/detailed/connections.go index cf252e134..e41dbd401 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -48,7 +48,6 @@ type Connection struct { NodeID string `json:"nodeId"` // ID of a node in the topology. Optional, must be set if linkable is true. Label string `json:"label"` LabelMinor string `json:"labelMinor,omitempty"` - Linkable bool `json:"linkable"` Metadata []report.MetadataRow `json:"metadata,omitempty"` } @@ -132,7 +131,6 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal NodeID: summary.ID, Label: summary.Label, LabelMinor: summary.LabelMinor, - Linkable: true, } if row.remoteAddr != "" { connection.Label = row.remoteAddr diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index aa6f86f39..c8e2d086c 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -154,7 +154,6 @@ func TestMakeDetailedHostNode(t *testing.T) { NodeID: fixture.ServerHostNodeID, Label: "server", LabelMinor: "hostname.com", - Linkable: true, Metadata: []report.MetadataRow{ { ID: "port", @@ -261,7 +260,6 @@ func TestMakeDetailedContainerNode(t *testing.T) { NodeID: fixture.ClientContainerNodeID, Label: "client", LabelMinor: "client.hostname.com", - Linkable: true, Metadata: []report.MetadataRow{ { ID: "port", @@ -274,10 +272,9 @@ func TestMakeDetailedContainerNode(t *testing.T) { }, }, { - ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), - NodeID: render.IncomingInternetID, - Label: fixture.RandomClientIP, - Linkable: true, + ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), + NodeID: render.IncomingInternetID, + Label: fixture.RandomClientIP, Metadata: []report.MetadataRow{ { ID: "port", @@ -378,7 +375,6 @@ func TestMakeDetailedPodNode(t *testing.T) { NodeID: fixture.ClientPodNodeID, Label: "pong-a", LabelMinor: "1 container", - Linkable: true, Metadata: []report.MetadataRow{ { ID: "port", @@ -391,10 +387,9 @@ func TestMakeDetailedPodNode(t *testing.T) { }, }, { - ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), - NodeID: render.IncomingInternetID, - Label: fixture.RandomClientIP, - Linkable: true, + ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), + NodeID: render.IncomingInternetID, + Label: fixture.RandomClientIP, Metadata: []report.MetadataRow{ { ID: "port", From d1bb4aa45ecb0412dc13d5700ebdc29d5fa6c7bd Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 2 Jan 2018 10:33:57 +0000 Subject: [PATCH 5/5] refactor: remove APITopologyDesc.filter It's always set to render.FilterUnconnectedPseudo, so we can simply use that constant in the one function (RendererForTopology) that looked at that value. --- app/api_topologies.go | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 6f5dd3fc6..2a28a40ae 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -200,7 +200,6 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: processesID, renderer: render.ProcessWithContainerNameRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Processes", Rank: 1, Options: unconnectedFilter, @@ -210,7 +209,6 @@ func MakeRegistry() *Registry { id: processesByNameID, parent: processesID, renderer: render.ProcessNameRenderer, - filter: render.FilterUnconnectedPseudo, Name: "by name", Options: unconnectedFilter, HideIfEmpty: true, @@ -218,7 +216,6 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: containersID, renderer: render.ContainerWithImageNameRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Containers", Rank: 2, Options: containerFilters, @@ -227,7 +224,6 @@ func MakeRegistry() *Registry { id: containersByHostnameID, parent: containersID, renderer: render.ContainerHostnameRenderer, - filter: render.FilterUnconnectedPseudo, Name: "by DNS name", Options: containerFilters, }, @@ -235,14 +231,12 @@ func MakeRegistry() *Registry { id: containersByImageID, parent: containersID, renderer: render.ContainerImageRenderer, - filter: render.FilterUnconnectedPseudo, Name: "by image", Options: containerFilters, }, APITopologyDesc{ id: podsID, renderer: render.PodRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Pods", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -252,7 +246,6 @@ func MakeRegistry() *Registry { id: kubeControllersID, parent: podsID, renderer: render.KubeControllerRenderer, - filter: render.FilterUnconnectedPseudo, Name: "controllers", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, @@ -261,7 +254,6 @@ func MakeRegistry() *Registry { id: servicesID, parent: podsID, renderer: render.PodServiceRenderer, - filter: render.FilterUnconnectedPseudo, Name: "services", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, @@ -269,7 +261,6 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: ecsTasksID, renderer: render.ECSTaskRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Tasks", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -279,7 +270,6 @@ func MakeRegistry() *Registry { id: ecsServicesID, parent: ecsTasksID, renderer: render.ECSServiceRenderer, - filter: render.FilterUnconnectedPseudo, Name: "services", Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, @@ -287,7 +277,6 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: swarmServicesID, renderer: render.SwarmServiceRenderer, - filter: render.FilterUnconnectedPseudo, Name: "services", Rank: 3, Options: []APITopologyOptionGroup{unmanagedFilter}, @@ -296,7 +285,6 @@ func MakeRegistry() *Registry { APITopologyDesc{ id: hostsID, renderer: render.HostRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Hosts", Rank: 4, }, @@ -304,7 +292,6 @@ func MakeRegistry() *Registry { id: weaveID, parent: hostsID, renderer: render.WeaveRenderer, - filter: render.FilterUnconnectedPseudo, Name: "Weave Net", }, ) @@ -317,7 +304,6 @@ type APITopologyDesc struct { id string parent string renderer render.Renderer - filter render.Transformer Name string `json:"name"` Rank int `json:"rank"` @@ -541,7 +527,7 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt if len(values) == 0 { // if no options where provided, only apply base filter - return topology.renderer, topology.filter, nil + return topology.renderer, render.FilterUnconnectedPseudo, nil } var filters []render.FilterFunc @@ -552,9 +538,9 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt } } if len(filters) > 0 { - return topology.renderer, render.Transformers([]render.Transformer{render.ComposeFilterFuncs(filters...), topology.filter}), nil + return topology.renderer, render.Transformers([]render.Transformer{render.ComposeFilterFuncs(filters...), render.FilterUnconnectedPseudo}), nil } - return topology.renderer, topology.filter, nil + return topology.renderer, render.FilterUnconnectedPseudo, nil } type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request)