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

migrate kubelet custom metrics to stability framework part 1 #84907

Conversation

RainbowMango
Copy link
Member

What type of PR is this?
/kind feature

What this PR does / why we need it:
Currently, custom metrics emitted from kubelet do not offer any stability guarantees.

And #83062 tries to make it possible to do so.

About metrics stability please refer to KEP .

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
There are still some custom metrics under pkg/kubelet/server that I'd prefer to do it later.
Because it's a little different(can't make it one-line migration) and I'm trying to find a way to deal with them.

Does this PR introduce a user-facing change?:

Following metrics from kubelet are now marked as with the ALPHA stability level:
kubelet_container_log_filesystem_used_bytes
kubelet_volume_stats_capacity_bytes
kubelet_volume_stats_available_bytes
kubelet_volume_stats_used_bytes
kubelet_volume_stats_inodes
kubelet_volume_stats_inodes_free
kubelet_volume_stats_inodes_used
plugin_manager_total_plugins
volume_manager_total_volumes

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/20190404-kubernetes-control-plane-metrics-stability.md

/priority important-soon
/milestone v1.17

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Nov 7, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Nov 7, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 7, 2019
@RainbowMango RainbowMango force-pushed the pr_migrate_custom_collector_kubelet branch 2 times, most recently from ccbf61c to 03d9592 Compare November 7, 2019 09:41
@RainbowMango
Copy link
Member Author

RainbowMango commented Nov 7, 2019

Unit test failed as we need another functionality, the one like CollectAndCompare but especially for custom collector.

/hold
for a while and get back soon.

[edit] This rely on some functionalities for custom collector testing and they are in #84919

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2019
Copy link
Contributor

@mattjmcnaughton mattjmcnaughton left a comment

Choose a reason for hiding this comment

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

Adding @dashpole for the initial review, as he is in the intersection of those who participate in sig-node and those who participated in the linked KEP.

cc @dashpole

Removing myself as I don't have much, if any, context on the overall effort.

/uncc

@dashpole
Copy link
Contributor

dashpole commented Nov 7, 2019

/assign

@RainbowMango RainbowMango force-pushed the pr_migrate_custom_collector_kubelet branch from 03d9592 to e4a128a Compare November 8, 2019 01:19
@RainbowMango
Copy link
Member Author

/test pull-kubernetes-e2e-gce-storage-slow
/hold cancel

/assign @saad-ali
for pluginmanager and voluemanager

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2019
@dashpole
Copy link
Contributor

dashpole commented Nov 8, 2019

Great work!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 8, 2019
@RainbowMango
Copy link
Member Author

/assign @saad-ali

@kubernetes/sig-node-pr-reviews

@josiahbjorgaard
Copy link
Contributor

@RainbowMango The v1.17 code freeze is coming up on 11/14 at 5PM pacific time. This PR will need to be merged before then for v1.17.

@derekwaynecarr
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, derekwaynecarr, RainbowMango

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2019
@k8s-ci-robot k8s-ci-robot merged commit b3dde20 into kubernetes:master Nov 11, 2019
@RainbowMango RainbowMango deleted the pr_migrate_custom_collector_kubelet branch November 11, 2019 07:30
@rajivmucheli
Copy link

Hi @RainbowMango , would these metrics be visible only in v1.17 ? not before or after ? I dont see ONLY these volume metrics in kubelet scrape on v1.15.

I had to ask here since i had no response to #89733

@RainbowMango
Copy link
Member Author

@rajivmucheli

would these metrics be visible only in v1.17 ?

No, this PR didn't change the visibility of metrics.

@rajivmucheli
Copy link

Oh ok, only these metrics fail to be visible on k8s v1.15, could you please assist on how to troubleshoot this issue on this PR or my PR? Are there any other metrics to determine pv consumption?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants