Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42

6 files changed, 150 insertions, 33 deletions

diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
index e096e7f6e91..1a7116e75e5 100644
--- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
@@ -581,4 +581,101 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
       model.delete_queued_jobs('BackgroundMigrationClassName')
     end
   end
+
+  describe '#finalized_background_migration' do
+    include_context 'background migration job class'
+
+    let!(:tracked_pending_job) { create(:background_migration_job, class_name: job_class_name, status: :pending, arguments: [1]) }
+    let!(:tracked_successful_job) { create(:background_migration_job, class_name: job_class_name, status: :succeeded, arguments: [2]) }
+
+    before do
+      Sidekiq::Testing.disable! do
+        BackgroundMigrationWorker.perform_async(job_class_name, [1, 2])
+        BackgroundMigrationWorker.perform_async(job_class_name, [3, 4])
+        BackgroundMigrationWorker.perform_in(10, job_class_name, [5, 6])
+        BackgroundMigrationWorker.perform_in(20, job_class_name, [7, 8])
+      end
+    end
+
+    it_behaves_like 'finalized tracked background migration' do
+      before do
+        model.finalize_background_migration(job_class_name)
+      end
+    end
+
+    context 'when removing all tracked job records' do
+      # Force pending jobs to remain pending.
+      let!(:job_perform_method) { ->(*arguments) { } }
+
+      before do
+        model.finalize_background_migration(job_class_name, delete_tracking_jobs: %w[pending succeeded])
+      end
+
+      it_behaves_like 'finalized tracked background migration'
+      it_behaves_like 'removed tracked jobs', 'pending'
+      it_behaves_like 'removed tracked jobs', 'succeeded'
+    end
+
+    context 'when retaining all tracked job records' do
+      before do
+        model.finalize_background_migration(job_class_name, delete_tracking_jobs: false)
+      end
+
+      it_behaves_like 'finalized background migration'
+      include_examples 'retained tracked jobs', 'succeeded'
+    end
+
+    context 'during retry race condition' do
+      let(:queue_items_added) { [] }
+      let!(:job_perform_method) do
+        ->(*arguments) do
+          Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded(
+            RSpec.current_example.example_group_instance.job_class_name,
+            arguments
+          )
+
+          # Mock another process pushing queue jobs.
+          queue_items_added = RSpec.current_example.example_group_instance.queue_items_added
+          if queue_items_added.count < 10
+            Sidekiq::Testing.disable! do
+              job_class_name = RSpec.current_example.example_group_instance.job_class_name
+              queue_items_added << BackgroundMigrationWorker.perform_async(job_class_name, [Time.current])
+              queue_items_added << BackgroundMigrationWorker.perform_in(10, job_class_name, [Time.current])
+            end
+          end
+        end
+      end
+
+      it_behaves_like 'finalized tracked background migration' do
+        before do
+          model.finalize_background_migration(job_class_name, delete_tracking_jobs: ['succeeded'])
+        end
+      end
+    end
+  end
+
+  describe '#delete_job_tracking' do
+    let!(:job_class_name) { 'TestJob' }
+
+    let!(:tracked_pending_job) { create(:background_migration_job, class_name: job_class_name, status: :pending, arguments: [1]) }
+    let!(:tracked_successful_job) { create(:background_migration_job, class_name: job_class_name, status: :succeeded, arguments: [2]) }
+
+    context 'with default status' do
+      before do
+        model.delete_job_tracking(job_class_name)
+      end
+
+      include_examples 'retained tracked jobs', 'pending'
+      include_examples 'removed tracked jobs', 'succeeded'
+    end
+
+    context 'with explicit status' do
+      before do
+        model.delete_job_tracking(job_class_name, status: %w[pending succeeded])
+      end
+
+      include_examples 'removed tracked jobs', 'pending'
+      include_examples 'removed tracked jobs', 'succeeded'
+    end
+  end
 end
diff --git a/spec/lib/gitlab/database/migrations/instrumentation_spec.rb b/spec/lib/gitlab/database/migrations/instrumentation_spec.rb
index 6d047eed3bb..5945e5a2039 100644
--- a/spec/lib/gitlab/database/migrations/instrumentation_spec.rb
+++ b/spec/lib/gitlab/database/migrations/instrumentation_spec.rb
@@ -5,24 +5,35 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
   describe '#observe' do
     subject { described_class.new }
 
-    let(:migration) { 1234 }
+    let(:migration_name) { 'test' }
+    let(:migration_version) { '12345' }
 
     it 'executes the given block' do
-      expect { |b| subject.observe(migration, &b) }.to yield_control
+      expect { |b| subject.observe(version: migration_version, name: migration_name, &b) }.to yield_control
     end
 
     context 'behavior with observers' do
-      subject { described_class.new(observers).observe(migration) {} }
+      subject { described_class.new([Gitlab::Database::Migrations::Observers::MigrationObserver]).observe(version: migration_version, name: migration_name) {} }
 
-      let(:observers) { [observer] }
       let(:observer) { instance_double('Gitlab::Database::Migrations::Observers::MigrationObserver', before: nil, after: nil, record: nil) }
 
+      before do
+        allow(Gitlab::Database::Migrations::Observers::MigrationObserver).to receive(:new).and_return(observer)
+      end
+
+      it 'instantiates observer with observation' do
+        expect(Gitlab::Database::Migrations::Observers::MigrationObserver)
+          .to receive(:new)
+          .with(instance_of(Gitlab::Database::Migrations::Observation)) { |observation| expect(observation.version).to eq(migration_version) }
+          .and_return(observer)
+
+        subject
+      end
+
       it 'calls #before, #after, #record on given observers' do
         expect(observer).to receive(:before).ordered
         expect(observer).to receive(:after).ordered
-        expect(observer).to receive(:record).ordered do |observation|
-          expect(observation.migration).to eq(migration)
-        end
+        expect(observer).to receive(:record).ordered
 
         subject
       end
@@ -47,7 +58,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
     end
 
     context 'on successful execution' do
-      subject { described_class.new.observe(migration) {} }
+      subject { described_class.new.observe(version: migration_version, name: migration_name) {} }
 
       it 'records walltime' do
         expect(subject.walltime).not_to be_nil
@@ -58,12 +69,16 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
       end
 
       it 'records the migration version' do
-        expect(subject.migration).to eq(migration)
+        expect(subject.version).to eq(migration_version)
+      end
+
+      it 'records the migration name' do
+        expect(subject.name).to eq(migration_name)
       end
     end
 
     context 'upon failure' do
-      subject { described_class.new.observe(migration) { raise 'something went wrong' } }
+      subject { described_class.new.observe(version: migration_version, name: migration_name) { raise 'something went wrong' } }
 
       it 'raises the exception' do
         expect { subject }.to raise_error(/something went wrong/)
@@ -73,7 +88,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
         subject { instance.observations.first }
 
         before do
-          instance.observe(migration) { raise 'something went wrong' }
+          instance.observe(version: migration_version, name: migration_name) { raise 'something went wrong' }
         rescue StandardError
           # ignore
         end
@@ -89,7 +104,11 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
         end
 
         it 'records the migration version' do
-          expect(subject.migration).to eq(migration)
+          expect(subject.version).to eq(migration_version)
+        end
+
+        it 'records the migration name' do
+          expect(subject.name).to eq(migration_name)
         end
       end
     end
@@ -101,8 +120,8 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
       let(:migration2) { double('migration2', call: nil) }
 
       it 'records observations for all migrations' do
-        subject.observe('migration1') {}
-        subject.observe('migration2') { raise 'something went wrong' } rescue nil
+        subject.observe(version: migration_version, name: migration_name) {}
+        subject.observe(version: migration_version, name: migration_name) { raise 'something went wrong' } rescue nil
 
         expect(subject.observations.size).to eq(2)
       end
diff --git a/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb
index 8aac3ed67c6..36885a1594f 100644
--- a/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb
+++ b/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb
@@ -2,16 +2,17 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do
-  subject { described_class.new }
+  subject { described_class.new(observation) }
 
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
   let(:connection) { ActiveRecord::Base.connection }
   let(:query) { "select date_trunc('day', $1::timestamptz) + $2 * (interval '1 hour')" }
   let(:query_binds) { [Time.current, 3] }
   let(:directory_path) { Dir.mktmpdir }
-  let(:log_file) { "#{directory_path}/#{migration}-query-details.json" }
+  let(:log_file) { "#{directory_path}/#{migration_version}_#{migration_name}-query-details.json" }
   let(:query_details) { Gitlab::Json.parse(File.read(log_file)) }
-  let(:migration) { 20210422152437 }
+  let(:migration_version) { 20210422152437 }
+  let(:migration_name) { 'test' }
 
   before do
     stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path)
@@ -49,7 +50,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do
     subject.before
     run_query
     subject.after
-    subject.record(observation)
+    subject.record
   end
 
   def run_query
diff --git a/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb
index 195e7114582..2a49d8e8b73 100644
--- a/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb
+++ b/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb
@@ -2,14 +2,14 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
-  subject { described_class.new }
+  subject { described_class.new(observation) }
 
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
   let(:connection) { ActiveRecord::Base.connection }
   let(:query) { 'select 1' }
   let(:directory_path) { Dir.mktmpdir }
-  let(:log_file) { "#{directory_path}/current.log" }
-  let(:migration) { 20210422152437 }
+  let(:migration_version) { 20210422152437 }
+  let(:migration_name) { 'test' }
 
   before do
     stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path)
@@ -22,7 +22,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
   it 'writes a file with the query log' do
     observe
 
-    expect(File.read("#{directory_path}/#{migration}.log")).to include(query)
+    expect(File.read("#{directory_path}/#{migration_version}_#{migration_name}.log")).to include(query)
   end
 
   it 'does not change the default logger' do
@@ -33,6 +33,6 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
     subject.before
     connection.execute(query)
     subject.after
-    subject.record(observation)
+    subject.record
   end
 end
diff --git a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb
index a3b03050b33..32a25fdaa28 100644
--- a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb
+++ b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb
@@ -2,8 +2,9 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do
-  subject { described_class.new }
+  subject { described_class.new(observation) }
 
+  let(:observation) { Gitlab::Database::Migrations::Observation.new }
   let(:connection) { ActiveRecord::Base.connection }
 
   def mock_pgss(enabled: true)
@@ -37,7 +38,6 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do
   end
 
   describe '#record' do
-    let(:observation) { Gitlab::Database::Migrations::Observation.new }
     let(:result) { double }
     let(:pgss_query) do
       <<~SQL
@@ -52,7 +52,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do
         mock_pgss(enabled: true)
         expect(connection).to receive(:execute).with(pgss_query).once.and_return(result)
 
-        expect { subject.record(observation) }.to change { observation.query_statistics }.from(nil).to(result)
+        expect { subject.record }.to change { observation.query_statistics }.from(nil).to(result)
       end
     end
 
@@ -61,7 +61,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do
         mock_pgss(enabled: false)
         expect(connection).not_to receive(:execute)
 
-        expect { subject.record(observation) }.not_to change { observation.query_statistics }
+        expect { subject.record }.not_to change { observation.query_statistics }
       end
     end
   end
diff --git a/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb b/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb
index 73466471944..61e28003e66 100644
--- a/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb
+++ b/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb
@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::Migrations::Observers::TotalDatabaseSizeChange do
-  subject { described_class.new }
+  subject { described_class.new(observation) }
 
   let(:observation) { Gitlab::Database::Migrations::Observation.new }
   let(:connection) { ActiveRecord::Base.connection }
@@ -14,7 +14,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::TotalDatabaseSizeChange
 
     subject.before
     subject.after
-    subject.record(observation)
+    subject.record
 
     expect(observation.total_database_size_change).to eq(256 - 1024)
   end
@@ -27,13 +27,13 @@ RSpec.describe Gitlab::Database::Migrations::Observers::TotalDatabaseSizeChange
     it 'does not record anything if before size is unknown' do
       subject.after
 
-      expect { subject.record(observation) }.not_to change { observation.total_database_size_change }
+      expect { subject.record }.not_to change { observation.total_database_size_change }
     end
 
     it 'does not record anything if after size is unknown' do
       subject.before
 
-      expect { subject.record(observation) }.not_to change { observation.total_database_size_change }
+      expect { subject.record }.not_to change { observation.total_database_size_change }
     end
   end
 end