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

Add rpm repo profile #2118

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Add rpm repo profile #2118

merged 1 commit into from
Sep 13, 2024

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Aug 16, 2024

This is to provide the code to match the structure -- #1937

puppet/modules/web/manifests/vhost/rpm.pp Show resolved Hide resolved
puppet/modules/web/manifests/vhost/rpm.pp Outdated Show resolved Hide resolved
puppet/modules/profiles/manifests/repo/rpm.pp Outdated Show resolved Hide resolved
@ehelms ehelms force-pushed the add-rpm-scaffolding branch 2 times, most recently from f6e4a4d to a8989ef Compare August 22, 2024 13:23
puppet/modules/web/files/stagingrpm/robots.txt Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
<h1>Pulpcore packages</h1>

These are RPM builds for <a href="https://pulpproject.org">Pulp 3</a> and various plugins for use by <a href="https://theforeman.org/plugins/katello/">Katello</a>. They are only intended to be used by Katello. Only branches used by Katello are maintained. No explicit end of life announcements will be made.
Copy link
Member

Choose a reason for hiding this comment

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

Should we also link to https://github.com/theforeman/forklift/blob/master/vagrant/config/versions.yaml to figure out that mapping?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think we should try to display this mapping on https://theforeman.github.io/foreman-plugin-overview/ ?

Copy link
Member

Choose a reason for hiding this comment

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

That's an interesting thought I hadn't considered. The URL might need to be changed to match the title (Foreman landscape) to reflect the broader scope.

Copy link
Member

Choose a reason for hiding this comment

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

I was recently talking to @archanaserver and she also expressed confusion that this important information is so well hidden in a config file in some almost random repo.

So yeah, big 👍 on exposing that data somewhere in a nice way, but obviously shouldn't block this PR.

puppet/modules/web/templates/deploy-rpmrepo.sh.epp Outdated Show resolved Hide resolved
puppet/modules/profiles/manifests/repo/rpm.pp Outdated Show resolved Hide resolved
puppet/modules/web/files/rpm/robots.txt Show resolved Hide resolved
puppet/modules/web/files/stagingrpm/HEADER.html Outdated Show resolved Hide resolved
puppet/modules/web/files/stagingrpm/HEADER.html Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/deploy-rpmrepo.sh.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/deploy-rpmrepo.sh.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
@ehelms ehelms force-pushed the add-rpm-scaffolding branch 2 times, most recently from d48916b to a4d0d27 Compare August 30, 2024 01:42
@ehelms
Copy link
Member Author

ehelms commented Aug 30, 2024

If we look at the structure (from #1937):

├── candlepin
│   ├── 4.1
│   └── 4.2
├── foreman
│   ├── 3.9
│   │   ├── client
│   │   ├── foreman
│   │   ├── katello
│   │   └── plugins
│   └── nightly
│       ├── client
│       ├── foreman
│       ├── katello
│       └── plugins
└── pulpcore
    ├── 3.22
    ├── 3.28
    └── nightly

Given we won't have per katello-version repositories, I wonder if having a Katello repository makes sense. Let's look at what is in the Katello repo:

  • katello RPMs
  • Pulp API gems
  • Plugins that require Katello
  • a few rubygem and nodejs dependencies

I believe the three primary reasons we keep Katello separate today:

  • Katello has a larger number of RPMs for itself and it's plugins
  • Easier to have dedicated pipelines with a dedicated repository
  • Katello RPMs are signed, plugins are not

I think the first point is not such a problem anymore, and could be reduced further through meta RPM elimination (perhaps through single scenario per installer RPM change).

We could start signing plugins as we've discussed.

The pipelines part we've discussed and never arrived at a good answer for.

So perhaps doing it through release RPMs is the easiest. And we can simplify a bit at least by aligning GPG signing key with this structure.

@ehelms ehelms marked this pull request as ready for review August 30, 2024 19:43
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I wouldn't mind dropping the specific katello repository. It does mean some additional preparation like updating procedures in theforeman-rel-eng and some RPMs, but it would resolve a long standing issue.

puppet/modules/web/manifests/vhost/rpm.pp Outdated Show resolved Hide resolved
puppet/modules/web/manifests/vhost/stagingrpm.pp Outdated Show resolved Hide resolved
puppet/modules/web/manifests/vhost/stagingrpm.pp Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
puppet/modules/web/templates/rpm/HEADER.html.epp Outdated Show resolved Hide resolved
@ehelms
Copy link
Member Author

ehelms commented Sep 4, 2024

I am feeling good about this and think it's ready for merge an getting applied to the host so that I can proceed with additional testing and updates.

@ehelms
Copy link
Member Author

ehelms commented Sep 13, 2024

Based on conversation here -- #1937 (comment) I would like to:

  • Merge this change to update rpm-repo1 to have the vhosts and structure for handling new releases
  • Follow up PR to add vhost to handle yum.theforeman.org

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

One small nit, but I think we can start rolling out the machine to experiment with. Then from there we can write up the exact migration plan.

puppet/data/common.yaml Outdated Show resolved Hide resolved
@@ -241,3 +242,14 @@ sudo::wheel_config: password
redmine::https: true

apache::default_vhost: false

rsync_usernames:
Copy link
Member

Choose a reason for hiding this comment

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

I should have added a comment, but I started this file with custom definitions first. Then around like 12 I started to map things. With the introduction of stable_release I think that already broke (and I should have noticed it there).

My question: how should we manage this? Do we want to keep things that we regularly change (i.e., real data) at the top while keeping more "internals" near the bottom or keep them close to each other since they're closely connected

Copy link
Member Author

Choose a reason for hiding this comment

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

Split it into two files? That would be easier for me to understand. And I imagine be more obvious to newcomers.

Copy link
Member

Choose a reason for hiding this comment

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

Two files sounds good to me!

Copy link
Member Author

Choose a reason for hiding this comment

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

I am going to treat this as a follow up and merge this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented this idea here

@ehelms ehelms merged commit 0a2b93c into theforeman:master Sep 13, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants