From 2af2b1f15a874d642b082be242332fecf1ebe1b0 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 27 Apr 2016 11:26:16 +0100 Subject: [PATCH 1/6] Filter by Kubernetes Namespaces --- app/api_topologies.go | 137 ++++++++++++++++++++++++---------------- app/api_topology.go | 50 +++++++++------ app/router.go | 6 +- probe/kubernetes/pod.go | 11 ++-- render/filters.go | 8 +++ render/pod.go | 56 ++++++++-------- 6 files changed, 157 insertions(+), 111 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 0d4a61fdd..7f841973c 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -1,6 +1,7 @@ package app import ( + "fmt" "net/http" "sort" "sync" @@ -8,6 +9,7 @@ import ( "github.com/gorilla/mux" "golang.org/x/net/context" + "github.com/weaveworks/scope/probe/kubernetes" "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" ) @@ -21,30 +23,6 @@ var ( ) func init() { - serviceFilters := []APITopologyOptionGroup{ - { - ID: "system", - Default: "application", - Options: []APITopologyOption{ - {"system", "System services", render.IsSystem}, - {"application", "Application services", render.IsApplication}, - {"both", "Both", render.Noop}, - }, - }, - } - - podFilters := []APITopologyOptionGroup{ - { - ID: "system", - Default: "application", - Options: []APITopologyOption{ - {"system", "System pods", render.IsSystem}, - {"application", "Application pods", render.IsApplication}, - {"both", "Both", render.Noop}, - }, - }, - } - containerFilters := []APITopologyOptionGroup{ { ID: "system", @@ -116,19 +94,12 @@ func init() { Name: "by DNS name", Options: containerFilters, }, - APITopologyDesc{ - id: "hosts", - renderer: render.HostRenderer, - Name: "Hosts", - Rank: 4, - }, APITopologyDesc{ id: "pods", renderer: render.PodRenderer, Name: "Pods", Rank: 3, HideIfEmpty: true, - Options: podFilters, }, APITopologyDesc{ id: "pods-by-service", @@ -136,11 +107,60 @@ func init() { renderer: render.PodServiceRenderer, Name: "by service", HideIfEmpty: true, - Options: serviceFilters, + }, + APITopologyDesc{ + id: "hosts", + renderer: render.HostRenderer, + Name: "Hosts", + Rank: 4, }, ) } +// kubernetesFilters generates the current kubernetes filters based on the +// available k8s topologies. +func kubernetesFilters(namespaces ...string) APITopologyOptionGroup { + options := APITopologyOptionGroup{ID: "namespace", Default: "all"} + for _, namespace := range namespaces { + options.Options = append(options.Options, APITopologyOption{namespace, namespace, render.IsNamespace(namespace)}) + } + options.Options = append(options.Options, APITopologyOption{"all", "All Namespaces", render.Noop}) + return options +} + +// updateFilters updates the available filters based on the current report. +// Currently only kubernetes changes. +func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopologyDesc { + namespaces := map[string]struct{}{} + for _, t := range []report.Topology{rpt.Pod, rpt.Service} { + for _, n := range t.Nodes { + if namespace, ok := n.Latest.Lookup(kubernetes.Namespace); ok { + namespaces[namespace] = struct{}{} + } + } + } + var ns []string + for namespace := range namespaces { + ns = append(ns, namespace) + } + sort.Strings(ns) + for i, t := range topologies { + if t.id == "pods" || t.id == "pods-by-service" { + topologies[i] = updateTopologyFilters(t, []APITopologyOptionGroup{kubernetesFilters(ns...)}) + } + } + return topologies +} + +// updateTopologyFilters recursively sets the options on a topology description +func updateTopologyFilters(t APITopologyDesc, options []APITopologyOptionGroup) APITopologyDesc { + t.Options = options + for i, sub := range t.SubTopologies { + t.SubTopologies[i] = updateTopologyFilters(sub, options) + } + return t +} + // registry is a threadsafe store of the available topologies type registry struct { sync.RWMutex @@ -239,23 +259,27 @@ func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc { respondWith(w, http.StatusInternalServerError, err.Error()) return } - topologies := r.renderTopologies(report, req) - respondWith(w, http.StatusOK, topologies) + respondWith(w, http.StatusOK, r.renderTopologies(report, req)) } } func (r *registry) renderTopologies(rpt report.Report, req *http.Request) []APITopologyDesc { topologies := []APITopologyDesc{} + req.ParseForm() + values := map[string]string{} + for k, vs := range req.Form { + values[k] = vs[0] + } r.walk(func(desc APITopologyDesc) { - renderer, decorator := renderedForRequest(req, desc) + renderer, decorator, _ := r.rendererForTopology(desc.id, values, rpt) desc.Stats = decorateWithStats(rpt, renderer, decorator) for i := range desc.SubTopologies { - renderer, decorator := renderedForRequest(req, desc.SubTopologies[i]) + renderer, decorator, _ := r.rendererForTopology(desc.id, values, rpt) desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator) } topologies = append(topologies, desc) }) - return topologies + return updateFilters(rpt, topologies) } func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator render.Decorator) topologyStats { @@ -280,10 +304,16 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re } } -func renderedForRequest(r *http.Request, topology APITopologyDesc) (render.Renderer, render.Decorator) { +func (r *registry) rendererForTopology(id string, values map[string]string, rpt report.Report) (render.Renderer, render.Decorator, error) { + topology, ok := r.get(id) + if !ok { + return nil, nil, fmt.Errorf("topology not found: %s", id) + } + topology = updateFilters(rpt, []APITopologyDesc{topology})[0] + var filters []render.FilterFunc for _, group := range topology.Options { - value := r.FormValue(group.ID) + value := values[group.ID] for _, opt := range group.Options { if opt.filter == nil { continue @@ -299,23 +329,22 @@ func renderedForRequest(r *http.Request, topology APITopologyDesc) (render.Rende return render.MakeFilter(render.ComposeFilterFuncs(filters...), renderer) } } - return topology.renderer, decorator + return topology.renderer, decorator, nil } -type reportRenderHandler func( - context.Context, - Reporter, render.Renderer, render.Decorator, - http.ResponseWriter, *http.Request, -) +type reportRenderHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) -func (r *registry) captureRenderer(rep Reporter, f reportRenderHandler) CtxHandlerFunc { - return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { - topology, ok := r.get(mux.Vars(req)["topology"]) - if !ok { - http.NotFound(w, req) - return - } - renderer, decorator := renderedForRequest(req, topology) - f(ctx, rep, renderer, decorator, w, req) +func (r *registry) rendererForRequest(req *http.Request, rpt report.Report) (render.Renderer, render.Decorator, error) { + req.ParseForm() + values := map[string]string{} + for k, vs := range req.Form { + values[k] = vs[0] + } + return r.rendererForTopology(mux.Vars(req)["topology"], values, rpt) +} + +func captureReporter(rep Reporter, f reportRenderHandler) CtxHandlerFunc { + return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { + f(ctx, rep, w, r) } } diff --git a/app/api_topology.go b/app/api_topology.go index d9ecd7378..da55bdd58 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -9,7 +9,6 @@ import ( "golang.org/x/net/context" "github.com/weaveworks/scope/common/xfer" - "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/render/detailed" ) @@ -28,27 +27,24 @@ type APINode struct { } // Full topology. -func handleTopology( - ctx context.Context, - rep Reporter, renderer render.Renderer, decorator render.Decorator, - w http.ResponseWriter, r *http.Request, -) { +func handleTopology(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { report, err := rep.Report(ctx) if err != nil { respondWith(w, http.StatusInternalServerError, err.Error()) return } + renderer, decorator, err := topologyRegistry.rendererForRequest(r, report) + if err != nil { + http.NotFound(w, r) + return + } respondWith(w, http.StatusOK, APITopology{ Nodes: detailed.Summaries(report, renderer.Render(report, decorator)), }) } // Websocket for the full topology. This route overlaps with the next. -func handleWs( - ctx context.Context, - rep Reporter, renderer render.Renderer, decorator render.Decorator, - w http.ResponseWriter, r *http.Request, -) { +func handleWs(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { if err := r.ParseForm(); err != nil { respondWith(w, http.StatusInternalServerError, err.Error()) return @@ -61,22 +57,31 @@ func handleWs( return } } - handleWebsocket(ctx, w, r, rep, renderer, decorator, loop) + handleWebsocket(ctx, w, r, rep, loop) } // Individual nodes. -func handleNode( - ctx context.Context, - rep Reporter, renderer render.Renderer, _ render.Decorator, - w http.ResponseWriter, r *http.Request, -) { +func handleNode(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] report, err = rep.Report(ctx) - rendered = renderer.Render(report, render.FilterNoop) - node, ok = rendered[nodeID] + ) + if err != nil { + respondWith(w, http.StatusInternalServerError, err.Error()) + return + } + + renderer, _, err := topologyRegistry.rendererForRequest(r, report) + if err != nil { + http.NotFound(w, r) + return + } + + var ( + rendered = renderer.Render(report, nil) + node, ok = rendered[nodeID] ) if err != nil { respondWith(w, http.StatusInternalServerError, err.Error()) @@ -94,8 +99,6 @@ func handleWebsocket( w http.ResponseWriter, r *http.Request, rep Reporter, - renderer render.Renderer, - decorator render.Decorator, loop time.Duration, ) { conn, err := xfer.Upgrade(w, r, nil) @@ -132,6 +135,11 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } + renderer, decorator, err := topologyRegistry.rendererForRequest(r, report) + if err != nil { + log.Errorf("Error generating report: %v", err) + return + } newTopo := detailed.Summaries(report, renderer.Render(report, decorator)) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/router.go b/app/router.go index d2ca136dd..d8467f4e7 100644 --- a/app/router.go +++ b/app/router.go @@ -89,11 +89,11 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter) { get.HandleFunc("/api/topology", gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get.HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))) + gzipHandler(requestContextDecorator(captureReporter(r, handleTopology)))) get.HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(topologyRegistry.captureRenderer(r, handleWs))) // NB not gzip! + requestContextDecorator(captureReporter(r, handleWs))) // NB not gzip! get.MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))) + gzipHandler(requestContextDecorator(captureReporter(r, handleNode)))) get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) get.HandleFunc("/api/probes", diff --git a/probe/kubernetes/pod.go b/probe/kubernetes/pod.go index 02db0e382..b5a116515 100644 --- a/probe/kubernetes/pod.go +++ b/probe/kubernetes/pod.go @@ -37,13 +37,13 @@ type Pod interface { type pod struct { *api.Pod - serviceIDs []string + serviceIDs report.StringSet Node *api.Node } // NewPod creates a new Pod func NewPod(p *api.Pod) Pod { - return &pod{Pod: p} + return &pod{Pod: p, serviceIDs: report.MakeStringSet()} } func (p *pod) UID() string { @@ -75,7 +75,7 @@ func (p *pod) Labels() labels.Labels { } func (p *pod) AddServiceID(id string) { - p.serviceIDs = append(p.serviceIDs, id) + p.serviceIDs = p.serviceIDs.Add(id) } func (p *pod) State() string { @@ -95,10 +95,7 @@ func (p *pod) GetNode(probeID string) report.Node { PodState: p.State(), PodIP: p.Status.PodIP, report.ControlProbeID: probeID, - }) - if len(p.serviceIDs) > 0 { - n = n.WithLatests(map[string]string{ServiceIDs: strings.Join(p.serviceIDs, " ")}) - } + }).WithSets(report.EmptySets.Add(ServiceIDs, p.serviceIDs)) for _, serviceID := range p.serviceIDs { segments := strings.SplitN(serviceID, "/", 2) if len(segments) != 2 { diff --git a/render/filters.go b/render/filters.go index 6b18ee459..0d075ce32 100644 --- a/render/filters.go +++ b/render/filters.go @@ -275,6 +275,14 @@ func HasChildren(topology string) FilterFunc { } } +// IsNamespace checks if the node is a pod/service in the specified namespace +func IsNamespace(namespace string) FilterFunc { + return func(n report.Node) bool { + gotNamespace, ok := n.Latest.Lookup(kubernetes.Namespace) + return !ok || namespace == gotNamespace + } +} + var systemContainerNames = map[string]struct{}{ "weavescope": {}, "weavedns": {}, diff --git a/render/pod.go b/render/pod.go index fdb1fda37..db90e20b8 100644 --- a/render/pod.go +++ b/render/pod.go @@ -16,37 +16,41 @@ const ( // PodRenderer is a Renderer which produces a renderable kubernetes // graph by merging the container graph and the pods topology. -var PodRenderer = MakeFilter( - func(n report.Node) bool { - // Drop deleted or empty pods - state, ok := n.Latest.Lookup(kubernetes.PodState) - return HasChildren(report.Container)(n) && (!ok || state != kubernetes.StateDeleted) - }, - MakeReduce( - MakeFilter( - func(n report.Node) bool { - // Drop unconnected pseudo nodes (could appear due to filtering) - _, isConnected := n.Latest.Lookup(IsConnected) - return n.Topology != Pseudo || isConnected - }, - ColorConnected(MakeMap( - MapContainer2Pod, - ContainerWithImageNameRenderer, - )), +var PodRenderer = ApplyDecorators( + MakeFilter( + func(n report.Node) bool { + // Drop deleted or empty pods + state, ok := n.Latest.Lookup(kubernetes.PodState) + return HasChildren(report.Container)(n) && (!ok || state != kubernetes.StateDeleted) + }, + MakeReduce( + MakeFilter( + func(n report.Node) bool { + // Drop unconnected pseudo nodes (could appear due to filtering) + _, isConnected := n.Latest.Lookup(IsConnected) + return n.Topology != Pseudo || isConnected + }, + ColorConnected(MakeMap( + MapContainer2Pod, + ContainerWithImageNameRenderer, + )), + ), + SelectPod, ), - SelectPod, ), ) // PodServiceRenderer is a Renderer which produces a renderable kubernetes services // graph by merging the pods graph and the services topology. -var PodServiceRenderer = FilterEmpty(report.Pod, - MakeReduce( - MakeMap( - MapPod2Service, - PodRenderer, +var PodServiceRenderer = ApplyDecorators( + FilterEmpty(report.Pod, + MakeReduce( + MakeMap( + MapPod2Service, + PodRenderer, + ), + SelectService, ), - SelectService, ), ) @@ -117,13 +121,13 @@ func MapPod2Service(pod report.Node, _ report.Networks) report.Nodes { if !ok { return report.Nodes{} } - ids, ok := pod.Latest.Lookup(kubernetes.ServiceIDs) + serviceIDs, ok := pod.Sets.Lookup(kubernetes.ServiceIDs) if !ok { return report.Nodes{} } result := report.Nodes{} - for _, serviceID := range strings.Fields(ids) { + for _, serviceID := range serviceIDs { serviceName := strings.TrimPrefix(serviceID, namespace+"/") id := report.MakeServiceNodeID(namespace, serviceName) node := NewDerivedNode(id, pod).WithTopology(report.Service) From 8758921215515c1ec587fd4ca852aee66f0dc510 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 29 Apr 2016 16:11:25 +0100 Subject: [PATCH 2/6] pass nil for Noop a few other places --- app/api_topologies.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 7f841973c..1515c10ad 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -30,7 +30,7 @@ func init() { Options: []APITopologyOption{ {"system", "System containers", render.IsSystem}, {"application", "Application containers", render.IsApplication}, - {"both", "Both", render.Noop}, + {"both", "Both", nil}, }, }, { @@ -39,7 +39,7 @@ func init() { Options: []APITopologyOption{ {"stopped", "Stopped containers", render.IsStopped}, {"running", "Running containers", render.IsRunning}, - {"both", "Both", render.Noop}, + {"both", "Both", nil}, }, }, } @@ -124,7 +124,7 @@ func kubernetesFilters(namespaces ...string) APITopologyOptionGroup { for _, namespace := range namespaces { options.Options = append(options.Options, APITopologyOption{namespace, namespace, render.IsNamespace(namespace)}) } - options.Options = append(options.Options, APITopologyOption{"all", "All Namespaces", render.Noop}) + options.Options = append(options.Options, APITopologyOption{"all", "All Namespaces", nil}) return options } From fe853e3f0f4de6229b071f527a63f17e4c1a0836 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 29 Apr 2016 16:36:13 +0100 Subject: [PATCH 3/6] filter out deleted pods when calculating available namespaces --- app/api_topologies.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/api_topologies.go b/app/api_topologies.go index 1515c10ad..acac489d5 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -134,6 +134,9 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog namespaces := map[string]struct{}{} for _, t := range []report.Topology{rpt.Pod, rpt.Service} { for _, n := range t.Nodes { + if state, ok := n.Latest.Lookup(kubernetes.PodState); ok && state == kubernetes.StateDeleted { + continue + } if namespace, ok := n.Latest.Lookup(kubernetes.Namespace); ok { namespaces[namespace] = struct{}{} } From 02a0e752e3b7290ae965ed2b795029cd7e40e76f Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 29 Apr 2016 16:42:43 +0100 Subject: [PATCH 4/6] fix up stats on sub-topologies --- app/api_topologies.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index acac489d5..adf261112 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -276,8 +276,8 @@ func (r *registry) renderTopologies(rpt report.Report, req *http.Request) []APIT r.walk(func(desc APITopologyDesc) { renderer, decorator, _ := r.rendererForTopology(desc.id, values, rpt) desc.Stats = decorateWithStats(rpt, renderer, decorator) - for i := range desc.SubTopologies { - renderer, decorator, _ := r.rendererForTopology(desc.id, values, rpt) + for i, sub := range desc.SubTopologies { + renderer, decorator, _ := r.rendererForTopology(sub.id, values, rpt) desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator) } topologies = append(topologies, desc) From 4187ea2107be94d96fe2c992a372f2fdc83ca4f3 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 29 Apr 2016 18:32:16 +0100 Subject: [PATCH 5/6] fixing up tests --- probe/kubernetes/reporter_test.go | 13 +++++++++++-- render/expected/expected.go | 8 ++++---- render/pod_test.go | 30 ++++++++---------------------- test/fixture/report_fixture.go | 24 +++++++++++++----------- 4 files changed, 36 insertions(+), 39 deletions(-) diff --git a/probe/kubernetes/reporter_test.go b/probe/kubernetes/reporter_test.go index aa7fc14a3..b7b65475d 100644 --- a/probe/kubernetes/reporter_test.go +++ b/probe/kubernetes/reporter_test.go @@ -174,20 +174,23 @@ func TestReporter(t *testing.T) { id string parentService string latest map[string]string + sets map[string]report.StringSet }{ {pod1ID, serviceID, map[string]string{ kubernetes.PodID: "ping/pong-a", kubernetes.PodName: "pong-a", kubernetes.Namespace: "ping", kubernetes.PodCreated: pod1.Created(), - kubernetes.ServiceIDs: "ping/pongservice", + }, map[string]report.StringSet{ + kubernetes.ServiceIDs: report.MakeStringSet("ping/pongservice"), }}, {pod2ID, serviceID, map[string]string{ kubernetes.PodID: "ping/pong-b", kubernetes.PodName: "pong-b", kubernetes.Namespace: "ping", kubernetes.PodCreated: pod1.Created(), - kubernetes.ServiceIDs: "ping/pongservice", + }, map[string]report.StringSet{ + kubernetes.ServiceIDs: report.MakeStringSet("ping/pongservice"), }}, } { node, ok := rpt.Pod.Nodes[pod.id] @@ -204,6 +207,12 @@ func TestReporter(t *testing.T) { t.Errorf("Expected pod %s latest %q: %q, got %q", pod.id, k, want, have) } } + + for k, want := range pod.sets { + if have, ok := node.Sets.Lookup(k); !ok || !reflect.DeepEqual(want, have) { + t.Errorf("Expected pod %s sets %q: %q, got %q", pod.id, k, want, have) + } + } } // Reporter should have added a service diff --git a/render/expected/expected.go b/render/expected/expected.go index 351f19c99..b436d72e5 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -238,8 +238,8 @@ var ( render.OutgoingInternetID: theOutgoingInternetNode, } - unmanagedServerID = render.MakePseudoNodeID(render.UnmanagedID, fixture.ServerHostID) - unmanagedServerNode = pseudo(unmanagedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet( + UnmanagedServerID = render.MakePseudoNodeID(render.UnmanagedID, fixture.ServerHostID) + unmanagedServerNode = pseudo(UnmanagedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet( uncontainedServerNode, RenderedEndpoints[fixture.NonContainerNodeID], RenderedProcesses[fixture.NonContainerProcessNodeID], @@ -262,7 +262,7 @@ var ( RenderedContainers[fixture.ServerContainerNodeID], )), - unmanagedServerID: unmanagedServerNode, + UnmanagedServerID: unmanagedServerNode, render.IncomingInternetID: theIncomingInternetNode(fixture.ServerPodNodeID), render.OutgoingInternetID: theOutgoingInternetNode, } @@ -282,7 +282,7 @@ var ( RenderedPods[fixture.ServerPodNodeID], )), - unmanagedServerID: unmanagedServerNode, + UnmanagedServerID: unmanagedServerNode, render.IncomingInternetID: theIncomingInternetNode(fixture.ServiceNodeID), render.OutgoingInternetID: theOutgoingInternetNode, } diff --git a/render/pod_test.go b/render/pod_test.go index d4776939e..f091f8c22 100644 --- a/render/pod_test.go +++ b/render/pod_test.go @@ -7,14 +7,13 @@ import ( "github.com/weaveworks/scope/probe/kubernetes" "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/render/expected" - "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" "github.com/weaveworks/scope/test/fixture" "github.com/weaveworks/scope/test/reflect" ) func TestPodRenderer(t *testing.T) { - have := Prune(render.PodRenderer.Render(fixture.Report, render.FilterNoop)) + have := Prune(render.PodRenderer.Render(fixture.Report, nil)) want := Prune(expected.RenderedPods) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -26,7 +25,7 @@ func TestPodFilterRenderer(t *testing.T) { // it is filtered out correctly. input := fixture.Report.Copy() input.Pod.Nodes[fixture.ClientPodNodeID] = input.Pod.Nodes[fixture.ClientPodNodeID].WithLatests(map[string]string{ - kubernetes.PodID: "pod:kube-system/foo", + kubernetes.PodID: "kube-system/foo", kubernetes.Namespace: "kube-system", kubernetes.PodName: "foo", }) @@ -43,7 +42,7 @@ func TestPodFilterRenderer(t *testing.T) { } func TestPodServiceRenderer(t *testing.T) { - have := Prune(render.PodServiceRenderer.Render(fixture.Report, render.FilterNoop)) + have := Prune(render.PodServiceRenderer.Render(fixture.Report, nil)) want := Prune(expected.RenderedPodServices) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -54,25 +53,12 @@ func TestPodServiceFilterRenderer(t *testing.T) { // tag on containers or pod namespace in the topology and ensure // it is filtered out correctly. input := fixture.Report.Copy() - input.Pod.Nodes[fixture.ClientPodNodeID] = input.Pod.Nodes[fixture.ClientPodNodeID].WithLatests(map[string]string{ - kubernetes.PodID: "pod:kube-system/foo", - kubernetes.Namespace: "kube-system", - kubernetes.PodName: "foo", - }) - input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{ - docker.LabelPrefix + "io.kubernetes.pod.name": "kube-system/foo", - }) - have := Prune(render.PodServiceRenderer.Render(input, render.FilterApplication)) + have := Prune(render.PodServiceRenderer.Render(input, render.FilterSystem)) want := Prune(expected.RenderedPodServices.Copy()) - wantNode := want[fixture.ServiceNodeID] - wantNode.Adjacency = nil - wantNode.Children = report.MakeNodeSet( - expected.RenderedEndpoints[fixture.Server80NodeID], - expected.RenderedProcesses[fixture.ServerProcessNodeID], - expected.RenderedContainers[fixture.ServerContainerNodeID], - expected.RenderedPods[fixture.ServerPodNodeID], - ) - want[fixture.ServiceNodeID] = wantNode + delete(want, fixture.ServiceNodeID) + delete(want, expected.UnmanagedServerID) + delete(want, render.IncomingInternetID) + delete(want, render.OutgoingInternetID) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } diff --git a/test/fixture/report_fixture.go b/test/fixture/report_fixture.go index 679d3b718..84b9d358d 100644 --- a/test/fixture/report_fixture.go +++ b/test/fixture/report_fixture.go @@ -367,25 +367,27 @@ var ( Nodes: report.Nodes{ ClientPodNodeID: report.MakeNodeWith( ClientPodNodeID, map[string]string{ - kubernetes.PodID: ClientPodID, - kubernetes.PodName: "pong-a", - kubernetes.Namespace: KubernetesNamespace, - kubernetes.ServiceIDs: ServiceID, - report.HostNodeID: ClientHostNodeID, + kubernetes.PodID: ClientPodID, + kubernetes.PodName: "pong-a", + kubernetes.Namespace: KubernetesNamespace, + report.HostNodeID: ClientHostNodeID, }). + WithSets(report.EmptySets. + Add(kubernetes.ServiceIDs, report.MakeStringSet(ServiceID))). WithTopology(report.Pod).WithParents(report.EmptySets. Add("host", report.MakeStringSet(ClientHostNodeID)). Add("service", report.MakeStringSet(ServiceID)), ), ServerPodNodeID: report.MakeNodeWith( ServerPodNodeID, map[string]string{ - kubernetes.PodID: ServerPodID, - kubernetes.PodName: "pong-b", - kubernetes.Namespace: KubernetesNamespace, - kubernetes.PodState: "running", - kubernetes.ServiceIDs: ServiceID, - report.HostNodeID: ServerHostNodeID, + kubernetes.PodID: ServerPodID, + kubernetes.PodName: "pong-b", + kubernetes.Namespace: KubernetesNamespace, + kubernetes.PodState: "running", + report.HostNodeID: ServerHostNodeID, }). + WithSets(report.EmptySets. + Add(kubernetes.ServiceIDs, report.MakeStringSet(ServiceID))). WithTopology(report.Pod).WithParents(report.EmptySets. Add("host", report.MakeStringSet(ServerHostNodeID)). Add("service", report.MakeStringSet(ServiceID)), From 0e70f70ffd8e51887a8a82ed6f76e1280b1931be Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Tue, 3 May 2016 11:59:31 +0100 Subject: [PATCH 6/6] Review feedback --- app/api_topologies.go | 54 ++++++++++++++++----------- app/api_topology.go | 85 ++++++++++++++----------------------------- app/router.go | 6 +-- render/pod.go | 3 +- 4 files changed, 65 insertions(+), 83 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index adf261112..ee02ad3eb 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -3,6 +3,7 @@ package app import ( "fmt" "net/http" + "net/url" "sort" "sync" @@ -269,15 +270,11 @@ func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc { func (r *registry) renderTopologies(rpt report.Report, req *http.Request) []APITopologyDesc { topologies := []APITopologyDesc{} req.ParseForm() - values := map[string]string{} - for k, vs := range req.Form { - values[k] = vs[0] - } r.walk(func(desc APITopologyDesc) { - renderer, decorator, _ := r.rendererForTopology(desc.id, values, rpt) + renderer, decorator, _ := r.rendererForTopology(desc.id, req.Form, rpt) desc.Stats = decorateWithStats(rpt, renderer, decorator) for i, sub := range desc.SubTopologies { - renderer, decorator, _ := r.rendererForTopology(sub.id, values, rpt) + renderer, decorator, _ := r.rendererForTopology(sub.id, req.Form, rpt) desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator) } topologies = append(topologies, desc) @@ -307,16 +304,16 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re } } -func (r *registry) rendererForTopology(id string, values map[string]string, rpt report.Report) (render.Renderer, render.Decorator, error) { - topology, ok := r.get(id) +func (r *registry) rendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Decorator, error) { + topology, ok := r.get(topologyID) if !ok { - return nil, nil, fmt.Errorf("topology not found: %s", id) + return nil, nil, fmt.Errorf("topology not found: %s", topologyID) } topology = updateFilters(rpt, []APITopologyDesc{topology})[0] var filters []render.FilterFunc for _, group := range topology.Options { - value := values[group.ID] + value := values.Get(group.ID) for _, opt := range group.Options { if opt.filter == nil { continue @@ -335,19 +332,34 @@ func (r *registry) rendererForTopology(id string, values map[string]string, rpt return topology.renderer, decorator, nil } -type reportRenderHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) +type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) -func (r *registry) rendererForRequest(req *http.Request, rpt report.Report) (render.Renderer, render.Decorator, error) { - req.ParseForm() - values := map[string]string{} - for k, vs := range req.Form { - values[k] = vs[0] - } - return r.rendererForTopology(mux.Vars(req)["topology"], values, rpt) -} - -func captureReporter(rep Reporter, f reportRenderHandler) CtxHandlerFunc { +func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { f(ctx, rep, w, r) } } + +type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, http.ResponseWriter, *http.Request) + +func (r *registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { + return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { + topologyID := mux.Vars(req)["topology"] + if _, ok := r.get(topologyID); !ok { + http.NotFound(w, req) + return + } + rpt, err := rep.Report(ctx) + if err != nil { + respondWith(w, http.StatusInternalServerError, err.Error()) + return + } + req.ParseForm() + renderer, decorator, err := r.rendererForTopology(topologyID, req.Form, rpt) + if err != nil { + respondWith(w, http.StatusInternalServerError, err.Error()) + return + } + f(ctx, renderer, decorator, rpt, w, req) + } +} diff --git a/app/api_topology.go b/app/api_topology.go index da55bdd58..3a6eabb89 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -9,7 +9,9 @@ import ( "golang.org/x/net/context" "github.com/weaveworks/scope/common/xfer" + "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/render/detailed" + "github.com/weaveworks/scope/report" ) const ( @@ -27,24 +29,35 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { - report, err := rep.Report(ctx) - if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) - return - } - renderer, decorator, err := topologyRegistry.rendererForRequest(r, report) - if err != nil { - http.NotFound(w, r) - return - } +func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ Nodes: detailed.Summaries(report, renderer.Render(report, decorator)), }) } -// Websocket for the full topology. This route overlaps with the next. -func handleWs(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { +// Individual nodes. +func handleNode(ctx context.Context, renderer render.Renderer, _ render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { + var ( + vars = mux.Vars(r) + topologyID = vars["topology"] + nodeID = vars["id"] + rendered = renderer.Render(report, nil) + node, ok = rendered[nodeID] + ) + if !ok { + http.NotFound(w, r) + return + } + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node)}) +} + +// Websocket for the full topology. +func handleWebsocket( + ctx context.Context, + rep Reporter, + w http.ResponseWriter, + r *http.Request, +) { if err := r.ParseForm(); err != nil { respondWith(w, http.StatusInternalServerError, err.Error()) return @@ -57,50 +70,7 @@ func handleWs(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http. return } } - handleWebsocket(ctx, w, r, rep, loop) -} -// Individual nodes. -func handleNode(ctx context.Context, rep Reporter, w http.ResponseWriter, r *http.Request) { - var ( - vars = mux.Vars(r) - topologyID = vars["topology"] - nodeID = vars["id"] - report, err = rep.Report(ctx) - ) - if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) - return - } - - renderer, _, err := topologyRegistry.rendererForRequest(r, report) - if err != nil { - http.NotFound(w, r) - return - } - - var ( - rendered = renderer.Render(report, nil) - node, ok = rendered[nodeID] - ) - if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) - return - } - if !ok { - http.NotFound(w, r) - return - } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node)}) -} - -func handleWebsocket( - ctx context.Context, - w http.ResponseWriter, - r *http.Request, - rep Reporter, - loop time.Duration, -) { conn, err := xfer.Upgrade(w, r, nil) if err != nil { // log.Info("Upgrade:", err) @@ -125,6 +95,7 @@ func handleWebsocket( previousTopo detailed.NodeSummaries tick = time.Tick(loop) wait = make(chan struct{}, 1) + topologyID = mux.Vars(r)["topology"] ) rep.WaitOn(ctx, wait) defer rep.UnWait(ctx, wait) @@ -135,7 +106,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - renderer, decorator, err := topologyRegistry.rendererForRequest(r, report) + renderer, decorator, err := topologyRegistry.rendererForTopology(topologyID, r.Form, report) if err != nil { log.Errorf("Error generating report: %v", err) return diff --git a/app/router.go b/app/router.go index d8467f4e7..960717ba5 100644 --- a/app/router.go +++ b/app/router.go @@ -89,11 +89,11 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter) { get.HandleFunc("/api/topology", gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get.HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(captureReporter(r, handleTopology)))) + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))) get.HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(captureReporter(r, handleWs))) // NB not gzip! + requestContextDecorator(captureReporter(r, handleWebsocket))) // NB not gzip! get.MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(captureReporter(r, handleNode)))) + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))) get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) get.HandleFunc("/api/probes", diff --git a/render/pod.go b/render/pod.go index db90e20b8..7f7c540ab 100644 --- a/render/pod.go +++ b/render/pod.go @@ -19,9 +19,8 @@ const ( var PodRenderer = ApplyDecorators( MakeFilter( func(n report.Node) bool { - // Drop deleted or empty pods state, ok := n.Latest.Lookup(kubernetes.PodState) - return HasChildren(report.Container)(n) && (!ok || state != kubernetes.StateDeleted) + return (!ok || state != kubernetes.StateDeleted) }, MakeReduce( MakeFilter(