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.

Sign up for GitHub

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

Jump to bottom

fixing issue with GetCPUUsageNanoCores on Windows #122999

Merged
merged 1 commit into from
Feb 5, 2024
Merged

fixing issue with GetCPUUsageNanoCores on Windows #122999

merged 1 commit into from
Feb 5, 2024

Conversation

marosset
Copy link
Contributor

@marosset marosset commented Jan 26, 2024 •
edited by liggitt

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR fixes some calculations used to determine node CPU usage for Windows

A bug was introduced by b3a67ab when the polling period for fetching current CPU usage was updated from 1 second to 10 seconds and the function GetCPUUsageNanoCores did not account for this

Which issue(s) this PR fixes:

Fixes # #122382

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixes a 1.27.0+ regression calculating total CPU usage reported for Windows nodes

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

/area kubelet
/sig windows node
/milestone v1.30

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 26, 2024
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Jan 26, 2024
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/kubelet sig/windows Categorizes an issue or PR as relevant to SIG Windows. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jan 26, 2024
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 26, 2024
@marosset
Copy link
Contributor Author

/assign @aravindhp @jsturtevant

aravindhp
aravindhp reviewed Jan 26, 2024
View reviewed changes
Copy link
Contributor

@aravindhp aravindhp left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 57afbf7e74f2fddd282887ddda7b62682dbc118f

@marosset
Copy link
Contributor Author

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 26, 2024
jsturtevant
jsturtevant approved these changes Jan 26, 2024
View reviewed changes
Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

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

/lgtm

@bart0sh bart0sh added this to Triage in SIG Node PR Triage Jan 26, 2024
@bart0sh bart0sh moved this from Triage to Needs Approver in SIG Node PR Triage Jan 26, 2024
@marosset
Copy link
Contributor Author

/assign @mrunalp

@Howard-Haiyang-Hao Howard-Haiyang-Hao mentioned this pull request Jan 30, 2024
Open
@marosset
Copy link
Contributor Author

marosset commented Feb 2, 2024

@yujuhong - Can you take a look at this? It is impacting users running K8s v1.27+
thanks!

@yujuhong
Copy link
Contributor

yujuhong commented Feb 5, 2024

/lgtm

@yujuhong
Copy link
Contributor

yujuhong commented Feb 5, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Feb 5, 2024
@k8s-ci-robot k8s-ci-robot merged commit 980033e into kubernetes:master Feb 5, 2024
14 checks passed
SIG Node PR Triage automation moved this from Needs Approver to Done Feb 5, 2024
This was referenced Feb 12, 2024
Merged
Merged
Merged
k8s-ci-robot added a commit that referenced this pull request Mar 2, 2024
@k8s-ci-robot
Merge pull request #123245 from marosset/automated-cherry-pick-of-#12…
d3b89a5 
…2999-upstream-release-1.28

Automated cherry pick of #122999: fixing issue with GetCPUUsageNanoCores on Windows
k8s-ci-robot added a commit that referenced this pull request Mar 2, 2024
@k8s-ci-robot
Merge pull request #123246 from marosset/automated-cherry-pick-of-#12…
da1393a 
…2999-upstream-release-1.29

Automated cherry pick of #122999: fixing issue with GetCPUUsageNanoCores on Windows
k8s-ci-robot added a commit that referenced this pull request Mar 2, 2024
@k8s-ci-robot
Merge pull request #123244 from marosset/automated-cherry-pick-of-#12…
aa1ad8d 
…2999-upstream-release-1.27

Automated cherry pick of #122999: fixing issue with GetCPUUsageNanoCores on Windows
@liggitt liggitt added the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Mar 27, 2024
@marosset marosset deleted the fix-windows-stats-cpu-nano-seconds-usage branch May 7, 2024 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aravindhp aravindhp aravindhp left review comments

@jsturtevant jsturtevant jsturtevant approved these changes

@krmayankk krmayankk Awaiting requested review from krmayankk

@matthyx matthyx Awaiting requested review from matthyx

Assignees

@mrunalp mrunalp

@jsturtevant jsturtevant

@yujuhong yujuhong

@aravindhp aravindhp

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/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. 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. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node PR Triage
Archived in project
[sig-release] Bug Triage
Archived in project
[sig-windows] PR tracking
Status: Done
SIG Node PR Triage
Done
Milestone
v1.30
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@marosset @k8s-ci-robot @yujuhong @jsturtevant @aravindhp @mrunalp @liggitt