From da8b8d5095c21288f0af6129ed689fd6ee7c4502 Mon Sep 17 00:00:00 2001 From: Mike Lang Date: Mon, 20 Mar 2017 10:05:10 -0700 Subject: [PATCH 1/3] Revert "Revert "Merge pull request #2285 from weaveworks/mike/k8s-ns-in-container-view"" This reverts commit d55c528fe2a510cb56e4b638470b25be9da5b6a9. --- app/api_topologies.go | 24 +++++++++++++++--------- render/filters.go | 15 +++++++++++++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index ecc44fc85..faa3160aa 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -80,21 +80,23 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog ns = append(ns, namespace) } sort.Strings(ns) - for i, t := range topologies { - if t.id == podsID || t.id == servicesID || t.id == deploymentsID || t.id == replicaSetsID { - topologies[i] = updateTopologyFilters(t, []APITopologyOptionGroup{ - kubernetesFilters(ns...), unmanagedFilter, - }) + if len(ns) > 0 { // We only want to apply k8s filters when we have k8s-related nodes + for i, t := range topologies { + if t.id == containersID || t.id == containersByImageID || t.id == containersByHostnameID || t.id == podsID || t.id == servicesID || t.id == deploymentsID || t.id == replicaSetsID { + topologies[i] = mergeTopologyFilters(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 +// mergeTopologyFilters recursively merges in new options on a topology description +func mergeTopologyFilters(t APITopologyDesc, options []APITopologyOptionGroup) APITopologyDesc { + t.Options = append(t.Options, options...) for i, sub := range t.SubTopologies { - t.SubTopologies[i] = updateTopologyFilters(sub, options) + t.SubTopologies[i] = mergeTopologyFilters(sub, options) } return t } @@ -200,6 +202,7 @@ func MakeRegistry() *Registry { renderer: render.PodRenderer, Name: "Pods", Rank: 3, + Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ @@ -207,6 +210,7 @@ func MakeRegistry() *Registry { parent: podsID, renderer: render.ReplicaSetRenderer, Name: "replica sets", + Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ @@ -214,6 +218,7 @@ func MakeRegistry() *Registry { parent: podsID, renderer: render.DeploymentRenderer, Name: "deployments", + Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ @@ -221,6 +226,7 @@ func MakeRegistry() *Registry { parent: podsID, renderer: render.PodServiceRenderer, Name: "services", + Options: []APITopologyOptionGroup{unmanagedFilter}, HideIfEmpty: true, }, APITopologyDesc{ diff --git a/render/filters.go b/render/filters.go index 28a0fb483..fc5425d46 100644 --- a/render/filters.go +++ b/render/filters.go @@ -10,6 +10,10 @@ import ( "github.com/weaveworks/scope/report" ) +const ( + k8sNamespaceLabel = "io.kubernetes.pod.namespace" +) + // PreciousNodeRenderer ensures a node is never filtered out by decorators type PreciousNodeRenderer struct { PreciousNodeID string @@ -281,7 +285,7 @@ func IsApplication(n report.Node) bool { if roleLabel == "system" { return false } - namespace, _ := n.Latest.Lookup(docker.LabelPrefix + "io.kubernetes.pod.namespace") + namespace, _ := n.Latest.Lookup(docker.LabelPrefix + k8sNamespaceLabel) if namespace == "kube-system" { return false } @@ -320,7 +324,14 @@ func IsNotPseudo(n report.Node) bool { // 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, _ := n.Latest.Lookup(kubernetes.Namespace) + tryKeys := []string{kubernetes.Namespace, docker.LabelPrefix + k8sNamespaceLabel} + gotNamespace := "" + for _, key := range tryKeys { + if value, ok := n.Latest.Lookup(key); ok { + gotNamespace = value + break + } + } return namespace == gotNamespace } } From b61d7027af6b94d8363cd9440e7c5e7d765b4395 Mon Sep 17 00:00:00 2001 From: Mike Lang Date: Mon, 20 Mar 2017 10:48:08 -0700 Subject: [PATCH 2/3] Fix various sources of mutation instead of copying in k8s filter merging code This was causing a large number of repeated appending of the same option group --- app/api_topologies.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index faa3160aa..41eb0e746 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -81,6 +81,7 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog } sort.Strings(ns) if len(ns) > 0 { // We only want to apply k8s filters when we have k8s-related nodes + topologies = append([]APITopologyDesc{}, topologies...) // Make a copy so we can make changes safely for i, t := range topologies { if t.id == containersID || t.id == containersByImageID || t.id == containersByHostnameID || t.id == podsID || t.id == servicesID || t.id == deploymentsID || t.id == replicaSetsID { topologies[i] = mergeTopologyFilters(t, []APITopologyOptionGroup{ @@ -94,10 +95,12 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog // mergeTopologyFilters recursively merges in new options on a topology description func mergeTopologyFilters(t APITopologyDesc, options []APITopologyOptionGroup) APITopologyDesc { - t.Options = append(t.Options, options...) + t.Options = append(append([]APITopologyOptionGroup{}, t.Options...), options...) + newSubTopologies := make([]APITopologyDesc, len(t.SubTopologies)) for i, sub := range t.SubTopologies { - t.SubTopologies[i] = mergeTopologyFilters(sub, options) + newSubTopologies[i] = mergeTopologyFilters(sub, options) } + t.SubTopologies = newSubTopologies return t } From c06e7582cb35442f2fc1694a794fabb1507bee8e Mon Sep 17 00:00:00 2001 From: Mike Lang Date: Tue, 21 Mar 2017 15:43:30 -0700 Subject: [PATCH 3/3] api_topologies k8s filters: some minor refactoring for niceness --- app/api_topologies.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 41eb0e746..4d0a38e2f 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -75,19 +75,22 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog } } } - var ns []string + if len(namespaces) == 0 { + // We only want to apply k8s filters when we have k8s-related nodes, + // so if we don't then return early + return topologies + } + ns := []string{} for namespace := range namespaces { ns = append(ns, namespace) } sort.Strings(ns) - if len(ns) > 0 { // We only want to apply k8s filters when we have k8s-related nodes - topologies = append([]APITopologyDesc{}, topologies...) // Make a copy so we can make changes safely - for i, t := range topologies { - if t.id == containersID || t.id == containersByImageID || t.id == containersByHostnameID || t.id == podsID || t.id == servicesID || t.id == deploymentsID || t.id == replicaSetsID { - topologies[i] = mergeTopologyFilters(t, []APITopologyOptionGroup{ - kubernetesFilters(ns...), - }) - } + topologies = append([]APITopologyDesc{}, topologies...) // Make a copy so we can make changes safely + for i, t := range topologies { + if t.id == containersID || t.id == containersByImageID || t.id == containersByHostnameID || t.id == podsID || t.id == servicesID || t.id == deploymentsID || t.id == replicaSetsID { + topologies[i] = mergeTopologyFilters(t, []APITopologyOptionGroup{ + kubernetesFilters(ns...), + }) } } return topologies