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

Add kubelet resource metrics v1alpha1 endpoint #73946

Merged
merged 2 commits into from Mar 8, 2019

Conversation

@dashpole
Copy link
Contributor

commented Feb 12, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
This implements the Kubelet Resource Metrics Endpoint as accepted in the KEP.

Which issue(s) this PR fixes:
Associated feature issue: kubernetes/enhancements#727

Special notes for your reviewer:
This separates the versioned metric definition (in /pkg/kubelet/apis/resourcemetrics/v1alpha1/config.go) from the implementation and format of the endpoint as suggested in kubernetes/enhancements#726 (comment). While the metric endpoint is alpha, it re-uses the SummaryProvider interface, which is well-tested and stable. Thus, I have not placed enabling this endpoint behind a feature gate.

I had up update the gomega dependency to be able to do assertions on a map.

Does this PR introduce a user-facing change?:

Add a new kubelet endpoint for serving first-class resource metrics

/assign @tallclair
for the config separation

@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

/priority important-soon
/sig node

@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

This should be added to the 1.14 milestone

@k8s-ci-robot k8s-ci-robot requested review from pmorie and yifan-gu Feb 12, 2019
@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch 3 times, most recently from 043e395 to 7a7134c Feb 12, 2019
@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

/assign @DirectXMan12

@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from 7a7134c to b6d2c32 Feb 13, 2019
@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

This is ready for review

@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from b6d2c32 to 96c7b3d Feb 13, 2019
Copy link
Contributor

left a comment

comments inline, overall looks good

pkg/kubelet/server/server.go Outdated Show resolved Hide resolved
test/e2e/framework/metrics/kubelet_metrics.go Outdated Show resolved Hide resolved
test/e2e_node/resource_metrics_test.go Outdated Show resolved Hide resolved
test/e2e_node/resource_metrics_test.go Show resolved Hide resolved
test/e2e_node/resource_metrics_test.go Outdated Show resolved Hide resolved
@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from c962fbe to c6ebc6c Feb 27, 2019
@nikopen

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

/milestone v1.14

@k8s-ci-robot k8s-ci-robot added this to the v1.14 milestone Mar 4, 2019
@nikopen

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

Copy link
Member

left a comment

Mostly lgtm, just a little test cleanup...

pkg/kubelet/server/server.go Outdated Show resolved Hide resolved
test/e2e_node/resource_metrics_test.go Outdated Show resolved Hide resolved
@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from c6ebc6c to c3f6c55 Mar 6, 2019
@nikopen

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

/retest

@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from c3f6c55 to fd2acfc Mar 7, 2019
@k8s-ci-robot k8s-ci-robot added size/XL and removed size/L labels Mar 7, 2019
@tallclair

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 7, 2019
@dchen1107

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

/lgtm only reviewed the commit of add kubelet prometheus resource metrics endpoint
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, dchen1107, tallclair

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

@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

/retest

@dashpole dashpole force-pushed the dashpole:prometheus_core_metrics branch from fd2acfc to 6051664 Mar 7, 2019
@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 7, 2019
@dchen1107

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

/lgtm

@dashpole

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

/retest

1 similar comment
@nikopen

This comment has been minimized.

Copy link
Member

commented Mar 8, 2019

/retest

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

@dashpole: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gce-device-plugin-gpu 6051664 link /test pull-kubernetes-e2e-gce-device-plugin-gpu

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot merged commit 5350647 into kubernetes:master Mar 8, 2019
15 of 17 checks passed
15 of 17 checks passed
pull-kubernetes-e2e-gce-device-plugin-gpu Job failed.
Details
pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
cla/linuxfoundation dashpole authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
@dashpole dashpole deleted the dashpole:prometheus_core_metrics branch Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.