Merge branch 'sh-disable-ldap-sync-in-geo-secondary' into 'master'

Disable all unnecessary Sidekiq-cron workers when node is a Geo secondary

See merge request !2261

app/workers/historical_data_worker.rb

@@ -3,7 +3,6 @@ class HistoricalDataWorker
   include CronjobQueue
 
   def perform
-    return if Gitlab::Geo.secondary?
     return if License.current.nil? || License.current&.trial?
 
     HistoricalData.track!

app/workers/prune_old_events_worker.rb

@@ -3,8 +3,6 @@ class PruneOldEventsWorker
   include CronjobQueue
 
   def perform
-    return if Gitlab::Geo.secondary?
-
     # Contribution calendar shows maximum 12 months of events.
     # Double nested query is used because MySQL doesn't allow DELETE subqueries
     # on the same table.

app/workers/schedule_update_user_activity_worker.rb

@@ -3,8 +3,6 @@ class ScheduleUpdateUserActivityWorker
   include CronjobQueue
 
   def perform(batch_size = 500)
-    return if Gitlab::Geo.secondary?
-
     Gitlab::UserActivities.new.each_slice(batch_size) do |batch|
       UpdateUserActivityWorker.perform_async(Hash[batch])
     end

app/workers/update_all_mirrors_worker.rb

@@ -6,10 +6,6 @@ class UpdateAllMirrorsWorker
   LEASE_KEY = 'update_all_mirrors'.freeze
 
   def perform
-    # This worker requires updating the database state, which we can't
-    # do on a Geo secondary
-    return if Gitlab::Geo.secondary?
-
     lease_uuid = try_obtain_lease
     return unless lease_uuid
 

app/workers/update_user_activity_worker.rb

@@ -3,8 +3,6 @@ class UpdateUserActivityWorker
   include DedicatedSidekiqQueue
 
   def perform(pairs)
-    return if Gitlab::Geo.secondary?
-
     pairs = cast_data(pairs)
     ids = pairs.keys
     conditions = 'WHEN id = ? THEN ? ' * ids.length

lib/gitlab/geo.rb

@@ -83,27 +83,40 @@ module Gitlab
     end
 
     def self.configure_primary_jobs!
-      PRIMARY_JOBS.each { |job| self.send(job).try(:enable!) }
+      self.enable_all_cron_jobs!
       SECONDARY_JOBS.each { |job| self.send(job).try(:disable!) }
     end
 
     def self.configure_secondary_jobs!
-      PRIMARY_JOBS.each { |job| self.send(job).try(:disable!) }
+      self.disable_all_cron_jobs!
       SECONDARY_JOBS.each { |job| self.send(job).try(:enable!) }
     end
 
-    def self.disable_all_jobs!
+    def self.disable_all_geo_jobs!
       PRIMARY_JOBS.each { |job| self.send(job).try(:disable!) }
       SECONDARY_JOBS.each { |job| self.send(job).try(:disable!) }
     end
 
+    def self.disable_all_cron_jobs!
+      self.cron_jobs.select(&:enabled?).each { |job| job.disable! }
+    end
+
+    def self.enable_all_cron_jobs!
+      self.cron_jobs.reject(&:enabled?).each { |job| job.enable! }
+    end
+
+    def self.cron_jobs
+      Sidekiq::Cron::Job.all
+    end
+
     def self.configure_cron_jobs!
       if self.primary_role_enabled?
         self.configure_primary_jobs!
       elsif self.secondary_role_enabled?
         self.configure_secondary_jobs!
       else
-        self.disable_all_jobs!
+        self.enable_all_cron_jobs!
+        self.disable_all_geo_jobs!
       end
     end
 

spec/lib/gitlab/geo_spec.rb

@@ -121,17 +121,24 @@ describe Gitlab::Geo, lib: true do
   end
 
   describe '.configure_cron_jobs!' do
+    JOBS = %w(ldap_test geo_bulk_notify_worker geo_repository_sync_worker geo_file_download_dispatch_worker).freeze
+
     def init_cron_job(job_name, class_name)
-      Sidekiq::Cron::Job.create(
+      job = Sidekiq::Cron::Job.new(
         name: job_name,
         cron: '0 * * * *',
         class: class_name
       )
+
+      job.enable!
     end
 
     before(:all) do
-      jobs = %w(geo_bulk_notify_worker geo_repository_sync_worker geo_file_download_dispatch_worker)
-      jobs.each { |job| init_cron_job(job, job.camelize) }
+      JOBS.each { |job| init_cron_job(job, job.camelize) }
+    end
+
+    after(:all) do
+      JOBS.each { |job| Sidekiq::Cron::Job.find(job)&.destroy }
     end
 
     it 'activates cron jobs for primary' do
@@ -143,6 +150,7 @@ describe Gitlab::Geo, lib: true do
       expect(described_class.bulk_notify_job).to be_enabled
       expect(described_class.repository_sync_job).not_to be_enabled
       expect(described_class.file_download_job).not_to be_enabled
+      expect(Sidekiq::Cron::Job.find('ldap_test')).to be_enabled
     end
 
     it 'activates cron jobs for secondary' do
@@ -151,6 +159,7 @@ describe Gitlab::Geo, lib: true do
 
       described_class.configure_cron_jobs!
 
+      expect(Sidekiq::Cron::Job.find('ldap_test')).not_to be_enabled
       expect(described_class.bulk_notify_job).not_to be_enabled
       expect(described_class.repository_sync_job).to be_enabled
       expect(described_class.file_download_job).to be_enabled
@@ -165,6 +174,21 @@ describe Gitlab::Geo, lib: true do
       expect(described_class.bulk_notify_job).not_to be_enabled
       expect(described_class.repository_sync_job).not_to be_enabled
       expect(described_class.file_download_job).not_to be_enabled
+      expect(Sidekiq::Cron::Job.find('ldap_test')).to be_enabled
+    end
+
+    it 'reactivates cron jobs when node turns off Geo' do
+      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
+      allow(described_class).to receive(:secondary_role_enabled?).and_return(true)
+
+      described_class.configure_cron_jobs!
+      expect(Sidekiq::Cron::Job.find('ldap_test')).not_to be_enabled
+
+      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
+      allow(described_class).to receive(:secondary_role_enabled?).and_return(false)
+
+      described_class.configure_cron_jobs!
+      expect(Sidekiq::Cron::Job.find('ldap_test')).to be_enabled
     end
   end
 end

spec/workers/historical_data_worker_spec.rb

@@ -28,16 +28,6 @@ describe HistoricalDataWorker do
       end
     end
 
-    context 'with a Geo secondary node' do
-      it 'does not track historical data' do
-        allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
-
-        expect(HistoricalData).not_to receive(:track!)
-
-        subject.perform
-      end
-    end
-
     context 'when there is not a license key' do
       it 'does not track historical data' do
         License.destroy_all