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

CRI: Add field to support CPU affinity on Windows #124285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kiashok
Copy link
Contributor

@kiashok kiashok commented Apr 11, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds CRI field 'affinity_cpus' to WindowsContainerResources struct to support CPU affinity on windows.
This field will be used by Windows CPU manager to set the logical processors to affinitize for a particular container down to containerd/hcsshim.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

No

CRI: Add field to support CPU affinity on Windows

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


/sig node
/sig windows

/cc @jsturtevant @marosset @aravindhp

@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 Apr 11, 2024
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows. labels Apr 11, 2024
@k8s-ci-robot
Copy link
Contributor

Please note that we're already in Test Freeze for the release-1.30 branch. This means every merged PR will be automatically fast-forwarded via the periodic ci-fast-forward job to the release branch of the upcoming v1.30.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Thu Apr 11 19:48:19 UTC 2024.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. area/kubelet labels Apr 11, 2024
@kiashok kiashok marked this pull request as draft April 11, 2024 21:22
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 11, 2024
@bart0sh bart0sh added this to Triage in SIG Node PR Triage Apr 11, 2024
@bart0sh bart0sh moved this from Triage to WIP in SIG Node PR Triage Apr 11, 2024
@ffromani
Copy link
Contributor

/cc

@swatisehgal
Copy link
Contributor

/retest

@aravindhp
Copy link
Contributor

@kiashok thank you for the PR.

This field will be used by Windows CPU manager to set the logical processors to affinitize for a particular container down to containerd/hcsshim.

Is any follow up work required for this? It will help if you can link to any docs about the Windows CPU manager.

@aravindhp
Copy link
Contributor

/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 Apr 16, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 30, 2024
@jsturtevant
Copy link
Contributor

xref: #125262

@kiashok kiashok closed this Jun 6, 2024
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 6, 2024
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
@ffromani
Copy link
Contributor

ffromani commented Jun 6, 2024

just curious why close? is there a better direction for this work?

@kiashok
Copy link
Contributor Author

kiashok commented Jun 6, 2024

just curious why close? is there a better direction for this work?

@ffromani all I wanted to do is push new changes and when I did so, it closed the PR automatically. Not sure why! Would ideally want to reopen this PR

@ffromani
Copy link
Contributor

ffromani commented Jun 6, 2024

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Jun 6, 2024
@k8s-ci-robot
Copy link
Contributor

@ffromani: Reopened this PR.

In response to this:

/reopen

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 6, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kiashok
Once this PR has been reviewed and has the lgtm label, please assign liggitt for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@ffromani
Copy link
Contributor

ffromani commented Jun 6, 2024

just curious why close? is there a better direction for this work?

@ffromani all I wanted to do is push new changes and when I did so, it closed the PR automatically. Not sure why! Would ideally want to reopen this PR

If you delete the remote branch (git push -d origin BRANCHNAME) it will automatically close the PR. Not sure this is what happened for you but I managed to shot myself in the foot many times with this.

@kiashok
Copy link
Contributor Author

kiashok commented Jun 6, 2024

just curious why close? is there a better direction for this work?

@ffromani all I wanted to do is push new changes and when I did so, it closed the PR automatically. Not sure why! Would ideally want to reopen this PR

If you delete the remote branch (git push -d origin BRANCHNAME) it will automatically close the PR. Not sure this is what happened for you but I managed to shot myself in the foot many times with this.

No, I did not delete the remote branch at all. Did a rebase, made new changes and pushed the branch.

@kiashok
Copy link
Contributor Author

kiashok commented Jun 6, 2024

/reopen

Thanks for reopening this PR!

@kiashok kiashok marked this pull request as ready for review June 6, 2024 14:54
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: Needs Reviewer
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants