From e1b45de7f64c2ca3fc46aab270d6001256895fb7 Mon Sep 17 00:00:00 2001 From: Miles Bryant Date: Tue, 18 Jun 2019 15:20:01 +0100 Subject: [PATCH] builder/main: allow collectors not enabled by default This fixes an issue where it was impossible to specify a collector that was available but not selected by default. Instead of checking whether chosen collectors are valid at flag parse time, this moves the check into the builder, where we can reference it against the availableStores in the builder. As a bonus, the error message also prints out a list of available collectors: ``` kube-state-metrics --collectors non-existent-collector I0618 15:23:34.517532 50719 main.go:88] Using collectors non-existent-collector F0618 15:23:34.519132 50719 main.go:90] Error: collector non-existent-collector does not exist. Available collectors: persistentvolumeclaims,configmaps,limitranges,nodes,namespaces,persistentvolumes,pods,replicasets,services,cronjobs,deployments,ingresses,horizontalpodautoscalers,jobs,poddisruptionbudgets,secrets,certificatesigningrequests,daemonsets,endpoints,storageclasses,replicationcontrollers,resourcequotas,statefulsets ``` --- go.sum | 3 --- internal/store/builder.go | 23 ++++++++++++++++++++++- main.go | 9 +++++++-- pkg/options/types.go | 6 ------ pkg/options/types_test.go | 6 ------ 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/go.sum b/go.sum index b75b07cfe8..77f7a9b5aa 100644 --- a/go.sum +++ b/go.sum @@ -74,9 +74,7 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20181201002055-351d144fa1fc h1:a3CU5tJYVj92DY2LaA1kUkrsqD5/3mLDhx2NcNqyW+0= golang.org/x/net v0.0.0-20181201002055-351d144fa1fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -86,7 +84,6 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= diff --git a/internal/store/builder.go b/internal/store/builder.go index f13102dd1b..d72fd2b1c5 100644 --- a/internal/store/builder.go +++ b/internal/store/builder.go @@ -20,6 +20,7 @@ import ( "sort" "strings" + "github.com/pkg/errors" "k8s.io/klog" "golang.org/x/net/context" @@ -64,13 +65,20 @@ func NewBuilder( } // WithEnabledResources sets the enabledResources property of a Builder. -func (b *Builder) WithEnabledResources(c []string) { +func (b *Builder) WithEnabledResources(c []string) error { + for _, col := range c { + if !collectorExists(col) { + return errors.Errorf("collector %s does not exist. Available collectors: %s", col, strings.Join(availableCollectors(), ",")) + } + } + var copy []string copy = append(copy, c...) sort.Strings(copy) b.enabledResources = copy + return nil } // WithNamespaces sets the namespaces property of a Builder. @@ -138,6 +146,19 @@ var availableStores = map[string]func(f *Builder) *metricsstore.MetricsStore{ "storageclasses": func(b *Builder) *metricsstore.MetricsStore { return b.buildStorageClassStore() }, } +func collectorExists(name string) bool { + _, ok := availableStores[name] + return ok +} + +func availableCollectors() []string { + c := []string{} + for name := range availableStores { + c = append(c, name) + } + return c +} + func (b *Builder) buildConfigMapStore() *metricsstore.MetricsStore { return b.buildStore(configMapMetricFamilies, &v1.ConfigMap{}, createConfigMapListWatch) } diff --git a/main.go b/main.go index d6b1128564..f7d8cf7e6d 100644 --- a/main.go +++ b/main.go @@ -81,12 +81,17 @@ func main() { storeBuilder := store.NewBuilder(ctx) + var collectors []string if len(opts.Collectors) == 0 { klog.Info("Using default collectors") - storeBuilder.WithEnabledResources(options.DefaultCollectors.AsSlice()) + collectors = options.DefaultCollectors.AsSlice() } else { klog.Infof("Using collectors %s", opts.Collectors.String()) - storeBuilder.WithEnabledResources(opts.Collectors.AsSlice()) + collectors = opts.Collectors.AsSlice() + } + + if err := storeBuilder.WithEnabledResources(collectors); err != nil { + klog.Fatalf("Failed to set up collectors: %v", err) } if len(opts.Namespaces) == 0 { diff --git a/pkg/options/types.go b/pkg/options/types.go index 2f263827d9..42e4b823a9 100644 --- a/pkg/options/types.go +++ b/pkg/options/types.go @@ -20,8 +20,6 @@ import ( "sort" "strings" - "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -84,10 +82,6 @@ func (c *CollectorSet) Set(value string) error { for _, col := range cols { col = strings.TrimSpace(col) if len(col) != 0 { - _, ok := DefaultCollectors[col] - if !ok { - return errors.Errorf("collector \"%s\" does not exist", col) - } s[col] = struct{}{} } } diff --git a/pkg/options/types_test.go b/pkg/options/types_test.go index a4dd1605de..53b3400a49 100644 --- a/pkg/options/types_test.go +++ b/pkg/options/types_test.go @@ -45,12 +45,6 @@ func TestCollectorSetSet(t *testing.T) { }), WantedError: false, }, - { - Desc: "none exist collectors", - Value: "none-exists", - Wanted: CollectorSet{}, - WantedError: true, - }, } for _, test := range tests {