Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

[metricbeat] Update doc with kube-state-metrics repository #1039

Merged
merged 5 commits into from
Feb 3, 2021

Conversation

yousafsyed
Copy link
Contributor

@yousafsyed yousafsyed commented Feb 2, 2021

Kube state metrics that were used in the metricbeat's charts were pointing towards the deprecated helm chart repository i.e "@stable" and in order to fix that we needed to point the chart towards @bitnami/kube-state-metrics which are stable and compatible with the metricbeat's deployment.

Without this small change, the charts are rendered useless since it breaks at the requirements phase.

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${CHART}/examples/*/test/goss.yaml

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@yousafsyed yousafsyed changed the title Fixing the respository of kube-state-metrics for metricbeats [metricbeat] Fixing the respository of kube-state-metrics for metricbeats Feb 2, 2021
Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

Hi @yousafsyed,

The current @stable requirements is still working as long as you configured stable repo to target the new location using helm repo add stable https://charts.helm.sh/stable.

This is a temporary solution as https://charts.helm.sh/stable is not anymore updated but this is fine for now as we pinned kube-state-metrics 1.2.0 for purpose.

We had a few bad experiences before where bumping to new kube-state-metrics version broke metricbeat chart and we won't update it until we have time to fully test it.

For reference, the work to update kube-state-metrics dependency is already tracked in #733 and we intend to use the official kube-state-metrics chart from https://github.com/kubernetes/kube-state-metrics/tree/master/charts/kube-state-metrics instead of bitnami chart.

What can be improved for now?

With Helm 2, stable repo was configured by default when installing Helm client, so there was no need to document it.

With Helm 3, stable repo is no more configured by default and it's URL changed to https://charts.helm.sh/stable.

We need to explicitely mention it in the chart install section, with something like:

  • Add the Elastic Helm charts repo (required for kube-state-metrics chart dependency):
    helm repo add stable https://charts.helm.sh/stable

Would you be interested to update your PR to add this instruction instead?

@jmlrt jmlrt added enhancement New feature or request metricbeat labels Feb 3, 2021
@yousafsyed
Copy link
Contributor Author

@jmlrt
I guess this is fine for now, It's working for me and I have updated the documentation instead of the change that I suggested earlier although the bitnami charts for kube-state-metric worked for me, but I agree we need to test them well before changing the version or charts.

metricbeat/README.md Outdated Show resolved Hide resolved
Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

LGTM⛴

@jmlrt jmlrt added docs need-backport and removed enhancement New feature or request labels Feb 3, 2021
@jmlrt jmlrt merged commit 71fbc93 into elastic:master Feb 3, 2021
@yousafsyed yousafsyed deleted the fix-kube-state-metrics branch February 7, 2021 15:57
@jmlrt jmlrt changed the title [metricbeat] Fixing the respository of kube-state-metrics for metricbeats [metricbeat] Update doc with kube-state-metrics repository Feb 8, 2021
@jmlrt jmlrt mentioned this pull request Feb 8, 2021
This was referenced Feb 12, 2021
@jmlrt jmlrt mentioned this pull request Feb 18, 2021
This was referenced Mar 15, 2021
@jmlrt jmlrt mentioned this pull request May 25, 2021
@jmlrt jmlrt mentioned this pull request Mar 8, 2022
@jmlrt jmlrt mentioned this pull request Apr 21, 2022
This was referenced Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants