Merge branch 'clarify-correct-migration-place-for-sidekiq' into 'master'

Add clarifying docs about sidekiq queue migration

See merge request gitlab-org/gitlab!64669

db/migrate/20201224144948_migrate_coverage_report_worker.rb

@@ -6,10 +6,10 @@ class MigrateCoverageReportWorker < ActiveRecord::Migration[6.0]
   DOWNTIME = false
 
   def up
-    sidekiq_queue_migrate 'ci_pipelines_create_artifact', to: 'ci_pipeline_artifacts_coverage_report'
+    sidekiq_queue_migrate 'ci_pipelines_create_artifact', to: 'ci_pipeline_artifacts_coverage_report' # rubocop:disable Migration/SidekiqQueueMigrate
   end
 
   def down
-    sidekiq_queue_migrate 'ci_pipeline_artifacts_coverage_report', to: 'ci_pipelines_create_artifact'
+    sidekiq_queue_migrate 'ci_pipeline_artifacts_coverage_report', to: 'ci_pipelines_create_artifact' # rubocop:disable Migration/SidekiqQueueMigrate
   end
 end

db/migrate/20210526155257_rename_sync_security_report_approval_rules_sidekiq_queue.rb

@@ -6,10 +6,10 @@ class RenameSyncSecurityReportApprovalRulesSidekiqQueue < ActiveRecord::Migratio
   DOWNTIME = false
 
   def up
-    sidekiq_queue_migrate 'sync_security_reports_to_report_approval_rules', to: 'ci_sync_reports_to_report_approval_rules'
+    sidekiq_queue_migrate 'sync_security_reports_to_report_approval_rules', to: 'ci_sync_reports_to_report_approval_rules' # rubocop:disable Migration/SidekiqQueueMigrate
   end
 
   def down
-    sidekiq_queue_migrate 'ci_sync_reports_to_report_approval_rules', to: 'sync_security_reports_to_report_approval_rules'
+    sidekiq_queue_migrate 'ci_sync_reports_to_report_approval_rules', to: 'sync_security_reports_to_report_approval_rules' # rubocop:disable Migration/SidekiqQueueMigrate
   end
 end

doc/development/sidekiq_style_guide.md

@@ -968,8 +968,8 @@ Sidekiq jobs, please consider removing the worker in a major release only.
 For the same reasons that removing workers is dangerous, care should be taken
 when renaming queues.
 
-When renaming queues, use the `sidekiq_queue_migrate` helper migration method,
-as shown in this example:
+When renaming queues, use the `sidekiq_queue_migrate` helper migration method
+in a **post-deployment migration**:
 
 ```ruby
 class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
@@ -985,3 +985,7 @@ class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
 end
 
 ```
+
+You must rename the queue in a post-deployment migration not in a normal
+migration. Otherwise, it runs too early, before all the workers that
+schedule these jobs have stopped running. See also [other examples](post_deployment_migrations.md#use-cases).

rubocop/cop/migration/sidekiq_queue_migrate.rb

+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if sidekiq_queue_migrate is used in a regular
+      # (not post-deployment) migration.
+      class SidekiqQueueMigrate < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = '`sidekiq_queue_migrate` must only be used in post-deployment migrations'
+
+        def on_def(node)
+          return unless in_migration?(node) && !in_post_deployment_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            send_method = send_node.children[1]
+
+            if send_method == :sidekiq_queue_migrate
+              add_offense(send_node, location: :selector)
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/migration/sidekiq_queue_migrate'
+
+RSpec.describe RuboCop::Cop::Migration::SidekiqQueueMigrate do
+  subject(:cop) { described_class.new }
+
+  def source(meth = 'change')
+    "def #{meth}; sidekiq_queue_migrate 'queue', to: 'new_queue'; end"
+  end
+
+  context 'when in a regular migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+      allow(cop).to receive(:in_post_deployment_migration?).and_return(false)
+    end
+
+    %w(up down change any_other_method).each do |method_name|
+      it "registers an offense when sidekiq_queue_migrate is used in ##{method_name}" do
+        expect_offense(<<~RUBY)
+          def #{method_name}
+            sidekiq_queue_migrate 'queue', to: 'new_queue'
+            ^^^^^^^^^^^^^^^^^^^^^ `sidekiq_queue_migrate` must only be used in post-deployment migrations
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'when in a post-deployment migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+      allow(cop).to receive(:in_post_deployment_migration?).and_return(true)
+    end
+
+    it 'registers no offense' do
+      expect_no_offenses(source)
+    end
+  end
+
+  context 'when outside of a migration' do
+    it 'registers no offense' do
+      expect_no_offenses(source)
+    end
+  end
+end