From f9d29ba76146108356ba707a28331e062437f818 Mon Sep 17 00:00:00 2001 From: Catherine Fang Date: Thu, 27 Oct 2022 11:35:22 -0400 Subject: [PATCH] Refactoring --- docs/cli-arguments.md | 2 +- internal/store/builder.go | 31 ++++++++++++------------------- main.go | 15 +-------------- pkg/app/server.go | 2 +- pkg/options/options.go | 16 +++++++++++++++- pkg/options/types.go | 17 +++++++++++++++-- pkg/options/types_test.go | 4 ++-- 7 files changed, 47 insertions(+), 40 deletions(-) diff --git a/docs/cli-arguments.md b/docs/cli-arguments.md index adbe0954..ab4a54d1 100644 --- a/docs/cli-arguments.md +++ b/docs/cli-arguments.md @@ -46,7 +46,7 @@ Usage of ./kube-state-metrics: --metric-opt-in-list string Comma-separated list of metrics which are opt-in and not enabled by default. This is in addition to the metric allow- and denylists --namespaces string Comma-separated list of namespaces to be enabled. Defaults to "" --namespaces-denylist string Comma-separated list of namespaces not to be enabled. If namespaces and namespaces-denylist are both set, only namespaces that are excluded in namespaces-denylist will be used. - --nodename string Set spec.nodeName=nodename when watching resources. Only available for resources which support nodeName filter. + --nodename string Name of the node that contains the kube-state-metrics pod, only available for resources (pod metrics) that support spec.nodeName fieldSelector. Each kube-state-metrics pod will only exposes metrics related to this node. --one_output If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true) --pod string Name of the pod that contains the kube-state-metrics container. When set, it is expected that --pod and --pod-namespace are both set. Most likely this should be passed via the downward API. This is used for auto-detecting sharding. If set, this has preference over statically configured sharding. This is experimental, it may be removed without notice. --pod-namespace string Name of the namespace of the pod specified by --pod. When set, it is expected that --pod and --pod-namespace are both set. Most likely this should be passed via the downward API. This is used for auto-detecting sharding. If set, this has preference over statically configured sharding. This is experimental, it may be removed without notice. diff --git a/internal/store/builder.go b/internal/store/builder.go index 034c8208..8dfb8534 100644 --- a/internal/store/builder.go +++ b/internal/store/builder.go @@ -58,10 +58,11 @@ var _ ksmtypes.BuilderInterface = &Builder{} // Builder helps to build store. It follows the builder pattern // (https://en.wikipedia.org/wiki/Builder_pattern). type Builder struct { - kubeClient clientset.Interface - customResourceClients map[string]interface{} - vpaClient vpaclientset.Interface - namespaces options.NamespaceList + kubeClient clientset.Interface + customResourceClients map[string]interface{} + vpaClient vpaclientset.Interface + namespaces options.NamespaceList + // namespaceFilter is inside fieldSelectorFilter fieldSelectorFilter string ctx context.Context enabledResources []string @@ -116,17 +117,9 @@ func (b *Builder) WithNamespaces(n options.NamespaceList) { b.namespaces = n } -// MergeFieldSelector merges multiple fieldSelectors using AND operator. -func (b *Builder) MergeFieldSelector(selectors []string) (string, error) { - var err error - merged := options.EmptyFieldSelector() - for _, s := range selectors { - merged, err = options.MergeFieldSelector(merged, s) - if err != nil { - return "", err - } - } - return merged, nil +// MergeFieldSelectors merges multiple fieldSelectors using AND operator. +func (b *Builder) MergeFieldSelectors(selectors []string) (string, error) { + return options.MergeFieldSelectors(selectors) } // WithSharding sets the shard and totalShards property of a Builder. @@ -485,7 +478,7 @@ func (b *Builder) buildStores( composedMetricGenFuncs, ) if b.fieldSelectorFilter != "" { - klog.Infof("FieldSelector is used ", b.fieldSelectorFilter) + klog.Infof("FieldSelector is used %s", b.fieldSelectorFilter) } listWatcher := listWatchFunc(b.kubeClient, v1.NamespaceAll, b.fieldSelectorFilter) b.startReflector(expectedType, store, listWatcher, useAPIServerCache) @@ -499,7 +492,7 @@ func (b *Builder) buildStores( composedMetricGenFuncs, ) if b.fieldSelectorFilter != "" { - klog.Infof("FieldSelector is used ", b.fieldSelectorFilter) + klog.Infof("FieldSelector is used %s", b.fieldSelectorFilter) } listWatcher := listWatchFunc(b.kubeClient, ns, b.fieldSelectorFilter) b.startReflector(expectedType, store, listWatcher, useAPIServerCache) @@ -532,7 +525,7 @@ func (b *Builder) buildCustomResourceStores(resourceName string, composedMetricGenFuncs, ) if b.fieldSelectorFilter != "" { - klog.Infof("FieldSelector is used ", b.fieldSelectorFilter) + klog.Infof("FieldSelector is used %s", b.fieldSelectorFilter) } listWatcher := listWatchFunc(customResourceClient, v1.NamespaceAll, b.fieldSelectorFilter) b.startReflector(expectedType, store, listWatcher, useAPIServerCache) @@ -545,7 +538,7 @@ func (b *Builder) buildCustomResourceStores(resourceName string, familyHeaders, composedMetricGenFuncs, ) - klog.Infof("FieldSelector is used ", b.fieldSelectorFilter) + klog.Infof("FieldSelector is used %s", b.fieldSelectorFilter) listWatcher := listWatchFunc(customResourceClient, ns, b.fieldSelectorFilter) b.startReflector(expectedType, store, listWatcher, useAPIServerCache) stores = append(stores, store) diff --git a/main.go b/main.go index 4afdd2c6..72c8f090 100644 --- a/main.go +++ b/main.go @@ -34,19 +34,6 @@ import ( "k8s.io/kube-state-metrics/v2/pkg/options" ) -func validate(opts *options.Options) error { - shardableResource := "pods" - if opts.NodeName == "" { - return nil - } - for _, x := range opts.Resources.AsSlice() { - if x != shardableResource { - return fmt.Errorf("Resource %s can't be sharding by field selector nodeName", x) - } - } - return nil -} - func main() { opts := options.NewOptions() opts.AddFlags() @@ -66,7 +53,7 @@ func main() { os.Exit(0) } - if err := validate(opts); err != nil { + if err := opts.Validate(); err != nil { klog.ErrorS(err, "Validating options error") klog.FlushAndExit(klog.ExitFlushTimeout, 1) } diff --git a/pkg/app/server.go b/pkg/app/server.go index d9b2d8a6..dbdf8354 100644 --- a/pkg/app/server.go +++ b/pkg/app/server.go @@ -107,7 +107,7 @@ func RunKubeStateMetrics(ctx context.Context, opts *options.Options, factories . namespaces := opts.Namespaces.GetNamespaces() nsFieldSelector := namespaces.GetExcludeNSFieldSelector(opts.NamespacesDenylist) nodeNameFieldSelector := opts.NodeName.GetNodeNameFieldSelector() - merged, err := storeBuilder.MergeFieldSelector([]string{nsFieldSelector, nodeNameFieldSelector}) + merged, err := storeBuilder.MergeFieldSelectors([]string{nsFieldSelector, nodeNameFieldSelector}) if err != nil { return err } diff --git a/pkg/options/options.go b/pkg/options/options.go index 845c5762..bdcede80 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -104,7 +104,7 @@ func (o *Options) AddFlags() { o.flags.Var(&o.Resources, "resources", fmt.Sprintf("Comma-separated list of Resources to be enabled. Defaults to %q", &DefaultResources)) o.flags.Var(&o.Namespaces, "namespaces", fmt.Sprintf("Comma-separated list of namespaces to be enabled. Defaults to %q", &DefaultNamespaces)) o.flags.Var(&o.NamespacesDenylist, "namespaces-denylist", "Comma-separated list of namespaces not to be enabled. If namespaces and namespaces-denylist are both set, only namespaces that are excluded in namespaces-denylist will be used.") - o.flags.StringVar((*string)(&o.NodeName), "nodename", "", "Set spec.nodeName=nodename when watching resources. Only available for resources which support nodeName filter.") + o.flags.StringVar((*string)(&o.NodeName), "nodename", "", "Name of the node that contains the kube-state-metrics pod, only available for resources (pod metrics) that support spec.nodeName fieldSelector. Each kube-state-metrics pod will only exposes metrics related to this node.") o.flags.Var(&o.MetricAllowlist, "metric-allowlist", "Comma-separated list of metrics to be exposed. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.") o.flags.Var(&o.MetricDenylist, "metric-denylist", "Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.") o.flags.Var(&o.MetricOptInList, "metric-opt-in-list", "Comma-separated list of metrics which are opt-in and not enabled by default. This is in addition to the metric allow- and denylists") @@ -134,3 +134,17 @@ func (o *Options) Parse() error { func (o *Options) Usage() { o.flags.Usage() } + +// Validate validates arguments +func (o *Options) Validate() error { + shardableResource := "pods" + if o.NodeName == "" { + return nil + } + for _, x := range o.Resources.AsSlice() { + if x != shardableResource { + return fmt.Errorf("Resource %s can't be sharding by field selector spec.nodeName", x) + } + } + return nil +} diff --git a/pkg/options/types.go b/pkg/options/types.go index 92d18870..5759a774 100644 --- a/pkg/options/types.go +++ b/pkg/options/types.go @@ -120,8 +120,21 @@ func EmptyFieldSelector() string { return fields.Nothing().String() } -// MergeFieldSelector returns AND of two field selectors. -func MergeFieldSelector(s1 string, s2 string) (string, error) { +// MergeFieldSelectors returns AND of a list of field selectors. +func MergeFieldSelectors(selectors []string) (string, error) { + var err error + merged := EmptyFieldSelector() + for _, s := range selectors { + merged, err = MergeTwoFieldSelectors(merged, s) + if err != nil { + return "", err + } + } + return merged, nil +} + +// MergeTwoFieldSelectors returns AND of two field selectors. +func MergeTwoFieldSelectors(s1 string, s2 string) (string, error) { selector1, err := fields.ParseSelector(s1) if err != nil { return EmptyFieldSelector(), err diff --git a/pkg/options/types_test.go b/pkg/options/types_test.go index adaa8026..85724239 100644 --- a/pkg/options/types_test.go +++ b/pkg/options/types_test.go @@ -182,7 +182,7 @@ func TestNodeNameFieldSelector(t *testing.T) { } } -func TestMergeFieldSelector(t *testing.T) { +func TestMergeFieldSelectors(t *testing.T) { tests := []struct { Desc string Namespaces NamespaceList @@ -239,7 +239,7 @@ func TestMergeFieldSelector(t *testing.T) { deniedNS := test.DeniedNamespaces selector1 := ns.GetExcludeNSFieldSelector(deniedNS) selector2 := test.NodeName.GetNodeNameFieldSelector() - actual, err := MergeFieldSelector(selector1, selector2) + actual, err := MergeFieldSelectors([]string{selector1, selector2}) if err != nil { t.Errorf("Test error for Desc: %s. Can't merge field selector %v.", test.Desc, err) }