-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
✨ Integrate kube-state-metrics and CR config into tilt. #7095
Conversation
@chrischdi: This issue is currently awaiting triage. If CAPI contributors determines this is a relevant issue, they will accept it by applying the The 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. |
hack/observability/capi-state-metrics/kube-state-metrics/values.yaml
Outdated
Show resolved
Hide resolved
hack/observability/capi-state-metrics/kube-state-metrics/values.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few initial questions. Please just resolve if findings are just related to WIP.
hack/observability/capi-state-metrics/kube-state-metrics/kustomization.yaml
Outdated
Show resolved
Hide resolved
hack/observability/capi-state-metrics/kube-state-metrics/values.yaml
Outdated
Show resolved
Hide resolved
image: | ||
# TODO(chrischdi): drop to default to released image as soon as it got released | ||
# repository: registry.k8s.io/kube-state-metrics/kube-state-metrics | ||
# tag: v2.5.0 | ||
# custom image which includes changes > v2.5.0 required for custom resource metrics | ||
repository: chrischdi/kube-state-metrics | ||
tag: v2.5.0-fe097b6c | ||
pullPolicy: IfNotPresent | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: remove as soon as ksm release + helm chart release is out
image: | |
# TODO(chrischdi): drop to default to released image as soon as it got released | |
# repository: registry.k8s.io/kube-state-metrics/kube-state-metrics | |
# tag: v2.5.0 | |
# custom image which includes changes > v2.5.0 required for custom resource metrics | |
repository: chrischdi/kube-state-metrics | |
tag: v2.5.0-fe097b6c | |
pullPolicy: IfNotPresent |
1b69a11
to
d87d785
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see this nearing the finish line!
Overall lgtm, I still need to make a detaied pass to all the metrics
/hold |
/hold pending squash + reviews :-) |
Made a pass on the metrics, and I think this is a good set to start with. |
lgtm pending #7095 (comment) |
@@ -0,0 +1,77 @@ | |||
image: | |||
tag: v2.6.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good if we can drop the pinned tag in a follow-up PR once the corresponding Helm chart is out
(just that when we pull the latest chart the pinned tag and the chart don't run out of sync)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the latest chart still refers 2.5.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's open an issue to track this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done: #7143
/lgtm @chrischdi hold cancel? |
/assign @fabriziopandini |
/lgtm |
/lgtm
I think there is an intermediate step to get there, that is to automate the generation of the metrics configuration by introducing a new set of markers; this will make adoption in the providers straight forward |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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:
Approvers can indicate their approval by writing |
/hold cancel |
What this PR does / why we need it:
kube-state-metrics
tohack/observability
which uses thekube-state-metrics
helm chart and adds configuration for CAPI CRsCurrently not added metrics:
*_labels
TODOs:
Split CR configuration to multiple files if Prevent definition of same gvk in custom resource configuration kubernetes/kube-state-metrics#1810 gets merged_labels
)*_spec_paused
and*_annotation_paused
metrics*_status_replicas_*
metrics and unifyWhich issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Part of #6458