Skip to content

Commit

Permalink
Refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
CatherineF-dev committed Nov 4, 2022
1 parent 813496d commit 43c6073
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 33 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ The downside of using an auto-sharded setup comes from the rollout strategy supp
### Daemonset sharding for pod metrics

For pod metrics, they can be sharded per node with the following flag:
* `--nodename`
* `--node`

Each kube-state-metrics pod uses FieldSelector (spec.nodeName) to watch/list pod metrics only on the same node.

Expand All @@ -255,7 +255,7 @@ spec:
name: kube-state-metrics
args:
- --resource=pods
- --nodename=$(NODE_NAME)
- --node=$(NODE_NAME)
env:
- name: NODE_NAME
valueFrom:
Expand Down
2 changes: 1 addition & 1 deletion docs/cli-arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
--node string Name of the node that contains the kube-state-metrics pod. Most likely it should be passed via the downward API. This is used for daemonset sharding. Only available for resources (pod metrics) that support spec.nodeName fieldSelector. This is experimental.
--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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ 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.MergeFieldSelectors([]string{nsFieldSelector, nodeNameFieldSelector})
nodeFieldSelector := opts.Node.GetNodeFieldSelector()
merged, err := storeBuilder.MergeFieldSelectors([]string{nsFieldSelector, nodeFieldSelector})
if err != nil {
return err
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type Options struct {
Resources ResourceSet
Namespaces NamespaceList
NamespacesDenylist NamespaceList
NodeName NodeNameType
Node NodeType
Shard int32
TotalShards int
Pod string
Expand Down Expand Up @@ -104,7 +104,6 @@ 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", "", "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")
Expand All @@ -113,6 +112,8 @@ func (o *Options) AddFlags() {
o.flags.Int32Var(&o.Shard, "shard", int32(0), "The instances shard nominal (zero indexed) within the total number of shards. (default 0)")
o.flags.IntVar(&o.TotalShards, "total-shards", 1, "The total number of shards. Sharding is disabled when total shards is set to 1.")

o.flags.StringVar((*string)(&o.Node), "node", "", "Name of the node that contains the kube-state-metrics pod. Most likely it should be passed via the downward API. This is used for daemonset sharding. Only available for resources (pod metrics) that support spec.nodeName fieldSelector. This is experimental.")

autoshardingNotice := "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."

o.flags.StringVar(&o.Pod, "pod", "", "Name of the pod that contains the kube-state-metrics container. "+autoshardingNotice)
Expand All @@ -138,12 +139,12 @@ func (o *Options) Usage() {
// Validate validates arguments
func (o *Options) Validate() error {
shardableResource := "pods"
if o.NodeName == "" {
if o.Node == "" {
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 fmt.Errorf("Resource %s can't be sharded by field selector spec.nodeName", x)
}
}
return nil
Expand Down
8 changes: 4 additions & 4 deletions pkg/options/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ func (r *ResourceSet) Type() string {
return "string"
}

// NodeNameType represents a nodeName to query from.
type NodeNameType string
// NodeType represents a nodeName to query from.
type NodeType string

// GetNodeNameFieldSelector returns a nodename field selector.
func (n *NodeNameType) GetNodeNameFieldSelector() string {
// GetNodeFieldSelector returns a nodename field selector.
func (n *NodeType) GetNodeFieldSelector() string {
if string(*n) != "" {
return fields.OneTermEqualSelector("spec.nodeName", string(*n)).String()
}
Expand Down
40 changes: 20 additions & 20 deletions pkg/options/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,27 @@ func TestNamespaceList_ExcludeNamespacesFieldSelector(t *testing.T) {
}
}

func TestNodeNameFieldSelector(t *testing.T) {
func TestNodeFieldSelector(t *testing.T) {
tests := []struct {
Desc string
NodeName NodeNameType
Wanted string
Desc string
Node NodeType
Wanted string
}{
{
Desc: "empty node name",
NodeName: "",
Wanted: "",
Desc: "empty node name",
Node: "",
Wanted: "",
},
{
Desc: "with node name",
NodeName: "k8s-node-1",
Wanted: "spec.nodeName=k8s-node-1",
Desc: "with node name",
Node: "k8s-node-1",
Wanted: "spec.nodeName=k8s-node-1",
},
}

for _, test := range tests {
node := test.NodeName
actual := node.GetNodeNameFieldSelector()
node := test.Node
actual := node.GetNodeFieldSelector()
if !reflect.DeepEqual(actual, test.Wanted) {
t.Errorf("Test error for Desc: %s. Want: %+v. Got: %+v.", test.Desc, test.Wanted, actual)
}
Expand All @@ -187,49 +187,49 @@ func TestMergeFieldSelectors(t *testing.T) {
Desc string
Namespaces NamespaceList
DeniedNamespaces NamespaceList
NodeName NodeNameType
Node NodeType
Wanted string
}{
{
Desc: "empty DeniedNamespaces",
Namespaces: NamespaceList{"default", "kube-system"},
DeniedNamespaces: NamespaceList{},
NodeName: "",
Node: "",
Wanted: "",
},
{
Desc: "all DeniedNamespaces",
Namespaces: DefaultNamespaces,
DeniedNamespaces: NamespaceList{"some-system"},
NodeName: "",
Node: "",
Wanted: "metadata.namespace!=some-system",
},
{
Desc: "general case",
Namespaces: DefaultNamespaces,
DeniedNamespaces: NamespaceList{"case1-system", "case2-system"},
NodeName: "",
Node: "",
Wanted: "metadata.namespace!=case1-system,metadata.namespace!=case2-system",
},
{
Desc: "empty DeniedNamespaces",
Namespaces: NamespaceList{"default", "kube-system"},
DeniedNamespaces: NamespaceList{},
NodeName: "k8s-node-1",
Node: "k8s-node-1",
Wanted: "spec.nodeName=k8s-node-1",
},
{
Desc: "all DeniedNamespaces",
Namespaces: DefaultNamespaces,
DeniedNamespaces: NamespaceList{"some-system"},
NodeName: "k8s-node-1",
Node: "k8s-node-1",
Wanted: "metadata.namespace!=some-system,spec.nodeName=k8s-node-1",
},
{
Desc: "general case",
Namespaces: DefaultNamespaces,
DeniedNamespaces: NamespaceList{"case1-system", "case2-system"},
NodeName: "k8s-node-1",
Node: "k8s-node-1",
Wanted: "metadata.namespace!=case1-system,metadata.namespace!=case2-system,spec.nodeName=k8s-node-1",
},
}
Expand All @@ -238,7 +238,7 @@ func TestMergeFieldSelectors(t *testing.T) {
ns := test.Namespaces
deniedNS := test.DeniedNamespaces
selector1 := ns.GetExcludeNSFieldSelector(deniedNS)
selector2 := test.NodeName.GetNodeNameFieldSelector()
selector2 := test.Node.GetNodeFieldSelector()
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)
Expand Down

0 comments on commit 43c6073

Please sign in to comment.