CNF-15505: Remove container runtime selection #1219

rbaturov · 2024-11-17T15:53:08Z

A new patch has been introduced for CRI-O, allowing the container runtime for the high-performance runtime class to be inherited from the system instead of being detected and explicitly set. cri-o/cri-o#8753

As a result, the controller no longer needs to identify the container runtime from the containerRuntimeConfig for setting it in the 99-runtimes.conf snippet.

This PR encompasses all (need to verify) required changes.

openshift-ci · 2024-11-17T15:53:11Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

assets/performanceprofile/configs/99-runtimes.conf

rbaturov · 2024-11-22T14:47:45Z

/test all

rbaturov · 2024-11-22T23:49:15Z

/test all

rbaturov · 2024-11-24T20:21:07Z

/test all

rbaturov · 2024-11-25T06:56:47Z

/test all

rbaturov · 2024-11-25T07:05:36Z

/test all

rbaturov · 2024-11-25T07:21:31Z

/test all

openshift-ci-robot · 2024-11-25T09:19:39Z

@rbaturov: This pull request references CNF-15505 which is a valid jira issue.

Details

In response to this:

A new patch has been introduced for CRI-O, allowing the container runtime for the high-performance runtime class to be inherited from the system instead of being detected and explicitly set. cri-o/cri-o#8753

As a result, the controller no longer needs to identify the container runtime from the containerRuntimeConfig for setting it in the 99-runtimes.conf snippet.

This PR encompasses all (need to verify) required changes.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-11-25T09:25:34Z

@rbaturov: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

Details

In response to this:

A new patch has been introduced for CRI-O, allowing the container runtime for the high-performance runtime class to be inherited from the system instead of being detected and explicitly set. cri-o/cri-o#8753

As a result, the controller no longer needs to identify the container runtime from the containerRuntimeConfig for setting it in the 99-runtimes.conf snippet.

This PR encompasses all (need to verify) required changes.

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 openshift-eng/jira-lifecycle-plugin repository.

A new patch has been introduced for CRI-O, allowing the container runtime for the high-performance runtime class to be inherited from the system instead of being detected and explicitly set. cri-o/cri-o#8753 As a result, the controller no longer needs to identify the container runtime from the containerRuntimeConfig for setting it in the 99-runtimes.conf snippet. Signed-off-by: Ronny Baturov <[email protected]>

rbaturov · 2024-11-27T13:41:08Z

/retest-required

ffromani

need to review the e2e test, and there are pending comments open.
Besides that, I'd like a clarification about a test utility.
The logic per se LGTM

test/e2e/performanceprofile/functests/utils/cgroup/runtime/runtime.go

The GetContainerRuntimeTypeFor function has been updated to provide more accurate output regarding the runtime class used on the node. Previously, it relied on reading the runtime path from the CRI-O configuration file, but this is no longer viable, as the runtime path is no longer written to the file. The new approach directly checks the container process on the node using the ps tool to determine the runtime in use. The new command outputs the conmon process of the container and greps the container runtime from the -r argument using regex. For example: /bin/ps aux | grep 'tuned-ncp4j' root 6317 0.0 0.0 8312 1600 ? Ss 12:25 0:00 /usr/bin/conmon -b /run/containers/storage/overlay-containers/f492c7c342b71.../userdata/conmon-pidfile -p /run/containers/storage/overlay-containers/f492c7c342b71e19e.../userdata/pidfile --persist-dir /var/lib/containers/storage/overlay-containers/f492c7c342b7..../userdata -r /usr/bin/crun --runtime-arg --root=/run/crun --socket-dir-path /var/run/crio --syslog -u f492c7c3... -s root 90775 0.0 0.0 3332 1600 ? S+ 13:57 0:00 grep tuned-ncp4j Signed-off-by: Ronny Baturov <[email protected]>

When we have no ContainerRuntimeConfig in the system, the high-performance class should use the runtime that is currently being used in the system. This should be crun for 4.18 systems, or runc if upgraded from older version and not explicitly switched to crun. Since the selected runtime is not directly exposed, we target the tuned pod to determine which runtime is handling it, using this as an indicator for the node's default runtime. Moreover, instead of checking the container runtime for the high-performance pods from the conf file (which no longer is possible), we will create a test pod and check the container runtime used in practice. Signed-off-by: Ronny Baturov <[email protected]>

rbaturov · 2024-11-27T22:35:48Z

/retest-required

rbaturov · 2024-11-28T06:03:54Z

/retest-required

rbaturov · 2024-11-28T08:45:23Z

/retest-required

test/e2e/performanceprofile/functests/utils/cgroup/runtime/runtime.go

rbaturov · 2024-11-28T12:06:31Z

/retest-required

ffromani

my comments are addressed, LGTM pending review of the e2e tests

rbaturov · 2024-11-28T15:35:47Z

/retest-required

rbaturov · 2024-11-28T15:36:42Z

my comments are addressed, LGTM pending review of the e2e tests

@mrniranjan Has reviewed.

Tal-or · 2024-11-28T15:40:21Z

LGTM from my side

rbaturov · 2024-11-28T22:45:25Z

/retest-required

yanirq · 2024-12-01T20:21:16Z

/approve

openshift-ci · 2024-12-01T20:21:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rbaturov, yanirq

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [yanirq]

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

rbaturov · 2024-12-02T12:24:40Z

/test e2e-pao-updating-profile-hypershift

openshift-ci · 2024-12-02T16:03:51Z

@rbaturov: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

yanirq · 2024-12-02T16:24:02Z

/lgtm

openshift-cherrypick-robot · 2024-12-02T16:33:32Z

@rbaturov: cannot checkout 4.17: error checking out "4.17": exit status 1 error: pathspec '4.17' did not match any file(s) known to git

Details

In response to this:

/cherry-pick 4.17

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.

openshift-cherrypick-robot · 2024-12-02T16:36:56Z

@rbaturov: #1219 failed to apply on top of branch "release-4.17":

Applying: Remove detecting runtime from containerRuntimeConfig
Using index info to reconstruct a base tree...
M	pkg/performanceprofile/controller/performanceprofile/hypershift/components/handler.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/performanceprofile/controller/performanceprofile/hypershift/components/handler.go
CONFLICT (content): Merge conflict in pkg/performanceprofile/controller/performanceprofile/hypershift/components/handler.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Remove detecting runtime from containerRuntimeConfig

Details

In response to this:

/cherry-pick release-4.17

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.

rbaturov · 2024-12-02T16:37:36Z

/cherry-pick release-4.18

openshift-cherrypick-robot · 2024-12-02T16:38:15Z

@rbaturov: new pull request created: #1233

Details

In response to this:

/cherry-pick release-4.18

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.

openshift-bot · 2024-12-02T18:49:22Z

[ART PR BUILD NOTIFIER]

Distgit: cluster-node-tuning-operator
This PR has been included in build cluster-node-tuning-operator-container-v4.19.0-202412021737.p0.g7d54582.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2024

rbaturov force-pushed the remove-container-runtime-selection branch 2 times, most recently from 58b8352 to 1b1e182 Compare November 17, 2024 16:21

ffromani reviewed Nov 18, 2024

View reviewed changes

assets/performanceprofile/configs/99-runtimes.conf Show resolved Hide resolved

rbaturov changed the title ~~Remove container runtime selection~~ wip: Remove container runtime selection Nov 22, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2024

rbaturov force-pushed the remove-container-runtime-selection branch from 1b1e182 to a529fc7 Compare November 22, 2024 23:48

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2024

rbaturov force-pushed the remove-container-runtime-selection branch from 95eaf2c to 75c64c0 Compare November 25, 2024 06:56

rbaturov force-pushed the remove-container-runtime-selection branch from 75c64c0 to f562028 Compare November 25, 2024 07:05

rbaturov force-pushed the remove-container-runtime-selection branch from f562028 to 0063564 Compare November 25, 2024 07:21

rbaturov changed the title ~~wip: Remove container runtime selection~~ CNF-15505: Remove container runtime selection Nov 25, 2024

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

rbaturov changed the title ~~CNF-15505: Remove container runtime selection~~ Remove container runtime selection Nov 25, 2024

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

rbaturov force-pushed the remove-container-runtime-selection branch from 0063564 to de20252 Compare November 25, 2024 13:25

rbaturov changed the title ~~Remove container runtime selection~~ WIP: Remove container runtime selection Nov 25, 2024

rbaturov marked this pull request as ready for review November 25, 2024 13:25

openshift-ci bot requested review from dagrayvid and kpouget November 25, 2024 13:26

ffromani reviewed Nov 27, 2024

View reviewed changes

test/e2e/performanceprofile/functests/utils/cgroup/runtime/runtime.go Outdated Show resolved Hide resolved

rbaturov added 2 commits November 27, 2024 16:03

rbaturov force-pushed the remove-container-runtime-selection branch from e86d851 to dec1553 Compare November 27, 2024 17:06

mrniranjan reviewed Nov 28, 2024

View reviewed changes

test/e2e/performanceprofile/functests/utils/cgroup/runtime/runtime.go Show resolved Hide resolved

ffromani reviewed Nov 28, 2024

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2024

openshift-ci bot assigned yanirq Dec 2, 2024

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

openshift-merge-bot bot merged commit 7d54582 into openshift:master Dec 2, 2024

openshift-cherrypick-robot mentioned this pull request Dec 2, 2024

OCPBUGS-45450: [release-4.18]: Remove container runtime selection #1233

Merged

CNF-15505: Remove container runtime selection #1219

CNF-15505: Remove container runtime selection #1219

Uh oh!

Conversation

rbaturov commented Nov 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Nov 17, 2024

Uh oh!

Uh oh!

rbaturov commented Nov 22, 2024

Uh oh!

rbaturov commented Nov 22, 2024

Uh oh!

rbaturov commented Nov 24, 2024

Uh oh!

rbaturov commented Nov 25, 2024

Uh oh!

rbaturov commented Nov 25, 2024

Uh oh!

rbaturov commented Nov 25, 2024

Uh oh!

openshift-ci-robot commented Nov 25, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 25, 2024

Uh oh!

rbaturov commented Nov 27, 2024

Uh oh!

ffromani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rbaturov commented Nov 27, 2024

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

ffromani left a comment

Choose a reason for hiding this comment

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

Tal-or commented Nov 28, 2024

Uh oh!

rbaturov commented Nov 28, 2024

Uh oh!

yanirq commented Dec 1, 2024

Uh oh!

openshift-ci bot commented Dec 1, 2024

Uh oh!

rbaturov commented Dec 2, 2024

Uh oh!

openshift-ci bot commented Dec 2, 2024

Uh oh!

yanirq commented Dec 2, 2024

Uh oh!

openshift-cherrypick-robot commented Dec 2, 2024

Uh oh!

openshift-cherrypick-robot commented Dec 2, 2024

Uh oh!

rbaturov commented Dec 2, 2024

Uh oh!

openshift-cherrypick-robot commented Dec 2, 2024

Uh oh!

openshift-bot commented Dec 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

rbaturov commented Nov 17, 2024 •

edited

Loading

openshift-ci-robot commented Nov 25, 2024 •

edited by openshift-ci bot

Loading