Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele #67829

krzysztof-jastrzebski · 2018-08-24T14:58:17Z

What this PR does / why we need it:
PR adds "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubelet. If parameter is true then only cpu and memory will be present in response. The parameter will be used by Metric Server to avoid sending/decoding unneeded data. Metric server needs only cpu and memory.

Release note:

Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubelet. If parameter is true then only cpu and memory will be present in response.

dims · 2018-08-24T17:27:27Z

@mwielgus this does not fall under sig node?

neolit123 · 2018-08-25T01:54:52Z

^ i have the same question. ;-)

@krzysztof-jastrzebski please add a release note explaining your change.
https://github.com/kubernetes/kubernetes/blob/master/.github/PULL_REQUEST_TEMPLATE.md

dashpole · 2018-08-27T17:59:29Z

@krzysztof-jastrzebski This seems to be a way to implement core metrics in the kubelet. Back when I proposed the set of core metrics, sig-instrumentation wasn't ready to consume scoped-down metrics in heapster (which has now become the metrics server), and thus we punted on adding the new endpoint. IMO, if the metrics-server is in a position to consume core metrics, we should define the core metrics endpoint, rather than making this change. That way we can also begin the process of removing dependencies on the summary api.

krzysztof-jastrzebski · 2018-08-29T08:41:42Z

Using new endpoint requires migrating all nodes in the cluster to the version with new endpoint. As we support 3 versions it can't be delivered before version 1.15. This could be a temporary solution before final solution is delivered.

dashpole · 2018-08-29T17:15:07Z

/ok-to-test

krzysztof-jastrzebski · 2018-08-29T18:09:07Z

/test pull-kubernetes-e2e-kops-aws

dashpole · 2018-08-30T17:00:54Z

I think this makes sense as a temporary solution. We can hopefully start discussions on the core metrics API for the 1.13 timeframe

krzysztof-jastrzebski · 2018-08-31T12:17:07Z

Is there anything I need to fix?

dashpole · 2018-08-31T16:43:12Z

/lgtm

yujuhong · 2018-09-06T00:10:05Z

pkg/kubelet/server/stats/summary.go

@@ -81,6 +81,31 @@ func (sp *summaryProviderImpl) Get(updateStats bool) (*statsapi.Summary, error)
 		Rlimit:           rlimit,
 		SystemContainers: sp.GetSystemContainersStats(nodeConfig, podStats, updateStats),
 	}
+
+	if onlyCPUAndMemory {


Could you create a new function instead? I think it'd be easier to test and maintain down the road.

Uh...sorry for not being clear. I was thinking that we could add a GetCPUMemoryStats function, as opposed to adding another flag in the Get function.

krzysztof-jastrzebski · 2018-09-06T14:10:56Z

/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-kops-aws

…r in kubelet. If parameter is true then only cpu and memory will be present in response. The parameter will be used by Metric Server to avoid sending/decoding unneeded data.

krzysztof-jastrzebski · 2018-09-06T20:27:39Z

/test pull-kubernetes-e2e-kops-aws

yujuhong · 2018-09-14T17:38:38Z

pkg/kubelet/server/stats/summary.go

@@ -87,3 +89,32 @@ func (sp *summaryProviderImpl) Get(updateStats bool) (*statsapi.Summary, error)
 	}
 	return &summary, nil
 }
+
+func (sp *summaryProviderImpl) GetCPUAndMemoryStats() (*statsapi.Summary, error) {
+	summary, err := sp.Get(false)


Will it be hard to actually bypass getting the imageFS (and other unused) stats? Setting them to nil after getting all the data seems sub-optimal...

Do you want me to bypass getting all unneeded stats or only some of them? Do you want me to add functions to StatsProvider that returns only needed stats?

Do you want me to bypass getting all unneeded stats or only some of them?

Bypassing all unneeded if possible.

Do you want me to add functions to StatsProvider that returns only needed stats?

If that's needed, then yes :-)

I have final #68841 but it is much bigger than this one.
Could we submit this PR and later submit final version? I would like to push this PR to 1.12 patch release and #68841 will be released with 1.13. It will be easier to release smaller PR in patch release.

guineveresaenger · 2018-09-18T23:03:49Z

As we are close to release, please continue this work in 1.13!
/milestone v1.13

yujuhong · 2018-09-25T23:53:17Z

/approve

yujuhong · 2018-09-26T18:53:57Z

/lgtm

based on #67829 (comment)

k8s-ci-robot · 2018-09-26T18:54:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, krzysztof-jastrzebski, yujuhong

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:

~~pkg/kubelet/OWNERS~~ [yujuhong]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

krzysztof-jastrzebski · 2018-09-26T21:27:42Z

/test pull-kubernetes-e2e-gce-100-performance

k8s-ci-robot requested review from dchen1107 and mtaufen August 24, 2018 14:59

k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 24, 2018

mwielgus assigned dchen1107 and yujuhong Aug 24, 2018

mwielgus removed the sig/node Categorizes an issue or PR as relevant to SIG Node. label Aug 24, 2018

mtaufen assigned dashpole Aug 27, 2018

krzysztof-jastrzebski force-pushed the cpuandmemory branch from 795d135 to c17df7a Compare August 27, 2018 07:37

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Aug 27, 2018

krzysztof-jastrzebski force-pushed the cpuandmemory branch from c17df7a to 5fba230 Compare August 27, 2018 07:38

krzysztof-jastrzebski changed the title ~~Add /stats/cpuandmemory http handller to kubelet.~~ Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele Aug 27, 2018

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 27, 2018

krzysztof-jastrzebski force-pushed the cpuandmemory branch from 5fba230 to 5d24448 Compare August 27, 2018 07:40

mwielgus added this to the v1.12 milestone Aug 28, 2018

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 29, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2018

krzysztof-jastrzebski force-pushed the cpuandmemory branch from 5d24448 to 26f7693 Compare September 5, 2018 21:30

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 5, 2018

yujuhong reviewed Sep 6, 2018

View reviewed changes

krzysztof-jastrzebski force-pushed the cpuandmemory branch from 26f7693 to c2f6ca5 Compare September 6, 2018 12:51

krzysztof-jastrzebski force-pushed the cpuandmemory branch from c2f6ca5 to b9ad1ad Compare September 6, 2018 19:26

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 6, 2018

krzysztof-jastrzebski force-pushed the cpuandmemory branch from b9ad1ad to 2b94c17 Compare September 6, 2018 19:28

Add "only_cpu_and_memory" GET parameter to /stats/summary http handle…

138a3c7

…r in kubelet. If parameter is true then only cpu and memory will be present in response. The parameter will be used by Metric Server to avoid sending/decoding unneeded data.

krzysztof-jastrzebski force-pushed the cpuandmemory branch from 2b94c17 to 138a3c7 Compare September 6, 2018 19:49

dashpole mentioned this pull request Sep 11, 2018

Reduce the set of metrics exposed by the kubelet #68522

Open

11 tasks

yujuhong reviewed Sep 14, 2018

View reviewed changes

k8s-ci-robot modified the milestones: v1.12, v1.13 Sep 18, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 26, 2018

k8s-ci-robot merged commit f263993 into kubernetes:master Sep 26, 2018

dashpole mentioned this pull request Sep 26, 2018

Optimizes calculating stats when only CPU and Memory stats are returned from Kubelet stats/summary http endpoint. #68841

Merged

serathius mentioned this pull request Aug 22, 2019

Pass only_cpu_and_memory parameter to Summary API call kubernetes-sigs/metrics-server#303

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele #67829

Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele #67829

krzysztof-jastrzebski commented Aug 24, 2018 •

edited

dims commented Aug 24, 2018

neolit123 commented Aug 25, 2018 •

edited

dashpole commented Aug 27, 2018

krzysztof-jastrzebski commented Aug 29, 2018

dashpole commented Aug 29, 2018

krzysztof-jastrzebski commented Aug 29, 2018

dashpole commented Aug 30, 2018

krzysztof-jastrzebski commented Aug 31, 2018

dashpole commented Aug 31, 2018

yujuhong Sep 6, 2018

krzysztof-jastrzebski Sep 6, 2018

yujuhong Sep 6, 2018

krzysztof-jastrzebski Sep 6, 2018

krzysztof-jastrzebski commented Sep 6, 2018

krzysztof-jastrzebski commented Sep 6, 2018

yujuhong Sep 14, 2018

krzysztof-jastrzebski Sep 17, 2018

yujuhong Sep 18, 2018

krzysztof-jastrzebski Sep 19, 2018

guineveresaenger commented Sep 18, 2018

yujuhong commented Sep 25, 2018

yujuhong commented Sep 26, 2018

k8s-ci-robot commented Sep 26, 2018

krzysztof-jastrzebski commented Sep 26, 2018

Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele #67829

Add "only_cpu_and_memory" GET parameter to /stats/summary http handler in kubele #67829

Conversation

krzysztof-jastrzebski commented Aug 24, 2018 • edited

dims commented Aug 24, 2018

neolit123 commented Aug 25, 2018 • edited

dashpole commented Aug 27, 2018

krzysztof-jastrzebski commented Aug 29, 2018

dashpole commented Aug 29, 2018

krzysztof-jastrzebski commented Aug 29, 2018

dashpole commented Aug 30, 2018

krzysztof-jastrzebski commented Aug 31, 2018

dashpole commented Aug 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzysztof-jastrzebski commented Sep 6, 2018

krzysztof-jastrzebski commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guineveresaenger commented Sep 18, 2018

yujuhong commented Sep 25, 2018

yujuhong commented Sep 26, 2018

k8s-ci-robot commented Sep 26, 2018

krzysztof-jastrzebski commented Sep 26, 2018

krzysztof-jastrzebski commented Aug 24, 2018 •

edited

neolit123 commented Aug 25, 2018 •

edited