From af714c7a6673ef05166014be392aafcc3765cb2c Mon Sep 17 00:00:00 2001 From: Max Leonard Inden Date: Tue, 20 Nov 2018 10:14:33 +0100 Subject: [PATCH] pkg/whiteblacklist: Encapsulate white- blacklisting logic into pkg --- main.go | 30 +++--- main_test.go | 13 +++ pkg/collectors/builder.go | 52 ++++------- pkg/whiteblacklist/whiteblacklist.go | 107 ++++++++++++++++++++++ pkg/whiteblacklist/whiteblacklist_test.go | 104 +++++++++++++++++++++ 5 files changed, 260 insertions(+), 46 deletions(-) create mode 100644 pkg/whiteblacklist/whiteblacklist.go create mode 100644 pkg/whiteblacklist/whiteblacklist_test.go diff --git a/main.go b/main.go index d0e11611..60ec150b 100644 --- a/main.go +++ b/main.go @@ -40,6 +40,7 @@ import ( kcollectors "k8s.io/kube-state-metrics/pkg/collectors" "k8s.io/kube-state-metrics/pkg/options" "k8s.io/kube-state-metrics/pkg/version" + "k8s.io/kube-state-metrics/pkg/whiteblacklist" ) const ( @@ -95,21 +96,24 @@ func main() { collectorBuilder.WithNamespaces(opts.Namespaces) } - if opts.MetricWhitelist.IsEmpty() && opts.MetricBlacklist.IsEmpty() { - glog.Info("No metric whitelist or blacklist set. No filtering of metrics will be done.") + whiteBlackList, err := whiteblacklist.New(opts.MetricWhitelist, opts.MetricBlacklist) + if err != nil { + glog.Fatal(err) } - if !opts.MetricWhitelist.IsEmpty() && !opts.MetricBlacklist.IsEmpty() { - glog.Fatal("Whitelist and blacklist are both set. They are mutually exclusive, only one of them can be set.") - } - if !opts.MetricWhitelist.IsEmpty() { - glog.Infof("A metric whitelist has been configured. Only the following metrics will be exposed: %s.", opts.MetricWhitelist.String()) - collectorBuilder.WithMetricWhitelist(opts.MetricWhitelist) - } - if !opts.MetricBlacklist.IsEmpty() { - glog.Infof("A metric blacklist has been configured. The following metrics will not be exposed: %s.", opts.MetricBlacklist.String()) - collectorBuilder.WithMetricBlacklist(opts.MetricBlacklist) + + if opts.DisablePodNonGenericResourceMetrics { + whiteBlackList.Exclude([]string{ + "kube_pod_container_resource_requests_cpu_cores", + "kube_pod_container_resource_requests_memory_bytes", + "kube_pod_container_resource_limits_cpu_cores", + "kube_pod_container_resource_limits_memory_bytes", + }) } + glog.Infof("metric white- blacklisting: %v", whiteBlackList.Status()) + + collectorBuilder.WithWhiteBlackList(whiteBlackList) + proc.StartReaper() kubeClient, err := createKubeClient(opts.Apiserver, opts.Kubeconfig) @@ -127,8 +131,6 @@ func main() { collectors := collectorBuilder.Build() - // TODO: Reenable white and blacklisting - // metricsServer(metrics.FilteredGatherer(registry, opts.MetricWhitelist, opts.MetricBlacklist), opts.Host, opts.Port) serveMetrics(collectors, opts.Host, opts.Port, opts.EnableGZIPEncoding) } diff --git a/main_test.go b/main_test.go index 1ea778d4..2471505f 100644 --- a/main_test.go +++ b/main_test.go @@ -31,6 +31,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" kcollectors "k8s.io/kube-state-metrics/pkg/collectors" + "k8s.io/kube-state-metrics/pkg/whiteblacklist" ) func BenchmarkKubeStateMetrics(b *testing.B) { @@ -57,6 +58,12 @@ func BenchmarkKubeStateMetrics(b *testing.B) { builder.WithKubeClient(kubeClient) builder.WithNamespaces(options.DefaultNamespaces) + l, err := whiteblacklist.New(map[string]struct{}{}, map[string]struct{}{}) + if err != nil { + b.Fatal(err) + } + builder.WithWhiteBlackList(l) + // This test is not suitable to be compared in terms of time, as it includes // a one second wait. Use for memory allocation comparisons, profiling, ... b.Run("GenerateMetrics", func(b *testing.B) { @@ -110,6 +117,12 @@ func TestFullScrapeCycle(t *testing.T) { builder.WithKubeClient(kubeClient) builder.WithNamespaces(options.DefaultNamespaces) + l, err := whiteblacklist.New(map[string]struct{}{}, map[string]struct{}{}) + if err != nil { + t.Fatal(err) + } + builder.WithWhiteBlackList(l) + collectors := builder.Build() // Wait for caches to fill diff --git a/pkg/collectors/builder.go b/pkg/collectors/builder.go index a921ebb3..54a62e50 100644 --- a/pkg/collectors/builder.go +++ b/pkg/collectors/builder.go @@ -37,16 +37,21 @@ import ( "k8s.io/kube-state-metrics/pkg/options" ) +type whiteBlackLister interface { + IsIncluded(string) bool + IsExcluded(string) bool +} + // Builder helps to build collectors. It follows the builder pattern // (https://en.wikipedia.org/wiki/Builder_pattern). type Builder struct { - kubeClient clientset.Interface - namespaces options.NamespaceList + kubeClient clientset.Interface + namespaces options.NamespaceList + // TODO: Are opts still needed anywhere? opts *options.Options ctx context.Context enabledCollectors options.CollectorSet - metricWhitelist map[string]struct{} - metricBlacklist map[string]struct{} + whiteBlackList whiteBlackLister } // NewBuilder returns a new builder. @@ -75,20 +80,17 @@ func (b *Builder) WithKubeClient(c clientset.Interface) { b.kubeClient = c } -// WithMetricWhitelist configures the whitelisted metrics to be exposed by the -// collectors build by the Builder -func (b *Builder) WithMetricWhitelist(l map[string]struct{}) { - b.metricWhitelist = l -} - -// WithMetricBlacklist configures the blacklisted metrics to be exposed by the -// collectors build by the Builder -func (b *Builder) WithMetricBlacklist(l map[string]struct{}) { - b.metricBlacklist = l +// WithWhiteBlackList configures the white or blacklisted metrics to be exposed +// by the collectors build by the Builder +func (b *Builder) WithWhiteBlackList(l whiteBlackLister) { + b.whiteBlackList = l } // Build initializes and registers all enabled collectors. func (b *Builder) Build() []*Collector { + if b.whiteBlackList == nil { + panic("whiteBlackList should not be nil") + } collectors := []*Collector{} activeCollectorNames := []string{} @@ -188,7 +190,7 @@ var availableCollectors = map[string]func(f *Builder) *Collector{ // return NewCollector(store) // } func (b *Builder) buildServiceCollector() *Collector { - filteredMetricFamilies := filterMetricFamilies(b.metricWhitelist, b.metricBlacklist, serviceMetricFamilies) + filteredMetricFamilies := filterMetricFamilies(b.whiteBlackList, serviceMetricFamilies) composedMetricGenFuncs := composeMetricGenFuncs(filteredMetricFamilies) helpTexts := extractHelpText(filteredMetricFamilies) @@ -203,7 +205,7 @@ func (b *Builder) buildServiceCollector() *Collector { } func (b *Builder) buildPodCollector() *Collector { - filteredMetricFamilies := filterMetricFamilies(b.metricWhitelist, b.metricBlacklist, podMetricFamilies) + filteredMetricFamilies := filterMetricFamilies(b.whiteBlackList, podMetricFamilies) composedMetricGenFuncs := composeMetricGenFuncs(filteredMetricFamilies) helpTexts := extractHelpText(filteredMetricFamilies) @@ -248,25 +250,11 @@ func composeMetricGenFuncs(families []metrics.FamilyGenerator) func(obj interfac // filterMetricFamilies takes a white- and a blacklist and a slice of metric // families and returns a filtered slice. -func filterMetricFamilies(white, black map[string]struct{}, families []metrics.FamilyGenerator) []metrics.FamilyGenerator { - if len(white) != 0 && len(black) != 0 { - panic("Whitelist and blacklist are both set. They are mutually exclusive, only one of them can be set.") - } - +func filterMetricFamilies(l whiteBlackLister, families []metrics.FamilyGenerator) []metrics.FamilyGenerator { filtered := []metrics.FamilyGenerator{} - if len(white) != 0 { - for _, f := range families { - if _, whitelisted := white[f.Name]; whitelisted { - filtered = append(filtered, f) - } - } - - return filtered - } - for _, f := range families { - if _, blacklisted := black[f.Name]; !blacklisted { + if l.IsIncluded(f.Name) { filtered = append(filtered, f) } } diff --git a/pkg/whiteblacklist/whiteblacklist.go b/pkg/whiteblacklist/whiteblacklist.go new file mode 100644 index 00000000..8509db62 --- /dev/null +++ b/pkg/whiteblacklist/whiteblacklist.go @@ -0,0 +1,107 @@ +package whiteblacklist + +import ( + "errors" + "strings" +) + +// WhiteBlackList encapsulates the logic needed to filter based on a string. +type WhiteBlackList struct { + list map[string]struct{} + isWhiteList bool +} + +// New constructs a new WhtieBlackList based on a white- and a +// blacklist. Only one of them can be not empty. +func New(w, b map[string]struct{}) (*WhiteBlackList, error) { + if len(w) != 0 && len(b) != 0 { + return nil, errors.New( + "whitelist and blacklist are both set, they are mutually exclusive, only one of them can be set", + ) + } + + white := copyList(w) + black := copyList(b) + + var list map[string]struct{} + var isWhiteList bool + + // Default to blacklisting + if len(white) != 0 { + list = white + isWhiteList = true + } else { + list = black + isWhiteList = false + } + + return &WhiteBlackList{ + list: list, + isWhiteList: isWhiteList, + }, nil +} + +// Include includes the given items in the list. +func (l *WhiteBlackList) Include(items []string) { + if l.isWhiteList { + for _, item := range items { + l.list[item] = struct{}{} + } + } else { + for _, item := range items { + delete(l.list, item) + } + } +} + +// Exclude excludes the given items from the list. +func (l *WhiteBlackList) Exclude(items []string) { + if l.isWhiteList { + for _, item := range items { + delete(l.list, item) + } + } else { + for _, item := range items { + l.list[item] = struct{}{} + } + } +} + +// IsIncluded returns if the given item is included. +func (l *WhiteBlackList) IsIncluded(item string) bool { + _, exists := l.list[item] + + if l.isWhiteList { + return exists + } + + return !exists +} + +// IsExcluded returns if the given item is excluded. +func (l *WhiteBlackList) IsExcluded(item string) bool { + return !l.IsIncluded(item) +} + +// Status returns the status of the WhtieBlackList that can e.g. be passed into +// a logger. +func (l *WhiteBlackList) Status() string { + items := []string{} + for key := range l.list { + items = append(items, key) + } + + if l.isWhiteList { + return "whitelisting the following items: " + strings.Join(items, ", ") + } + + return "blacklisting the following items: " + strings.Join(items, ", ") +} + +func copyList(l map[string]struct{}) map[string]struct{} { + newList := map[string]struct{}{} + for k, v := range l { + newList[k] = v + } + return newList +} diff --git a/pkg/whiteblacklist/whiteblacklist_test.go b/pkg/whiteblacklist/whiteblacklist_test.go new file mode 100644 index 00000000..dfd866f1 --- /dev/null +++ b/pkg/whiteblacklist/whiteblacklist_test.go @@ -0,0 +1,104 @@ +package whiteblacklist + +import ( + "testing" +) + +func TestNew(t *testing.T) { + t.Run("fails with two non empty maps", func(t *testing.T) { + _, err := New(map[string]struct{}{"not-empty": struct{}{}}, map[string]struct{}{"not-empty": struct{}{}}) + if err == nil { + t.Fatal("expected New() to fail with two non-empty maps") + } + }) + + t.Run("defaults to blacklisting", func(t *testing.T) { + l, err := New(map[string]struct{}{}, map[string]struct{}{}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + if l.isWhiteList { + t.Fatal("expected whiteBlackList to default to blacklist") + } + }) + + t.Run("if whitelist set, should be whitelist", func(t *testing.T) { + list, err := New(map[string]struct{}{"not-empty": struct{}{}}, map[string]struct{}{}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + if !list.isWhiteList { + t.Fatal("expected list to be whitelist") + } + }) + + t.Run("if blacklist set, should be blacklist", func(t *testing.T) { + list, err := New(map[string]struct{}{}, map[string]struct{}{"not-empty": struct{}{}}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + if list.isWhiteList { + t.Fatal("expected list to be blacklist") + } + }) +} + +func TestInclude(t *testing.T) { + t.Run("adds when whitelist", func(t *testing.T) { + whitelist, err := New(map[string]struct{}{"not-empty": struct{}{}}, map[string]struct{}{}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + whitelist.Include([]string{"item1"}) + + if !whitelist.IsIncluded("item1") { + t.Fatal("expected included item to be included") + } + }) + t.Run("removes when blacklist", func(t *testing.T) { + item1 := "item1" + blacklist, err := New(map[string]struct{}{}, map[string]struct{}{item1: struct{}{}}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + blacklist.Include([]string{item1}) + + if !blacklist.IsIncluded(item1) { + t.Fatal("expected included item to be included") + } + }) +} + +func TestExclude(t *testing.T) { + t.Run("removes when whitelist", func(t *testing.T) { + item1 := "item1" + whitelist, err := New(map[string]struct{}{item1: struct{}{}}, map[string]struct{}{}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + whitelist.Exclude([]string{item1}) + + if whitelist.IsIncluded(item1) { + t.Fatal("expected excluded item to be excluded") + } + }) + t.Run("removes when blacklist", func(t *testing.T) { + item1 := "item1" + blacklist, err := New(map[string]struct{}{}, map[string]struct{}{"not-empty": struct{}{}}) + if err != nil { + t.Fatal("expected New() to not fail") + } + + blacklist.Exclude([]string{item1}) + + if blacklist.IsIncluded(item1) { + t.Fatal("expected excluded item to be excluded") + } + }) +}