Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KSM in v2.7.0 lead to some conflicts with the existing CustomResourceMetricConfiguration #7832

Closed
bavarianbidi opened this issue Jan 3, 2023 · 9 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@bavarianbidi
Copy link
Contributor

What steps did you take and what happened:

As this PR has landed in v2.7.0 of kube-state-metrics,
for each custom resource metric, the labels group, version and kind are some kind of "reserved labels" and will be overwritten by the values from the groupVersionKind of the CR itself.

from the KSM docs:

NOTE: The group, version, and kind common labels are reserved, and will be overwritten by the values from the groupVersionKind field.

Unfourtanetly this statement isn't true.
With the current implementation the version from the configuration (if one is defined) will overwrite the version from the groupVersionKind field.

This lead to the behavior that e.g. the current config of the capi_kubeadmcontrolplane_info metric

    metricNamePrefix: capi_kubeadmcontrolplane
    metrics:
    - name: info
      help: Information about a kubeadmcontrolplane.
      each:
        info:
          labelsFromPath:
            version:
            - spec
            - version
        type: Info

will generate the following metric

 capi_kubeadmcontrolplane_info{cluster_name="glippy",group="controlplane.cluster.x-k8s.io",kind="KubeadmControlPlane",name="glippy",namespace="org-giantswarm",uid="bd2c772e-2140-480f-a051-3ecedc2ed02b",version="v1.22.7"}

group and kind are generated from the corresponding CR and version gets overwritten by the configuration from above.

The corresponding capi_kubeadmcontrolplane_owner configuration will lead to

 capi_kubeadmcontrolplane_owner{cluster_name="glippy",group="controlplane.cluster.x-k8s.io",kind="KubeadmControlPlane",name="glippy",namespace="org-giantswarm",owner_is_controller="true",owner_kind="Cluster",owner_name="glippy",owner_uid="a2b4fcfe-8db5-4c37-bfee-19334fc36f67",uid="bd2c772e-2140-480f-a051-3ecedc2ed02b",version="v1beta1"}

As there is no explicit version label defined in the configuration, the version label will get generated from the CR.

What did you expect to happen:

We could either think about renaming version to kubernetes_version or we wait until kubernetes/kube-state-metrics#1942 got accepted and merged.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Cluster-api version:
  • minikube/kind version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):

/kind bug
[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 3, 2023
@k8s-ci-robot
Copy link
Contributor

@bavarianbidi: This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@chrischdi
Copy link
Member

I'd prefer to see the outcome of kubernetes/kube-state-metrics#1942 (I like the proposed approach) 👍 .

Thanks for collecting all the information and bringing up this topic 🎉

@sbueringer
Copy link
Member

+1
Nice catch, thx!
Let's see what happens to the KSM PR.

@sbueringer
Copy link
Member

@bavarianbidi Nice work!

As the PR has been merged, If I understood correctly with the next KSM release this issue would be resolved?

@bavarianbidi
Copy link
Contributor Author

@bavarianbidi Nice work!

As the PR has been merged, If I understood correctly with the next KSM release this issue would be resolved?

Yes, the upcoming KSM release will solve the issue.
As i still have to fix the comments from @chrischdi in #7886 i will use KSM from main for my local testing and add some examples in this issue to keep it more transparent.

@bavarianbidi
Copy link
Contributor Author

Tested the existing main configuration with a KSM image (build from current main - b7a7070e87ba4299a90587c8c2d5c7647db1ba42) generates the following metrics:

capi_machinedeployment_owner where no custom version label is configured:

# HELP capi_machinedeployment_owner Owner references.
# TYPE capi_machinedeployment_owner info
capi_machinedeployment_owner{cluster_name="glippy",customresource_group="cluster.x-k8s.io",customresource_kind="MachineDeployment",customresource_version="v1beta1",name="glippy-md",namespace="org-giantswarm",owner_kind="Cluster",owner_name="glippy",owner_uid="a2b4fcfe-8db5-4c37-bfee-19334fc36f67",uid="94e82c4e-d30c-4d83-8663-499a91b8cd1b"} 1

capi_machine_info with a version label configured:

capi_machine_info{cluster_name="glippy",container_runtime_version="containerd://1.6.0",customresource_group="cluster.x-k8s.io",customresource_kind="Machine",customresource_version="v1beta1",failure_domain="3",kernel_version="5.11.0-1028-azure",kube_proxy_version="v1.22.7",kubelet_version="v1.22.7",name="glippy-dnxdb",namespace="org-giantswarm",os_image="Ubuntu 20.04.3 LTS",provider_id="azure:///subscriptions/<random-subscription-id>/resourceGroups/glippy/providers/Microsoft.Compute/virtualMachines/glippy-control-plane-4fg59",uid="998fe651-704e-4d72-b447-291cc8778544",version="v1.22.7"} 1

@sbueringer / @chrischdi feel free to close the issue

@chrischdi
Copy link
Member

🆗 , thanks a lot for investigating and addressing it on the kube-state-metrics side 👍

/close

@k8s-ci-robot
Copy link
Contributor

@chrischdi: Closing this issue.

In response to this:

🆗 , thanks a lot for investigating and addressing it on the kube-state-metrics side 👍

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sbueringer
Copy link
Member

+1 Thank you very much!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

4 participants