rake db:migrate:status emits an exit status of 2 when migrations are required

[gee-forr]

Motivation / Background

This Pull Request has been created because previously during deployments I would need to write a hairy grep on the output of rails db:migrate:status to determine if I needed to run a migration. I've taken inspiration from bundle check which outputs a status code of 1 if a bundle install is required.

Now, when running a rails db:migrate:status, people can check the exit status, and make a decision to migrate or not based on that value.

Detail

This Pull Request changes the rake task db:migrate:status found in activerecord/lib/active_record/tasks/database_tasks.rb

Now, as part of rendering the list of migrations, it checks if any migrations are needed. If so, it will exit with a status code of 2. No migrations required will exit as normal with a status code of 0. There is an abort check before the task runs which will exit with a status code of 1, which is why I elected to use 2.

Additional information

Here's how I was checking for outstanding migrations previously with Ansible:

---
- block:
  - name: Check for outstanding migrations
    ansible.builtin.shell:
      cmd:   bundle exec rails db:migrate:status  | grep --perl-regex "^\s+down\s+\d+"
    register:     migration_status
    changed_when: migration_status.rc == 0
    failed_when:  migration_status.rc > 1

  - name: Run any outstanding migrations
    ansible.builtin.shell:
      cmd:   bundle exec rails db:migrate
    when: migration_status.changed

This can now be simplified to:

---
- block:
  - name: Check for outstanding migrations
    ansible.builtin.shell:
      cmd:   bundle exec rails db:migrate:status
    register:     migration_status
    changed_when: migration_status.rc == 2
# 8< snip other tasks

Checklist

I could not find the documentation for this, and I'm not sure if this requires a CHANGELOG entry. Please let me know and I'll update.

Please also let me know if I must backport these changes to 6.1.x and 6.0.x?

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
• CI is passing. (unrelated tests are failing, i.e. mysql connection pools, and another suite is being killed after the 30 mins runtime timeout)

[eileencodes]

Hi @gee-forr thanks for the PR and welcome to Rail!

This Pull Request has been created because previously during deployments I would need to write a hairy grep on the output of rails db:migrate:status to determine if I needed to run a migration

I'd like to better understand why you need this. Why aren't you able to run db:migrate every time rather than checking the status? If a migration ran, it won't be re-run.

[gee-forr]

Hey @eileencodes, long time listener, first time caller :)

I'd like to better understand why you need this. Why aren't you able to run db:migrate every time rather than checking the status? If a migration ran, it won't be re-run.

This may be more specific to the way Ansible works, but I can see a use case for knowing whether or not to run a migrate, vs. blindly running it every deploy.

When an Ansible playbook runs, it provides a summary of changed elements upon completion. We currently do a regex on migrate:status's output to determine if migrations are required, and if not, skip the migrate task. This ensures that the amount of "changed" things during the deploy is as accurate as possible.

If we blindly ran a migrate every time, we also wouldn't be able to detect if the migration ran anything because it also doesn't provide an exit status, putting us back at square 1.

We're also moving over to K8S, and having db:migrate:status provide an exit status means that we can programatically kick off a K8S Job pod or not based on said exit status to run a migration. This would mean quicker deploys when no migrations are needed, and less K8S Job pods kicked off.

This is pretty much the same energy as what Dave Copeland is doing here with bundle install which can also be safely run over and over like migrate.

Also just ran some benchmarks... for a project with a fair number (200+) migrations, running a noop db:migrate on my machine (M1 Max 64GB) takes on average 7 secs. A db:migrate:status takes around 1s. This means we can save quite a bit of time by avoiding running noop migrations each deploy.

app@b5c9db693a8b:~$ time bundle exec rails db:migrate
Model files unchanged.

real	0m7.745s
user	0m0.284s
sys	0m0.065s

app@b5c9db693a8b:~$ time bundle exec rails db:migrate:status

database: xxxxxx_development

 Status   Migration ID    Migration Name
--------------------------------------------------
   up     20190926173858  Devise create users
8< snip around 200 migrations... >8
   up     20221014155516  Add quantity formula to rate card rule


real	0m0.844s
user	0m0.270s
sys	0m0.073s

[gee-forr]

Hey @eileencodes - I know you're hella busy, but I was wondering if you've had some time to go over my justification for having db:migrate:status emit a non-zero exit code when migrations are required?

[gee-forr]

Hi @eileencodes - happy new year... I hope you had a relaxing festive season?

Please let me know if there's anything I can do to get this PR over the finish line?

[eileencodes]

To be honest, I don't like the implementation but I don't have another suggestion at the moment. So I'm not sure what to tell you to work on - it just feels too complex for a simple status check. It also changes the behavior of applications and that might be surprising / undesirable.

[gee-forr]

Hey @eileencodes - thanks for your feedback :)

I totally see your point... as an alternative, would you be amenable to the following:

• reverting db:migrate:status to its original functionality
• adding a new task of db:migrate:check with stripped down functionality?

This will enable the following workflow:

rails db:migrate:check || rails db:migrate

This ☝🏻 approach has utility outside of just my Ansible use case above. Container entrypoints that include a migrate prior to boot would save time starting up, and k8s migration Jobs would also execute faster, as migrations - in my experience at least - happen way less per deploy compared to vanilla code changes.

Let me know :) And once again - thanks for the feedback, I appreciate you taking time out to respond to me.