Fix computing of cpu nano core usage

[yujuhong]

CRI runtimes do not supply cpu nano core usage as it is not part of CRI
stats. However, there are upstream components that still rely on such
stats to function. The previous fix was faulty because the multiple
callers could compete and update the stats, causing
inconsistent/incoherent metrics. This change, instead, creates a
separate call for updating the usage, and rely on eviction manager,
which runs periodically, to trigger the updates. The caveat is that if
eviction manager is completley turned off, no one would compute the
usage.

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug

/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #74667

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

[yujuhong]

/cc @michmike @PatrickLang @neolit123 @adelina-t @feiskyer @hpandeycodeit

[k8s-ci-robot]

@yujuhong: GitHub didn't allow me to request PR reviews from the following users: hpandeycodeit.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @michmike @PatrickLang @neolit123 @adelina-t @feiskyer @hpandeycodeit

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.

[neolit123]

thanks a lot for looking into this @yujuhong
found a couple of minor nits.

i still have the concern that we have reduced precision on Windows, but let's see it it's not a problem.

@adelina-t do you think you will be able to test this patch by building a custom kubelet?

wget https://github.com/kubernetes/kubernetes/pull/74933.diff && git apply 74933.diff
KUBE_BUILD_PLATFORMS=windows/amd64 make all WHAT=cmd/kubelet
# result should be at '_output/local/bin/windows/amd64/kubelet.exe'
# 'git reset --hard HEAD' to remove the patch

[neolit123]

/sig windows

[michmike]

/LGTM
fix the couple of nits that were pointed out and this is good

[michmike]

/lgtm

[michmike]

/hold cancel

[yujuhong]

/hold

[yujuhong]

@yujuhong , did you ran this one in gce ci ?

Our CI job tracks the master branch right now, so not yet. I manually tested and see no kubelet crash, and also kubectl top pods reporting meaningful numbers.

[adelina-t]

@yujuhong

Our CI job tracks the master branch right now, so not yet. I manually tested and see no kubelet crash, and also kubectl top pods reporting meaningful numbers.

Yeah, I was thinking manual runs :) I have jobs running as we speak, seeing consistent runs as well, but haven't seen logs yet, will look tomorrow.

[dashpole]

This should work well, and is as clean a way of implementing something like this as I think we will find. Ill expand on @yujuhong comment above to document it here.

The eviction manager does not run if:

• The list of hard eviction thresholds is explicitly set to empty
• No soft eviction thresholds are set
• The feature gate --LocalStorageCapacityIsolation is explicitly disabled.

/lgtm

[yujuhong]

Performance of the /stats/summary endpoint is pretty bad compared to Linux, but not sure if that's important. This does definitely fix the panic issue. Usage during the load test was consistently low.

@benmoss could you help file an issue about the performance for future reference? Thanks!

@dashpole thanks for reviewing.

/hold cancel

[yujuhong]

i still have the concern that we have reduced precision on Windows, but let's see it it's not a problem.

The reduced precision should not cause problems with the current interval (updating every 10s), but it's worth more investigation in the future.
@neolit123 could you file an issue summarizing your findings, so that we can come back and look at them later? Thanks!

[neolit123]

@neolit123 could you file an issue summarizing your findings, so that we can come back and look at them later? Thanks!

sure, here it is:
#74999

[tedyu]

Should nil be returned in this case ?

[SleepyBrett]

Is this going to get cherry picked back into the 1.13 and others?

[mattjmcnaughton]

Is this going to get cherry picked back into the 1.13 and others?

I think that's a good idea :) I answered in your issue.

[SleepyBrett]

Didn't seem to fix my problem on 1.14.1