CNF-8808: perfruntime: add shared cpus to container's cgroup #7502

Tal-or · 2023-11-21T14:55:34Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Given an annotation CRI-O appends additional CPUs (named shared CPUs)
into the container's cgroups.

The operation is done via a performance runtime hook.

This will allow the container to run light-weight tasks
on the shared cpus, while all the other task that
requires full isolation will keep running on the guaranteed CPUs.

It also injects env variables in to the container
in order to distinguish between the isolated and shared CPUs.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Related to:

Depends on:

runtime handler hooks: add support for cgroupv2 cpu load balancing #7485

EP: openshift/enhancements@76ea48a

Does this PR introduce a user-facing change?

openshift-ci-robot · 2023-11-21T14:55:39Z

@Tal-or: This pull request references CNF-8808 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

What type of PR is this?

/kind feature

What this PR does / why we need it:

Given an annotation CRI-O appends additional CPUs (named shared CPUs)
into the container's cgroups.

The operation is done via a performance runtime hook.

This will allow the container to run light-weight tasks
on the shared cpus, while all the other task that
requires full isolation will keep running on the guaranteed CPUs.

It also injects env variables in to the container
in order to distinguish between the isolated and shared CPUs.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Related to:

CNF-8326: advertise shared cpus for mixed cpus feature openshift/kubernetes#1795

CNF-8809: admission: add new admission for handling shared cpus request openshift/kubernetes#1799

CNF-7610: Mixed CPUs plugin deployment support openshift/cluster-node-tuning-operator#853

Depends on:

runtime handler hooks: add support for cgroupv2 cpu load balancing #7485

EP: openshift/enhancements@76ea48a

Does this PR introduce a user-facing change?
None

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.

openshift-ci · 2023-11-21T14:55:51Z

Hi @Tal-or. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Tal-or · 2023-11-21T14:55:51Z

Depends on: #7485

codecov · 2023-11-26T17:08:06Z

Codecov Report

Merging #7502 (14e6af6) into main (87c53ac) will decrease coverage by 0.18%.
Report is 13 commits behind head on main.
The diff coverage is 12.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7502      +/-   ##
==========================================
- Coverage   48.69%   48.52%   -0.18%     
==========================================
  Files         145      145              
  Lines       15926    16055     +129     
==========================================
+ Hits         7755     7790      +35     
- Misses       7236     7327      +91     
- Partials      935      938       +3

ffromani · 2023-11-29T13:23:23Z

/ok-to-test

haircommander · 2023-12-01T16:52:11Z

internal/config/node/cgroups_linux.go

@@ -26,6 +27,38 @@ var (
 	cgroupIsV2Err error
 )

+// CgroupHierarchy is a cgroup version agnostic struct that allows


I don't think this is best here. This package has a pretty particular use (for initializing variables that CRI-O needs to react to on the node). If it's not being used anywhere else, I'd prefer it in runtime_handler_hooks

The common case is that mixed CPUs will be used along side load-balancing disable. prep up a cgroup-child as part of the `setSharedCPUs` to save extra work and more conditioning when the `setCPULoadBalancing` function is being called. Signed-off-by: Talor Itzhak <titzhak@redhat.com>

Tal-or · 2023-12-06T22:31:58Z

/retest

Tal-or · 2023-12-07T09:48:50Z

/retest

haircommander · 2023-12-07T15:23:23Z

/retest

haircommander · 2023-12-07T15:23:43Z

Failed to download metadata for repo 'rhel-9-for-x86_64-appstream-rpms': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried

looks unrelated

haircommander · 2023-12-07T16:56:41Z

needs #7566

haircommander · 2023-12-07T18:50:33Z

/lgtm
/approve

openshift-ci · 2023-12-07T18:52:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, Tal-or

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:

~~OWNERS~~ [haircommander]

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

haircommander · 2023-12-07T19:11:35Z

/lgtm

haircommander · 2023-12-07T19:16:54Z

/hold cancel

haircommander · 2023-12-07T19:37:36Z

/retest

haircommander · 2023-12-07T20:17:54Z

/override ci/prow/ci-fedora-integration

openshift-ci · 2023-12-07T20:18:00Z

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-fedora-integration

In response to this:

/override ci/prow/ci-fedora-integration

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.

haircommander · 2023-12-07T21:15:59Z

/override ci/prow/e2e-gcp-ovn

aws passed

openshift-ci · 2023-12-07T21:16:06Z

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/e2e-gcp-ovn

In response to this:

/override ci/prow/e2e-gcp-ovn

aws passed

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.

Tal-or requested a review from mrunalp as a code owner November 21, 2023 14:55

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 21, 2023

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Nov 21, 2023

openshift-ci bot requested review from hasan4791 and klihub November 21, 2023 14:55

openshift-ci bot added kind/feature Categorizes issue or PR as related to a new feature. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 21, 2023

Tal-or mentioned this pull request Nov 21, 2023

CNF-7610: Mixed CPUs plugin deployment support openshift/cluster-node-tuning-operator#853

Merged

4 tasks

Tal-or force-pushed the lbv2_mixed_cpus branch from 37b7869 to fd8ddf7 Compare November 26, 2023 17:00

Tal-or force-pushed the lbv2_mixed_cpus branch 9 times, most recently from 36db7f4 to 2dd2faa Compare November 27, 2023 12:37

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 29, 2023

Tal-or force-pushed the lbv2_mixed_cpus branch 2 times, most recently from 9bdb2a6 to d16cec9 Compare November 29, 2023 13:54

haircommander reviewed Dec 1, 2023

View reviewed changes

Tal-or force-pushed the lbv2_mixed_cpus branch from 1cc1f96 to 20a631a Compare December 6, 2023 21:24

Tal-or force-pushed the lbv2_mixed_cpus branch from 20a631a to 14e6af6 Compare December 6, 2023 21:37

haircommander force-pushed the lbv2_mixed_cpus branch from 14e6af6 to 408feca Compare December 7, 2023 18:49

openshift-ci bot assigned haircommander Dec 7, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Dec 7, 2023

haircommander force-pushed the lbv2_mixed_cpus branch from 87f1c78 to 14e6af6 Compare December 7, 2023 19:05

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

haircommander mentioned this pull request Dec 7, 2023

[1.28] perfruntime: add shared cpus to container's cgroup #7567

Merged

openshift-merge-bot bot merged commit f40da54 into cri-o:main Dec 7, 2023
88 of 95 checks passed

Tal-or mentioned this pull request Jan 3, 2024

REQUEST: New organization membership for Tal-or #7652

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNF-8808: perfruntime: add shared cpus to container's cgroup #7502

CNF-8808: perfruntime: add shared cpus to container's cgroup #7502

Tal-or commented Nov 21, 2023 •

edited by haircommander

openshift-ci-robot commented Nov 21, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

openshift-ci bot commented Nov 21, 2023

Tal-or commented Nov 21, 2023

codecov bot commented Nov 26, 2023 •

edited

ffromani commented Nov 29, 2023

haircommander Dec 1, 2023

Tal-or commented Dec 6, 2023

Tal-or commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023 •

edited

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

CNF-8808: perfruntime: add shared cpus to container's cgroup #7502

CNF-8808: perfruntime: add shared cpus to container's cgroup #7502

Conversation

Tal-or commented Nov 21, 2023 • edited by haircommander

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

openshift-ci-robot commented Nov 21, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

openshift-ci bot commented Nov 21, 2023

Tal-or commented Nov 21, 2023

codecov bot commented Nov 26, 2023 • edited

Codecov Report

ffromani commented Nov 29, 2023

haircommander Dec 1, 2023

Choose a reason for hiding this comment

Tal-or commented Dec 6, 2023

Tal-or commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023 • edited

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

haircommander commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

Tal-or commented Nov 21, 2023 •

edited by haircommander

codecov bot commented Nov 26, 2023 •

edited

haircommander commented Dec 7, 2023 •

edited