Remove migration downtime check since we do not allow downtime

For 4 years we've had logic in our database migrations that required you
to set a constant `DOWNTIME = true` if you required downtime and seek
approval from the VP of Engineering. We have never once used this
process as we've always found a way around the problem using a different
approach.

As such we decided in
https://gitlab.com/gitlab-org/gitlab/-/issues/326495 that we should just
remove this `DOWNTIME` constant and the extra checks here to reduce
noise in our database code review and give less processes for people to
learn.

This MR removes a lot of things and here is a high level summary:

1. Remove DowntimeCheck class and the rake task that invoked and the CI
job that invoked that rake task and any related tests, helper classes
2. Update documentation to make it clearer that downtime is not allowed
and therefore remove the approval process
3. Update a page called `what_requires_downtime` to be called
`avoiding_downtime_in_migrations` since it was already a set of patterns
to avoid downtime and now it's worded more strongly to imply that we can
always get away without downtime
4. Various other docs pages that had examples of migrations that used
the `DOWNTIME` constant
5. Various rubocop specs that had migrations in them which used the
`DOWNTIME` constant

The one thing I did not do is go back and remove `DOWNTIME = false` from
all the existing migrations. In general we should not be updating
migrations once they've run and this would have made this MR change many
thousands of files so it's not worth it.

.gitlab/ci/rails.gitlab-ci.yml

@@ -257,17 +257,6 @@ static-analysis:
     - run_timed_command "retry yarn install --frozen-lockfile"
     - scripts/static-analysis
 
-downtime_check:
-  extends:
-    - .rails-job-base
-    - .rails:rules:downtime_check
-  needs: []
-  stage: test
-  variables:
-    SETUP_DB: "false"
-  script:
-    - bundle exec rake downtime_check
-
 rspec migration pg11:
   extends:
     - .rspec-base-pg11

.gitlab/ci/rules.gitlab-ci.yml

@@ -880,11 +880,6 @@
       changes: *code-backstage-patterns
       when: on_failure
 
-.rails:rules:downtime_check:
-  rules:
-    - <<: *if-merge-request
-      changes: *code-backstage-patterns
-
 .rails:rules:deprecations:
   rules:
     - <<: *if-not-ee

doc/development/application_limits.md

@@ -43,8 +43,6 @@ It's recommended to create two separate migration script files.
    class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      def up
        create_or_update_plan_limit('project_hooks', 'default', 0)
        create_or_update_plan_limit('project_hooks', 'free', 10)

doc/development/what_requires_downtime.md → doc/development/avoiding_downtime_in_migrations.md

@@ -4,12 +4,13 @@ group: Database
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
 ---
 
-# What requires downtime?
+# Avoiding downtime in migrations
 
-When working with a database certain operations can be performed without taking
-GitLab offline, others do require a downtime period. This guide describes
-various operations, their impact, and how to perform them without requiring
-downtime.
+When working with a database certain operations may require downtime. Since we
+cannot have downtime in migrations we need to use a set of steps to get the
+same end result without downtime. This guide describes various operations that
+may appear to need downtime, their impact, and how to perform them without
+requiring downtime.
 
 ## Dropping Columns
 

doc/development/database/add_foreign_key_to_existing_column.md

@@ -66,8 +66,6 @@ Migration file for adding `NOT VALID` foreign key:
 class AddNotValidForeignKeyToEmailsUser < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     # safe to use: it requires short lock on the table since we don't validate the foreign key
     add_foreign_key :emails, :users, on_delete: :cascade, validate: false
@@ -94,8 +92,6 @@ Example for cleaning up records in the `emails` table within a database migratio
 class RemoveRecordsWithoutUserFromEmailsTable < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   class Email < ActiveRecord::Base
@@ -130,8 +126,6 @@ Migration file for validating the foreign key:
 class ValidateForeignKeyOnEmailUsers < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     validate_foreign_key :emails, :user_id
   end

doc/development/database/database_reviewer_guidelines.md

@@ -66,7 +66,7 @@ Finally, you can find various guides in the [Database guides](index.md) page tha
 topics and use cases. The most frequently required during database reviewing are the following:
 
 - [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations.
-- [What requires downtime?](../what_requires_downtime.md).
+- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md).
 - [SQL guidelines](../sql.md) for working with SQL queries.
 
 ## How to apply for becoming a database maintainer

doc/development/database/index.md

@@ -22,7 +22,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 ## Migrations
 
-- [What requires downtime?](../what_requires_downtime.md)
+- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md)
 - [SQL guidelines](../sql.md) for working with SQL queries
 - [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations
 - [Testing Rails migrations](../testing_guide/testing_migrations_guide.md) guide

doc/development/database/not_null_constraints.md

@@ -26,8 +26,6 @@ For example, consider a migration that creates a table with two `NOT NULL` colum
 
 ```ruby
 class CreateDbGuides < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
-
   def change
     create_table :db_guides do |t|
       t.bigint :stars, default: 0, null: false
@@ -47,8 +45,6 @@ For example, consider a migration that adds a new `NOT NULL` column `active` to
 
 ```ruby
 class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
-
   def change
     add_column :db_guides, :active, :boolean, default: true, null: false
   end
@@ -117,7 +113,6 @@ with `validate: false` in a post-deployment migration,
 ```ruby
 class AddNotNullConstraintToEpicsDescription < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 
@@ -191,7 +186,6 @@ migration helper in a final post-deployment migration,
 ```ruby
 class ValidateNotNullConstraintOnEpicsDescription < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 

doc/development/database/strings_and_the_text_data_type.md

@@ -52,8 +52,6 @@ For example, consider a migration that creates a table with two text columns,
 class CreateDbGuides < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     create_table_with_constraints :db_guides do |t|
       t.bigint :stars, default: 0, null: false
@@ -89,7 +87,6 @@ For example, consider a migration that adds a new text column `extended_title` t
 
 ```ruby
 class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
 
   # rubocop:disable Migration/AddLimitToTextColumns
   # limit is added in 20200501000002_add_text_limit_to_sprints_extended_title
@@ -106,8 +103,6 @@ A second migration should follow the first one with a limit added to `extended_t
 ```ruby
 class AddTextLimitToSprintsExtendedTitle < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   def up
@@ -185,8 +180,6 @@ in a post-deployment migration,
 class AddTextLimitMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   def up
@@ -266,7 +259,6 @@ helper in a final post-deployment migration,
 ```ruby
 class ValidateTextLimitMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 

doc/development/database_review.md

@@ -176,7 +176,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
 
 #### Preparation when removing columns, tables, indexes, or other structures
 
-- Follow the [guidelines on dropping columns](what_requires_downtime.md#dropping-columns).
+- Follow the [guidelines on dropping columns](avoiding_downtime_in_migrations.md#dropping-columns).
 - Generally it's best practice (but not a hard rule) to remove indexes and foreign keys in a post-deployment migration.
   - Exceptions include removing indexes and foreign keys for small tables.
 - If you're adding a composite index, another index might become redundant, so remove that in the same migration.
@@ -199,7 +199,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
   - Check that the relevant version files under `db/schema_migrations` were added or removed.
   - Check queries timing (If any): In a single transaction, cumulative query time executed in a migration
     needs to fit comfortably within `15s` - preferably much less than that - on GitLab.com.
-  - For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns)
+  - For column removals, make sure the column has been [ignored in a previous release](avoiding_downtime_in_migrations.md#dropping-columns)
 - Check [background migrations](background_migrations.md):
   - Establish a time estimate for execution on GitLab.com. For historical purposes,
     it's highly recommended to include this estimation on the merge request description.

doc/development/geo/framework.md

@@ -266,8 +266,6 @@ For example, to add support for files referenced by a `Widget` model with a
    class CreateWidgetRegistry < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up
@@ -399,8 +397,6 @@ can track verification state.
    # frozen_string_literal: true
 
    class AddVerificationStateToWidgets < ActiveRecord::Migration[6.0]
-     DOWNTIME = false
-
      def change
        change_table(:widgets) do |t|
          t.integer :verification_state, default: 0, limit: 2, null: false
@@ -427,8 +423,6 @@ can track verification state.
    class AddVerificationFailureLimitToWidgets < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      CONSTRAINT_NAME = 'widget_verification_failure_text_limit'
@@ -453,8 +447,6 @@ can track verification state.
    class AddVerificationIndexesToWidgets < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-     VERIFICATION_STATE_INDEX_NAME = "index_widgets_verification_state"
      PENDING_VERIFICATION_INDEX_NAME = "index_widgets_pending_verification"
      FAILED_VERIFICATION_INDEX_NAME = "index_widgets_failed_verification"
      NEEDS_VERIFICATION_INDEX_NAME = "index_widgets_needs_verification"
@@ -497,8 +489,6 @@ can track verification state.
    class CreateWidgetStates < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up
@@ -926,8 +916,6 @@ For example, to add support for files referenced by a `Gizmos` model with a
    class CreateGizmoRegistry < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up

doc/development/merge_request_performance_guidelines.md

@@ -18,7 +18,7 @@ To measure the impact of a merge request you can use
 the following guides:
 
 - [Performance Guidelines](performance.md)
-- [What requires downtime?](what_requires_downtime.md)
+- [Avoiding downtime in migrations](avoiding_downtime_in_migrations.md)
 
 ## Definition
 

doc/development/migration_style_guide.md

@@ -16,16 +16,12 @@ migrations are written carefully, can be applied online, and adhere to the style
 guide below.
 
 Migrations are **not** allowed to require GitLab installations to be taken
-offline unless _absolutely necessary_.
-
-When downtime is necessary the migration has to be approved by:
-
-1. The VP of Engineering
-1. A Backend Maintainer
-1. A Database Maintainer
-
-An up-to-date list of people holding these titles can be found at
-<https://about.gitlab.com/company/team/>.
+offline ever. Migrations always must be written in such a way to avoid
+downtime. In the past we had a process for defining migrations that allowed for
+downtime by setting a `DOWNTIME` constant. You may see this when looking at
+older migrations. This process was in place for 4 years without every being
+used and as such we've learnt we can always figure out how to write a migration
+differently to avoid downtime.
 
 When writing your migrations, also consider that databases might have stale data
 or inconsistencies and guard for that. Try to make as few assumptions as
@@ -65,47 +61,16 @@ scripts/regenerate-schema
 TARGET=12-9-stable-ee scripts/regenerate-schema
 ```
 
-## What Requires Downtime?
-
-The document ["What Requires Downtime?"](what_requires_downtime.md) specifies
-various database operations, such as
-
-- [dropping and renaming columns](what_requires_downtime.md#dropping-columns)
-- [changing column constraints and types](what_requires_downtime.md#changing-column-constraints)
-- [adding and dropping indexes, tables, and foreign keys](what_requires_downtime.md#adding-indexes)
-
-and whether they require downtime and how to work around that whenever possible.
-
-## Downtime Tagging
-
-Every migration must specify if it requires downtime or not, and if it should
-require downtime it must also specify a reason for this. This is required even
-if 99% of the migrations don't require downtime as this makes it easier to find
-the migrations that _do_ require downtime.
-
-To tag a migration, add the following two constants to the migration class'
-body:
-
-- `DOWNTIME`: a boolean that when set to `true` indicates the migration requires
-  downtime.
-- `DOWNTIME_REASON`: a String containing the reason for the migration requiring
-  downtime. This constant **must** be set when `DOWNTIME` is set to `true`.
+## Avoiding downtime
 
-For example:
+The document ["Avoiding downtime in migrations"](avoiding_downtime_in_migrations.md) specifies
+various database operations, such as:
 
-```ruby
-class MyMigration < ActiveRecord::Migration[6.0]
-  DOWNTIME = true
-  DOWNTIME_REASON = 'This migration requires downtime because ...'
-
-  def change
-    ...
-  end
-end
-```
+- [dropping and renaming columns](avoiding_downtime_in_migrations.md#dropping-columns)
+- [changing column constraints and types](avoiding_downtime_in_migrations.md#changing-column-constraints)
+- [adding and dropping indexes, tables, and foreign keys](avoiding_downtime_in_migrations.md#adding-indexes)
 
-It is an error (that is, CI fails) if the `DOWNTIME` constant is missing
-from a migration class.
+and explains how to perform them without requiring downtime.
 
 ## Reversibility
 
@@ -512,8 +477,6 @@ class like so:
 class MyMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   INDEX_NAME = 'index_name'
@@ -640,8 +603,6 @@ Take the following migration as an example:
 
 ```ruby
 class DefaultRequestAccessGroups < ActiveRecord::Migration[5.2]
-  DOWNTIME = false
-
   def change
     change_column_default(:namespaces, :request_access_enabled, from: false, to: true)
   end
@@ -849,8 +810,6 @@ Example migration adding this column:
 
 ```ruby
 class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0]
-  DOWNTIME = false
-
   def change
     add_column :ci_builds_metadata, :config_options, :jsonb
   end

doc/development/prometheus_metrics.md

@@ -39,8 +39,6 @@ Or, you can create a database migration:
 class ImportCommonMetrics < ActiveRecord::Migration[4.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end

doc/development/scalability.md

@@ -36,7 +36,7 @@ application starts, Rails queries the database schema, caching the tables and
 column types for the data requested. Because of this schema cache, dropping a
 column or table while the application is running can produce 500 errors to the
 user. This is why we have a [process for dropping columns and other
-no-downtime changes](what_requires_downtime.md).
+no-downtime changes](avoiding_downtime_in_migrations.md).
 
 #### Multi-tenancy
 

doc/development/sidekiq_style_guide.md

@@ -831,8 +831,6 @@ as shown in this example:
 class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name'
   end

generator_templates/active_record/migration/create_table_migration.rb

@@ -6,14 +6,6 @@
 class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>]
   include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
-  DOWNTIME = false
-
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
   # When using the methods "add_concurrent_index" or "remove_concurrent_index"
   # you must disable the use of transactions
   # as these methods can not run in an existing transaction.

generator_templates/active_record/migration/migration.rb

@@ -7,14 +7,6 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi
   # Uncomment the following include if you require helper functions:
   # include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
-  DOWNTIME = false
-
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
   # When using the methods "add_concurrent_index" or "remove_concurrent_index"
   # you must disable the use of transactions
   # as these methods can not run in an existing transaction.

generator_templates/rails/post_deployment_migration/migration.rb

@@ -7,8 +7,6 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi
   # Uncomment the following include if you require helper functions:
   # include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   # When using the methods "add_concurrent_index" or "remove_concurrent_index"
   # you must disable the use of transactions
   # as these methods can not run in an existing transaction.

lib/gitlab/downtime_check.rb

-# frozen_string_literal: true
-
-module Gitlab
-  # Checks if a set of migrations requires downtime or not.
-  class DowntimeCheck
-    # The constant containing the boolean that indicates if downtime is needed
-    # or not.
-    DOWNTIME_CONST = :DOWNTIME
-
-    # The constant that specifies the reason for the migration requiring
-    # downtime.
-    DOWNTIME_REASON_CONST = :DOWNTIME_REASON
-
-    # Checks the given migration paths and returns an Array of
-    # `Gitlab::DowntimeCheck::Message` instances.
-    #
-    # migrations - The migration file paths to check.
-    def check(migrations)
-      migrations.map do |path|
-        require(path)
-
-        migration_class = class_for_migration_file(path)
-
-        unless migration_class.const_defined?(DOWNTIME_CONST)
-          raise "The migration in #{path} does not specify if it requires " \
-            "downtime or not"
-        end
-
-        if online?(migration_class)
-          Message.new(path)
-        else
-          reason = downtime_reason(migration_class)
-
-          unless reason
-            raise "The migration in #{path} requires downtime but no reason " \
-              "was given"
-          end
-
-          Message.new(path, true, reason)
-        end
-      end
-    end
-
-    # Checks the given migrations and prints the results to STDOUT/STDERR.
-    #
-    # migrations - The migration file paths to check.
-    def check_and_print(migrations)
-      check(migrations).each do |message|
-        puts message.to_s # rubocop: disable Rails/Output
-      end
-    end
-
-    # Returns the class for the given migration file path.
-    def class_for_migration_file(path)
-      File.basename(path, File.extname(path)).split('_', 2).last.camelize
-        .constantize
-    end
-
-    # Returns true if the given migration can be performed without downtime.
-    def online?(migration)
-      migration.const_get(DOWNTIME_CONST, false) == false
-    end
-
-    # Returns the downtime reason, or nil if none was defined.
-    def downtime_reason(migration)
-      if migration.const_defined?(DOWNTIME_REASON_CONST)
-        migration.const_get(DOWNTIME_REASON_CONST, false)
-      else
-        nil
-      end
-    end
-  end
-end

lib/gitlab/downtime_check/message.rb

-# frozen_string_literal: true
-
-module Gitlab
-  class DowntimeCheck
-    class Message
-      attr_reader :path, :offline
-
-      OFFLINE = "\e[31moffline\e[0m"
-      ONLINE = "\e[32monline\e[0m"
-
-      # path - The file path of the migration.
-      # offline - When set to `true` the migration will require downtime.
-      # reason - The reason as to why the migration requires downtime.
-      def initialize(path, offline = false, reason = nil)
-        @path = path
-        @offline = offline
-        @reason = reason
-      end
-
-      def to_s
-        label = offline ? OFFLINE : ONLINE
-
-        message = ["[#{label}]: #{path}"]
-
-        if reason?
-          message << ":\n\n#{reason}\n\n"
-        end
-
-        message.join
-      end
-
-      def reason?
-        @reason.present?
-      end
-
-      def reason
-        @reason.strip.lines.map(&:strip).join("\n")
-      end
-    end
-  end
-end

lib/tasks/db_obsolete_ignored_columns.rake

@@ -20,7 +20,7 @@ task 'db:obsolete_ignored_columns' => :environment do
 
       WARNING: Removing columns is tricky because running GitLab processes may still be using the columns.
 
-      See also https://docs.gitlab.com/ee/development/what_requires_downtime.html#dropping-columns
+      See also https://docs.gitlab.com/ee/development/avoiding_downtime_in_migrations.html#dropping-columns
     TEXT
   end
 end

lib/tasks/downtime_check.rake

-# frozen_string_literal: true
-
-desc 'Checks if migrations in a branch require downtime'
-task downtime_check: :environment do
-  repo = if defined?(Gitlab::License)
-           'gitlab'
-         else
-           'gitlab-foss'
-         end
-
-  `git fetch https://gitlab.com/gitlab-org/#{repo}.git --depth 1`
-
-  Rake::Task['gitlab:db:downtime_check'].invoke('FETCH_HEAD')
-end

lib/tasks/gitlab/db.rake

@@ -80,22 +80,6 @@ namespace :gitlab do
       end
     end
 
-    desc 'GitLab | DB | Checks if migrations require downtime or not'
-    task :downtime_check, [:ref] => :environment do |_, args|
-      abort 'You must specify a Git reference to compare with' unless args[:ref]
-
-      require 'shellwords'
-
-      ref = Shellwords.escape(args[:ref])
-
-      migrations = `git diff #{ref}.. --diff-filter=A --name-only -- db/migrate`.lines
-        .map { |file| Rails.root.join(file.strip).to_s }
-        .select { |file| File.file?(file) }
-        .select { |file| /\A[0-9]+.*\.rb\z/ =~ File.basename(file) }
-
-      Gitlab::DowntimeCheck.new.check_and_print(migrations)
-    end
-
     desc 'GitLab | DB | Sets up EE specific database functionality'
 
     if Gitlab.ee?

spec/lib/gitlab/downtime_check/message_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::DowntimeCheck::Message do
-  describe '#to_s' do
-    it 'returns an ANSI formatted String for an offline migration' do
-      message = described_class.new('foo.rb', true, 'hello')
-
-      expect(message.to_s).to eq("[\e[31moffline\e[0m]: foo.rb:\n\nhello\n\n")
-    end
-
-    it 'returns an ANSI formatted String for an online migration' do
-      message = described_class.new('foo.rb')
-
-      expect(message.to_s).to eq("[\e[32monline\e[0m]: foo.rb")
-    end
-  end
-
-  describe '#reason?' do
-    it 'returns false when no reason is specified' do
-      message = described_class.new('foo.rb')
-
-      expect(message.reason?).to eq(false)
-    end
-
-    it 'returns true when a reason is specified' do
-      message = described_class.new('foo.rb', true, 'hello')
-
-      expect(message.reason?).to eq(true)
-    end
-  end
-
-  describe '#reason' do
-    it 'strips excessive whitespace from the returned String' do
-      message = described_class.new('foo.rb', true, " hello\n world\n\n foo")
-
-      expect(message.reason).to eq("hello\nworld\n\nfoo")
-    end
-  end
-end

spec/lib/gitlab/downtime_check_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::DowntimeCheck do
-  subject { described_class.new }
-
-  let(:path) { 'foo.rb' }
-
-  describe '#check' do
-    before do
-      expect(subject).to receive(:require).with(path)
-    end
-
-    context 'when a migration does not specify if downtime is required' do
-      it 'raises RuntimeError' do
-        expect(subject).to receive(:class_for_migration_file)
-          .with(path)
-          .and_return(Class.new)
-
-        expect { subject.check([path]) }
-          .to raise_error(RuntimeError, /it requires downtime/)
-      end
-    end
-
-    context 'when a migration requires downtime' do
-      context 'when no reason is specified' do
-        it 'raises RuntimeError' do
-          stub_const('TestMigration::DOWNTIME', true)
-
-          expect(subject).to receive(:class_for_migration_file)
-            .with(path)
-            .and_return(TestMigration)
-
-          expect { subject.check([path]) }
-            .to raise_error(RuntimeError, /no reason was given/)
-        end
-      end
-
-      context 'when a reason is specified' do
-        it 'returns an Array of messages' do
-          stub_const('TestMigration::DOWNTIME', true)
-          stub_const('TestMigration::DOWNTIME_REASON', 'foo')
-
-          expect(subject).to receive(:class_for_migration_file)
-            .with(path)
-            .and_return(TestMigration)
-
-          messages = subject.check([path])
-
-          expect(messages).to be_an_instance_of(Array)
-          expect(messages[0]).to be_an_instance_of(Gitlab::DowntimeCheck::Message)
-
-          message = messages[0]
-
-          expect(message.path).to eq(path)
-          expect(message.offline).to eq(true)
-          expect(message.reason).to eq('foo')
-        end
-      end
-    end
-  end
-
-  describe '#check_and_print' do
-    it 'checks the migrations and prints the results to STDOUT' do
-      stub_const('TestMigration::DOWNTIME', true)
-      stub_const('TestMigration::DOWNTIME_REASON', 'foo')
-
-      expect(subject).to receive(:require).with(path)
-
-      expect(subject).to receive(:class_for_migration_file)
-        .with(path)
-        .and_return(TestMigration)
-
-      expect(subject).to receive(:puts).with(an_instance_of(String))
-
-      subject.check_and_print([path])
-    end
-  end
-
-  describe '#class_for_migration_file' do
-    it 'returns the class for a migration file path' do
-      expect(subject.class_for_migration_file('123_string.rb')).to eq(String)
-    end
-  end
-
-  describe '#online?' do
-    it 'returns true when a migration can be performed online' do
-      stub_const('TestMigration::DOWNTIME', false)
-
-      expect(subject.online?(TestMigration)).to eq(true)
-    end
-
-    it 'returns false when a migration can not be performed online' do
-      stub_const('TestMigration::DOWNTIME', true)
-
-      expect(subject.online?(TestMigration)).to eq(false)
-    end
-  end
-
-  describe '#downtime_reason' do
-    context 'when a reason is defined' do
-      it 'returns the downtime reason' do
-        stub_const('TestMigration::DOWNTIME_REASON', 'hello')
-
-        expect(subject.downtime_reason(TestMigration)).to eq('hello')
-      end
-    end
-
-    context 'when a reason is not defined' do
-      it 'returns nil' do
-        expect(subject.downtime_reason(Class.new)).to be_nil
-      end
-    end
-  end
-end

spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers an offense' do
         expect_offense(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
             disable_ddl_transaction!
 
             def up
@@ -54,7 +53,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
             disable_ddl_transaction!
 
             def up
@@ -90,8 +88,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               create_table :test_text_limits, id: false do |t|
                 t.integer :test_id, null: false
@@ -113,7 +109,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers an offense' do
         expect_offense(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
             disable_ddl_transaction!
 
             def up
@@ -146,7 +141,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers no offenses' do
         expect_no_offenses(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
             disable_ddl_transaction!
 
             def up
@@ -168,8 +162,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class TestTextLimits < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               drop_table :no_offense_on_down
             end
@@ -194,7 +186,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
     it 'registers no offense' do
       expect_no_offenses(<<~RUBY)
         class TestTextLimits < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
           disable_ddl_transaction!
 
           def up

spec/rubocop/cop/migration/add_timestamps_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
   let(:migration_with_add_timestamps) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
           add_timestamps(:users)
@@ -22,8 +20,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
   let(:migration_without_add_timestamps) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
         end
@@ -34,8 +30,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
   let(:migration_with_add_timestamps_with_timezone) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
           add_timestamps_with_timezone(:users)
@@ -52,8 +46,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
     it 'registers an offense when the "add_timestamps" method is used' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[4.2]
-          DOWNTIME = false
-
           def change
             add_column(:users, :username, :text)
             add_timestamps(:users)

spec/rubocop/cop/migration/complex_indexes_require_name_spec.rb

@@ -18,8 +18,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
         it 'registers an offense' do
           expect_offense(<<~RUBY)
             class TestComplexIndexes < ActiveRecord::Migration[6.0]
-              DOWNTIME = false
-
               def up
                 create_table :test_table do |t|
                   t.integer :column1, null: false
@@ -46,8 +44,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
         it 'registers no offense' do
           expect_no_offenses(<<~RUBY)
             class TestComplexIndexes < ActiveRecord::Migration[6.0]
-              DOWNTIME = false
-
               def up
                 create_table :test_table do |t|
                   t.integer :column1, null: false
@@ -74,8 +70,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
         it 'registers an offense' do
           expect_offense(<<~RUBY)
             class TestComplexIndexes < ActiveRecord::Migration[6.0]
-              DOWNTIME = false
-
               disable_ddl_transaction!
 
               def up
@@ -101,8 +95,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
         it 'registers no offenses' do
           expect_no_offenses(<<~RUBY)
             class TestComplexIndexes < ActiveRecord::Migration[6.0]
-              DOWNTIME = false
-
               INDEX_NAME = 'my_test_name'
 
               disable_ddl_transaction!
@@ -135,8 +127,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
     it 'registers no offenses' do
       expect_no_offenses(<<~RUBY)
         class TestComplexIndexes < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           disable_ddl_transaction!
 
           def up

spec/rubocop/cop/migration/datetime_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:create_table_migration_without_datetime) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           create_table :users do |t|
             t.string :username, null: false
@@ -24,8 +22,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:create_table_migration_with_datetime_with_timezone) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           create_table :users do |t|
             t.string :username, null: false
@@ -39,8 +35,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:add_column_migration_with_datetime) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
           add_column(:users, :last_sign_in, :datetime)
@@ -52,8 +46,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:add_column_migration_with_timestamp) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
           add_column(:users, :last_sign_in, :timestamp)
@@ -65,8 +57,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:add_column_migration_without_datetime) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
         end
@@ -77,8 +67,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
   let(:add_column_migration_with_datetime_with_timezone) do
     %q(
       class Users < ActiveRecord::Migration[6.0]
-        DOWNTIME = false
-
         def change
           add_column(:users, :username, :text)
           add_column(:users, :last_sign_in, :datetime_with_timezone)
@@ -95,8 +83,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
     it 'registers an offense when the ":datetime" data type is used on create_table' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           def change
             create_table :users do |t|
               t.string :username, null: false
@@ -111,8 +97,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
     it 'registers an offense when the ":timestamp" data type is used on create_table' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           def change
             create_table :users do |t|
               t.string :username, null: false
@@ -135,8 +119,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
     it 'registers an offense when the ":datetime" data type is used on add_column' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           def change
             add_column(:users, :username, :text)
             add_column(:users, :last_sign_in, :datetime)
@@ -149,8 +131,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
     it 'registers an offense when the ":timestamp" data type is used on add_column' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           def change
             add_column(:users, :username, :text)
             add_column(:users, :last_sign_in, :timestamp)

spec/rubocop/cop/migration/prevent_strings_spec.rb

@@ -15,8 +15,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
       it 'registers an offense' do
         expect_offense(<<~RUBY, msg: "Do not use the `string` data type, use `text` instead.[...]")
           class Users < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               create_table :users do |t|
                 t.string :username, null: false
@@ -46,8 +44,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class Users < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               create_table :users do |t|
                 t.integer :not_a_string, null: false
@@ -65,8 +61,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class Users < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               create_table :users do |t|
                 t.text :username, null: false
@@ -87,8 +81,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class TestStringArrays < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               create_table :test_string_arrays, id: false do |t|
                 t.integer :test_id, null: false
@@ -108,8 +100,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
       it 'registers no offense' do
         expect_no_offenses(<<~RUBY)
           class Users < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             def up
               remove_column :users, :bio
               remove_column :users, :url
@@ -137,8 +127,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
     it 'registers no offense' do
       expect_no_offenses(<<~RUBY)
         class Users < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           def up
             create_table :users do |t|
               t.string :username, null: false

spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb

@@ -15,8 +15,6 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
       it 'registers an offense' do
         expect_offense(<<~RUBY, msg: 'migration methods that refer to existing indexes must do so by name')
           class TestReferToIndexByName < ActiveRecord::Migration[6.0]
-            DOWNTIME = false
-
             INDEX_NAME = 'my_test_name'
 
             disable_ddl_transaction!
@@ -63,8 +61,6 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
     it 'registers no offenses' do
       expect_no_offenses(<<~RUBY)
         class TestReferToIndexByName < ActiveRecord::Migration[6.0]
-          DOWNTIME = false
-
           disable_ddl_transaction!
 
           def up

spec/rubocop/cop/migration/timestamps_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
   let(:migration_with_timestamps) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           create_table :users do |t|
             t.string :username, null: false
@@ -25,8 +23,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
   let(:migration_without_timestamps) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           create_table :users do |t|
             t.string :username, null: false
@@ -40,8 +36,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
   let(:migration_with_timestamps_with_timezone) do
     %q(
       class Users < ActiveRecord::Migration[4.2]
-        DOWNTIME = false
-
         def change
           create_table :users do |t|
             t.string :username, null: false
@@ -61,8 +55,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
     it 'registers an offense when the "timestamps" method is used' do
       expect_offense(<<~RUBY)
         class Users < ActiveRecord::Migration[4.2]
-          DOWNTIME = false
-
           def change
             create_table :users do |t|
               t.string :username, null: false