-
Notifications
You must be signed in to change notification settings - Fork 137
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
CNF-15235: cnf-tests: move to rhel 9 #2139
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Tal-or 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 |
@Tal-or: This pull request references CNF-15235 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.19.0" version, but no target version was set. 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. |
More work is needed in https://github.com/openshift/release/blob/master/ci-operator/config/openshift-kni/cnf-features-deploy/openshift-kni-cnf-features-deploy-master.yaml |
cnf-tests/Dockerfile
Outdated
WORKDIR $TOOL_PATH | ||
|
||
RUN go build -mod=vendor -o /hugepages-allocator | ||
|
||
# build latency-test's runner binaries | ||
FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.22-openshift-4.18 AS builder-latency-test-runners |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we keeping this as rhel8 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yes, this is the standard Dockerfile
not the Dockerfile.openshift
I wasn't sure whether we want to change images here as well, because the cnf-tests image is centos7 based
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must say I am not sure who use this file but if you can please update everything to rhel 9 base.
I don't think this dockerfile really work as we build stuff on rhel 8 and 9 but put everything inside a centos7 image...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work!!!
I left a few comments :)
cnf-tests/Dockerfile
Outdated
WORKDIR $TOOL_PATH | ||
|
||
RUN go build -mod=vendor -o /hugepages-allocator | ||
|
||
# build latency-test's runner binaries | ||
FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.22-openshift-4.18 AS builder-latency-test-runners |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must say I am not sure who use this file but if you can please update everything to rhel 9 base.
I don't think this dockerfile really work as we build stuff on rhel 8 and 9 but put everything inside a centos7 image...
Signed-off-by: Talor Itzhak <[email protected]>
Align oc binary and pull it from ocp 4.19 Signed-off-by: Talor Itzhak <[email protected]>
Signed-off-by: Talor Itzhak <[email protected]>
The `libhugetlbfs` is not available on rhel-9 Signed-off-by: Talor Itzhak <[email protected]>
Some of the tests running under cnf-tests are using the `libhugetlbfs` for allocating hugepages. `libhugetlbfs` has been removed from rhel9, so instead we introduce a small program named hugepages-allocator that will be used by the tests for hugepages allocations. The program built like any other tool we already have in cnf-tests and the binary delivered as part of the final image. Signed-off-by: Talor Itzhak <[email protected]>
Same tests are runnning for both images so we should have the tool in both images to be able to run the same tests. Signed-off-by: Talor Itzhak <[email protected]>
Align `Dockerfile` with `Dockerfile.openshift` and use the same builder images based on rhel9 Signed-off-by: Talor Itzhak <[email protected]>
Change the test to use the tool instead of libhugetlbfs library Signed-off-by: Talor Itzhak <[email protected]>
related to #openshift/release#60244 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm just a small change in the regular dockerfile :)
@@ -59,7 +73,7 @@ RUN yum install -y numactl-devel make gcc && \ | |||
cp hwlatdetect /hwlatdetect && \ | |||
cp cyclictest /cyclictest | |||
|
|||
FROM quay.io/openshift/origin-oc-rpms:4.16 AS oc | |||
FROM quay.io/openshift/origin-oc-rpms:4.19 AS oc | |||
|
|||
# Final image | |||
FROM centos:7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to update this one also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was on purpose actually, if we're changing to rhel9 then both Dockerfile
and Dockerfile.openshift
are identical. So one can be deleted
/retest |
1 similar comment
/retest |
Can you please update also:
These defaults are needed to use this PR's changes during tests. Also, it should fix the test:
as if flakes because of the cnf-tests image size. See |
Signed-off-by: Talor Itzhak <[email protected]>
/retest |
1 similar comment
/retest |
|
/retest |
@Tal-or: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
depends on: openshift/release#60333 |
libhugetlbfs
,which is not present in rhel9, with a local tool for allocating hugepages (more details in the commits)Signed-off-by: Talor Itzhak [email protected]