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

[PLAT-520] Added kube-state-metrics support #126

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

baba230896
Copy link
Contributor

@baba230896 baba230896 commented Mar 16, 2022

Summary

  • Users can provide the custom endpoint for kube-state-metrics.
  • Users can install the kube-state-metrics and the platform in the same namespace with a single flag modification.

Testing

I have done the following testing -

  1. Custom kube-state-metrics endpoint.
  2. kube-state-metrics installation.
  3. Platform chart will use the custom endpoint if the user provides both the installation flag and custom endpoint. I have provided a higher priority to the custom endpoint than the installation.
  4. Default platform chart installation works as intended.

For second commit which has changes related to prometheus scrape configuration.

Summary

  • The kube-state-metric changed the following CPU metric name. And the Yugabyte Platform java code still uses the old metric name to show the CPU metric on Platform UI. At this place, we had two ways to solve this. Either update the java code or handle it in the Prometheus scrape configuration. We have chosen the second approach and fixed the Prometheus scrape configuration.

  • I have updated the Platform code to use the new CPU metric (kube_pod_container_resource_requests) and added the relabel in Prometheus to convert the old CPU metric (kube_pod_container_resource_requests_cpu_cores) to new.

Test

  • Deployed the platform using without this change and created the universe. We didn't have CPU metrics in the Platform UI at this time. Once I modified the Prometheus config using the helm upgrade, it started showing the CPU metrics.

image
image

Summary

- Users can provide the custom endpoint for kube-state-metrics.
- Users can install the kube-state-metrics and the platform in the same namespace with a single flag modification.

Testing

I have done the following testing -

- Custom kube-state-metrics endpoint.
- kube-state-metrics installation.
- Platform chart will use the custom endpoint if the user provides both installation flag and custom endpoint.
  I have provided a higher priority to the custom endpoint than the installation.
- Default platform chart installation works as intended.
Summary -

The kube-state-metric changed the following CPU metric name.
And the Yugabyte Platform java code still uses the old metric name to show the CPU metric on Platform UI.
At this place, we had two ways to solve this. Either update the java code or handle it in the Prometheus scrape configuration.
We have chosen the second approach and fixed the Prometheus scrape configuration.

Test -
Deployed the platform using without this change and created the universe.
We didn't have CPU metrics in the Platform UI at this time.
Once I modified the Prometheus config using the helm upgrade, it started showing the CPU metrics.
Copy link
Contributor

@anmalysh-yb anmalysh-yb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
One question.

stable/yugaware/templates/configs.yaml Outdated Show resolved Hide resolved
@baba230896
Copy link
Contributor Author

@anmalysh-yb I have modified the changes as per the suggestion. Could you please review the latest change and this related PR?

@baba230896
Copy link
Contributor Author

@anmalysh-yb Please check it once.

Copy link
Contributor

@anmalysh-yb anmalysh-yb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

@bhavin192 bhavin192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, I have suggested few changes.

stable/yugaware/templates/configs.yaml Show resolved Hide resolved
stable/yugaware/templates/kube-state-metrics-service.yaml Outdated Show resolved Hide resolved
stable/yugaware/values.yaml Outdated Show resolved Hide resolved
stable/yugaware/values.yaml Outdated Show resolved Hide resolved
stable/yugaware/templates/_helpers.tpl Outdated Show resolved Hide resolved
stable/yugaware/values.yaml Outdated Show resolved Hide resolved
baba230896 and others added 2 commits May 6, 2022 13:12
1. kubeStateMetric to kubeStateMetrics
2. Added a few explanatory comments

Co-authored-by: Bhavin Gandhi <bhavin192@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants