-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Packit/TMT: Exclude podman packages from podman-next for tests #27488
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
base: main
Are you sure you want to change the base?
Conversation
|
let me check if dnf exclusion can solve this issue for good. |
8f26c20 to
a7da75c
Compare
|
Alright, this is good to go. Podman is only installed from the packit copr in TMT envs, which is the one we want to test. The cockpit tests never used the podman-next copr so there's no reason to keep it in the packit config. @containers/podman-maintainers @martinpitt PTAL |
Luap99
left a comment
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: lsm5, Luap99 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 |
| - type: repository-file | ||
| id: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/repo/fedora-$releasever/rhcontainerbot-podman-next-fedora-$releasever.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.
I don't like this TBH -- seems to be prone to installing podman without matching dependencies? Isn't it often the case that newer podman versions need a newer crun/selinux policy etc.?
Your cited TF run is weird indeed. The dnf update -y podman crun conmon criu call is meant to upgrade to newer dependencies from the -next copr. And e.g. crun is much newer, it has a "102:" epoch in COPR and no epoch in Fedora. I can debug this, but it's certainly meant to be used.
Or asked the other way around: Do you expect -next to be tested as a group and want to have non-released dependencies in podman? Or expect podman main to work on all released distros without newer deps?
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.
@martinpitt I have used the ridiculous Epoch number on most, if not all, podman-next packages, but builds for all other coprs (like Packit) should use the default distro Epoch.
To use podman-next deps, I found simply adding the repo to packit config didn't suffice. We also needed to bump their priority in the TF env like this . I didn't see any such step in cockpit-podman.fmf and the TF log did show the packit copr rpm being tested. So, the podman-next repo in the cockpit job felt like a NOOP. That's why the removal here.
Yes, ideally, everything on main should be tested with podman-next, so how about I reuse the same dnf priority modification block in cockpit-podman.fmf? LMK.
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.
Ah, thanks for the explanation! So TF adds these extra repo files with a high prio number (i.e. low priority, ignored) by default? Ugh..
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.
In other words, adding the same fix to cockpit-podman.fmf sounds great!
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.
@martinpitt great, I'll update this PR likewise. Thanks!
|
LGTM |
450e8c3 to
ca47f08
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
1 similar comment
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
ca47f08 to
d41c37a
Compare
d41c37a to
8bea3e5
Compare
9559592 to
8b68207
Compare
| @@ -0,0 +1,12 @@ | |||
| #!/usr/bin/env bash | |||
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.
should this script be moved into test/tmt/ instead?
The reason I am saying this is because hack/ is a used a lot in cirrus testing so we have a strict rule to trigger all tests anytime someone touches hack/
Given this is a tmt specific file maybe the location there would be better? I think that should trigger much less cirrus tests.
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.
oh, good point RE: hack/ . Are there similar restrictions for contrib ? That script isn't being used by anything in test/tmt but used in plans, and I'd prefer not to put it in plans. If contrib is good, I'll prefer contrib.
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.
worst case, I think I can just put in plans and move on.
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.
Long term, might even be nice to put these in a separate template repo for reuse by all projects as this setup should largely be the same for our projects.
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.
contrib is fine except for contrib/cirrus
32260af to
8b3eb34
Compare
37e0c86 to
571dbb9
Compare
571dbb9 to
b87b73e
Compare
|
I think this is good to go. Instead of bumping podman-next priority, I only removed the priority from testing-farm repo. Both cockpit and podman jobs show the expected rpms. PTAL. I'll keep this in draft until we merge #27271 |
This will fetch the highest packages from all repos present on the environment. TMT_TREE envvar is ok to use in this case as it will only be used on upstream packit tests. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
b87b73e to
6b1064b
Compare
See individual commits.
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?