From 4729c5697a0f81f5727cbaf205f6d6d48828c7b8 Mon Sep 17 00:00:00 2001 From: Daniel Nelson Date: Thu, 12 Oct 2017 15:50:09 -0700 Subject: [PATCH] Fix container name filters in docker input (#3331) --- filter/filter.go | 37 +++ plugins/inputs/docker/docker.go | 75 ++--- plugins/inputs/docker/docker_test.go | 401 ++++++++++++++++++++------- 3 files changed, 353 insertions(+), 160 deletions(-) diff --git a/filter/filter.go b/filter/filter.go index 9a28c2627..df171257b 100644 --- a/filter/filter.go +++ b/filter/filter.go @@ -77,3 +77,40 @@ func compileFilterNoGlob(filters []string) Filter { } return &out } + +type IncludeExcludeFilter struct { + include Filter + exclude Filter +} + +func NewIncludeExcludeFilter( + include []string, + exclude []string, +) (Filter, error) { + in, err := Compile(include) + if err != nil { + return nil, err + } + + ex, err := Compile(exclude) + if err != nil { + return nil, err + } + + return &IncludeExcludeFilter{in, ex}, nil +} + +func (f *IncludeExcludeFilter) Match(s string) bool { + if f.include != nil { + if !f.include.Match(s) { + return false + } + } + + if f.exclude != nil { + if f.exclude.Match(s) { + return false + } + } + return true +} diff --git a/plugins/inputs/docker/docker.go b/plugins/inputs/docker/docker.go index 171097621..5f39fa0d9 100644 --- a/plugins/inputs/docker/docker.go +++ b/plugins/inputs/docker/docker.go @@ -22,16 +22,6 @@ import ( "github.com/influxdata/telegraf/plugins/inputs" ) -type DockerLabelFilter struct { - labelInclude filter.Filter - labelExclude filter.Filter -} - -type DockerContainerFilter struct { - containerInclude filter.Filter - containerExclude filter.Filter -} - // Docker object type Docker struct { Endpoint string @@ -45,11 +35,9 @@ type Docker struct { TagEnvironment []string `toml:"tag_env"` LabelInclude []string `toml:"docker_label_include"` LabelExclude []string `toml:"docker_label_exclude"` - LabelFilter DockerLabelFilter ContainerInclude []string `toml:"container_name_include"` ContainerExclude []string `toml:"container_name_exclude"` - ContainerFilter DockerContainerFilter SSLCA string `toml:"ssl_ca"` SSLCert string `toml:"ssl_cert"` @@ -59,10 +47,12 @@ type Docker struct { newEnvClient func() (Client, error) newClient func(string, *tls.Config) (Client, error) - client Client - httpClient *http.Client - engine_host string - filtersCreated bool + client Client + httpClient *http.Client + engine_host string + filtersCreated bool + labelFilter filter.Filter + containerFilter filter.Filter } // KB, MB, GB, TB, PB...human friendly @@ -374,12 +364,8 @@ func (d *Docker) gatherContainer( "container_version": imageVersion, } - if len(d.ContainerInclude) > 0 || len(d.ContainerExclude) > 0 { - if len(d.ContainerInclude) == 0 || !d.ContainerFilter.containerInclude.Match(cname) { - if len(d.ContainerExclude) == 0 || d.ContainerFilter.containerExclude.Match(cname) { - return nil - } - } + if !d.containerFilter.Match(cname) { + return nil } ctx, cancel := context.WithTimeout(context.Background(), d.Timeout.Duration) @@ -400,10 +386,8 @@ func (d *Docker) gatherContainer( // Add labels to tags for k, label := range container.Labels { - if len(d.LabelInclude) == 0 || d.LabelFilter.labelInclude.Match(k) { - if len(d.LabelExclude) == 0 || !d.LabelFilter.labelExclude.Match(k) { - tags[k] = label - } + if d.labelFilter.Match(k) { + tags[k] = label } } @@ -749,46 +733,25 @@ func parseSize(sizeStr string) (int64, error) { } func (d *Docker) createContainerFilters() error { + // Backwards compatibility for deprecated `container_names` parameter. if len(d.ContainerNames) > 0 { d.ContainerInclude = append(d.ContainerInclude, d.ContainerNames...) } - if len(d.ContainerInclude) != 0 { - var err error - d.ContainerFilter.containerInclude, err = filter.Compile(d.ContainerInclude) - if err != nil { - return err - } + filter, err := filter.NewIncludeExcludeFilter(d.ContainerInclude, d.ContainerExclude) + if err != nil { + return err } - - if len(d.ContainerExclude) != 0 { - var err error - d.ContainerFilter.containerExclude, err = filter.Compile(d.ContainerExclude) - if err != nil { - return err - } - } - + d.containerFilter = filter return nil } func (d *Docker) createLabelFilters() error { - if len(d.LabelInclude) != 0 { - var err error - d.LabelFilter.labelInclude, err = filter.Compile(d.LabelInclude) - if err != nil { - return err - } + filter, err := filter.NewIncludeExcludeFilter(d.LabelInclude, d.LabelExclude) + if err != nil { + return err } - - if len(d.LabelExclude) != 0 { - var err error - d.LabelFilter.labelExclude, err = filter.Compile(d.LabelExclude) - if err != nil { - return err - } - } - + d.labelFilter = filter return nil } diff --git a/plugins/inputs/docker/docker_test.go b/plugins/inputs/docker/docker_test.go index b18274136..088c94597 100644 --- a/plugins/inputs/docker/docker_test.go +++ b/plugins/inputs/docker/docker_test.go @@ -69,30 +69,32 @@ func (c *MockClient) NodeList( return c.NodeListF(ctx, options) } +var baseClient = MockClient{ + InfoF: func(context.Context) (types.Info, error) { + return info, nil + }, + ContainerListF: func(context.Context, types.ContainerListOptions) ([]types.Container, error) { + return containerList, nil + }, + ContainerStatsF: func(context.Context, string, bool) (types.ContainerStats, error) { + return containerStats(), nil + }, + ContainerInspectF: func(context.Context, string) (types.ContainerJSON, error) { + return containerInspect, nil + }, + ServiceListF: func(context.Context, types.ServiceListOptions) ([]swarm.Service, error) { + return ServiceList, nil + }, + TaskListF: func(context.Context, types.TaskListOptions) ([]swarm.Task, error) { + return TaskList, nil + }, + NodeListF: func(context.Context, types.NodeListOptions) ([]swarm.Node, error) { + return NodeList, nil + }, +} + func newClient(host string, tlsConfig *tls.Config) (Client, error) { - return &MockClient{ - InfoF: func(context.Context) (types.Info, error) { - return info, nil - }, - ContainerListF: func(context.Context, types.ContainerListOptions) ([]types.Container, error) { - return containerList, nil - }, - ContainerStatsF: func(context.Context, string, bool) (types.ContainerStats, error) { - return containerStats(), nil - }, - ContainerInspectF: func(context.Context, string) (types.ContainerJSON, error) { - return containerInspect, nil - }, - ServiceListF: func(context.Context, types.ServiceListOptions) ([]swarm.Service, error) { - return ServiceList, nil - }, - TaskListF: func(context.Context, types.TaskListOptions) ([]swarm.Task, error) { - return TaskList, nil - }, - NodeListF: func(context.Context, types.NodeListOptions) ([]swarm.Node, error) { - return NodeList, nil - }, - }, nil + return &baseClient, nil } func TestDockerGatherContainerStats(t *testing.T) { @@ -277,82 +279,291 @@ func TestDocker_WindowsMemoryContainerStats(t *testing.T) { require.NoError(t, err) } -func TestDockerGatherLabels(t *testing.T) { - var gatherLabelsTests = []struct { - include []string - exclude []string - expected []string - notexpected []string +func TestContainerLabels(t *testing.T) { + var tests = []struct { + name string + container types.Container + include []string + exclude []string + expected map[string]string }{ - {[]string{}, []string{}, []string{"label1", "label2"}, []string{}}, - {[]string{"*"}, []string{}, []string{"label1", "label2"}, []string{}}, - {[]string{"lab*"}, []string{}, []string{"label1", "label2"}, []string{}}, - {[]string{"label1"}, []string{}, []string{"label1"}, []string{"label2"}}, - {[]string{"label1*"}, []string{}, []string{"label1"}, []string{"label2"}}, - {[]string{}, []string{"*"}, []string{}, []string{"label1", "label2"}}, - {[]string{}, []string{"lab*"}, []string{}, []string{"label1", "label2"}}, - {[]string{}, []string{"label1"}, []string{"label2"}, []string{"label1"}}, - {[]string{"*"}, []string{"*"}, []string{}, []string{"label1", "label2"}}, + { + name: "Nil filters matches all", + container: types.Container{ + Labels: map[string]string{ + "a": "x", + }, + }, + include: nil, + exclude: nil, + expected: map[string]string{ + "a": "x", + }, + }, + { + name: "Empty filters matches all", + container: types.Container{ + Labels: map[string]string{ + "a": "x", + }, + }, + include: []string{}, + exclude: []string{}, + expected: map[string]string{ + "a": "x", + }, + }, + { + name: "Must match include", + container: types.Container{ + Labels: map[string]string{ + "a": "x", + "b": "y", + }, + }, + include: []string{"a"}, + exclude: []string{}, + expected: map[string]string{ + "a": "x", + }, + }, + { + name: "Must not match exclude", + container: types.Container{ + Labels: map[string]string{ + "a": "x", + "b": "y", + }, + }, + include: []string{}, + exclude: []string{"b"}, + expected: map[string]string{ + "a": "x", + }, + }, + { + name: "Include Glob", + container: types.Container{ + Labels: map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }, + }, + include: []string{"a*"}, + exclude: []string{}, + expected: map[string]string{ + "aa": "x", + "ab": "y", + }, + }, + { + name: "Exclude Glob", + container: types.Container{ + Labels: map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }, + }, + include: []string{}, + exclude: []string{"a*"}, + expected: map[string]string{ + "bb": "z", + }, + }, + { + name: "Excluded Includes", + container: types.Container{ + Labels: map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }, + }, + include: []string{"a*"}, + exclude: []string{"*b"}, + expected: map[string]string{ + "aa": "x", + }, + }, } - - for _, tt := range gatherLabelsTests { - t.Run("", func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { var acc testutil.Accumulator - d := Docker{ - newClient: newClient, + + newClientFunc := func(host string, tlsConfig *tls.Config) (Client, error) { + client := baseClient + client.ContainerListF = func(context.Context, types.ContainerListOptions) ([]types.Container, error) { + return []types.Container{tt.container}, nil + } + return &client, nil } - for _, label := range tt.include { - d.LabelInclude = append(d.LabelInclude, label) - } - for _, label := range tt.exclude { - d.LabelExclude = append(d.LabelExclude, label) + d := Docker{ + newClient: newClientFunc, + LabelInclude: tt.include, + LabelExclude: tt.exclude, } err := d.Gather(&acc) require.NoError(t, err) - for _, label := range tt.expected { - if !acc.HasTag("docker_container_cpu", label) { - t.Errorf("Didn't get expected label of %s. Test was: Include: %s Exclude %s", - label, tt.include, tt.exclude) + // Grab tags from a container metric + var actual map[string]string + for _, metric := range acc.Metrics { + if metric.Measurement == "docker_container_cpu" { + actual = metric.Tags } } - for _, label := range tt.notexpected { - if acc.HasTag("docker_container_cpu", label) { - t.Errorf("Got unexpected label of %s. Test was: Include: %s Exclude %s", - label, tt.include, tt.exclude) - } + for k, v := range tt.expected { + require.Equal(t, v, actual[k]) } }) } } func TestContainerNames(t *testing.T) { - var gatherContainerNames = []struct { - include []string - exclude []string - expected []string - notexpected []string + var tests = []struct { + name string + containers [][]string + include []string + exclude []string + expected []string }{ - {[]string{}, []string{}, []string{"etcd", "etcd2"}, []string{}}, - {[]string{"*"}, []string{}, []string{"etcd", "etcd2"}, []string{}}, - {[]string{"etc*"}, []string{}, []string{"etcd", "etcd2"}, []string{}}, - {[]string{"etcd"}, []string{}, []string{"etcd"}, []string{"etcd2"}}, - {[]string{"etcd2*"}, []string{}, []string{"etcd2"}, []string{"etcd"}}, - {[]string{}, []string{"etc*"}, []string{}, []string{"etcd", "etcd2"}}, - {[]string{}, []string{"etcd"}, []string{"etcd2"}, []string{"etcd"}}, - {[]string{"*"}, []string{"*"}, []string{"etcd", "etcd2"}, []string{}}, - {[]string{}, []string{"*"}, []string{""}, []string{"etcd", "etcd2"}}, + { + name: "Nil filters matches all", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: nil, + exclude: nil, + expected: []string{"etcd", "etcd2"}, + }, + { + name: "Empty filters matches all", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{}, + exclude: []string{}, + expected: []string{"etcd", "etcd2"}, + }, + { + name: "Match all containers", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{"*"}, + exclude: []string{}, + expected: []string{"etcd", "etcd2"}, + }, + { + name: "Include prefix match", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{"etc*"}, + exclude: []string{}, + expected: []string{"etcd", "etcd2"}, + }, + { + name: "Exact match", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{"etcd"}, + exclude: []string{}, + expected: []string{"etcd"}, + }, + { + name: "Star matches zero length", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{"etcd2*"}, + exclude: []string{}, + expected: []string{"etcd2"}, + }, + { + name: "Exclude matches all", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{}, + exclude: []string{"etc*"}, + expected: []string{}, + }, + { + name: "Exclude single", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{}, + exclude: []string{"etcd"}, + expected: []string{"etcd2"}, + }, + { + name: "Exclude all", + containers: [][]string{ + {"/etcd"}, + {"/etcd2"}, + }, + include: []string{"*"}, + exclude: []string{"*"}, + expected: []string{}, + }, + { + name: "Exclude item matching include", + containers: [][]string{ + {"acme"}, + {"foo"}, + {"acme-test"}, + }, + include: []string{"acme*"}, + exclude: []string{"*test*"}, + expected: []string{"acme"}, + }, + { + name: "Exclude item no wildcards", + containers: [][]string{ + {"acme"}, + {"acme-test"}, + }, + include: []string{"acme*"}, + exclude: []string{"test"}, + expected: []string{"acme", "acme-test"}, + }, } - - for _, tt := range gatherContainerNames { - t.Run("", func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { var acc testutil.Accumulator + newClientFunc := func(host string, tlsConfig *tls.Config) (Client, error) { + client := baseClient + client.ContainerListF = func(context.Context, types.ContainerListOptions) ([]types.Container, error) { + var containers []types.Container + for _, names := range tt.containers { + containers = append(containers, types.Container{ + Names: names, + }) + } + return containers, nil + } + return &client, nil + } + d := Docker{ - newClient: newClient, + newClient: newClientFunc, ContainerInclude: tt.include, ContainerExclude: tt.exclude, } @@ -360,39 +571,21 @@ func TestContainerNames(t *testing.T) { err := d.Gather(&acc) require.NoError(t, err) + // Set of expected names + var expected = make(map[string]bool) + for _, v := range tt.expected { + expected[v] = true + } + + // Set of actual names + var actual = make(map[string]bool) for _, metric := range acc.Metrics { - if metric.Measurement == "docker_container_cpu" { - if val, ok := metric.Tags["container_name"]; ok { - var found bool = false - for _, cname := range tt.expected { - if val == cname { - found = true - break - } - } - if !found { - t.Errorf("Got unexpected container of %s. Test was -> Include: %s, Exclude: %s", val, tt.include, tt.exclude) - } - } + if name, ok := metric.Tags["container_name"]; ok { + actual[name] = true } } - for _, metric := range acc.Metrics { - if metric.Measurement == "docker_container_cpu" { - if val, ok := metric.Tags["container_name"]; ok { - var found bool = false - for _, cname := range tt.notexpected { - if val == cname { - found = true - break - } - } - if found { - t.Errorf("Got unexpected container of %s. Test was -> Include: %s, Exclude: %s", val, tt.include, tt.exclude) - } - } - } - } + require.Equal(t, expected, actual) }) } }