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

Fixes #36579 - Split the installer into multiple subpackages #9564

Open
wants to merge 1 commit into
base: rpm/develop
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jul 11, 2023

This creates separate RPMs for each scenario. This allows the user to install just a single scenario and never use the --scenario parameter.

Packages created:

  • foreman-installer-scenario-$scenario
    foreman/katello/foreman-proxy-content scenarios
  • foreman-installer-common
    Puppet modules, Hiera data and other shared files
  • foreman-installer
    Transitional package that requires foreman-installer-scenario-foreman for a smooth migration

Split off from #8814 and requires theforeman/foreman-installer#870

This creates separate RPMs for each scenario. This allows the user to
install just a single scenario and never use the --scenario parameter.

Packages created:

foreman-installer-scenario-$scenario
  foreman/katello/foreman-proxy-content scenarios
foreman-installer-common
  Puppet modules, Hiera data and other shared files
foreman-installer
  Transitional package that requires foreman-installer-scenario-foreman
  for a smooth migration
@ekohl ekohl force-pushed the rpm/develop-36579-split-installer branch from 5575b24 to 7989af7 Compare July 11, 2023 12:35
@ekohl
Copy link
Member Author

ekohl commented Jul 11, 2023

Before I do the same to Debian packages I'd like some agreement on that this is a good structure.

@ekohl ekohl requested a review from evgeni July 11, 2023 14:49
Requires: rubygem(kafo) >= 6.5.0
Requires: rubygem(kafo) < 8.0.0
Requires: ruby(release)
# As a migration foreman-installer ensures the foreman scenario is installed
Copy link
Member

Choose a reason for hiding this comment

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

Two options/questions:

  • Could foreman-installer-foreman Obsolete foreman-installer and avoid this?
  • Could foreman-installer become the common package rather than a -common sub-package?

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 considered that, but I don't think you can do that.

Could foreman-installer-foreman Obsolete foreman-installer and avoid this?

Perhaps foreman-installer-scenario can have:

Provides: foreman-installer = %{epoch}:%{version}-%release
Obsoletes: foreman-installer < 1:3.8.0-somerelease

And that could replace it. I'm never sure so would need to test it out. But I think then we can't have a foreman-installer package.

I just realized we need a similar solution for katello, but there we may need to pull in both katello and foreman-proxy-content scenarios to remain compatible.

Could foreman-installer become the common package rather than a -common sub-package?

Yes, but then I fear we can't do the migration well. Perhaps we can list them as soft dependencies with Suggests, but I really don't trust that if I'm honest. Especially if we take downstream into account.

@evgeni any more ideas?

Copy link
Member

Choose a reason for hiding this comment

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

Hard no on "Suggests", that implies the package would work without the "suggested" one, but I don't think the installer will without the common bits ;-)

Can you elaborate more on your fear wrt "foreman-installer" is the "common" package? My brain suggests it should work, but its also partially in PTO mode.

And for me, to recap the plan and whether I understood it correctly:

  1. new users would just install foreman-installer-scenario-<thing> and that would make things work as needed
  2. existing foreman users would get foreman-installer-scenario-foreman and would still not have to pass --scenario and things will work as before
  3. existing katello users would get foreman-installer-scenario-katello AND foreman-installer-scenario-fpc (as we don't know, package wise, what they have deployed) and will have multiple scenarios available as before (but as they already installed, they don't need to pass --scenario anymore)
  4. existing satellite users (even if not modeled here) would get the same as katello, plus the satellite and capsule scenarios, again, with no behavioral changes.
  5. then, after a few releases, we can drop those transitional packages (the ones depending on the new foreman-installer-scenario-<thing>) and Obsolete them with the "right" <thing> package

One thing to consider: I know that we use the "is foreman-installer-katello installed" fact to differentiate between foreman and katello installs in a few places (like LEAPP), so we gotta go hunting for those and update them :)

Copy link
Member Author

Choose a reason for hiding this comment

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

new users would just install foreman-installer-scenario-<thing> and that would make things work as needed

Yes, we will modify the installation instructions. I also hope to introduce foreman-installer-scenario-foreman-proxy to provide a non-content proxy so there would be consistency.

existing foreman users would get foreman-installer-scenario-foreman and would still not have to pass --scenario and things will work as before

Correct. I hope to get to a point where dnf upgrade will ensure foreman-installer-scenario-foreman is installed when they now have foreman-installer installed.

existing katello users would get foreman-installer-scenario-katello AND foreman-installer-scenario-fpc (as we don't know, package wise, what they have deployed) and will have multiple scenarios available as before (but as they already installed, they don't need to pass --scenario anymore)

Correct, and because they have foreman-installer installed they will also get foreman-installer-scenario-foreman which they also don't need. Perhaps rich dependencies could help here.

In foreman_maintain we can suggest cleanups by looking at the symlink which scenario is active, but that's not going to work in plain dnf.

existing satellite users (even if not modeled here) would get the same as katello, plus the satellite and capsule scenarios, again, with no behavioral changes.

Correct. As above, satellite-maintain can suggest redundant packages. Though it depends on how much we want to patch downstream. In downstream we have satellite and capsule meta packages which could do the migration as well.

One thing to consider: I know that we use the "is foreman-installer-katello installed" fact to differentiate between foreman and katello installs in a few places (like LEAPP), so we gotta go hunting for those and update them :)

I don't see it show up in foreman_maintain. As for LEAPP: do we still support that? We don't support skipping over releases and we dropped EL7 a while back so by the time they get here, they should already be on EL8.

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 thinking ahead when we will do LEAPP for 8 to 9, reusing the code :)

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 like your optimism.

Copy link
Member

Choose a reason for hiding this comment

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

Correct. As above, satellite-maintain can suggest redundant packages. Though it depends on how much we want to patch downstream. In downstream we have satellite and capsule meta packages which could do the migration as well.

My thinking basically, the fact Satellite has meta packages to represent each type means they can require the respective scenario package and do the cleanup.

In all cases, we will know what scenario is installed on the system, and can use that information to do cleanup. We could consider using this as the same opportunity to move the answer files (theforeman/foreman-installer#698) but that might be scope creep. Figured I'd throw it out there as this new packaging provides an opportunity to signal other changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

In all cases, we will know what scenario is installed on the system, and can use that information to do cleanup. We could consider using this as the same opportunity to move the answer files (theforeman/foreman-installer#698) but that might be scope creep. Figured I'd throw it out there as this new packaging provides an opportunity to signal other changes.

It is something I also thought about. I mostly worry about the additional tooling that reads it, such as foreman_maintain. Perhaps we should provide a way to output the answers, like some script or some installer output option.

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

I think we can use rich deps for a slightly cleaner end result, but overall this should get us to where we want

Requires: rubygem(kafo) < 8.0.0
Requires: ruby(release)
# As a migration foreman-installer ensures the foreman scenario is installed
Requires: %{name}-scenario-foreman = %{epoch}:%{version}-%{release}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Requires: %{name}-scenario-foreman = %{epoch}:%{version}-%{release}
Requires: (%{name}-scenario-foreman = %{epoch}:%{version}-%{release} unless (%{name}-katello or %{name}-scenario-katello or %{name}-scenario-foreman-proxy-content))

I think this should allow users to not get the foreman scenario if they already have something katello-ish

Comment on lines +62 to +63
Requires: foreman-installer-scenario-katello = %{epoch}:%{version}-%{release}
Requires: foreman-installer-scenario-foreman-proxy-content = %{epoch}:%{version}-%{release}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Requires: foreman-installer-scenario-katello = %{epoch}:%{version}-%{release}
Requires: foreman-installer-scenario-foreman-proxy-content = %{epoch}:%{version}-%{release}
Requires: (foreman-installer-scenario-katello = %{epoch}:%{version}-%{release} unless foreman-installer-scenario-foreman-proxy-content)
Requires: (foreman-installer-scenario-foreman-proxy-content = %{epoch}:%{version}-%{release} unless foreman-installer-scenario-katello)

but I am not sure those two would cancel themself out and you'd end with a weird result?

Comment on lines +142 to +146
%{configdir}/foreman.migrations
%config(noreplace) %attr(600, root, root) %{scenariodir}/foreman.yaml
%config(noreplace) %attr(600, root, root) %{scenariodir}/foreman-answers.yaml
%config(noreplace) %{scenariodir}/foreman-migrations-applied
%{parser_cache}/foreman.yaml
Copy link
Member

Choose a reason for hiding this comment

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

this could become a macro files_for_scenario(name), but not sure if that'd be over-optimization or not

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 tried to write a macro but couldn't make it work. There's a good chance that's on my side but RPM documentation was insufficient.

@evgeni
Copy link
Member

evgeni commented Aug 8, 2023

You asked for Debian… I think there the situation is a tad easier. Given we have no Katello/FPC there, and there is no standalone-proxy scenario today, we can rather safely split things up into foreman-installer-common and foreman-installer-scenario-foreman (as here), make foreman-installer an empty, transitional package only pulling in f-i-s-foreman. Then, a few releases later, we can force the removal of the transitional package (using Breaks/Replaces) and drop it completely.

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