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

Require copr build job definition for running tests #1775

Closed
4 tasks
lbarcziova opened this issue Nov 24, 2022 · 8 comments
Closed
4 tasks

Require copr build job definition for running tests #1775

lbarcziova opened this issue Nov 24, 2022 · 8 comments
Assignees
Labels
area/user-experience Usability issue

Comments

@lbarcziova
Copy link
Member

lbarcziova commented Nov 24, 2022

Currently, users can define only the test job and we will run Copr build before triggering the tests (if skip_build=False). With adding new features (identifiers, monorepo support, Image builder, ..) this has been causing a lot of "guessing" and updating code to match both the explicit definition of the build job and implicit.

We discussed we should start requiring a Copr build job definition for running tests:

  • open PRs with updated Packit configurations of projects where the build job definition is missing
  • update the documentation
  • (have some "deprecation" period: in this period we could give e.g. a comment to PRs/commits if the Packit configuration misses build job and warn users about upcoming change) - maybe not needed if we do the updating of configs
  • after that, if there will be a test job defined (skip_build=False) without any build job, we will report to user that the build definition is required via statuses and not run the build+tests
@lbarcziova lbarcziova moved this from new to backlog in Packit Kanban Board Nov 24, 2022
@TomasTomecek TomasTomecek added the area/user-experience Usability issue label Dec 13, 2022
@lbarcziova lbarcziova moved this from backlog to ready-to-refine in Packit Kanban Board Jan 6, 2023
@majamassarini majamassarini moved this from ready-to-refine to refined [10] in Packit Kanban Board Jan 12, 2023
lbarcziova added a commit to lbarcziova/tmt that referenced this issue Jan 12, 2023
As described in teemtee#1760 (comment),
Packit is planning to require explicit build job definition for the testing jobs
(see packit/packit-service#1775).
Adding the explicit Copr build job should also prevent the current bug in Packit
related to usage of implicit build job and usage of identifiers.
lbarcziova added a commit to lbarcziova/tmt that referenced this issue Jan 13, 2023
As described in teemtee#1760 (comment),
Packit is planning to require explicit build job definition for the testing jobs
(see packit/packit-service#1775).
Adding the explicit Copr build job should also prevent the current bug in Packit
related to usage of implicit build job and usage of identifiers.
@FrNecas FrNecas self-assigned this Jan 30, 2023
@FrNecas FrNecas moved this from refined [10] to in-progress [10] in Packit Kanban Board Jan 30, 2023
@FrNecas
Copy link

FrNecas commented Feb 21, 2023

Since we have to introduce a mechanism for the config updates, there are some issues that need to be further discussed, mainly concerning rate limiting. I am putting my notes here (in case I don't manage to finish the card and someone else started working on this).

We want to store the packit configs (the current state on the main/master branch) in the database:

  • introduce a handler that reacts to git push, this is simple. Can we somehow detect the default branch or do we have to hardcode master/main and not support other possible names?
  • to start with, we do not want to wait for projects to have a push/merge to main, we want to know the current state of configs without any user action so that we can find projects that need modifications.

This means that we have to fetch the configs. This is problematic due to rate limiting:

  • on prod there are ~33,000 git projects enabled, for each we have to check 4 possible packit names using /repos/{owner}/{repo}/contents/{path} Github endpoint (or Gitlab equivalent). This requires at least 120,000 requests.
  • Github organization token seems to have a limit of 1000 requests per hour, personal token 5000 per hour according to the docs.
  • It does not seem feasible to try to fetch the configs during database migration (this would take too long). The migration should just introduce an empty table and then we need to import it "on the side"
  • One possible solution that came to my mind is fetching the configs using personal token (the script may run for a day or two but it shouldn't be a problem) and storing it in a format that can then be imported into the DB later.

@csomh
Copy link
Contributor

csomh commented Feb 21, 2023

> * to start with, we do not want to wait for projects to have a push/merge to main, we want to know the current state of configs without any user action so that we can find projects that need modifications.

Sorry if I missed something: why wouldn't we wait for user actions?

I thought the idea is to store the config on a per-pipeline basis, something like: when a GitHub event triggers a pipeline, the package config in that specific commit is saved in the pipeline and reused when subsequent events (Copr build progress, TF progress) move that pipeline forwards.

Apologies, wrong context 😄

@FrNecas
Copy link

FrNecas commented Feb 21, 2023

Yup, I was about to write that we are probably talking about a different thing :D No worries ;)

@lbarcziova
Copy link
Member Author

  • on prod there are ~33,000 git projects enabled

😮
How did you come to this number?

@FrNecas
Copy link

FrNecas commented Feb 21, 2023

SELECT COUNT(*) from git_projects on db dump from prod. Most of these won't have config enabled but we should probably check them anyway, right? Or maybe we could go based on projects which have pipelines attached, that would significantly reduce the number...

@lbarcziova
Copy link
Member Author

Or maybe we could go based on projects which have pipelines attached, that would significantly reduce the number...

Agreed to do some kind of filtering (probably not all 33000 are really "relevant" projects).

@lachmanfrantisek
Copy link
Member

lachmanfrantisek commented Feb 22, 2023

Regarding the numbers, we have all the dist-git projects saved in the database. (Because of processing the messages from the Fedora-messaging bus..)

Here are the numbers since 2022-01-01 (https://prod.packit.dev/api/usage?from=2022-01-01 yes, it's slow and can timeout on the first run... but caching helps for the second try...;)

{
  "all_projects": {
    "project_count": 30328,
    "instances": {
      "salsa.debian.org": 1,
      "gitlab.gnome.org": 2,
      "src.fedoraproject.org": 22175,
      "git.centos.org": 25,
      "gitlab.freedesktop.org": 4,
      "gitlab.com": 9,
      "github.com": 8112
    }
  },
  "active_projects": {
    "project_count": 254,
    "top_projects_by_events_handled": SKIPPED
    "instances": {
      "github.com": 246,
      "gitlab.freedesktop.org": 3,
      "gitlab.com": 3,
      "gitlab.gnome.org": 2
    }
  },

Active projects = at least one pipeline in the time period.


edit: In the API output, there is a list of active projects that should have the config in a repo.

@lachmanfrantisek lachmanfrantisek moved this from in-progress [10] to refined [10] in Packit Kanban Board Mar 8, 2023
@lbarcziova lbarcziova self-assigned this Mar 30, 2023
@lbarcziova lbarcziova moved this from refined to in-progress in Packit Kanban Board Mar 30, 2023
lbarcziova added a commit to lbarcziova/research that referenced this issue Apr 12, 2023
The script provides 3 commands (see particular command's --help for more info):
1. download-configs - to download the configs of the repos that used Packit Service in the past year
2. list-affected - to list the projects affected by particular migration
3. migrate - to update the config and create PR

Related to packit#159
Related to packit/packit-service#1775
lbarcziova added a commit to lbarcziova/research that referenced this issue Apr 12, 2023
@lbarcziova
Copy link
Member Author

lbarcziova commented Apr 12, 2023

With the script introduced in packit/research#176:

$ ./packit_config_checker.py list-affected migrations/tests_without_build_definition.py packit_configurations

Listing affected URLs from directory packit_configurations...
Affected repos (18): 
https://github.com/Dirout/depi
https://github.com/cockpit-project/cockpit-podman
https://github.com/pcahyna/tmt
https://github.com/martinpitt/cockpit-podman
https://github.com/Dirout/dokkoo
https://github.com/thrix/fmf
https://github.com/psss/python-nitrate
https://github.com/martinpitt/python-dbusmock
https://github.com/Dirout/ticky
https://github.com/cockpit-project/cockpit-machines
https://github.com/neoave/mrack
https://github.com/cockpit-project/starter-kit
https://github.com/martinpitt/cockpit-machines
https://github.com/fedora-iot/zezere
https://github.com/fedora-iot/greenboot
https://github.com/teemtee/fmf
https://github.com/Dirout/larz
https://github.com/cockpit-project/cockpit-certificates

lbarcziova added a commit to lbarcziova/packit-service that referenced this issue Apr 18, 2023
Add pre-check for TestingFarmHandler that checks whether there is a copr
build job definition in the config, if not, report it to user and do
not run the tests.

Related to packit#1775
lbarcziova added a commit to lbarcziova/ticky that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/larz that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/depi that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/greenboot that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.

Signed-off-by: Laura Barcziova <[email protected]>
martinpitt pushed a commit to cockpit-project/starter-kit that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/mrack that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
martinpitt pushed a commit to cockpit-project/cockpit-certificates that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/zezere that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.

Signed-off-by: Laura Barcziova <[email protected]>
martinpitt pushed a commit to martinpitt/python-dbusmock that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
martinpitt pushed a commit to cockpit-project/cockpit-podman that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/python-nitrate that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
martinpitt pushed a commit to cockpit-project/cockpit-machines that referenced this issue Apr 19, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
Tiboris pushed a commit to neoave/mrack that referenced this issue Apr 20, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
lbarcziova added a commit to lbarcziova/research that referenced this issue Apr 20, 2023
The script provides 3 commands (see particular command's --help for more info):
1. download-configs - to download the configs of the repos that used Packit Service in the past year
2. list-affected - to list the projects affected by particular migration
3. migrate - to update the config and create PR

Related to packit#159
Related to packit/packit-service#1775
lbarcziova added a commit to lbarcziova/research that referenced this issue Apr 20, 2023
lbarcziova added a commit to packit/configuration-migrations that referenced this issue Apr 25, 2023
The script provides 3 commands (see particular command's --help for more info):
1. download-configs - to download the configs of the repos that used Packit Service in the past year
2. list-affected - to list the projects affected by particular migration
3. migrate - to update the config and create PR

Related to packit/packit-service#1775
lbarcziova added a commit to packit/configuration-migrations that referenced this issue Apr 25, 2023
@lbarcziova lbarcziova moved this from in-progress to in-review in Packit Kanban Board Apr 25, 2023
lbarcziova added a commit to lbarcziova/packit.dev that referenced this issue May 4, 2023
softwarefactory-project-zuul bot added a commit to packit/packit.dev that referenced this issue May 4, 2023
Require copr build job definition for tests

Related to packit/packit-service#1775
Do not merge this before the configurations in affected repos are updated (or at least wait some time after PRs are opened)

Reviewed-by: Jiri Popelka
Reviewed-by: Matej Focko
Tiboris pushed a commit to Tiboris/mrack that referenced this issue May 4, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.

Signed-off-by: Tibor Dudlák <[email protected]>
@lbarcziova lbarcziova added the blocked We are blocked! label May 10, 2023
lbarcziova added a commit to lbarcziova/packit-service that referenced this issue May 17, 2023
Add pre-check for TestingFarmHandler that checks whether there is a copr
build job definition in the config, if not, report it to user and do
not run the tests.

Related to packit#1775
softwarefactory-project-zuul bot added a commit that referenced this issue May 17, 2023
Require copr build job definition for tests

Add pre-check for TestingFarmHandler that checks whether there is a copr build job definition in the config, if not, report it to user and do not run the tests.
Related to #1775
Do not merge this before the configurations in affected repos are updated (or at least wait some time after PRs are opened)

RELEASE NOTES BEGIN
Packit will now additionally require for each test job requiring build a build job definition to be present in the Packit configuration file.
RELEASE NOTES END

Reviewed-by: Jiri Popelka
Reviewed-by: František Lachman
@lbarcziova lbarcziova removed the blocked We are blocked! label May 17, 2023
@github-project-automation github-project-automation bot moved this from in-review to done in Packit Kanban Board May 17, 2023
psss pushed a commit to lbarcziova/fmf that referenced this issue May 25, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
psss pushed a commit to lbarcziova/fmf that referenced this issue May 25, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.
nullr0ute pushed a commit to fedora-iot/greenboot that referenced this issue Jun 5, 2023
Packit will now additionally require for each test job requiring build
a build job definition to be present in the Packit configuration file. See details
in packit/packit-service#1775.

Signed-off-by: Laura Barcziova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Usability issue
Projects
Archived in project
Development

No branches or pull requests

5 participants