Add latest changes from gitlab-org/gitlab@13-12-stable-eev13.12.0-rc42

3 files changed, 261 insertions, 36 deletions

diff --git a/spec/lib/gitlab/sidekiq_config/worker_matcher_spec.rb b/spec/lib/gitlab/sidekiq_config/worker_matcher_spec.rb
index 75e9c8c100b..dfe9358f70b 100644
--- a/spec/lib/gitlab/sidekiq_config/worker_matcher_spec.rb
+++ b/spec/lib/gitlab/sidekiq_config/worker_matcher_spec.rb
@@ -11,6 +11,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
       [
         {
           name: 'a',
+          worker_name: 'WorkerA',
           feature_category: :category_a,
           has_external_dependencies: false,
           urgency: :low,
@@ -19,6 +20,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
         },
         {
           name: 'a:2',
+          worker_name: 'WorkerA2',
           feature_category: :category_a,
           has_external_dependencies: false,
           urgency: :high,
@@ -27,6 +29,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
         },
         {
           name: 'b',
+          worker_name: 'WorkerB',
           feature_category: :category_b,
           has_external_dependencies: true,
           urgency: :high,
@@ -35,6 +38,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
         },
         {
           name: 'c',
+          worker_name: 'WorkerC',
           feature_category: :category_c,
           has_external_dependencies: false,
           urgency: :throttled,
@@ -46,56 +50,62 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
 
     context 'with valid input' do
       where(:query, :expected_metadatas) do
+        # worker_name
+        'worker_name=WorkerA' | %w(WorkerA)
+        'worker_name=WorkerA2' | %w(WorkerA2)
+        'worker_name=WorkerB|worker_name=WorkerD' | %w(WorkerB)
+        'worker_name!=WorkerA' | %w(WorkerA2 WorkerB WorkerC)
+
         # feature_category
-        'feature_category=category_a' | %w(a a:2)
-        'feature_category=category_a,category_c' | %w(a a:2 c)
-        'feature_category=category_a|feature_category=category_c' | %w(a a:2 c)
-        'feature_category!=category_a' | %w(b c)
+        'feature_category=category_a' | %w(WorkerA WorkerA2)
+        'feature_category=category_a,category_c' | %w(WorkerA WorkerA2 WorkerC)
+        'feature_category=category_a|feature_category=category_c' | %w(WorkerA WorkerA2 WorkerC)
+        'feature_category!=category_a' | %w(WorkerB WorkerC)
 
         # has_external_dependencies
-        'has_external_dependencies=true' | %w(b)
-        'has_external_dependencies=false' | %w(a a:2 c)
-        'has_external_dependencies=true,false' | %w(a a:2 b c)
-        'has_external_dependencies=true|has_external_dependencies=false' | %w(a a:2 b c)
-        'has_external_dependencies!=true' | %w(a a:2 c)
+        'has_external_dependencies=true' | %w(WorkerB)
+        'has_external_dependencies=false' | %w(WorkerA WorkerA2 WorkerC)
+        'has_external_dependencies=true,false' | %w(WorkerA WorkerA2 WorkerB WorkerC)
+        'has_external_dependencies=true|has_external_dependencies=false' | %w(WorkerA WorkerA2 WorkerB WorkerC)
+        'has_external_dependencies!=true' | %w(WorkerA WorkerA2 WorkerC)
 
         # urgency
-        'urgency=high' | %w(a:2 b)
-        'urgency=low' | %w(a)
-        'urgency=high,low,throttled' | %w(a a:2 b c)
-        'urgency=low|urgency=throttled' | %w(a c)
-        'urgency!=high' | %w(a c)
+        'urgency=high' | %w(WorkerA2 WorkerB)
+        'urgency=low' | %w(WorkerA)
+        'urgency=high,low,throttled' | %w(WorkerA WorkerA2 WorkerB WorkerC)
+        'urgency=low|urgency=throttled' | %w(WorkerA WorkerC)
+        'urgency!=high' | %w(WorkerA WorkerC)
 
         # name
-        'name=a' | %w(a)
-        'name=a,b' | %w(a b)
-        'name=a,a:2|name=b' | %w(a a:2 b)
-        'name!=a,a:2' | %w(b c)
+        'name=a' | %w(WorkerA)
+        'name=a,b' | %w(WorkerA WorkerB)
+        'name=a,a:2|name=b' | %w(WorkerA WorkerA2 WorkerB)
+        'name!=a,a:2' | %w(WorkerB WorkerC)
 
         # resource_boundary
-        'resource_boundary=memory' | %w(b c)
-        'resource_boundary=memory,cpu' | %w(a b c)
-        'resource_boundary=memory|resource_boundary=cpu' | %w(a b c)
-        'resource_boundary!=memory,cpu' | %w(a:2)
+        'resource_boundary=memory' | %w(WorkerB WorkerC)
+        'resource_boundary=memory,cpu' | %w(WorkerA WorkerB WorkerC)
+        'resource_boundary=memory|resource_boundary=cpu' | %w(WorkerA WorkerB WorkerC)
+        'resource_boundary!=memory,cpu' | %w(WorkerA2)
 
         # tags
-        'tags=no_disk_io' | %w(a b)
-        'tags=no_disk_io,git_access' | %w(a a:2 b)
-        'tags=no_disk_io|tags=git_access' | %w(a a:2 b)
-        'tags=no_disk_io&tags=git_access' | %w(a)
-        'tags!=no_disk_io' | %w(a:2 c)
-        'tags!=no_disk_io,git_access' | %w(c)
+        'tags=no_disk_io' | %w(WorkerA WorkerB)
+        'tags=no_disk_io,git_access' | %w(WorkerA WorkerA2 WorkerB)
+        'tags=no_disk_io|tags=git_access' | %w(WorkerA WorkerA2 WorkerB)
+        'tags=no_disk_io&tags=git_access' | %w(WorkerA)
+        'tags!=no_disk_io' | %w(WorkerA2 WorkerC)
+        'tags!=no_disk_io,git_access' | %w(WorkerC)
         'tags=unknown_tag' | []
-        'tags!=no_disk_io' | %w(a:2 c)
-        'tags!=no_disk_io,git_access' | %w(c)
-        'tags!=unknown_tag' | %w(a a:2 b c)
+        'tags!=no_disk_io' | %w(WorkerA2 WorkerC)
+        'tags!=no_disk_io,git_access' | %w(WorkerC)
+        'tags!=unknown_tag' | %w(WorkerA WorkerA2 WorkerB WorkerC)
 
         # combinations
-        'feature_category=category_a&urgency=high' | %w(a:2)
-        'feature_category=category_a&urgency=high|feature_category=category_c' | %w(a:2 c)
+        'feature_category=category_a&urgency=high' | %w(WorkerA2)
+        'feature_category=category_a&urgency=high|feature_category=category_c' | %w(WorkerA2 WorkerC)
 
         # Match all
-        '*' | %w(a a:2 b c)
+        '*' | %w(WorkerA WorkerA2 WorkerB WorkerC)
       end
 
       with_them do
@@ -103,7 +113,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
           matched_metadatas = worker_metadatas.select do |metadata|
             described_class.new(query).match?(metadata)
           end
-          expect(matched_metadatas.map { |m| m[:name] }).to match_array(expected_metadatas)
+          expect(matched_metadatas.map { |m| m[:worker_name] }).to match_array(expected_metadatas)
         end
       end
     end
@@ -113,7 +123,7 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerMatcher do
         'feature_category="category_a"' | described_class::InvalidTerm
         'feature_category=' | described_class::InvalidTerm
         'feature_category~category_a' | described_class::InvalidTerm
-        'worker_name=a' | described_class::UnknownPredicate
+        'invalid_term=a' | described_class::UnknownPredicate
       end
 
       with_them do
diff --git a/spec/lib/gitlab/sidekiq_config/worker_router_spec.rb b/spec/lib/gitlab/sidekiq_config/worker_router_spec.rb
new file mode 100644
index 00000000000..687e35813b1
--- /dev/null
+++ b/spec/lib/gitlab/sidekiq_config/worker_router_spec.rb
@@ -0,0 +1,212 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rspec-parameterized'
+
+RSpec.describe Gitlab::SidekiqConfig::WorkerRouter do
+  describe '.queue_name_from_worker_name' do
+    using RSpec::Parameterized::TableSyntax
+
+    def create_worker(name, namespace = nil)
+      Class.new.tap do |worker|
+        worker.define_singleton_method(:name) { name }
+        worker.define_singleton_method(:queue_namespace) { namespace }
+      end
+    end
+
+    where(:worker, :expected_name) do
+      create_worker('PagesWorker') | 'pages'
+      create_worker('PipelineNotificationWorker') | 'pipeline_notification'
+      create_worker('PostReceive') | 'post_receive'
+      create_worker('PostReceive', :git) | 'git:post_receive'
+      create_worker('PipelineHooksWorker', :pipeline_hooks) | 'pipeline_hooks:pipeline_hooks'
+      create_worker('Gitlab::JiraImport::AdvanceStageWorker') | 'jira_import_advance_stage'
+      create_worker('Gitlab::PhabricatorImport::ImportTasksWorker', :importer) | 'importer:phabricator_import_import_tasks'
+    end
+
+    with_them do
+      it 'generates a valid queue name from worker name' do
+        expect(described_class.queue_name_from_worker_name(worker)).to eql(expected_name)
+      end
+    end
+  end
+
+  shared_context 'router examples setup' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:worker) do
+      Class.new do
+        def self.name
+          'Gitlab::Foo::BarWorker'
+        end
+
+        include ApplicationWorker
+
+        feature_category :feature_a
+        urgency :low
+        worker_resource_boundary :cpu
+        tags :expensive
+      end
+    end
+
+    where(:routing_rules, :expected_queue) do
+      # Default, no configuration
+      [] | 'foo_bar'
+      # Does not match, fallback to the named queue
+      [
+        ['feature_category=feature_b|urgency=high', 'queue_a'],
+        ['resource_boundary=memory', 'queue_b'],
+        ['tags=cheap', 'queue_c']
+      ] | 'foo_bar'
+      # Match a nil queue, fallback to named queue
+      [
+        ['feature_category=feature_b|urgency=high', 'queue_a'],
+        ['resource_boundary=cpu', nil],
+        ['tags=cheap', 'queue_c']
+      ] | 'foo_bar'
+      # Match an empty string, fallback to named queue
+      [
+        ['feature_category=feature_b|urgency=high', 'queue_a'],
+        ['resource_boundary=cpu', ''],
+        ['tags=cheap', 'queue_c']
+      ] | 'foo_bar'
+      # Match the first rule
+      [
+        ['feature_category=feature_a|urgency=high', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=cheap', 'queue_c']
+      ] | 'queue_a'
+      # Match the first rule 2
+      [
+        ['feature_category=feature_b|urgency=low', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=cheap', 'queue_c']
+      ] | 'queue_a'
+      # Match the third rule
+      [
+        ['feature_category=feature_b|urgency=high', 'queue_a'],
+        ['resource_boundary=memory', 'queue_b'],
+        ['tags=expensive', 'queue_c']
+      ] | 'queue_c'
+      # Match all, first match wins
+      [
+        ['feature_category=feature_a|urgency=low', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=expensive', 'queue_c']
+      ] | 'queue_a'
+      # Match the same rule multiple times, the first match wins
+      [
+        ['feature_category=feature_a', 'queue_a'],
+        ['feature_category=feature_a', 'queue_b'],
+        ['feature_category=feature_a', 'queue_c']
+      ] | 'queue_a'
+      # Match wildcard
+      [
+        ['feature_category=feature_b|urgency=high', 'queue_a'],
+        ['resource_boundary=memory', 'queue_b'],
+        ['tags=cheap', 'queue_c'],
+        ['*', 'default']
+      ] | 'default'
+      # Match wildcard at the top of the chain. It makes the following rules useless
+      [
+        ['*', 'queue_foo'],
+        ['feature_category=feature_a|urgency=low', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=expensive', 'queue_c']
+      ] | 'queue_foo'
+    end
+  end
+
+  describe '.global' do
+    before do
+      described_class.remove_instance_variable(:@global_worker_router) if described_class.instance_variable_defined?(:@global_worker_router)
+    end
+
+    after do
+      described_class.remove_instance_variable(:@global_worker_router)
+    end
+
+    context 'valid routing rules' do
+      include_context 'router examples setup'
+
+      with_them do
+        before do
+          stub_config(sidekiq: { routing_rules: routing_rules })
+        end
+
+        it 'routes the worker to the correct queue' do
+          expect(described_class.global.route(worker)).to eql(expected_queue)
+        end
+      end
+    end
+
+    context 'invalid routing rules' do
+      let(:worker) do
+        Class.new do
+          def self.name
+            'Gitlab::Foo::BarWorker'
+          end
+
+          include ApplicationWorker
+        end
+      end
+
+      before do
+        stub_config(sidekiq: { routing_rules: routing_rules })
+      end
+
+      context 'invalid routing rules format' do
+        let(:routing_rules) { ['feature_category=a'] }
+
+        it 'captures the error and falls back to an empty route' do
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(be_a(described_class::InvalidRoutingRuleError))
+
+          expect(described_class.global.route(worker)).to eql('foo_bar')
+        end
+      end
+
+      context 'invalid predicate' do
+        let(:routing_rules) { [['invalid_term=a', 'queue_a']] }
+
+        it 'captures the error and falls back to an empty route' do
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(
+            be_a(Gitlab::SidekiqConfig::WorkerMatcher::UnknownPredicate)
+          )
+
+          expect(described_class.global.route(worker)).to eql('foo_bar')
+        end
+      end
+    end
+  end
+
+  describe '#route' do
+    context 'valid routing rules' do
+      include_context 'router examples setup'
+
+      with_them do
+        it 'routes the worker to the correct queue' do
+          router = described_class.new(routing_rules)
+
+          expect(router.route(worker)).to eql(expected_queue)
+        end
+      end
+    end
+
+    context 'invalid routing rules' do
+      it 'raises an exception' do
+        expect { described_class.new(nil) }.to raise_error(described_class::InvalidRoutingRuleError)
+        expect { described_class.new(['feature_category=a']) }.to raise_error(described_class::InvalidRoutingRuleError)
+        expect { described_class.new([['feature_category=a', 'queue_a', 'queue_b']]) }.to raise_error(described_class::InvalidRoutingRuleError)
+        expect do
+          described_class.new(
+            [
+              ['feature_category=a', 'queue_b'],
+              ['feature_category=b']
+            ]
+          )
+        end.to raise_error(described_class::InvalidRoutingRuleError)
+        expect { described_class.new([['invalid_term=a', 'queue_a']]) }.to raise_error(Gitlab::SidekiqConfig::WorkerMatcher::UnknownPredicate)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/sidekiq_config/worker_spec.rb b/spec/lib/gitlab/sidekiq_config/worker_spec.rb
index 05987f95b33..0c43c33ff8c 100644
--- a/spec/lib/gitlab/sidekiq_config/worker_spec.rb
+++ b/spec/lib/gitlab/sidekiq_config/worker_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
   def create_worker(queue:, **attributes)
     namespace = queue.include?(':') && queue.split(':').first
     inner_worker = double(
+      name: attributes[:worker_name] || 'Foo::BarWorker',
       queue: queue,
       queue_namespace: namespace,
       get_feature_category: attributes[:feature_category],
@@ -87,6 +88,7 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
   describe 'YAML encoding' do
     it 'encodes the worker in YAML as a hash of the queue' do
       attributes_a = {
+        worker_name: 'WorkerA',
         feature_category: :source_code_management,
         has_external_dependencies: false,
         urgency: :low,
@@ -97,6 +99,7 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
       }
 
       attributes_b = {
+        worker_name: 'WorkerB',
         feature_category: :not_owned,
         has_external_dependencies: true,
         urgency: :high,