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

Reimplement repoquery in a more generic way #10514

Draft
wants to merge 2 commits into
base: rpm/develop
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Feb 29, 2024

This approach is based on rpmdist-repoquery where a certain directory structure is used to track repositories. That is then selected, together with a releasever.

Our own repositories are made generic by using DNF_VAR_$var, which makes branching easier.

It also uses true modularity. For that it hacks together a temporary installroot and uses fakeroot to pretend to be root, because dnf insists you need to be root to enable a module. Due to only using real DNF, we can rely on the modulary metadata and dependency resolution to select the correct modules. Again, reducing duplication.

A major motivation for this is the upcoming CentOS Stream 8 EOL where the repositories will be archived. This new repository structure allows easy switching to another Enterprise Linux for repoclosure checks.

This needs a different approach in how we configure repoclosure in obal and the wrapper scripts should probably live in another repository as well. Another excercise is the EL7 repoclosure check.

dnfwrapper Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

This is essentially the same as rpmdistro-repoquery but without the repoquery in the last line. We can talk to the maintainers to provide this. While talking about this I was pointed to fedrq as well, which may be nicer.

Comment on lines +3 to +6
DNF_VAR_foremanver=nightly
DNF_VAR_katellover=nightly
DNF_VAR_candlepinver=nightly
DNF_VAR_puppetver=7
Copy link
Member Author

Choose a reason for hiding this comment

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

It would be nice if these were provided by the package manifest.

ekohl added 2 commits May 31, 2024 12:42
This makes it easier to update to version 8.
This approach is based on rpmdist-repoquery where a certain directory
structure is used to track repositories. That is then selected, together
with a releasever.

Our own repositories are made generic by using DNF_VAR_$var, which makes
branching easier.

It also uses true modularity. For that it hacks together a temporary
installroot and uses fakeroot to pretend to be root, because dnf insists
you need to be root to enable a module. Due to only using real DNF, we
can rely on the modulary metadata and dependency resolution to select
the correct modules. Again, reducing duplication.

A major motivation for this is the upcoming CentOS Stream 8 EOL where
the repositories will be archived. This new repository structure allows
easy switching to another Enterprise Linux for repoclosure checks.

This needs a different approach in how we configure repoclosure in obal
and the wrapper scripts should probably live in another repository as
well. Another excercise is the EL7 repoclosure check.
@ekohl ekohl force-pushed the rpm/develop-improve-repos branch from d1746d1 to 486bb75 Compare May 31, 2024 10:43
@ekohl
Copy link
Member Author

ekohl commented May 31, 2024

Rebased to resolve conflicts. I've also opened #10869 for the first commit.

@@ -0,0 +1 @@
/usr/share/rpmdistro-repoquery/distros/centos-stream-legacy/centos-stream-legacy.repo
Copy link
Member

Choose a reason for hiding this comment

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

This file comes from rpmdistro-repoquery, right?

checks https://src.fedoraproject.org/rpms/rpmdistro-repoquery

So EPEL9 will be required on the systems, OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. My idea was that we would push off the actual repo configs to something common, rather than maintaining it ourselves. We can also choose to copy them of course.

[foreman-$foremanver-staging]
name=Foreman $foremanver staging EL$releasever
baseurl=https://stagingyum.theforeman.org/foreman/$foremanver/el$releasever/$basearch/
module_hotfixes=1
Copy link
Member

Choose a reason for hiding this comment

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

is this required given you actually enable modules?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe not, probably a leftover from earlier testing.

Comment on lines +29 to +30
repoclosure centos-stream 9 "" --check foreman-${DNF_VAR_foremanver}-staging --enablerepo puppet7
repoclosure centos-stream-legacy 8 foreman-devel:el8 --check foreman-${DNF_VAR_foremanver}-staging --enablerepo puppet7
Copy link
Member

Choose a reason for hiding this comment

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

this hard-codes closure for foreman.
how would I call it for katello?
how for the case where we add a repo on the CLI to perform closure on a copr build?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the area where I ended the day of learning. I imagine the next step would be to something work this into obal so we can use it for our *-packaging repositories. Ideally set the variables from package_manifest.yaml.

Still, I think this script gives a good start to see the general idea.

@evgeni
Copy link
Member

evgeni commented Jun 3, 2024

My biggest concern here is that this lives in the wrong place.

Today we have three places that do repoclosure: foreman-packaging, pulpcore-packaging, candlepin-packaging.
They all use the same pattern: repoclosure/yum.conf defines the repos available, package_manifest.yml the repos used for each individual target and obal has a playbook that stitches that all together to execute it.

We certainly shouldn't have to have copies of dnfwrapper and repoclosure in each of those to make it work.

As for fedrq: it more reads as a "different" repoquery, not repoclosure, which is another beast in itself, imho

@ekohl
Copy link
Member Author

ekohl commented Jun 3, 2024

As I wrote in #10514 (comment), I agree. My ideal is that in package_manifest.yaml we can specify the operating systems to run on (AlmaLinux 8, 9, CentOS Stream 9) at the top level and repositories to enable (foreman-staging, foreman-plugins-staging, katello, puppet, etc) for each package group

But then we also have COPR that may replace parts, and my brain was full.

@ekohl
Copy link
Member Author

ekohl commented Jun 3, 2024

Notes to self: jenkins jobs invokes obal in 2 ways.

First of all, there is repoclosure in RPM pipelines:

obal(
    action: 'repoclosure',
    packages: "${repo}-repoclosure-${dist}"
)

These correspond to:

repoclosures:
hosts:
foreman-staging-repoclosure-el8:
repoclosure_target_repos:
el8:
- "el8-foreman-{{ foreman_version }}-staging"
repoclosure_target_dist: el8
repoclosure_lookaside_repos:
el8:
- el8-baseos
- el8-appstream
- el8-powertools
- el8-extras
- el8-puppet-7
- "el8-candlepin-{{ candlepin_version }}-staging"
foreman-staging-repoclosure-el9:
repoclosure_target_repos:
el9:
- "el9-foreman-{{ foreman_version }}-staging"
repoclosure_target_dist: el9
repoclosure_lookaside_repos:
el9:
- el9-baseos
- el9-appstream
- el9-puppet-7
- "el9-candlepin-{{ candlepin_version }}-staging"
plugins-staging-repoclosure-el8:
repoclosure_target_repos:
el8:
- "el8-foreman-plugins-{{ foreman_version }}-staging"
repoclosure_target_dist: el8
repoclosure_lookaside_repos:
el8:
- el8-baseos
- el8-appstream
- el8-powertools
- el8-configmanagement-salt
- el8-extras
- el8-puppet-7
- "el8-foreman-{{ foreman_version }}-staging"
plugins-staging-repoclosure-el9:
repoclosure_target_repos:
el9:
- "el9-foreman-plugins-{{ foreman_version }}-staging"
repoclosure_target_dist: el9
repoclosure_lookaside_repos:
el9:
- el9-baseos
- el9-appstream
- el9-configmanagement-salt
- el9-puppet-7
- "el9-foreman-{{ foreman_version }}-staging"
katello-staging-repoclosure-el8:
repoclosure_target_repos:
el8:
- "el8-katello-{{ katello_version }}-staging"
repoclosure_target_dist: el8
repoclosure_lookaside_repos:
el8:
- el8-baseos
- el8-appstream
- el8-powertools
- el8-extras
- el8-puppet-7
- "el8-foreman-{{ foreman_version }}-staging"
- "el8-foreman-plugins-{{ foreman_version }}-staging"
- "el8-candlepin-{{ candlepin_version }}-staging"
katello-staging-repoclosure-el9:
repoclosure_target_repos:
el9:
- "el9-katello-{{ katello_version }}-staging"
repoclosure_target_dist: el9
repoclosure_lookaside_repos:
el9:
- el9-baseos
- el9-appstream
- el9-puppet-7
- "el9-foreman-{{ foreman_version }}-staging"
- "el9-foreman-plugins-{{ foreman_version }}-staging"
- "el9-candlepin-{{ candlepin_version }}-staging"
foreman-client-staging-repoclosure-el9:
repoclosure_target_repos:
el9:
- "el9-foreman-client-{{ foreman_version }}-staging"
repoclosure_target_dist: el9
repoclosure_lookaside_repos:
el9:
- el9-baseos
- el9-appstream
- el9-puppet-7
foreman-client-staging-repoclosure-el8:
repoclosure_target_repos:
el8:
- "el8-foreman-client-{{ foreman_version }}-staging"
repoclosure_target_dist: el8
repoclosure_lookaside_repos:
el8:
- el8-baseos
- el8-appstream
- el8-powertools
- el8-epel
- el8-extras
- el8-puppet-7
foreman-client-staging-repoclosure-el7:
repoclosure_target_repos:
el7:
- "el7-foreman-client-{{ foreman_version }}-staging"
repoclosure_target_dist: el7
repoclosure_lookaside_repos:
el7:
- el7-base
- el7-updates
- el7-epel
- el7-extras
- el7-puppet-7

And then for individual packages when they've been built:

obal(
    action: "repoclosure",
    packages: package_name,
    extraVars: [
        'repoclosure_check_repos': [repo['url']],
        'repoclosure_target_dist': repo['dist']
    ]
)

These correspond to (among others):

repoclosure_target_repos:
el9:
- "el9-foreman-client-{{ foreman_version }}-staging"
el8:
- "el8-foreman-client-{{ foreman_version }}-staging"
el7:
- "el7-foreman-client-{{ foreman_version }}-staging"
repoclosure_lookaside_repos:
el9:
- el9-baseos
- el9-appstream
- el9-puppet-7
el8:
- el8-baseos
- el8-appstream
- el8-powertools
- el8-extras
- el8-puppet-7
el7:
- el7-base
- el7-updates
- el7-epel
- el7-extras
- el7-puppet-7

@ekohl
Copy link
Member Author

ekohl commented Jun 3, 2024

Let's first start with deduplicating the configs: #10881 reuses settings in the manifest.

@ehelms
Copy link
Member

ehelms commented Jun 3, 2024

My first impression is we make dnfwrapper a helper function within Obal's module_utils, and then we adapt the repoclosure module logic to do what the repoclosure script in here is doing. And we'll need some way to intelligently decide if we do it the old or the new way.

@ekohl
Copy link
Member Author

ekohl commented Jun 3, 2024

I was starting to lean to the same thing. Some further notes for myself below.

Essentially one use case calls it with:

repoclosure_target_repos:
  el8:
    - "foreman-{{ foreman_version }}-staging"
repoclosure_lookaside_repos:
  el8:
    - $OS_REPOS
    - puppet-7
repoclosure_target_dist: el8

Then the other sets repoclosure_target_dist and adds repoclosure_check_repos:

repoclosure_target_repos:
  el8:
    - "foreman-{{ foreman_version }}-staging"
repoclosure_lookaside_repos:
  el8:
    - $OS_REPOS
    - puppet-7
repoclosure_target_dist: el8
repoclosure_check_repos:
  - $url

The design I'm starting to lean to (and that's where #10881 tries to advance to) is to always refer to some "host" for indirection. Each group can have a basename which gets appended with a repoclosure_target_dist. Then for each package we only need to add repoclosure_check_repos.

This may require some redesign in obal itself, but I think it'll reduce duplication.

@ekohl
Copy link
Member Author

ekohl commented Jun 13, 2024

Further notes to self: from #10918 (comment)

We apply repoclosure 1-by-1. I think we should consider an approach where we group things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants