Merge branch...

Merge branch '326495-remove-downtime-check-for-migrations-and-always-disallow-downtime' into 'master'

Remove migration downtime check since we do not allow downtime [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!58237

.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

@@ -69,8 +69,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
@@ -97,8 +95,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
@@ -133,8 +129,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