RFC: Remove Database Actions in RPM Post Scripts

What about driving these settings through the installer or foreman-maintain? Generally, I don’t think the RPMs should be messing around with the databases. This requires that the database be active at the time of the yum install/update and that Foreman is configured to talk to the proper database if for example upgrading the database during a version upgrade.

I wasn’t saying it needs to be in the database:

It can also be as simple as touching a marker file. As long as we have a clear way of detecting that the database needs attention I think we can make it work. The installer calls foreman-rake on the command line so that can look at files. Perhaps Rails also has similar functionality built in.

From the installer we want to query something if the database migration/seeding is needed. The implementation isn’t that important.

If I recall correctly it was to signal the installer it should run migrations/seed as well as letting user know, the app is not ready to be used yet.

https://github.com/theforeman/foreman/commit/8f9b6994f9ca1fcbb55d2e827193ad1dd46bf388

I’m not 100% sure the latter part was also implemented though I think I’ve seen some page where the app refused to start because the DB was not migrated. I can’t find the code for that though.

What about making use of native Rails code to detect pending migrations? (https://stackoverflow.com/questions/1349047/show-pending-migrations-in-rails)

I don’t know how to do this for seeds given they should by definition be idempotent and have no tracking mechanism.

There are two situations where this occurs:

  • New plugin install
  • Upgrade

The installer/maintain workflows make these easy. I get the puppet module users it’s harder. I think if anything I might like the file the best since its the cleanest and least dependent on other systems.

I suspect that when this was built in Foreman, either Rails didn’t have it or we didn’t know it. I’d be all in favor of relying on our framework as much as possible.

Right, so I think here we should keep a mechanism. In the puppet module you don’t want to run rake db:seed every 30 minutes. I think the current db_pending_seed works well for new installs and the Puppet use case. However, we do need to think about upgrades.

I’m thinking about a if File.exist? /var/lib/foreman/needs-seed could be a good addition. Then the %foreman_db_seed macro would basically be a touch /var/lib/foreman/needs-seed.

My main issue with local files is that if you have multiple application servers you might end up with multiple seeds. However, that’s already an issue and implementing this would actually reduce migrating/seeding.

I hear alot from support and customers that having something in the app or hte cli that says “DB needs to be upgraded, yo” would be very beneficial.

Pleeeease take the DB items out of the rpm POST tasks. I have 30 foreman servers clustered with 10 plugins. Upgrades take me “forever” because the same DB rake tasks run ~300 times per upgrade

https://projects.theforeman.org/issues/19228

That bug is super old, i’ve since taken steps to ensure foreman installer only “manages rake” on one node, but the rpms do it no matter what still every server every plugin…

I’m actually trying with a full A/B setup now. Goal being I just:
setup a new version/clusters entirely
backup DB
copy it to “B side”
Run rake “once” to upgrade it.
Failover all loadbalancers to use B side.
Migrate/re register dedicated Puppet CA smart proxy over to B side for new servers and update it. (Once we hit puppet 6 I’m hopeful loadbalanced CA can make this A/B as well…

I’ll probably orchestrate it all with ansible if it works. I’ll lose a few reports, but it will be better than the 18 to 24 hours it takes me now simply waiting for all the redundant rakes to finish…

2 Likes

Is this something we can get as a change for 2.0? Sitting here again and waiting for foreman-rake to finish for hours! :frowning:

That is my goal. I’m starting with migrations since they are easier and able to be made natively idempotent within the puppet module [1]. Once I merge this I’ll be able to remove the migrate from the RPMs all together.

The database seeding is a little harder given the seeds themselves by Rails definition should be idempotent so we can’t explicitly detect if seeds need to run. For this, I think we plan to take the creating a file approach to trigger the installer when an RPM updates.

[1] https://github.com/theforeman/puppet-foreman/pull/778

2 Likes

We do have a couple of settings that might be useful in this regard - DB pending migration and DB pending seed. iiuc they were introduced (in https://github.com/theforeman/foreman/commit/8f9b6994f9ca1fcbb55d2e827193ad1dd46bf388) exactly for the purpose of knowing if the migrations and seed should be run. Maybe we can use them better to avoid these cases of running the same tasks multiple times?

For migrations we don’t need to since Rails provides a task db:abort_if_pending_migrations which should also provide idempotency. With seeding I agree we should build something in Foreman (core) since I don’t think Rails provides something similar. There is a discussion about that and I’d welcome insights from the Foreman core perspective.

The requirement from the installer is simple: there needs to be a way to verify if a migration/seed needs to happen.

Looks like nice bigger changes are stacking up for 2.0 :slight_smile:

I will add to that my goal to get subscription API + webhooks + shell hooks plugins.

Big up to @ehelms to push this, it’s a long overdue.

1 Like

I am adding an update here to point folks at a change in behavior proposal for the way seeding is handled. The last part of this puzzle is how to make database seeding happen only if there is a change. The solution a few of us are aligning to can be found in this pr.

Essentially, a check is being added on Rails startup that computes a hash of known seed files. If this hash differs from the previously stored hash, seeding is ran, if the hash is the same, seeding is skipped. This avoids needing to set any configuration values to trigger seeding and moves it out of the installer entirely. The traditional use of rake db:seed is preserved to force seeds to run.

1 Like

This work is effectively complete with nightlies having been updated with a Foreman RPM and half the available plugins dropping all posttrans RPM actions.

There is a running list of plugins still to be updated in a PR that I will be removing plugins from as they are updateable. In some cases the plugins do not build due to age or dependency changes (e.g. JS stack changes) and will need to be updated to drop the posttrans from.

As a quick recap:

  • Migrations are now detected by puppet-foreman and ran if any are pending every time
  • Seeds are now executed on application start but are only ran when changes are detected
  • Apipie cache indexing is handled by the installer
  • RPMs will no longer restart the Foreman application

TLDR for RPM environments, either use and trust the installer or puppet modules directly or perform these actions manually when updating Foreman or a plugin.

This should make all yum commands, especially yum update, quicker as well when updating Foreman or 1 or more plugins.

1 Like

I wonder, we already detect if migrations are pending and present a message to users to if they try to access the application in https://github.com/theforeman/foreman/blob/develop/app/controllers/concerns/foreman/controller/migration_checker.rb#L5 - Why not also do that on initialization and run the migrations if needed?
Thinking about it, the migrations should run prior to the seeds so if we seed on startup we should make sure that there are no pending migrations first or the seed might fail.

Every bullet point you described makes me SO HAPPY!!

How far “back” will these changes go? Or will it be only 1.25/2.0 and higher?

I’ve been effectively “stuck” on 1.19 because upgrades take 24+ hours of downtime to complete in my environment. But the light at the end of the tunnel makes me want to get moving on upgrades!! Wondering how many I’ll need to get relief?

Last time my workaround for this was:

  1. Add exit 0 on top of foreman-rake
  2. Run yum update
  3. After it updates the Foreman package, add exit 0 again to foreman-rake
  4. After yum completed remove the exit 0
  5. Run foreman-rake manually

It perhaps managed to run foreman-rake one time during yum before I could re-add the exit 0, this is why I ran it manually to ensure and it is not perfect as workaround.

Note that this is only done when it’s done when it’s refreshed. This happens after a DB migrate/seed and installing a plugin. If there’s only a plugin update that’s already installed and has no DB changes then we’re very likely to not update the apipie cache index.