-
Notifications
You must be signed in to change notification settings - Fork 107
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
kola/rpm-ostree/replace-rt-kernel: do not hardcode kernel versions #1309
kola/rpm-ostree/replace-rt-kernel: do not hardcode kernel versions #1309
Conversation
Skipping CI for Draft Pull Request. |
Continuing this thread #1306 (comment) on this PR: Agree eventually we should stop parsing HTML for the previous kernel test but doing that is going to require handling the "repoquery" stuff either in rpm-ostree or temporarily layering dnf or something else As far as the |
bdb6084
to
88e659d
Compare
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
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
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
/retest |
/test scos-9-build-test-qemu |
I'm also thinking this is a memory issue. In the SCOS community builds we give the pods running COSA 4gb ram and 1 full CPU. |
Let's bump this to 4GB here for this test only, master only as we don't run SCOS on other branches: https://github.com/openshift/release/blob/master/ci-operator/config/openshift/os/openshift-os-master.yaml#L78 |
lowering to 2GB and making this RHCOS only for now. I'll add this test for SCOS to my backlog. |
## # We've seen some OOM when 1024M is used in similar tests: | ||
## # https://github.com/coreos/fedora-coreos-tracker/issues/1506 | ||
## minMemory: 2048 | ||
## distros: rhcos |
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.
We don't have an scos switch here, rhcos
is aliased to scos
. You'll have to keep the denylist entry above but just for SCOS so c9s
.
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 would be nice if we could drop the denylist entry.. Maybe we use is_scos()
inside the test?
If the reason it's failing is because of #1309 (comment) then we could just do the rt test on SCOS, which is still a pretty good test.
what failures are we seeing with SCOS? Could it be that there is nothing to override because we are now using the latest version of the kernel in c9s and it matches the already baked in kernel in the OSTree? |
|
/retest |
runv rm -rf /etc/yum.repos.d/* | ||
runv curl -sSLf "${repo_url}" -o /etc/yum.repos.d/cs.repo | ||
runv curl -sSLf https://centos.org/keys/RPM-GPG-KEY-CentOS-Official -o /etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-Official | ||
runv sed -i 's|^gpgkey.*|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-Official|' /etc/yum.repos.d/cs.repo |
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.
This sed invocation also replaces other unrelated keys.
Analog to https://github.com/openshift/os/blob/master/extensions/Dockerfile#L13-L17:
runv sed -i 's|^gpgkey.*|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-Official|' /etc/yum.repos.d/cs.repo | |
runv sed -i 's|/usr/share/distribution-gpg-keys/centos|/etc/pki/rpm-gpg|' /etc/yum.repos.d/cs.repo |
Edit: Edited for clarity.
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'm a bit confused here. This is running inside the RHCOS machine (throw away test machine). What change are you suggesting and why is it important?
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.
To me it looks like rpm-ostree may be failing when trying to verify the repos in c9s.repo. The keys for all of them have to be available. Blanket replacing all of them with one key won't work.
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.
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.
And we do have repo_gpgcheck=0
on the repos in c9s.repo. I might've pulled a false alarm here.
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.
Correct this passes on RHCOS both on CI & locally
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.
The problem here is that those reference distribution-gpg-keys
which we don't ship in the OS by default. I think @LorbusChris 's suggestion makes sense...
But I don't understand actually why we're removing the repo files by default?
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 is because this is a test for RHCOS only at the moment, I am just making sure there nothing on yum.repos.d before downloading the centos repo. When I get to the SCOS test, the plan is to just use the provided repos/keys and just turn on the rt repo if it's disabled.
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.
see also #1316
/hold |
What do the journal logs say? |
My previous hold was probably a false alarm, but I'm still unsure why this fails on SCOS |
I opened #1315 to investigate the SCOS failure, CI is not saving the journal for this test (unless I am looking in the wrong place, there is a link to the logs in the issue). On a SCOS VM running the test manually I could not get it to fail, so I need to build scos and run the test against it locally to see if it fails and debug further. |
/retest |
/test rhcos-92-build-test-qemu |
@jmarrero: all tests passed! 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/test-infra repository. I understand the commands that are listed here. |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, dustymabe, jmarrero, travier 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:
Approvers can indicate their approval by writing |
closes #1099