-
Notifications
You must be signed in to change notification settings - Fork 309
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
packit: Build PRs into default packit COPRs #1260
Conversation
@martinpitt the main reason for having RE: Responding to comment from #1256:
So, what I'm trying to understand is if we change from |
Moving to draft, this still needs fixing to include the packit-next COPR. But I didn't want to spend a lot of work on this before discussing.
How do you mean? Each PR gets its own COPR, https://copr.fedorainfracloud.org/coprs/packit/containers-crun-1260 in this case (note the PR number), and they get deleted after a month or so.
Possibly -- the test could do some dnf incantation with the right amount of
The "special" bit is just that they get auto-created per PR. Not quite -- you switch from a single COPR for all PRs of all projects to one COPR per project PR. That's the whole point of it, to isolate PRs from one another. Note that podman-next is already past the point of "collisions" -- main commits are serialized and vetted (hopefully) to not break other bits; while open PRs are parallel and can be arbitrarily broken. |
It'd been quite a while since I used the default copr, so I'd forgotten it created a unique copr for every PR. My bad.
In cockpit project, do you do anything to get a quick view of packit failures or do you simply navigate to failures from the PR status? I'm kinda tempted to try some dnf stuff, but I do see it becoming a problem often. So, all things considered, I'm fine with the ephemeral unique copr for @martinpitt I welcome PRs from you if you got the time. If not, I can make the changes myself as and when I get the chance. |
The latter -- we rarely feel the need for such a "quick view". Unless COPR has an infra problem (which is very rarely the case), our fedora-stable and centos builds are always green when landing a PR, i.e. build failures in a PR are the fault of the PR and get sorted out right there. Rawhide is different -- there things do "spontaneously" (from our POV) combust, and if that is outside of our reach, we just have to complain/file a bug/wait until it is resolved. But then that failure will happen in all PRs anyway, so it's very easy to find an instance. That is a decision that your team has to make:
As you currently enable rawhide, I kept it enabled as well. If/once it gets too much on your nerve, it's easy to change the "fedora-all" to "fedora-stable".
Yes, makes sense. I suppose you mean "for all projects", so let's get working PRs for all projects that currently build into the packit-builds repo, and then land them together only once they work. So far this was just a proposal to gauge if you even want this. |
Building all PRs of all container projects into the same COPR does not properly isolate PRs from each other: E.g. a podman PR currently runs against whichever crun PR was opened/updated last; in other words, sending a broken crun PR will instantly break tests for all subsequent podman runs. To avoid that, change the copr_build configuration to use the packit default COPRs, which are specific to the particular PR, and disappear after a few weeks. Depending projects like podman should only run against what landed in crun/main, i.e. the podman-next COPR. Note that this does not preclude testing a podman PR against a crun PR: This can be explicitly requested [1]. But most PRs don't change the API and thus should default to isolation. [1] https://packit.dev/posts/testing-farm-triggering Signed-off-by: Martin Pitt <[email protected]>
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 This actually works out better given we can now easily add rhel-N-$ARCH
buildroots without fighting the copr settings.
LGTM.
@lsm5 : FTR, I have a half-done gvisor-tap-vsock on my disk (just in a meeting). I think I can do skopeo (the last one) too still today. |
sgtm, no rush, thanks! Appreciate the quick turnaround. I'm gonna add rhel targets to these soon as we have rhel testing on our agenda as well. |
Cool, I didn't know that we could run RHEL testing in public upstream PRs. Do you need to send @thrix some special 🍺 💝 for that? 😁 |
containers/container-selinux#260
i'll need to buy him a year's supply at least. I hope I can file it as a company expense 😆 |
Building all PRs of all container projects into the same COPR does not properly isolate PRs from each other: E.g. a podman PR currently runs against whichever crun PR was opened/updated last; in other words, sending a broken crun PR will instantly break tests for all subsequent podman runs.
To avoid that, change the copr_build configuration to use the packit default COPRs, which are specific to the particular PR, and disappear after a few weeks. Depending projects like podman should only run against what landed in crun/main, i.e. the podman-next COPR.
Note that this does not preclude testing a podman PR against a crun PR: This can be explicitly requested [1]. But most PRs don't change the API and thus should default to isolation.
[1] https://packit.dev/posts/testing-farm-triggering
See the discussion in #1256 for the background, hello @lsm5 . I kept the same COPR targets as https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/ . If you agree, I'm happy to send the same change to podman, container-selinux (other projects?), afterwards the packit-builds COPR can be removed. However, I'm not aware why you chose to do it that way in the first place? I.e. do you need the packit-builds COPR for anything else? Then I'd recommend to at least having one PR build COPR per project, not one for the whole org and cross-project testing.
Working PRs for all current packages in the packit-builds COPR. Once they are all ready, they need to land together. The green ones (ready for review) need review and work; the gray ones (draft) still fail, I'm looking into them: