RFC: Remove Database Actions in RPM Post Scripts

ehelms · September 22, 2017, 8:19pm

Howdy,

There have been recent conversations that have popped up on PRs, for
example [1], and IRC conversations around whether or not our RPM packages
should be performing database actions and restarting services. This thread
is intended to gather feedback and view points to arrive a community
decision on whether or not we should continue this behavior, alter it with
limitation or out right get rid of it.

This mostly happens within Foreman and some plugins, and the actions
performed today:

database migrations
database seeds
apipie cache
httpd restart
foreman-tasks restart

There may be others, these are the ones I am aware of. The history of these
actions, as I understand it, is so that in theory you can yum install a
plugin and, without further action, the Foreman server continue to run now
with your plugin.

Now, for my personal view point. Our application stack is fairly complex,
and there are a decently large number high percentage install plugins and
ecosystem of plugins in general. Plugins performing these sorta actions as
part of yum install has the potential to create unintended consequences. We
have created an idempotent installer to manage our server installations for
a reason, to help orchestrate changes, provide a framework for known and
coordinated change. And that these types of actions should be strictly
relegated to it.

I encourage all Foreman and plugin developers to please weigh in so that we
may reach consensus.

Thanks for your time and consideration.

[1] https://github.com/theforeman/foreman-packaging/pull/1761

···

-- Eric D. Helms Red Hat Engineering

Chris_Duryee · September 23, 2017, 12:28am

I would like if the only code in the RPM scripts was to land the bits on
the system's disk and nothing more. If the RPM scripts break, it is
difficult to find out what happened, and we already provide
foreman-installer to handle updating the system. The RPM scripts doing
things like db:seed and restarts can cause confusion when the application
starts up in a half-ready state during a maintenance window. Many users are
not aware that these types of things can even happen via %post, and are
surprised by it. They are also surprised if application error messages or
unusual return codes appear during RPM install.

···

On Fri, Sep 22, 2017 at 4:19 PM, Eric D Helms wrote:

Howdy,

There have been recent conversations that have popped up on PRs, for
example [1], and IRC conversations around whether or not our RPM packages
should be performing database actions and restarting services. This thread
is intended to gather feedback and view points to arrive a community
decision on whether or not we should continue this behavior, alter it with
limitation or out right get rid of it.

This mostly happens within Foreman and some plugins, and the actions
performed today:

database migrations

database seeds

apipie cache

httpd restart

foreman-tasks restart

There may be others, these are the ones I am aware of. The history of
these actions, as I understand it, is so that in theory you can yum install
a plugin and, without further action, the Foreman server continue to run
now with your plugin.

Now, for my personal view point. Our application stack is fairly complex,
and there are a decently large number high percentage install plugins and
ecosystem of plugins in general. Plugins performing these sorta actions as
part of yum install has the potential to create unintended consequences. We
have created an idempotent installer to manage our server installations for
a reason, to help orchestrate changes, provide a framework for known and
coordinated change. And that these types of actions should be strictly
relegated to it.

ekohl · September 23, 2017, 2:02pm

While I generally like it when a yum install just works™, I do get
the feeling that doing it for every plugin causes a lot of extra load
that shouldn't be needed. Ideally you could queue up tasks like a rake
db:migrate, apipie:cache:index and restart until everything is done but
I'm not sure yum can do this and if so how well that'll work.

Another question I have is whether the apipie cache can be incrementally
generated or if it's a full regenerate every time.

···

On Fri, Sep 22, 2017 at 04:19:43PM -0400, Eric D Helms wrote: >Howdy, > >There have been recent conversations that have popped up on PRs, for >example [1], and IRC conversations around whether or not our RPM packages >should be performing database actions and restarting services. This thread >is intended to gather feedback and view points to arrive a community >decision on whether or not we should continue this behavior, alter it with >limitation or out right get rid of it. > >This mostly happens within Foreman and some plugins, and the actions >performed today: > > * database migrations > * database seeds > * apipie cache > * httpd restart > * foreman-tasks restart > >There may be others, these are the ones I am aware of. The history of these >actions, as I understand it, is so that in theory you can yum install a >plugin and, without further action, the Foreman server continue to run now >with your plugin. > >Now, for my personal view point. Our application stack is fairly complex, >and there are a decently large number high percentage install plugins and >ecosystem of plugins in general. Plugins performing these sorta actions as >part of yum install has the potential to create unintended consequences. We >have created an idempotent installer to manage our server installations for >a reason, to help orchestrate changes, provide a framework for known and >coordinated change. And that these types of actions should be strictly >relegated to it. > >I encourage all Foreman and plugin developers to please weigh in so that we >may reach consensus. > >Thanks for your time and consideration. > > >[1] https://github.com/theforeman/foreman-packaging/pull/1761

Andrew_Schofield · September 24, 2017, 10:54pm

[I'm a user, not a developer]

I'd suggest that the RPM's simply drop the files onto the file system and
the installer then does the required actions. There are a lot of moving
parts to foreman and restarting one component can have impact on other
components. They also duplicate actions which take place in the installer.
The actions taken by users are (should be) yum update then a run of the
installer.

···

On Friday, September 22, 2017 at 4:19:46 PM UTC-4, Eric Helms wrote: > > Howdy, > > There have been recent conversations that have popped up on PRs, for > example [1], and IRC conversations around whether or not our RPM packages > should be performing database actions and restarting services. This thread > is intended to gather feedback and view points to arrive a community > decision on whether or not we should continue this behavior, alter it with > limitation or out right get rid of it. > > This mostly happens within Foreman and some plugins, and the actions > performed today: > > * database migrations > * database seeds > * apipie cache > * httpd restart > * foreman-tasks restart > > There may be others, these are the ones I am aware of. The history of > these actions, as I understand it, is so that in theory you can yum install > a plugin and, without further action, the Foreman server continue to run > now with your plugin. > > Now, for my personal view point. Our application stack is fairly complex, > and there are a decently large number high percentage install plugins and > ecosystem of plugins in general. Plugins performing these sorta actions as > part of yum install has the potential to create unintended consequences. We > have created an idempotent installer to manage our server installations for > a reason, to help orchestrate changes, provide a framework for known and > coordinated change. And that these types of actions should be strictly > relegated to it. > > I encourage all Foreman and plugin developers to please weigh in so that > we may reach consensus. > > Thanks for your time and consideration. > > > [1] https://github.com/theforeman/foreman-packaging/pull/1761 > > -- > Eric D. Helms > Red Hat Engineering >

Marek_Hulan · September 25, 2017, 6:02am

I'd be in favor of a change and avoid running scripts in post scripts. This is
the reason why we added "Optional Step 3 © - Run foreman-installer" to our
manual [1] a long time ago. We recommend running installer after upgrade if
users use it for initial setup. If this is too heavy step, maybe foreman-
maintain task could be added that would ensure all is up to date.

[1] Foreman :: Manual

···

-- Marek

On pondělí 25. září 2017 0:54:10 CEST Andrew Schofield wrote:

[I’m a user, not a developer]

I’d suggest that the RPM’s simply drop the files onto the file system and
the installer then does the required actions. There are a lot of moving
parts to foreman and restarting one component can have impact on other
components. They also duplicate actions which take place in the installer.
The actions taken by users are (should be) yum update then a run of the
installer.

On Friday, September 22, 2017 at 4:19:46 PM UTC-4, Eric Helms wrote:

Howdy,

There have been recent conversations that have popped up on PRs, for
example [1], and IRC conversations around whether or not our RPM packages
should be performing database actions and restarting services. This thread
is intended to gather feedback and view points to arrive a community
decision on whether or not we should continue this behavior, alter it with
limitation or out right get rid of it.

This mostly happens within Foreman and some plugins, and the actions

performed today:

database migrations

database seeds

apipie cache

httpd restart

foreman-tasks restart

There may be others, these are the ones I am aware of. The history of
these actions, as I understand it, is so that in theory you can yum
install
a plugin and, without further action, the Foreman server continue to run
now with your plugin.

Now, for my personal view point. Our application stack is fairly complex,
and there are a decently large number high percentage install plugins and
ecosystem of plugins in general. Plugins performing these sorta actions as
part of yum install has the potential to create unintended consequences.
We
have created an idempotent installer to manage our server installations
for
a reason, to help orchestrate changes, provide a framework for known and
coordinated change. And that these types of actions should be strictly
relegated to it.

I encourage all Foreman and plugin developers to please weigh in so that
we may reach consensus.

Thanks for your time and consideration.

[1] https://github.com/theforeman/foreman-packaging/pull/1761

lzap · September 25, 2017, 6:30am

I am all for pulling all complex changes out of post scriplets.

On the other hand, I like that not calling an installer was always an
option, at least for minor releases. We have lots of users (mainly in
downstream) who did not buy into Puppet and they tend to modify lots
of configs manually trying to avoid installer runs as possible. This
change would make this required otherwise database wont be migrated.

I would like to have an external script that would do the work but you
could still run it outside of the installer. This is kinda Satellite 5
experience, which is not bad at all I think.

LZ

···

On Mon, Sep 25, 2017 at 8:02 AM, Marek Hulán wrote: > I'd be in favor of a change and avoid running scripts in post scripts. This is > the reason why we added "Optional Step 3 (C) - Run foreman-installer" to our > manual [1] a long time ago. We recommend running installer after upgrade if > users use it for initial setup. If this is too heavy step, maybe foreman- > maintain task could be added that would ensure all is up to date. > > [1] https://theforeman.org/manuals/1.15/#UpgradingtoForeman1.15 > > -- > Marek > > On pondělí 25. září 2017 0:54:10 CEST Andrew Schofield wrote: >> [I'm a user, not a developer] >> >> I'd suggest that the RPM's *simply* drop the files onto the file system and >> the installer then does the required actions. There are a lot of moving >> parts to foreman and restarting one component can have impact on other >> components. They also duplicate actions which take place in the installer. >> The actions taken by users are (should be) yum update then a run of the >> installer. >> >> On Friday, September 22, 2017 at 4:19:46 PM UTC-4, Eric Helms wrote: >> > Howdy, >> > >> > There have been recent conversations that have popped up on PRs, for >> > example [1], and IRC conversations around whether or not our RPM packages >> > should be performing database actions and restarting services. This thread >> > is intended to gather feedback and view points to arrive a community >> > decision on whether or not we should continue this behavior, alter it with >> > limitation or out right get rid of it. >> > >> > This mostly happens within Foreman and some plugins, and the actions >> > >> > performed today: >> > * database migrations >> > * database seeds >> > * apipie cache >> > * httpd restart >> > * foreman-tasks restart >> > >> > There may be others, these are the ones I am aware of. The history of >> > these actions, as I understand it, is so that in theory you can yum >> > install >> > a plugin and, without further action, the Foreman server continue to run >> > now with your plugin. >> > >> > Now, for my personal view point. Our application stack is fairly complex, >> > and there are a decently large number high percentage install plugins and >> > ecosystem of plugins in general. Plugins performing these sorta actions as >> > part of yum install has the potential to create unintended consequences. >> > We >> > have created an idempotent installer to manage our server installations >> > for >> > a reason, to help orchestrate changes, provide a framework for known and >> > coordinated change. And that these types of actions should be strictly >> > relegated to it. >> > >> > I encourage all Foreman and plugin developers to please weigh in so that >> > we may reach consensus. >> > >> > Thanks for your time and consideration. >> > >> > >> > [1] https://github.com/theforeman/foreman-packaging/pull/1761 > > > -- > You received this message because you are subscribed to the Google Groups "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

–
Later,
Lukas @lzap Zapletal

ohadlevy · September 25, 2017, 7:51am

> I am all for pulling all complex changes out of post scriplets.
>
> On the other hand, I like that not calling an installer was always an
> option, at least for minor releases. We have lots of users (mainly in
> downstream) who did not buy into Puppet and they tend to modify lots
> of configs manually trying to avoid installer runs as possible. This
> change would make this required otherwise database wont be migrated.
>
> I would like to have an external script that would do the work but you
> could still run it outside of the installer. This is kinda Satellite 5
> experience, which is not bad at all I think.
>

Can we start by extracting the code into a script, and then executing the
script from post rpm trans? I dislike the fact that we are now introducing
a must have step (rake db:migrate db:seed apipie:cache:index ) which was
not required before.

This will also can be reflected in the UI where if we have a pending db
migration for example, we can ask the user to execute that script manually
if needed.

Ohad

···

On Mon, Sep 25, 2017 at 9:30 AM, Lukas Zapletal wrote:

LZ

On Mon, Sep 25, 2017 at 8:02 AM, Marek Hulán mhulan@redhat.com wrote:

I’d be in favor of a change and avoid running scripts in post scripts.
This is
the reason why we added “Optional Step 3 © - Run foreman-installer” to
our
manual [1] a long time ago. We recommend running installer after upgrade
if
users use it for initial setup. If this is too heavy step, maybe foreman-
maintain task could be added that would ensure all is up to date.

[1] Foreman :: Manual

–
Marek

On pondělí 25. září 2017 0:54:10 CEST Andrew Schofield wrote:

[I’m a user, not a developer]

I’d suggest that the RPM’s simply drop the files onto the file system
and

the installer then does the required actions. There are a lot of moving
parts to foreman and restarting one component can have impact on other
components. They also duplicate actions which take place in the
installer.

The actions taken by users are (should be) yum update then a run of the
installer.

On Friday, September 22, 2017 at 4:19:46 PM UTC-4, Eric Helms wrote:

Howdy,

There have been recent conversations that have popped up on PRs, for
example [1], and IRC conversations around whether or not our RPM
packages

should be performing database actions and restarting services. This
thread

is intended to gather feedback and view points to arrive a community
decision on whether or not we should continue this behavior, alter it
with

limitation or out right get rid of it.

This mostly happens within Foreman and some plugins, and the actions

performed today:

database migrations

database seeds

apipie cache

httpd restart

foreman-tasks restart

There may be others, these are the ones I am aware of. The history of
these actions, as I understand it, is so that in theory you can yum
install
a plugin and, without further action, the Foreman server continue to
run

now with your plugin.

Now, for my personal view point. Our application stack is fairly
complex,

and there are a decently large number high percentage install plugins
and

ecosystem of plugins in general. Plugins performing these sorta
actions as

part of yum install has the potential to create unintended
consequences.

We
have created an idempotent installer to manage our server
installations

for
a reason, to help orchestrate changes, provide a framework for known
and

coordinated change. And that these types of actions should be strictly
relegated to it.

I encourage all Foreman and plugin developers to please weigh in so
that

we may reach consensus.

Thanks for your time and consideration.

[1] https://github.com/theforeman/foreman-packaging/pull/1761

–
You received this message because you are subscribed to the Google
Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

–
Later,
Lukas @lzap Zapletal

–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

iNecas · September 25, 2017, 9:18am

The script sounds like a good start. If every rpm in post runs the
script, and the script
would be clever enough to know it it needs to run it twice or was
already run in the transaction.
We would have a win. Also we could have there some check (such as
properly placed file somewhere),
that could influence, if the script should or should not be run. Not
sure it I'm not over-complicating
the things by that. But anyway, having the script gives us the power
to control the logic from one place
as opposed to relying on the plugin specs.

– Ivan

···

On Mon, Sep 25, 2017 at 9:51 AM, Ohad Levy wrote: > > > On Mon, Sep 25, 2017 at 9:30 AM, Lukas Zapletal wrote: >> >> I am all for pulling all complex changes out of post scriplets. >> >> On the other hand, I like that not calling an installer was always an >> option, at least for minor releases. We have lots of users (mainly in >> downstream) who did not buy into Puppet and they tend to modify lots >> of configs manually trying to avoid installer runs as possible. This >> change would make this required otherwise database wont be migrated. >> >> I would like to have an external script that would do the work but you >> could still run it outside of the installer. This is kinda Satellite 5 >> experience, which is not bad at all I think. > > > Can we start by extracting the code into a script, and then executing the > script from post rpm trans? I dislike the fact that we are now introducing a > must have step (rake db:migrate db:seed apipie:cache:index ) which was not > required before. > > This will also can be reflected in the UI where if we have a pending db > migration for example, we can ask the user to execute that script manually > if needed. > > Ohad > >> >> LZ >> >> On Mon, Sep 25, 2017 at 8:02 AM, Marek Hulán wrote: >> > I'd be in favor of a change and avoid running scripts in post scripts. >> > This is >> > the reason why we added "Optional Step 3 (C) - Run foreman-installer" to >> > our >> > manual [1] a long time ago. We recommend running installer after upgrade >> > if >> > users use it for initial setup. If this is too heavy step, maybe >> > foreman- >> > maintain task could be added that would ensure all is up to date. >> > >> > [1] https://theforeman.org/manuals/1.15/#UpgradingtoForeman1.15 >> > >> > -- >> > Marek >> > >> > On pondělí 25. září 2017 0:54:10 CEST Andrew Schofield wrote: >> >> [I'm a user, not a developer] >> >> >> >> I'd suggest that the RPM's *simply* drop the files onto the file system >> >> and >> >> the installer then does the required actions. There are a lot of moving >> >> parts to foreman and restarting one component can have impact on other >> >> components. They also duplicate actions which take place in the >> >> installer. >> >> The actions taken by users are (should be) yum update then a run of the >> >> installer. >> >> >> >> On Friday, September 22, 2017 at 4:19:46 PM UTC-4, Eric Helms wrote: >> >> > Howdy, >> >> > >> >> > There have been recent conversations that have popped up on PRs, for >> >> > example [1], and IRC conversations around whether or not our RPM >> >> > packages >> >> > should be performing database actions and restarting services. This >> >> > thread >> >> > is intended to gather feedback and view points to arrive a community >> >> > decision on whether or not we should continue this behavior, alter it >> >> > with >> >> > limitation or out right get rid of it. >> >> > >> >> > This mostly happens within Foreman and some plugins, and the actions >> >> > >> >> > performed today: >> >> > * database migrations >> >> > * database seeds >> >> > * apipie cache >> >> > * httpd restart >> >> > * foreman-tasks restart >> >> > >> >> > There may be others, these are the ones I am aware of. The history of >> >> > these actions, as I understand it, is so that in theory you can yum >> >> > install >> >> > a plugin and, without further action, the Foreman server continue to >> >> > run >> >> > now with your plugin. >> >> > >> >> > Now, for my personal view point. Our application stack is fairly >> >> > complex, >> >> > and there are a decently large number high percentage install plugins >> >> > and >> >> > ecosystem of plugins in general. Plugins performing these sorta >> >> > actions as >> >> > part of yum install has the potential to create unintended >> >> > consequences. >> >> > We >> >> > have created an idempotent installer to manage our server >> >> > installations >> >> > for >> >> > a reason, to help orchestrate changes, provide a framework for known >> >> > and >> >> > coordinated change. And that these types of actions should be >> >> > strictly >> >> > relegated to it. >> >> > >> >> > I encourage all Foreman and plugin developers to please weigh in so >> >> > that >> >> > we may reach consensus. >> >> > >> >> > Thanks for your time and consideration. >> >> > >> >> > >> >> > [1] https://github.com/theforeman/foreman-packaging/pull/1761 >> > >> > >> > -- >> > You received this message because you are subscribed to the Google >> > Groups "foreman-dev" group. >> > To unsubscribe from this group and stop receiving emails from it, send >> > an email to foreman-dev+unsubscribe@googlegroups.com. >> > For more options, visit https://groups.google.com/d/optout. >> >> >> >> -- >> Later, >> Lukas @lzap Zapletal >> >> -- >> You received this message because you are subscribed to the Google Groups >> "foreman-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to foreman-dev+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. > > > -- > You received this message because you are subscribed to the Google Groups > "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

LangJV · February 4, 2018, 1:36am

This is an older thread - but i wanted to add my opinion as i was linked to it recently from here: Foreman-db-manage-rake usage?

I currently have:
18 puppet masters (smart proxy) with Foreman role installed
12 Foreman (no smart proxy) “UI” servers
6 “PXE” smart proxies (dhcp/tftp) with nothing else installed.

This is definitely “a bit overkill” but suffices to fill our need for full resiliency and full capacity across multiple (3 total) datacenters for 12,000 nodes with a lot (800 or so puppet classes) of puppet code being run, adding approximately 1-2k more nodes every month

My last few upgrades have been a nightmare due to the above RPM “issue”. From previous experience, i know that while Foreman “can” run in mixed mode with some roles upgraded and others not, with as many nodes (and api scripts/automation jobs/stuff running) as we have, i quickly hit our MYSQL database’s “max error count” for which the database starts blocking connections. Therefore, I tend to take everything offline during the upgrade until everything is completely upgraded. I can make the error count higher, but at some point that’s a losing battle versus just upgrading all at once, and really has no bearing on the overall “it takes forever for the same tasks to run hundreds of times” argument below

My first Foreman node takes about 2 hours, as with multiple plugins being upgraded, the RPM installers loop through the “migrate/seed/apipie cache” 5-6 times. I just watch the logs and watch the database keep running the “same.freaking.queries” in a 20-25 minute cycle.
Then i get to do this again, and again, and again. At some point, I just do the remainder in parallel, which causes the DB to get overloaded and “fail” on the seeds and migrates. I don’t generally care because its already all done…but still “scary” to be deliberately banking on errors to speed things up… However, the last few don’t error and i get to sit around and wait for them to finish, about 3-4 hours later.

My whole upgrade time now ends up being 13 hours of literally staring at nothing while i wait for the same migrate/seed/apipie cache process to run hundreds of times.
30 “Foreman” servers x 6 run’s per server for each plugin = 180 repetitive “runs” of migrate/seed/apipie cache…(while I bash my head against the wall dying of boredom)

Generally speaking my 2c (I’m not a developer, and I’m fine with this being disregarded…)

Need supported way(s) to clear large tables, reports has a rake task, as does sessions, but logs and audit don’t. Much of the seed tasks revolve around a ton of select statements around the audit table. I’d love to truncate it but i’m pretty sure that would cause issues. Same with logs and a few other tables that just seem to grow forever… the seed tasks are just slow with a giant audit table. If i could manage them - the seed jobs themselves might finish faster and speed things up significantly.
The seed and migrate rake tasks themselves should be augmented. upon success of each step (I’m thinking each migrate “step”, and each seed “step” as defined in the code currently), the step, as well as exact foreman version (1.15.6 or similar) should be recorded within the database itself within a specific (new) table within the database (only if it completes without error). Then the rake task(s) could also further be augmented to “check” the database/table prior to execution. If the specific task has already been run for the specific foreman version - skip it! This could easily allow as many RPM runs for each plugin, across many servers, as needed. The rake and migrate steps would be called, but would exit after a few seconds if everything already completed.
With the above - Install a new plugin, and it will have new migrate/seed tasks, which will still run as expected (once) and be recorded in the database for future use. Install it on multiple servers and the remaining servers “skip” the migrate/seed runs (if they did not fail).
This would then necessitate the seed and migrate rake tasks being augmented to support a “force” parameter/argument to allow manually re-running the whole shebang in case of some unforeseen issue…

This would allow you to keep the “RPM” running the rake tasks, while adding the needed intelligence to properly allow rake/migrate to run plugin specific changes, as well as across multiple servers, intelligently (and much much much quicker)…

Again, there may be factors here im not thinking of, but at the end of the day, making the database authoritative for which specific tasks need to run/rerun seems best, as it solves the multi-plugin loops as well as multi-server loops…

~Jason Lang

ehelms · February 18, 2018, 1:09pm

This script could be as simple as a foreman-rake-db-migrate script that does this logic and uses foreman-rake under the hood to run tasks. However, I do find myself asking could this script be foreman_maintain? Would that be overkill or “just right” for having a script that maintains the state of the database?

LangJV · February 18, 2018, 7:03pm

The only way i see it being Viable exclusively with foreman_maintain (which I’m not super experienced with) is to then remove the foreman-rake run (or replacement) altogether from the vanilla RPM’s. The whole argument for having the foreman-rake behavior there (in the RPM’s) is to allow for multiple use-cases. If the rake tasks are removed from the RPM’s - any installations or upgrades now require the use of foreman-installer (or foreman_maintain) negating the benefits of RPM’s, yum repositories, etc… Unless i’m misunderstanding how it all comes together?

lzap · February 19, 2018, 8:50am

I think that migration/seed are important parts of core and should be part of the core package. While I support moving most scripts under one roof, I think it is logical when you install a package you should not be asked to install a different one to finish the installation.

You may want to compare to our installer which also ships separately, but my argument is that you actually don’t need our installer to get the app and running. Everything is included with the core package at the moment (yes it’s bunch of work indeed but it’s possible).

ekohl · February 19, 2018, 10:11am

There is an issue open in katello to perform migrations. This also needs tracking of which tasks have been completed and is currently stored in the foreman-installer. It’d be worth checking if a more generic soution is possible. See Refactor #21938: Single upgrade rake task - Katello - Foreman

mbacovsky · February 19, 2018, 11:35pm

I can imagine foreman-maintain to call foreman-rake under the hood and performing some additional checks and turning maintainance mode on perhaps. Similar approach we use for the installer in foreman-maintain upgrade.
This way the raw logic can stay with the core while the foreman-maintain provides better UX.

ehelms · November 14, 2019, 9:11pm

I would like to revive this topic and scope it to database actions happening within the RPM post scripts. This actions are triggered when Foreman is installed or updated and when most plugins are installed and updated. I would like to drop this from our RPMs all together. Are there any strong objections to this?

Users that don’t use the installer or puppet modules directly would be responsible for running these commands manually after upgrade or installation.

ekohl · November 14, 2019, 9:44pm

I think that the RPMs do need a post script, but just one that marks it as “needs migration” / “needs seeding”.

Currently we have the following code in Foreman itself. There the settings default to true:

github.com

theforeman/foreman/blob/23dbf2cd3f4a2aee9e4ed4aaffa294aecd90e591/app/models/setting/general.rb#L19-L20


self.set('db_pending_migration', N_("Should the `foreman-rake db:migrate` be executed on the next run of the installer modules?"), true, N_('DB pending migration')),
self.set('db_pending_seed', N_("Should the `foreman-rake db:seed` be executed on the next run of the installer modules?"), true, N_('DB pending seed')),

Then the installer looks at this setting whether a migration or seeding is needed. Since it’s default true even without a migrated database, these are executed:

github.com

theforeman/puppet-foreman/blob/f5b31e0f43dcbc0df7ab4a3713f16db42f5959af/manifests/database.pp#L24-L35


foreman_config_entry { 'db_pending_migration':
  value => false,
  dry   => true,
}
~> foreman::rake { 'db:migrate': }
~> foreman_config_entry { 'db_pending_seed':
  value => false,
  dry   => true,
}
~> foreman::rake { 'db:seed':
  environment => delete_undef_values($seed_env),
}

Then when the database is seeded, the settings are initialized in the database with the following code:

github.com

theforeman/foreman/blob/23dbf2cd3f4a2aee9e4ed4aaffa294aecd90e591/lib/tasks/config.rake#L171-L184


# The db_pending_* settings are handled by the installer. For case
# of not installing with installer, we want to ensure the settings to false
# at the end of the seeding
Rake::Task['db:seed'].enhance do
  %w(db_pending_migration db_pending_seed).each do |setting_name|
    if !Setting[setting_name].nil?
      Setting[setting_name] = false
    else
      setting = Setting::General.default_settings.detect { |s| s[:name] == setting_name }
      setting[:value] = false
      Setting.create! setting.update(:category => "Setting::General")
    end
  end
end

Now we have no code that ever sets it to true again. I would propose that we implement a mechanism where we replace the current %post hooks with a mechanism that makes sure these values are set to true again. That way, if the installer is ran the existing code will already pick and runs them.

Not sure if we should keep an optimized (opt in) implementation that still does the migrations. It can be useful in nightly or with plugins but I’m not sure how much this is still needed in production deployments. Do people use tools like yum-cron on their servers?

magnus · November 15, 2019, 7:54am

Yes we do on all of them.

ehelms · November 15, 2019, 1:19pm

Can you remind me why we introduced these settings rather than simply running migrations and seeds every time we run the installer?

ekohl · November 15, 2019, 1:31pm

This way the installer is actually idempotent. That’s very important for people who use the Puppet modules directory to manage their servers but also for the normal installer use case. It means that if nothing changes, the installer won’t just restart various services resulting in a quicker execution.

ekohl · November 15, 2019, 1:38pm

To add to that: it’s also used to make sure the upgrade rake task is never executed on an uninitialized database:

github.com

theforeman/foreman/blob/85c0b771faa278a46e3c5604158f0756a70d5c66/lib/tasks/upgrade.rake#L15-L16


raise "DB migration has not run" if Setting['db_pending_migration']
raise "DB seed has not run" if Setting['db_pending_seed']

Making sure this mechanism is correct would also prevent users from upgrading when it’s likely that will result in a broken setup.