store: add kube_node_role metric

[abursavich]

What this PR does / why we need it:
This adds a kube_node_role metric for node roles based on node-role.kubernetes.io/<ROLE> node labels.

This is implemented like kube_node_spec_taint where each taint is a separate metric in the same family, as opposed to kube_node_labels where each label is mapped to the same metric and family.

I've tried to implement this with recording rules on top of kube_node_labels, but an empty node-role.kubernetes.io/<ROLE> label value is valid and, as far as i can tell, there's no way to differentiate between a missing label and an empty value in a query. Further, every possible role must be known in advance to query for the expected labels.

Which issue(s) this PR fixes
N/A

[k8s-ci-robot]

Welcome @abursavich!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[brancz]

Nominally this is not really aligned with how kube state metrics works, however I’m inclined to accept something like this as node labels are treated with stability guarantees in kubernetes (at least this label) and it is indeed difficult to do what you are trying to accomplish.

@lilic @tariq1890

[abursavich]

@brancz, I'm not sure this would be more palatable, but in the interest of exploration... another option would be to export a kube_node_label metric with stable label dimensions and higher series cardinality. This could be in addition to or in place of the kube_node_labels metric.

# BEFORE
kube_node_labels{label_foo="bar",label_node_role_kubernetes_io_master="",node="127.0.0.1"} 1

# AFTER
kube_node_label{label="foo",value="bar",node="127.0.0.1"} 1
kube_node_label{label="node-role.kubernetes.io/master",value="",node="127.0.0.1"} 1

I have actually wished it was that way for the purposes of other queries in the past, but not just for nodes... for deployments, pods, etc.

As an additional benefit, you would get the real label names without special characters replaced.

[tariq1890]

I am on the fence here on this. While I wish this was made more contractually bound via APIObject, I also don't want it to be a blocker. I'll leave it to @lilic to provide her opinions on this.

[lilic]

While I wish this was made more contractually bound via APIObject

I agree!

Asking the node team on this, to see if they have plans to change this. If it will stay this way I would say let's add it, unless there is any other place in k8s we think this metric might be better off?

something like this as node labels are treated with stability guarantees in kubernetes

Dear @kubernetes/sig-node-api-reviews can you let us know if you have any plans to change the node-role.kubernetes.io/<ROLE> in the near or far future, thank you!

[fejta-bot]

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

[abursavich]

I would like to be very explicit that this is a specific case of two generic issues which are the result of mapping dynamic object labels to dynamic metric dimensions, instead of fixing the dimensions and allowing the series cardinality to be dynamic per object.

1. It is not possible to query for objects with labels that exist but have empty values, as prometheus treats these the same as metrics without the labels.
2. It is not possible to use prometheus' value matching and parsing functionality on object label keys since they are munged into prometheus label keys.

Reiterating my previous comment, this is an alternative solution to the underlying problems without tying kube-state-metrics to any particular label conventions:

# BEFORE
kube_node_labels{label_foo="bar",label_node_role_kubernetes_io_master="",node="127.0.0.1"} 1

# AFTER
kube_node_label{label="foo",value="bar",node="127.0.0.1"} 1
kube_node_label{label="node-role.kubernetes.io/master",value="",node="127.0.0.1"} 1

One downside would be that queries on multiple labels would become more complicated joins:

# BEFORE
kube_node_labels{label_foo="bar",label_baz="qux"}

# AFTER
kube_node_label{label="foo",value="bar"} and on(node) kube_foo_label{label="baz",value="qux"}

[derekwaynecarr]

Related discussion about appropriate use of node role labels:
kubernetes/enhancements#1144

[derekwaynecarr]

The tldr is that the node role label is not going away, but its an implementation detail of the cluster topology and not something that kubernetes components should imply behavior around in favor of more intent based labels (exclude-disruption, etc.). That said, the role of the node should be stable for the life of the node.

[brancz]

Got it, thanks @derekwaynecarr. I think we're good with moving forward with this. The only thing I would like to do as this is a new metric and in general a new method for kube-state-metrics, mark this as experimental for now. Then this lgtm to merge. Can you mark this metric as experimental in the docs @abursavich ? 🙂

[lilic]

Few comments but we decided we want to proceed with this, thank you so much for the PR!

[brancz]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abursavich, brancz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brancz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment