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 check for file to migrate and seed database #776

Closed
wants to merge 1 commit into from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Nov 26, 2019

This is working to replace RPMs running migration steps by driving the database migration and seed via the existence of files. There are two approaches that could be taken:

  1. Create a file when migration or seed occurs, have users or packages remove those files to signal migration/seed is needed
  2. Have users or packages create files to signal a migration/seed is needed and have the intsaller only run these steps if those files exist and then remove them

I went back and forth and landed on a more installer centric view via #1. I perceived benefits were:

  • the installer just works initially since it does not rely on something creating a file first
  • this allows some tracking of the last time migrations or seeds were ran via the file timestamp
  • packages can remove the files to signal the need for installation

I'm open to option #2 as an alternative this just puts a burden on external creation of these files for the initial installer run (which can reasonably be expected but feels bad).

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 was thinking of two helpers that we'll ship in /usr/libexec. The first one that returns whether a migration is needed via an exit code so you can also script foreman-should-migrate || foreman-rake db:migrate. The other would clear the migration marker. Something like foreman-request-migration. For all of these we'll also need something for seeding. Then the %post packaging macro will essentially run foreman-request-migration instead of actually migrating. These scripts can be simple shell scripts.

I would prefer keeping these wrappers in foreman.git. By keeping them in foreman.git, we can later easily decide to change the implementation. If other configuration management tools (like Ansible) want to use the same mechanism, they can. We can even decide to make sure db:migrate and db:seed actually creates the files.

I also think the markers should live in /var/lib/foreman.

@@ -14,6 +15,7 @@
logoutput => 'on_failure',
refreshonly => true,
timeout => $timeout,
unless => $unless,
Copy link
Member

Choose a reason for hiding this comment

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

You can also expose the creates parameter which is closer to what you want now

~> foreman_config_entry { 'db_pending_seed':
value => false,
dry => true,
} ~>
file { '/etc/foreman/database_seeded':
Copy link
Member

Choose a reason for hiding this comment

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

You should do this after seeding.

@ekohl
Copy link
Member

ekohl commented Dec 9, 2019

Given #778, should we close this out?

@ehelms ehelms closed this Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants