Skip to content

USHIFT-5818: Generic Device Plugin - Sanity Smoke Test#5121

Merged
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
pmtk:gen-dev-plug/rf-test
Jul 8, 2025
Merged

USHIFT-5818: Generic Device Plugin - Sanity Smoke Test#5121
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
pmtk:gen-dev-plug/rf-test

Conversation

@pmtk
Copy link
Copy Markdown
Member

@pmtk pmtk commented Jun 30, 2025

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 30, 2025
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Jun 30, 2025

@pmtk: This pull request references USHIFT-5818 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 task to target the "4.20.0" version, but no target version was set.

Details

In response to this:

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 openshift-ci Bot requested review from eslutsky and vanhalenar June 30, 2025 11:25
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2025
@pmtk pmtk force-pushed the gen-dev-plug/rf-test branch from 08c436f to 62d3191 Compare June 30, 2025 11:28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think we should use uname -r / pass one single kernel if there is more than one being listed ? if the system has more than one kernel i see this command is failing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We cannot use uname -r because it returns the kernel of the host, bootc container can (and had when I tested it) contain different kernel.
It's true that rpm -q --qf will return all matching pkgs, but the problem is that what you're running might not be the newest one.

Because these commands are inside containerfile, not really reusable automatically (by importing a file) anywhere else, I would recommend that for QE testing you can copy and tweak them to your needs.

Copy link
Copy Markdown
Contributor

@ggiguash ggiguash Jul 6, 2025

Choose a reason for hiding this comment

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

This is not clear for me. Containers are using kernel of the host, aren't they?
What would happen if the kernel in the container is different from the kernel on the host (majority of cases)?

Should we install both the kernel and the kernel-devel of the host?

Copy link
Copy Markdown
Member Author

@pmtk pmtk Jul 7, 2025

Choose a reason for hiding this comment

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

Yes, containers are using kernel from the host and that's why there's a difference between uname -m and version returned by rpm.

There's already a kernel installed in the base bootc image, so I decided to install matching kernel-devel (rather to upgrade kernel and install kernel-devel). The kernel in the image is not active when building an image, but it will be when the image is installed onto the disk.

Personally, I wouldn't attempt to install the kernel of the host because there's no guarantee what someone has on their hypervisor (maybe it's older one). I feel like keeping the kernel might be easier if there's some kind of bug to report (I mean: image tag is pointing to kernel we're not changing).
Though maybe there's value in upgrading the kernel, so we can do it.
WDYT?

Copy link
Copy Markdown
Contributor

@ggiguash ggiguash Jul 8, 2025

Choose a reason for hiding this comment

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

Let's think what users would do in this case. Wouldn't they likely try to sync the kernel version between the host and the container?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What would they get by syncing the, for examples, CI's host kernel with the kernel of the image that will be installed on another host? At most, I would expect them to run dnf upgrade to get latest packages (including kernel).

I feel like this discussion is out of scope for this PR - current solution works and is completely valid: it installs kernel-devel for kernel already included in the bootc image, then compiles a driver for that particular kernel, there's no runtime involved, so there's no need to involve host's kernel.
Whether we should upgrade all packages or match kernel versions if more of a testing approach question and is, imho, applicable to all images we build (like, let's take rhel96-bootc-source image for example, question of upgrading the pkgs and/or kernel is just as valid).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, we can continue this interesting discussion elsewhere.
I'll approve the PR for now.

@pmtk pmtk force-pushed the gen-dev-plug/rf-test branch 2 times, most recently from b58d89d to 7c243d7 Compare June 30, 2025 15:06
@pmtk pmtk force-pushed the gen-dev-plug/rf-test branch from 7c243d7 to 0036578 Compare July 1, 2025 09:26
@pmtk
Copy link
Copy Markdown
Member Author

pmtk commented Jul 1, 2025

/test e2e-aws-tests-bootc

Comment thread test/assets/generic-device-plugin/job.yaml Outdated
Comment thread test/suites/optional/generic-device-plugin.robot Outdated
@pmtk pmtk force-pushed the gen-dev-plug/rf-test branch from 3983070 to 228222f Compare July 7, 2025 12:45
@ggiguash
Copy link
Copy Markdown
Contributor

ggiguash commented Jul 8, 2025

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ggiguash, pmtk

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:

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

@pacevedom
Copy link
Copy Markdown
Contributor

/retest

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 0d1d7d3 and 2 for PR HEAD 228222f in total

@pmtk
Copy link
Copy Markdown
Member Author

pmtk commented Jul 8, 2025

/test e2e-aws-tests-bootc-arm

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 8ae81eb and 1 for PR HEAD 228222f in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 8ae81eb and 2 for PR HEAD 228222f in total

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 8, 2025

@pmtk: 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.

@openshift-merge-bot openshift-merge-bot Bot merged commit 0f5c2a1 into openshift:main Jul 8, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants