Skip to content
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

Cirrus: remove centos9 build job #1015

Closed
wants to merge 1 commit into from
Closed

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Jun 28, 2024

Packit ensures c9s buildability, so there's no need for this job.

Packit ensures c9s buildability, so there's no need for this job.

Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
@lsm5
Copy link
Member Author

lsm5 commented Jun 28, 2024

@cevich @Luap99 @mheon PTAL

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

In principle yes that is the right thing to do.

However merge protection is not setup for any packit task, mainly because packit tasks are failing to often with random failures (this results in us maintainers ignoring them).

Given nobdy looks after quay.io/libpod/centos9rust image either I am still in favor of merging this so LGTM

Copy link
Contributor

openshift-ci bot commented Jul 1, 2024

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lsm5
Copy link
Member Author

lsm5 commented Jul 1, 2024

i've begun talks with the go maintainers for centos stream about adding reverse dependency tests on golang rpms before they enter centos stream build environments. So, we should be able to block on packit tasks sometime soon.

@Luap99
Copy link
Member

Luap99 commented Jul 1, 2024

I have not looked at enough to errors to tell but I don't see how blocking golang rpms is helping us here.
There are packit/tmt/whatever flakes in this pipeline, i.e. #1016 (comment), random dnf install failures at runtime, etc...

And our tests contain flakes too of course so the total number of flakes perceived by me looks high. Of course I have no tool to measure the actual numbers.

@lsm5
Copy link
Member Author

lsm5 commented Jul 1, 2024

I have not looked at enough to errors to tell but I don't see how blocking golang rpms is helping us here. There are packit/tmt/whatever flakes in this pipeline, i.e. #1016 (comment), random dnf install failures at runtime, etc...

uh ya, sorry i meant along with golang, I have plans to check with rust maintainers as well

RE: flakes on that PR, I'll report those to packit.

RE: dnf install failures, please let me know next time you see them. Quite likely something can be done to fix issues in COPR or how we install packages.

@Luap99
Copy link
Member

Luap99 commented Jul 1, 2024

RE: dnf install failures, please let me know next time you see them

There are permanently broken on aardvark-dns rawhide currently as an example, https://artifacts.dev.testing-farm.io/d4eac9f2-f05c-4585-8324-0e29a55ed479/
I assume you are already aware of this.
Note I am overall in favor to expand packit/tmt test on aardvark/netavark repos but we cannot setup merge protection for these tasks if we have to to overwrite them to often. And without merge protection we may merge broken changes as most of us are used to ignore the packit/tmt jobs entirely.

@lsm5
Copy link
Member Author

lsm5 commented Jul 1, 2024

RE: dnf install failures, please let me know next time you see them

There are permanently broken on aardvark-dns rawhide currently as an example, https://artifacts.dev.testing-farm.io/d4eac9f2-f05c-4585-8324-0e29a55ed479/ I assume you are already aware of this.

Yes, I've learned a few things since the last change to aardvark-dns PR. Things have been fixed in container-selinux to account for this and I'll be replicating that to aardvark-dns.

Note I am overall in favor to expand packit/tmt test on aardvark/netavark repos but we cannot setup merge protection for these tasks if we have to to overwrite them to often. And without merge protection we may merge broken changes as most of us are used to ignore the packit/tmt jobs entirely.

About merge protection, we'll have to start somewhere. I'll start on container-selinux right away, and soon as we have revdep testing on golang and rust, we can start blocking on build jobs (and later on test jobs) on other repos.

@Luap99 Luap99 mentioned this pull request Jul 12, 2024
@Luap99
Copy link
Member

Luap99 commented Jul 30, 2024

@mheon @baude PTAL

@mheon
Copy link
Member

mheon commented Jul 30, 2024

I am LGTM on this

@Luap99
Copy link
Member

Luap99 commented Oct 17, 2024

was removed in #1098

@lsm5 lsm5 deleted the trim-cirrus branch October 17, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants