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

Discover targets when building in users Copr repo #1499

Closed
csomh opened this issue May 11, 2022 · 13 comments · Fixed by #1673
Closed

Discover targets when building in users Copr repo #1499

csomh opened this issue May 11, 2022 · 13 comments · Fixed by #1673
Assignees

Comments

@csomh
Copy link
Contributor

csomh commented May 11, 2022

It's pretty cumbersome for users who are using their own Copr repositories to keep the list of chroots in sync with the target lists in the jobs.

This could be improved if the service could be configured to set the targets for jobs based on what chroots are enabled in the user provided Copr repo.

Something like:

jobs:
- job: copr_build
  trigger: release
  owner: "@cockpit"
  project: "cockpit-preview"
  targets: copr_chroots

Or going further: targets could become optional when a user provided Copr repo is used, with the default behaviour to get the list from the Copr repo.

(The example above uses the metadata-less config variant, which is not yet released.)

@csomh
Copy link
Contributor Author

csomh commented May 11, 2022

Is this what you meant, @martinpitt?

@martinpitt
Copy link

Or going further: targets could become optional when a user provided Copr repo is used

Right, that would be my intuition. Right now it is required, and thus one needs to keep the copr config and all 5 repos that use it in perfect sync.

I ran into this today in our releases, the first time that packit did these: https://github.com/cockpit-project/cockpit-ostree/runs/6386292898 ("Submit of the build failed: Only owners and admins may update their projects"). Apparently packit tried to reconfigure our copr for the build targets mentioned in packit.yaml, and failed because it is a "builder". It never asked for admin privs, and I wouldn't be that comfortable with handing them out to a bot.

I manually synced the copr's build targets with the list in packit.yaml, and then it worked. My main worry is how to maintain that in the future -- for each new fedora/c9s release we'd need to update all packit.yml files in lockstep and configure the COPR in the same way.

If possible, it'd be much nicer to just drop targets: for a custom COPR, and just let the creator of the COPR maintain the target list there.

Thank you!

@mfocko
Copy link
Member

mfocko commented May 11, 2022

Dropping it with the check for custom repository makes more sense and IMO might be easier to implement

martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue May 13, 2022
We immediately release new versions directly to Fedora anyway, so it
does not make sense to also build the COPR for Fedora.

This will avoid some churn with the (much more frequent) new Fedora
releases, until packit/packit-service#1499
gets fixed.
martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue May 13, 2022
We immediately release new versions directly to Fedora anyway, so it
does not make sense to also build the COPR for Fedora.

This will avoid some churn with the (much more frequent) new Fedora
releases, until packit/packit-service#1499
gets fixed.
martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue May 18, 2022
We immediately release new versions directly to Fedora anyway, so it
does not make sense to also build the COPR for Fedora.

This will avoid some churn with the (much more frequent) new Fedora
releases, until packit/packit-service#1499
gets fixed.
@mfocko mfocko self-assigned this May 24, 2022
mfocko added a commit to mfocko/packit-service that referenced this issue May 31, 2022
mfocko added a commit to mfocko/packit that referenced this issue Aug 24, 2022
Add method that returns set of chroots configured on the given Copr
project.

Related to packit/packit-service#1499

Signed-off-by: Matej Focko <[email protected]>
@lachmanfrantisek lachmanfrantisek moved this to 📋 Backlog 🔟 in Packit Kanban Board Sep 6, 2022
@csomh csomh moved this from backlog to in-review in Packit Kanban Board Sep 6, 2022
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been marked as stale because it hasn't seen any
activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed
by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! 🦄 🚀 🤖

(Note: issues labeled with pinned or EPIC are
never marked as stale.)

@stale stale bot added the stale Is the issue still valid? label Sep 21, 2022
mfocko added a commit to mfocko/packit that referenced this issue Sep 21, 2022
Add method that returns set of chroots configured on the given Copr
project.

Related to packit/packit-service#1499

Signed-off-by: Matej Focko <[email protected]>
softwarefactory-project-zuul bot added a commit to packit/packit that referenced this issue Sep 21, 2022
feat(copr): add method for getting chroots

Add method that returns set of chroots configured on the given Copr
project.
Signed-off-by: Matej Focko [email protected]
TODO:

 Write new tests or update the old ones to cover new functionality.
 Update doc-strings where appropriate.
 Update or write new documentation in packit/packit.dev.

Related to packit/packit-service#1499
Merge before packit/packit-service#1530
RELEASE NOTES BEGIN
RELEASE NOTES END

Reviewed-by: None <None>
@mfocko
Copy link
Member

mfocko commented Sep 22, 2022

Part of this was done in packit/packit#1694. I have a smallish issue with triggering the Copr build via /packit test

@stale stale bot removed the stale Is the issue still valid? label Sep 22, 2022
mfocko added a commit to mfocko/packit-service that referenced this issue Sep 22, 2022
If there are no configured build targets and custom Copr project is
configured, then fetch the targets and use them for the build.

Fixes packit#1499

Signed-off-by: Matej Focko <[email protected]>
@mfocko mfocko mentioned this issue Sep 22, 2022
3 tasks
softwarefactory-project-zuul bot added a commit that referenced this issue Sep 22, 2022
Deduce copr targets

TODO:

 Write new tests or update the old ones to cover new functionality.
 Update doc-strings where appropriate.
 Update or write new documentation in packit/packit.dev.



Fixes #1499

RELEASE NOTES BEGIN
Packit now deduces Copr targets for Copr builds when you have set your custom Copr project to be used.
RELEASE NOTES END

Reviewed-by: Tomas Tomecek <[email protected]>
Repository owner moved this from in-review to done in Packit Kanban Board Sep 22, 2022
@martinpitt
Copy link

Yay, thank you so much for fixing that! We use the same copr across all cockpit projects, and making changes to the build targets has been really painful until now (up to the point that we just didn't do it, and shrugged off issue reports). I'll try it soon!

@mfocko
Copy link
Member

mfocko commented Sep 23, 2022

@martinpitt just an FYI that it will be deployed on production on Tuesday

martinpitt added a commit to martinpitt/cockpit-ostree that referenced this issue Sep 28, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
martinpitt added a commit to martinpitt/cockpit-ostree that referenced this issue Sep 28, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
@martinpitt
Copy link

martinpitt commented Sep 28, 2022

@mfocko:

I wanted to test this today on my forked project. I dropped the copr targets list and added an extra debug commit that diverts the COPR from the official cockpit one to my own one.

However, nothing at all happens with packit -- usually I get some packit job statuses on the tagged release commit, but I only see the upstream release (which worked). At the first attempt I got an automated comment from packit about configuring allowed github repos in COPR, which I did -- both in my martinpitt/test-fixes and the official cockpit one. Then I force-pushed and re-attempted the release, and silence. There was no issue filed, no job started, and https://dashboard.packit.dev/jobs does not have anything about martinpitt/cockpit-ostree either.

Is there some infrastructure outage, or something wrong with my packit.yaml that doesn't get reported anywhere?

Thank you!

@martinpitt
Copy link

martinpitt/cockpit-ostree@c348b72 was the last "test release" commit that I did on the same project, 4 months ago. That commit has rpm-build packit statuses. However, these may have been for the propose_downstream job, which I deleted now in my debug commit as this really isn't something for Fedora. So strictly speaking I did not see a copr_build github status back then either. But where are these logged, if not on https://dashboard.packit.dev/jobs ?

@mfocko
Copy link
Member

mfocko commented Sep 29, 2022

@martinpitt sorry about the trouble, I have a guess it's related to the deployment, I will check and fix it

@mfocko
Copy link
Member

mfocko commented Sep 29, 2022

@martinpitt it should be fixed already, @majamassarini rebuilt and imported image with the correct version of Packit :)

@martinpitt
Copy link

@mfocko : Niiice! I re-did the test release, and this time I see the workflow runs from copr/packit, the packit dashboard page, and the build in my COPR. That has the correct targets, so all is well now. So now, only the documentation needs to be updated.

Yay, now off to cleaning this up for our real projects 🎉

Thanks a lot! This really makes maintenance a lot easier on our side.

@mfocko
Copy link
Member

mfocko commented Sep 29, 2022

I know about the documentation, Friday work :) :D

martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Sep 29, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue Sep 29, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
martinpitt added a commit to martinpitt/cockpit that referenced this issue Sep 29, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
marusak pushed a commit to cockpit-project/cockpit-podman that referenced this issue Oct 3, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
marusak pushed a commit to cockpit-project/cockpit-machines that referenced this issue Oct 3, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
marusak pushed a commit to cockpit-project/cockpit that referenced this issue Oct 3, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
marusak pushed a commit to cockpit-project/cockpit-ostree that referenced this issue Oct 3, 2022
Hardcoding this list is redundant and hard to change: changing the COPR
config and updating all our projects' packit.yml files needs to happen
in lockstep.

packit now learned to just read the configuration from an existing COPR
repository, so drop the target list entirely. See
packit/packit-service#1499
lsm5 pushed a commit to containers/qm that referenced this issue Aug 8, 2023
* packit: Drop explicit target lists for rhcontainerbot COPRs

If they differ from the actually configured target list in the COPR, the
build will either fail (if the packit user wasn't granted admin rights), or
actually reconfigure the COPR -- neither is desirable.

Specifying the target list is not necessary any more since
packit/packit-service#1499 got fixed.

Signed-off-by: Martin Pitt <[email protected]>

* packit: Build PRs into default packit COPRs

Building all PRs of all container projects into the same COPR does not
properly isolate PRs from each other.

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 should only run against what
landed in qm/main i.e. the podman-next COPR.

Signed-off-by: Martin Pitt <[email protected]>

---------

Signed-off-by: Martin Pitt <[email protected]>
Signed-off-by: Martin Pitt <[email protected]>
cfergeau pushed a commit to containers/gvisor-tap-vsock that referenced this issue Aug 16, 2023
Building all PRs of all container projects into the same COPR does not
properly isolate PRs from each other.

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 should only run against what
landed in gvisor-tap-vsock/main i.e. the podman-next COPR.

Unmerging the two copr jobs allows us the redundant targets: list for the
"main" builds into podman-next. Specifying the target list is not necessary
any more since packit/packit-service#1499 got fixed.

Signed-off-by: Martin Pitt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
3 participants