Make kube-state-metrics optional for installation #387
Conversation
|
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? |
|
jenkins test this please |
| @@ -92,7 +92,11 @@ spec: | |||
| fieldRef: | |||
| fieldPath: metadata.namespace | |||
| - name: KUBE_STATE_METRICS_HOSTS | |||
| {{- if .Values.kube_state_metrics.enabled }} | |||
| value: "$({{ .Release.Name | replace "-" "_" | upper }}_KUBE_STATE_METRICS_SERVICE_HOST):$({{ .Release.Name | replace "-" "_" | upper }}_KUBE_STATE_METRICS_SERVICE_PORT_HTTP)" | |||
There was a problem hiding this comment.
Would it make more sense here to just use the kube_state_metrics_host value even if we are installing the sub-chart? In that case, what is the value that gets passed to the subchart as the service name?
There was a problem hiding this comment.
Hi @pbecotte . The general idea is to preserve upstream behavior as much as possible,
so when user disables kube_state_metrics installation then he/she needs to provide
existing endpoint for kube_state_metrics
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. To track this PR (even if closed), please open a corresponding issue if one does not already exist. |
|
Is there something needed to get progress on this feature? |
|
Hi @erihanse
The first thing is to have time for a proper review, tests and impacts analysis. This one is pretty old and important, it is in the top of the backlog and we should be able to review it soon. @SergK, I'm sorry for the delay, Metricbeat had a lot of changes since you created this PR. Can you merge master on your branch and fix the conflicts? |
|
@jmlrt done! |
|
jenkins test this please |
|
Some changes are required to make Black formatter happy in elastic+helm-charts+pull-request+lint-python/361. Can you run |
|
Sure! |
|
jenkins test this please |
There was a problem hiding this comment.
My tests are OK, I have a last small request to replace kube_state_metrics_host by kube_state_metrics.hostso we can have nested values like that:
kube_state_metrics:
enabled: true
# is used only when kube_state_metrics.enabled: false
host: ""instead of:
# is used only when kube_state_metrics.enabled: false
kube_state_metrics_host: ""
kube_state_metrics:
enabled: true
In the most cases kube-state-metrics is already installed on the cluster, so let's just re-use it, instead of installing another one copy. By default, kube-state-metrics are installed as dependency, so this PR doesn't change the default behaviour Signed-off-by: Sergiy Kulanov <[email protected]>
|
jenkins test this please |
In the most cases kube-state-metrics is already installed on the cluster, so let's just re-use it, instead of installing another one copy. By default, kube-state-metrics are installed as dependency, so this PR doesn't change the default behaviour Signed-off-by: Sergiy Kulanov <[email protected]>
In the most cases kube-state-metrics is already installed on the cluster, so let's just re-use it, instead of installing another one copy. By default, kube-state-metrics are installed as dependency, so this PR doesn't change the default behaviour Signed-off-by: Sergiy Kulanov <[email protected]>
In the most cases kube-state-metrics is already installed on the cluster, so let's just re-use it, instead of installing another one copy. By default, kube-state-metrics are installed as dependency, so this PR doesn't change the default behaviour Signed-off-by: Sergiy Kulanov <[email protected]>
|
backported to |
In the most cases kube-state-metrics is already installed on the
cluster, so let's just re-use it, instead of installing another
one copy. By default, kube-state-metrics are installed as dependency,
so this PR doesn't change the default behaviour
Signed-off-by: Sergiy Kulanov [email protected]
${CHART}/tests/*.py${CHART}/examples/*/test/goss.yaml