From 9833a854b1638d8bd3270323c8aa0041016ebb98 Mon Sep 17 00:00:00 2001 From: CarltonSemple Date: Mon, 26 Sep 2016 15:17:06 +0000 Subject: [PATCH 1/2] Added container filters as CLI arguments gofmt load_container_filters.go removed the environment variable for container label filters Added the --app.container-label-filter command line argument, and load_container_filters.go now uses the results from that Changed init() to InitializeTopologies() Changed init() to InitializeTopologies() so that it can be called after the container filters are loaded from the command line argument. init() executes before main() in prog/main.go, so the flag parsing isn't finished before init() is called Applied lint fixes fixed lint issues brought back the init function for api_topologies.go Addressed many of the PR comments, except escaping colons Renamed IsDesired to HasLabel in render/filters.go Allows for the user to escape colons added registry function for modifying the container filters created a separate function that parses the container filter flags simplified registry.addContainerFilters() addressed review comments switched API Topology Description IDs to constants addressed review comments joined constants added test functions addressed most of the review comments Changed containerLabelFilters to an array of APItopologyOptions, placing the parsing in the Set() function. Removed parsing from HasLabel in render/filters.go refactored code added test that applies to the container filtering by labels applied golint made Registry items private and added a MakeRegistry() function fixed usage of topologyRegistry.RendererForTopology Added container label filters by exclusion minor update to report_fixture Modified container labels test to use existing report I added labels to the existing containers in the fixed report for testing. refactored code refactored code further code refactoring addressed @ijsnellf's review comments unexported Registry, and reduced duplicate code addressed @ijsnellf's review comments Addressed review comments Addressed final review comments --- app/api_topologies.go | 137 ++++++++++++++++++++++----------- app/api_topologies_test.go | 58 ++++++++++++++ app/api_topology.go | 2 +- prog/main.go | 70 +++++++++++++++-- prog/main_test.go | 26 +++++++ render/filters.go | 16 ++++ test/fixture/report_fixture.go | 7 ++ 7 files changed, 264 insertions(+), 52 deletions(-) create mode 100644 prog/main_test.go diff --git a/app/api_topologies.go b/app/api_topologies.go index ece945363..4f4ee2abd 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -15,48 +15,61 @@ import ( "github.com/weaveworks/scope/report" ) -const apiTopologyURL = "/api/topology/" +const ( + apiTopologyURL = "/api/topology/" + processesTopologyDescID = "processes" + processesByNameTopologyDescID = "processes-by-name" + containerLabelFiltersGroupID = "container_label_filters_group" + containersTopologyDescID = "containers" + containersByHostnameTopologyDescID = "containers-by-hostname" + containersByImageTopologyDescID = "containers-by-image" + podsTopologyDescID = "pods" + replicaSetsTopologyDescID = "replica-sets" + deploymentsTopologyDescID = "deployments" + servicesTopologyDescID = "services" + hostsTopologyDescID = "hosts" +) var ( - topologyRegistry = ®istry{ - items: map[string]APITopologyDesc{}, - } - k8sPseudoFilter = APITopologyOptionGroup{ + topologyRegistry = MakeRegistry() + k8sPseudoFilter = APITopologyOptionGroup{ ID: "pseudo", Default: "hide", Options: []APITopologyOption{ - {"show", "Show Unmanaged", nil, false}, - {"hide", "Hide Unmanaged", render.IsNotPseudo, true}, + {Value: "show", Label: "Show Unmanaged", filter: nil, filterPseudo: false}, + {Value: "hide", Label: "Hide Unmanaged", filter: render.IsNotPseudo, filterPseudo: true}, }, } ) func init() { + AddInitialTopologiesToRegistry(topologyRegistry) +} + +// AddInitialTopologiesToRegistry does the initial setup for a Registry. +// This is needed for testing. +func AddInitialTopologiesToRegistry(registry *Registry) { containerFilters := []APITopologyOptionGroup{ { - ID: "system", + ID: containerLabelFiltersGroupID, Default: "application", - Options: []APITopologyOption{ - {"system", "System containers", render.IsSystem, false}, - {"application", "Application containers", render.IsApplication, false}, - {"both", "Both", nil, false}, - }, + Options: []APITopologyOption{{Value: "all", Label: "All", filter: nil, filterPseudo: false}, {Value: "system", Label: "System Containers", filter: render.IsSystem, filterPseudo: false}, {Value: "notsystem", Label: "Application Containers", filter: render.IsApplication, filterPseudo: false}}, }, { ID: "stopped", Default: "running", Options: []APITopologyOption{ - {"stopped", "Stopped containers", render.IsStopped, false}, - {"running", "Running containers", render.IsRunning, false}, - {"both", "Both", nil, false}, + {Value: "stopped", Label: "Stopped containers", filter: render.IsStopped, filterPseudo: false}, + {Value: "running", Label: "Running containers", filter: render.IsRunning, filterPseudo: false}, + {Value: "both", Label: "Both", filter: nil, filterPseudo: false}, }, }, { ID: "pseudo", Default: "hide", Options: []APITopologyOption{ - {"show", "Show Uncontained", nil, false}, - {"hide", "Hide Uncontained", render.IsNotPseudo, true}, + {Value: "show", Label: "Show Uncontained", filter: nil, filterPseudo: false}, + {Value: "hide", Label: "Hide Uncontained", filter: render.IsNotPseudo, filterPseudo: true}, }, }, } @@ -68,16 +81,16 @@ func init() { Options: []APITopologyOption{ // Show the user why there are filtered nodes in this view. // Don't give them the option to show those nodes. - {"hide", "Unconnected nodes hidden", nil, false}, + {Value: "hide", Label: "Unconnected nodes hidden", filter: nil, filterPseudo: false}, }, }, } // Topology option labels should tell the current state. The first item must // be the verb to get to that state - topologyRegistry.add( + registry.Add( APITopologyDesc{ - id: "processes", + id: processesTopologyDescID, renderer: render.FilterUnconnected(render.ProcessWithContainerNameRenderer), Name: "Processes", Rank: 1, @@ -85,7 +98,7 @@ func init() { HideIfEmpty: true, }, APITopologyDesc{ - id: "processes-by-name", + id: processesByNameTopologyDescID, parent: "processes", renderer: render.FilterUnconnected(render.ProcessNameRenderer), Name: "by name", @@ -93,56 +106,56 @@ func init() { HideIfEmpty: true, }, APITopologyDesc{ - id: "containers", + id: containersTopologyDescID, renderer: render.ContainerWithImageNameRenderer, Name: "Containers", Rank: 2, Options: containerFilters, }, APITopologyDesc{ - id: "containers-by-hostname", + id: containersByHostnameTopologyDescID, parent: "containers", renderer: render.ContainerHostnameRenderer, Name: "by DNS name", Options: containerFilters, }, APITopologyDesc{ - id: "containers-by-image", + id: containersByImageTopologyDescID, parent: "containers", renderer: render.ContainerImageRenderer, Name: "by image", Options: containerFilters, }, APITopologyDesc{ - id: "pods", + id: podsTopologyDescID, renderer: render.PodRenderer, Name: "Pods", Rank: 3, HideIfEmpty: true, }, APITopologyDesc{ - id: "replica-sets", + id: replicaSetsTopologyDescID, parent: "pods", renderer: render.ReplicaSetRenderer, Name: "replica sets", HideIfEmpty: true, }, APITopologyDesc{ - id: "deployments", + id: deploymentsTopologyDescID, parent: "pods", renderer: render.DeploymentRenderer, Name: "deployments", HideIfEmpty: true, }, APITopologyDesc{ - id: "services", + id: servicesTopologyDescID, parent: "pods", renderer: render.PodServiceRenderer, Name: "services", HideIfEmpty: true, }, APITopologyDesc{ - id: "hosts", + id: hostsTopologyDescID, renderer: render.HostRenderer, Name: "Hosts", Rank: 4, @@ -165,10 +178,10 @@ func kubernetesFilters(namespaces ...string) APITopologyOptionGroup { options.Default = namespace } options.Options = append(options.Options, APITopologyOption{ - namespace, namespace, render.IsNamespace(namespace), false, + Value: namespace, Label: namespace, filter: render.IsNamespace(namespace), filterPseudo: false, }) } - options.Options = append(options.Options, APITopologyOption{"all", "All Namespaces", nil, false}) + options.Options = append(options.Options, APITopologyOption{Value: "all", Label: "All Namespaces", filter: nil, filterPseudo: false}) return options } @@ -192,7 +205,7 @@ func updateFilters(rpt report.Report, topologies []APITopologyDesc) []APITopolog } sort.Strings(ns) for i, t := range topologies { - if t.id == "pods" || t.id == "services" || t.id == "deployments" || t.id == "replica-sets" { + if t.id == podsTopologyDescID || t.id == servicesTopologyDescID || t.id == deploymentsTopologyDescID || t.id == replicaSetsTopologyDescID { topologies[i] = updateTopologyFilters(t, []APITopologyOptionGroup{ kubernetesFilters(ns...), k8sPseudoFilter, }) @@ -210,12 +223,25 @@ func updateTopologyFilters(t APITopologyDesc, options []APITopologyOptionGroup) return t } -// registry is a threadsafe store of the available topologies -type registry struct { +// MakeAPITopologyOption provides an external interface to the package for creating an APITopologyOption. +func MakeAPITopologyOption(value string, label string, filterFunc render.FilterFunc, pseudo bool) APITopologyOption { + return APITopologyOption{Value: value, Label: label, filter: filterFunc, filterPseudo: pseudo} +} + +// Registry is a threadsafe store of the available topologies +type Registry struct { sync.RWMutex items map[string]APITopologyDesc } +// MakeRegistry returns a new Registry +func MakeRegistry() *Registry { + newRegistry := &Registry{ + items: map[string]APITopologyDesc{}, + } + return newRegistry +} + // APITopologyDesc is returned in a list by the /api/topology handler. type APITopologyDesc struct { id string @@ -261,7 +287,27 @@ type topologyStats struct { FilteredNodes int `json:"filtered_nodes"` } -func (r *registry) add(ts ...APITopologyDesc) { +// AddContainerFilters adds to the default Registry (topologyRegistry)'s containerFilters +func AddContainerFilters(newFilters ...APITopologyOption) { + topologyRegistry.AddContainerFilters(newFilters...) +} + +// AddContainerFilters adds container filters to this Registry +func (r *Registry) AddContainerFilters(newFilters ...APITopologyOption) { + r.Lock() + defer r.Unlock() + for _, key := range []string{containersTopologyDescID, containersByHostnameTopologyDescID, containersByImageTopologyDescID} { + for i := range r.items[key].Options { + if r.items[key].Options[i].ID == containerLabelFiltersGroupID { + r.items[key].Options[i].Options = append(r.items[key].Options[i].Options, newFilters...) + break + } + } + } +} + +// Add inserts a topologyDesc to the Registry's items map +func (r *Registry) Add(ts ...APITopologyDesc) { r.Lock() defer r.Unlock() for _, t := range ts { @@ -277,14 +323,14 @@ func (r *registry) add(ts ...APITopologyDesc) { } } -func (r *registry) get(name string) (APITopologyDesc, bool) { +func (r *Registry) get(name string) (APITopologyDesc, bool) { r.RLock() defer r.RUnlock() t, ok := r.items[name] return t, ok } -func (r *registry) walk(f func(APITopologyDesc)) { +func (r *Registry) walk(f func(APITopologyDesc)) { r.RLock() defer r.RUnlock() descs := []APITopologyDesc{} @@ -301,7 +347,7 @@ func (r *registry) walk(f func(APITopologyDesc)) { } // makeTopologyList returns a handler that yields an APITopologyList. -func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc { +func (r *Registry) makeTopologyList(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { report, err := rep.Report(ctx) if err != nil { @@ -312,14 +358,14 @@ func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc { } } -func (r *registry) renderTopologies(rpt report.Report, req *http.Request) []APITopologyDesc { +func (r *Registry) renderTopologies(rpt report.Report, req *http.Request) []APITopologyDesc { topologies := []APITopologyDesc{} req.ParseForm() r.walk(func(desc APITopologyDesc) { - renderer, decorator, _ := r.rendererForTopology(desc.id, req.Form, 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, req.Form, rpt) + renderer, decorator, _ := r.RendererForTopology(sub.id, req.Form, rpt) desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator) } topologies = append(topologies, desc) @@ -349,7 +395,8 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re } } -func (r *registry) rendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Decorator, error) { +// RendererForTopology .. +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", topologyID) @@ -393,7 +440,7 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, http.ResponseWriter, *http.Request) -func (r *registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { +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 { @@ -406,7 +453,7 @@ func (r *registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu return } req.ParseForm() - renderer, decorator, err := r.rendererForTopology(topologyID, req.Form, rpt) + renderer, decorator, err := r.RendererForTopology(topologyID, req.Form, rpt) if err != nil { respondWith(w, http.StatusInternalServerError, err) return diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 54aabae55..cf075f41c 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -3,6 +3,7 @@ package app_test import ( "bytes" "net/http/httptest" + "net/url" "testing" "time" @@ -12,10 +13,17 @@ import ( "github.com/weaveworks/scope/app" "github.com/weaveworks/scope/probe/kubernetes" + "github.com/weaveworks/scope/render" + "github.com/weaveworks/scope/render/detailed" "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test/fixture" ) +const ( + containerLabelFiltersGroupID = "container_label_filters_group" + customAPITopologyOptionFilterID = "containerLabelFilter0" +) + func TestAPITopology(t *testing.T) { ts := topologyServer() defer ts.Close() @@ -50,6 +58,56 @@ func TestAPITopology(t *testing.T) { } } +func TestContainerLabelFilter(t *testing.T) { + topologySummaries, err := getTestContainerLabelFilterTopologySummary(t, false) + if err != nil { + t.Fatalf("Topology Registry Report error: %s", err) + } + + // only the filtered container with fixture.TestLabelKey1 should be present + equals(t, 1, len(topologySummaries)) + for key := range topologySummaries { + equals(t, fixture.ClientContainerID+";", key) + } +} + +func TestContainerLabelFilterExclude(t *testing.T) { + topologySummaries, err := getTestContainerLabelFilterTopologySummary(t, true) + if err != nil { + t.Fatalf("Topology Registry Report error: %s", err) + } + + // all containers but the excluded container should be present + for key := range topologySummaries { + if fixture.ServerContainerNodeID+";" == key { + t.Errorf("TestAPITopologyNegativeContainerLabelFilter Failed. Expected to not find " + fixture.ServerContainerNodeID + "; in report") + } + } +} + +func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (detailed.NodeSummaries, error) { + ts := topologyServer() + defer ts.Close() + + topologyRegistry := app.MakeRegistry() + app.AddInitialTopologiesToRegistry(topologyRegistry) + + if exclude == true { + topologyRegistry.AddContainerFilters(app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", render.DoesNotHaveLabel(fixture.TestLabelKey2, fixture.ApplicationLabelValue2), false)) + } else { + topologyRegistry.AddContainerFilters(app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", render.HasLabel(fixture.TestLabelKey1, fixture.ApplicationLabelValue1), false)) + } + + urlvalues := url.Values{} + urlvalues.Set(containerLabelFiltersGroupID, customAPITopologyOptionFilterID) + renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) + if err != nil { + return nil, err + } + + return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator)), nil +} + func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) diff --git a/app/api_topology.go b/app/api_topology.go index 08bfe229c..36d46ae16 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -107,7 +107,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - renderer, decorator, err := topologyRegistry.rendererForTopology(topologyID, r.Form, report) + renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, r.Form, report) if err != nil { log.Errorf("Error generating report: %v", err) return diff --git a/prog/main.go b/prog/main.go index d43c29065..16334a70d 100644 --- a/prog/main.go +++ b/prog/main.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "os" + "regexp" "strconv" "strings" "time" @@ -16,6 +17,7 @@ import ( "github.com/weaveworks/scope/common/xfer" "github.com/weaveworks/scope/probe/appclient" "github.com/weaveworks/scope/probe/kubernetes" + "github.com/weaveworks/scope/render" "github.com/weaveworks/weave/common" ) @@ -33,6 +35,8 @@ var ( kubernetesPasswordFlag, kubernetesTokenFlag, } + colonFinder = regexp.MustCompile(`[^\\](:)`) + unescapeBackslashes = regexp.MustCompile(`\\(.)`) ) type prefixFormatter struct { @@ -138,6 +142,54 @@ type appFlags struct { consulInf string } +type containerLabelFiltersFlag struct { + apiTopologyOptions []app.APITopologyOption + filterNumber int + filterIDPrefix string + exclude bool +} + +func (c *containerLabelFiltersFlag) String() string { + return fmt.Sprint(c.apiTopologyOptions) +} + +func (c *containerLabelFiltersFlag) Set(flagValue string) error { + filterID := fmt.Sprintf(c.filterIDPrefix+"%d", c.filterNumber) + newAPITopologyOption, err := c.toAPITopologyOption(flagValue, filterID) + if err != nil { + return err + } + c.filterNumber++ + + c.apiTopologyOptions = append(c.apiTopologyOptions, newAPITopologyOption) + return nil +} + +func (c *containerLabelFiltersFlag) toAPITopologyOption(flagValue string, filterID string) (app.APITopologyOption, error) { + indexRanges := colonFinder.FindAllStringIndex(flagValue, -1) + if len(indexRanges) != 1 { + if len(indexRanges) == 0 { + return app.APITopologyOption{}, fmt.Errorf("No unescaped colon found. This is needed to separate the title from the label") + } + return app.APITopologyOption{}, fmt.Errorf("Multiple unescaped colons. Escape colons that are part of the title and label") + } + splitIndices := indexRanges[0] + titleStringEscaped := flagValue[:splitIndices[0]+1] + labelStringEscaped := flagValue[splitIndices[1]:] + containerFilterTitle := unescapeBackslashes.ReplaceAllString(titleStringEscaped, `$1`) + containerFilterLabel := unescapeBackslashes.ReplaceAllString(labelStringEscaped, `$1`) + labelKeyValuePair := strings.Split(containerFilterLabel, "=") + if len(labelKeyValuePair) != 2 { + return app.APITopologyOption{}, fmt.Errorf("Docker label in the app.container-label-filter(-exclude) flag isn't in the correct key=value format") + } + + filterFunction := render.HasLabel + if c.exclude { + filterFunction = render.DoesNotHaveLabel + } + return app.MakeAPITopologyOption(filterID, containerFilterTitle, filterFunction(labelKeyValuePair[0], labelKeyValuePair[1]), false), nil +} + func logCensoredArgs() { var prettyPrintedArgs string // We show the flags followed by the args. This may change the original @@ -162,12 +214,14 @@ func logCensoredArgs() { func main() { var ( - flags = flags{} - mode string - debug bool - weaveEnabled bool - weaveHostname string - dryRun bool + flags = flags{} + mode string + debug bool + weaveEnabled bool + weaveHostname string + dryRun bool + containerLabelFilterFlags = containerLabelFiltersFlag{exclude: false, filterIDPrefix: "containerLabelFilterExclude"} + containerLabelFilterFlagsExclude = containerLabelFiltersFlag{exclude: true, filterIDPrefix: "containerLabelFilter"} ) // Flags that apply to both probe and app @@ -244,6 +298,8 @@ func main() { flag.StringVar(&flags.app.weaveHostname, "app.weave.hostname", app.DefaultHostname, "Hostname to advertise in WeaveDNS") flag.StringVar(&flags.app.containerName, "app.container.name", app.DefaultContainerName, "Name of this container (to lookup container ID)") flag.StringVar(&flags.app.dockerEndpoint, "app.docker", app.DefaultDockerEndpoint, "Location of docker endpoint (to lookup container ID)") + flag.Var(&containerLabelFilterFlags, "app.container-label-filter", "Add container label-based view filter, specified as title:label. Multiple flags are accepted. Example: --app.container-label-filter='Database Containers:role=db'") + flag.Var(&containerLabelFilterFlagsExclude, "app.container-label-filter-exclude", "Add container label-based view filter that excludes containers with the given label, specified as title:label. Multiple flags are accepted. Example: --app.container-label-filter-exclude='Database Containers:role=db'") flag.StringVar(&flags.app.collectorURL, "app.collector", "local", "Collector to use (local, dynamodb, or file)") flag.StringVar(&flags.app.s3URL, "app.collector.s3", "local", "S3 URL to use (when collector is dynamodb)") @@ -265,6 +321,8 @@ func main() { flag.Parse() + app.AddContainerFilters(append(containerLabelFilterFlags.apiTopologyOptions, containerLabelFilterFlagsExclude.apiTopologyOptions...)...) + // Deal with common args if debug { flags.probe.logLevel = "debug" diff --git a/prog/main_test.go b/prog/main_test.go new file mode 100644 index 000000000..30a75a7e4 --- /dev/null +++ b/prog/main_test.go @@ -0,0 +1,26 @@ +package main_test + +import ( + "fmt" + "github.com/weaveworks/scope/app" + "testing" +) + +func TestMakeContainerFiltersFromFlags(t *testing.T) { + containerLabelFlags := containerLabelFiltersFlag{exclude: false} + containerLabelFlags.Set(`title1:label=1`) + containerLabelFlags.Set(`ti\:tle2:lab\:el=2`) + containerLabelFlags.Set(`ti tile3:label=3`) + err := containerLabelFlags.Set(`just a string`) + if err == nil { + t.Fatalf("Invalid container label flag not detected") + } + apiTopologyOptions := containerLabelFlags.apiTopologyOptions + equals(t, 3, len(apiTopologyOptions)) + equals(t, `title1`, apiTopologyOptions[0].Value) + equals(t, `label=1`, apiTopologyOptions[0].Label) + equals(t, `ti:tle2`, apiTopologyOptions[1].Value) + equals(t, `lab:el=2`, apiTopologyOptions[1].Label) + equals(t, `ti tle3`, apiTopologyOptions[2].Value) + equals(t, `label=3`, apiTopologyOptions[2].Label) +} diff --git a/render/filters.go b/render/filters.go index 071d68818..3838ad32e 100644 --- a/render/filters.go +++ b/render/filters.go @@ -268,6 +268,22 @@ func IsApplication(n report.Node) bool { // IsSystem checks if the node is a "system" node var IsSystem = Complement(IsApplication) +// HasLabel checks if the node has the desired docker label +func HasLabel(labelKey string, labelValue string) FilterFunc { + return func(n report.Node) bool { + value, _ := n.Latest.Lookup(docker.LabelPrefix + labelKey) + if value == labelValue { + return true + } + return false + } +} + +// DoesNotHaveLabel checks if the node does NOT have the specified docker label +func DoesNotHaveLabel(labelKey string, labelValue string) FilterFunc { + return Complement(HasLabel(labelKey, labelValue)) +} + // IsNotPseudo returns true if the node is not a pseudo node // or internet/service nodes. func IsNotPseudo(n report.Node) bool { diff --git a/test/fixture/report_fixture.go b/test/fixture/report_fixture.go index f87c5631a..de660eda5 100644 --- a/test/fixture/report_fixture.go +++ b/test/fixture/report_fixture.go @@ -79,6 +79,11 @@ var ( ClientContainerNodeID = report.MakeContainerNodeID(ClientContainerID) ServerContainerNodeID = report.MakeContainerNodeID(ServerContainerID) + TestLabelKey1 = "myrole" + ApplicationLabelValue1 = "customapplication1" + TestLabelKey2 = "myrole2" + ApplicationLabelValue2 = "customapplication2" + ClientContainerHostname = ClientContainerName + ".hostname.com" ServerContainerHostname = ServerContainerName + ".hostname.com" @@ -262,6 +267,7 @@ var ( docker.ImageID: ClientContainerImageID, report.HostNodeID: ClientHostNodeID, docker.LabelPrefix + "io.kubernetes.pod.uid": ClientPodUID, + docker.LabelPrefix + TestLabelKey1: ApplicationLabelValue1, kubernetes.Namespace: KubernetesNamespace, docker.ContainerState: docker.StateRunning, docker.ContainerStateHuman: docker.StateRunning, @@ -288,6 +294,7 @@ var ( docker.LabelPrefix + "foo1": "bar1", docker.LabelPrefix + "foo2": "bar2", docker.LabelPrefix + "io.kubernetes.pod.uid": ServerPodUID, + docker.LabelPrefix + TestLabelKey2: ApplicationLabelValue2, kubernetes.Namespace: KubernetesNamespace, }). WithTopology(report.Container).WithParents(report.EmptySets. From a5859ba2189f01f8978a94ff8f9d6b376d789af7 Mon Sep 17 00:00:00 2001 From: CarltonSemple Date: Tue, 15 Nov 2016 22:18:55 +0000 Subject: [PATCH 2/2] Addressed final comments --- app/api_topologies_test.go | 6 +++--- prog/main.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index cf075f41c..ea23b7945 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -67,7 +67,7 @@ func TestContainerLabelFilter(t *testing.T) { // only the filtered container with fixture.TestLabelKey1 should be present equals(t, 1, len(topologySummaries)) for key := range topologySummaries { - equals(t, fixture.ClientContainerID+";", key) + equals(t, report.MakeContainerNodeID(fixture.ClientContainerID), key) } } @@ -79,8 +79,8 @@ func TestContainerLabelFilterExclude(t *testing.T) { // all containers but the excluded container should be present for key := range topologySummaries { - if fixture.ServerContainerNodeID+";" == key { - t.Errorf("TestAPITopologyNegativeContainerLabelFilter Failed. Expected to not find " + fixture.ServerContainerNodeID + "; in report") + if report.MakeContainerNodeID(fixture.ServerContainerNodeID) == key { + t.Errorf("TestAPITopologyNegativeContainerLabelFilter Failed. Expected to not find " + report.MakeContainerNodeID(fixture.ServerContainerNodeID) + " in report") } } } diff --git a/prog/main.go b/prog/main.go index 16334a70d..956549cea 100644 --- a/prog/main.go +++ b/prog/main.go @@ -180,7 +180,7 @@ func (c *containerLabelFiltersFlag) toAPITopologyOption(flagValue string, filter containerFilterLabel := unescapeBackslashes.ReplaceAllString(labelStringEscaped, `$1`) labelKeyValuePair := strings.Split(containerFilterLabel, "=") if len(labelKeyValuePair) != 2 { - return app.APITopologyOption{}, fmt.Errorf("Docker label in the app.container-label-filter(-exclude) flag isn't in the correct key=value format") + return app.APITopologyOption{}, fmt.Errorf("Docker label isn't in the correct key=value format") } filterFunction := render.HasLabel