Merge branch 'pb-fix-ci-batched-bg-worker' into 'master'

Fix batched bg migration for missing ci model

See merge request gitlab-org/gitlab!82127

app/workers/database/batched_background_migration/single_database_worker.rb

@@ -31,6 +31,15 @@ module Database
       end
 
       def perform
+        unless base_model
+          Sidekiq.logger.info(
+            class: self.class.name,
+            database: self.class.tracking_database,
+            message: 'skipping migration execution for unconfigured database')
+
+          return
+        end
+
         Gitlab::Database::SharedModel.using_connection(base_model.connection) do
           break unless Feature.enabled?(:execute_batched_migrations_on_schedule, type: :ops, default_enabled: :yaml) && active_migration
 

spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb

@@ -42,122 +42,155 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d
   describe '#perform' do
     subject(:worker) { described_class.new }
 
-    context 'when the feature flag is disabled' do
+    context 'when the base model does not exist' do
       before do
-        stub_feature_flags(execute_batched_migrations_on_schedule: false)
+        if Gitlab::Database.has_config?(tracking_database)
+          skip "because the base model for #{tracking_database} exists"
+        end
       end
 
       it 'does nothing' do
         expect(worker).not_to receive(:active_migration)
         expect(worker).not_to receive(:run_active_migration)
 
-        worker.perform
+        expect { worker.perform }.not_to raise_error
+      end
+
+      it 'logs a message indicating execution is skipped' do
+        expect(Sidekiq.logger).to receive(:info) do |payload|
+          expect(payload[:class]).to eq(described_class.name)
+          expect(payload[:database]).to eq(tracking_database)
+          expect(payload[:message]).to match(/skipping migration execution/)
+        end
+
+        expect { worker.perform }.not_to raise_error
       end
     end
 
-    context 'when the feature flag is enabled' do
+    context 'when the base model does exist' do
       before do
-        stub_feature_flags(execute_batched_migrations_on_schedule: true)
-
-        allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration).and_return(nil)
+        unless Gitlab::Database.has_config?(tracking_database)
+          skip "because the base model for #{tracking_database} does not exist"
+        end
       end
 
-      context 'when no active migrations exist' do
+      context 'when the feature flag is disabled' do
+        before do
+          stub_feature_flags(execute_batched_migrations_on_schedule: false)
+        end
+
         it 'does nothing' do
+          expect(worker).not_to receive(:active_migration)
           expect(worker).not_to receive(:run_active_migration)
 
           worker.perform
         end
       end
 
-      context 'when active migrations exist' do
-        let(:job_interval) { 5.minutes }
-        let(:lease_timeout) { 15.minutes }
-        let(:lease_key) { described_class.name.demodulize.underscore }
-        let(:migration) { build(:batched_background_migration, :active, interval: job_interval) }
-        let(:interval_variance) { described_class::INTERVAL_VARIANCE }
-
+      context 'when the feature flag is enabled' do
         before do
-          allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration)
-            .and_return(migration)
+          stub_feature_flags(execute_batched_migrations_on_schedule: true)
 
-          allow(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(true)
-          allow(migration).to receive(:reload)
+          allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration).and_return(nil)
         end
 
-        context 'when the reloaded migration is no longer active' do
-          it 'does not run the migration' do
-            expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
-
-            expect(migration).to receive(:reload)
-            expect(migration).to receive(:active?).and_return(false)
-
+        context 'when no active migrations exist' do
+          it 'does nothing' do
             expect(worker).not_to receive(:run_active_migration)
 
             worker.perform
           end
         end
 
-        context 'when the interval has not elapsed' do
-          it 'does not run the migration' do
-            expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
+        context 'when active migrations exist' do
+          let(:job_interval) { 5.minutes }
+          let(:lease_timeout) { 15.minutes }
+          let(:lease_key) { described_class.name.demodulize.underscore }
+          let(:migration) { build(:batched_background_migration, :active, interval: job_interval) }
+          let(:interval_variance) { described_class::INTERVAL_VARIANCE }
 
-            expect(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(false)
-
-            expect(worker).not_to receive(:run_active_migration)
+          before do
+            allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration)
+              .and_return(migration)
 
-            worker.perform
+            allow(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(true)
+            allow(migration).to receive(:reload)
           end
-        end
 
-        context 'when the reloaded migration is still active and the interval has elapsed' do
-          it 'runs the migration' do
-            expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
+          context 'when the reloaded migration is no longer active' do
+            it 'does not run the migration' do
+              expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
+
+              expect(migration).to receive(:reload)
+              expect(migration).to receive(:active?).and_return(false)
+
+              expect(worker).not_to receive(:run_active_migration)
 
-            expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance|
-              expect(instance).to receive(:run_migration_job).with(migration)
+              worker.perform
             end
+          end
 
-            expect(worker).to receive(:run_active_migration).and_call_original
+          context 'when the interval has not elapsed' do
+            it 'does not run the migration' do
+              expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
 
-            worker.perform
+              expect(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(false)
+
+              expect(worker).not_to receive(:run_active_migration)
+
+              worker.perform
+            end
           end
-        end
 
-        context 'when the calculated timeout is less than the minimum allowed' do
-          let(:minimum_timeout) { described_class::MINIMUM_LEASE_TIMEOUT }
-          let(:job_interval) { 2.minutes }
+          context 'when the reloaded migration is still active and the interval has elapsed' do
+            it 'runs the migration' do
+              expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout)
+
+              expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance|
+                expect(instance).to receive(:run_migration_job).with(migration)
+              end
 
-          it 'sets the lease timeout to the minimum value' do
-            expect_to_obtain_exclusive_lease(lease_key, timeout: minimum_timeout)
+              expect(worker).to receive(:run_active_migration).and_call_original
 
-            expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance|
-              expect(instance).to receive(:run_migration_job).with(migration)
+              worker.perform
             end
+          end
 
-            expect(worker).to receive(:run_active_migration).and_call_original
+          context 'when the calculated timeout is less than the minimum allowed' do
+            let(:minimum_timeout) { described_class::MINIMUM_LEASE_TIMEOUT }
+            let(:job_interval) { 2.minutes }
 
-            worker.perform
+            it 'sets the lease timeout to the minimum value' do
+              expect_to_obtain_exclusive_lease(lease_key, timeout: minimum_timeout)
+
+              expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance|
+                expect(instance).to receive(:run_migration_job).with(migration)
+              end
+
+              expect(worker).to receive(:run_active_migration).and_call_original
+
+              worker.perform
+            end
           end
-        end
 
-        it 'always cleans up the exclusive lease' do
-          lease = stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
+          it 'always cleans up the exclusive lease' do
+            lease = stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
 
-          expect(lease).to receive(:try_obtain).and_return(true)
+            expect(lease).to receive(:try_obtain).and_return(true)
 
-          expect(worker).to receive(:run_active_migration).and_raise(RuntimeError, 'I broke')
-          expect(lease).to receive(:cancel)
+            expect(worker).to receive(:run_active_migration).and_raise(RuntimeError, 'I broke')
+            expect(lease).to receive(:cancel)
 
-          expect { worker.perform }.to raise_error(RuntimeError, 'I broke')
-        end
+            expect { worker.perform }.to raise_error(RuntimeError, 'I broke')
+          end
 
-        it 'receives the correct connection' do
-          base_model = Gitlab::Database.database_base_models[tracking_database]
+          it 'receives the correct connection' do
+            base_model = Gitlab::Database.database_base_models[tracking_database]
 
-          expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield
+            expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield
 
-          worker.perform
+            worker.perform
+          end
         end
       end
     end

spec/workers/database/batched_background_migration/ci_database_worker_spec.rb

@@ -2,6 +2,6 @@
 
 require 'spec_helper'
 
-RSpec.describe Database::BatchedBackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, if: Gitlab::Database.has_config?(:ci) do
+RSpec.describe Database::BatchedBackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state do
   it_behaves_like 'it runs batched background migration jobs', 'ci'
 end