Merge branch 'use-generated-queue-name-for-worker-routing' into 'master'

Always use generated queue name for worker matching

See merge request gitlab-org/gitlab!66285

app/workers/concerns/application_worker.rb

@@ -54,6 +54,10 @@ module ApplicationWorker
       subclass.after_set_class_attribute { subclass.set_queue }
     end
 
+    def generated_queue_name
+      Gitlab::SidekiqConfig::WorkerRouter.queue_name_from_worker_name(self)
+    end
+
     override :validate_worker_attributes!
     def validate_worker_attributes!
       super

doc/administration/operations/extra_sidekiq_routing.md

@@ -103,9 +103,11 @@ based on a subset of worker attributes:
 - `worker_name` - the worker name. The other attributes are typically more useful as
   they are more general, but this is available in case a particular worker needs
   to be selected.
-- `name` - the queue name. The other attributes are typically more useful as
-  they are more general, but this is available in case a particular queue needs
-  to be selected.
+- `name` - the queue name generated from the worker name. The other attributes
+  are typically more useful as they are more general, but this is available in
+  case a particular queue needs to be selected. Because this is generated from
+  the worker name, it does not change based on the result of other routing
+  rules.
 - `resource_boundary` - if the queue is bound by `cpu`, `memory`, or
   `unknown`. For example, the `ProjectExportWorker` is memory bound as it has
   to load data in memory before saving it for export.

ee/spec/lib/gitlab/sidekiq_config_spec.rb

@@ -46,7 +46,7 @@ RSpec.describe Gitlab::SidekiqConfig do
   describe '.workers_for_all_queues_yml' do
     it 'returns a tuple with EE workers second' do
       expect(described_class.workers_for_all_queues_yml.second)
-        .to include(an_object_having_attributes(queue: 'repository_update_mirror'))
+        .to include(an_object_having_attributes(generated_queue_name: 'repository_update_mirror'))
     end
   end
 

lib/gitlab/sidekiq_config/dummy_worker.rb

@@ -5,7 +5,6 @@ module Gitlab
     # For queues that don't have explicit workers - default and mailers
     class DummyWorker
       ATTRIBUTE_METHODS = {
-        queue: :queue,
         name: :name,
         feature_category: :get_feature_category,
         has_external_dependencies: :worker_has_external_dependencies?,
@@ -20,6 +19,10 @@ module Gitlab
         @attributes = attributes
       end
 
+      def generated_queue_name
+        @attributes[:queue]
+      end
+
       def queue_namespace
         nil
       end

lib/gitlab/sidekiq_config/worker.rb

@@ -6,9 +6,11 @@ module Gitlab
       include Comparable
 
       attr_reader :klass
-      delegate :feature_category_not_owned?, :get_feature_category, :get_sidekiq_options,
-               :get_tags, :get_urgency, :get_weight, :get_worker_resource_boundary,
-               :idempotent?, :queue, :queue_namespace, :worker_has_external_dependencies?,
+
+      delegate :feature_category_not_owned?, :generated_queue_name, :get_feature_category,
+               :get_sidekiq_options, :get_tags, :get_urgency, :get_weight,
+               :get_worker_resource_boundary, :idempotent?, :queue_namespace,
+               :worker_has_external_dependencies?,
                to: :klass
 
       def initialize(klass, ee:)
@@ -35,7 +37,7 @@ module Gitlab
 
       # Put namespaced queues first
       def to_sort
-        [queue_namespace ? 0 : 1, queue]
+        [queue_namespace ? 0 : 1, generated_queue_name]
       end
 
       # YAML representation
@@ -45,7 +47,7 @@ module Gitlab
 
       def to_yaml
         {
-          name: queue,
+          name: generated_queue_name,
           worker_name: klass.name,
           feature_category: get_feature_category,
           has_external_dependencies: worker_has_external_dependencies?,
@@ -62,7 +64,7 @@ module Gitlab
       end
 
       def queue_and_weight
-        [queue, get_weight]
+        [generated_queue_name, get_weight]
       end
 
       def retries

spec/lib/gitlab/sidekiq_config/worker_router_spec.rb

@@ -114,6 +114,13 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerRouter do
         ['resource_boundary=cpu', 'queue_b'],
         ['tags=expensive', 'queue_c']
       ] | 'queue_foo'
+      # Match by generated queue name
+      [
+        ['name=foo_bar', 'queue_foo'],
+        ['feature_category=feature_a|urgency=low', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=expensive', 'queue_c']
+      ] | 'queue_foo'
     end
   end
 

spec/lib/gitlab/sidekiq_config/worker_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
     namespace = queue.include?(':') && queue.split(':').first
     inner_worker = double(
       name: attributes[:worker_name] || 'Foo::BarWorker',
-      queue: queue,
+      generated_queue_name: queue,
       queue_namespace: namespace,
       get_feature_category: attributes[:feature_category],
       get_weight: attributes[:weight],
@@ -48,9 +48,9 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
 
   describe 'delegations' do
     [
-      :feature_category_not_owned?, :get_feature_category, :get_weight,
-      :get_worker_resource_boundary, :get_urgency, :queue,
-      :queue_namespace, :worker_has_external_dependencies?
+      :feature_category_not_owned?, :generated_queue_name,
+      :get_feature_category, :get_weight, :get_worker_resource_boundary,
+      :get_urgency, :queue_namespace, :worker_has_external_dependencies?
     ].each do |meth|
       it "delegates #{meth} to the worker class" do
         worker = double

spec/lib/gitlab/sidekiq_config_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Gitlab::SidekiqConfig do
   describe '.workers_for_all_queues_yml' do
     it 'returns a tuple with FOSS workers first' do
       expect(described_class.workers_for_all_queues_yml.first)
-        .to include(an_object_having_attributes(queue: 'post_receive'))
+        .to include(an_object_having_attributes(generated_queue_name: 'post_receive'))
     end
   end
 

spec/workers/every_sidekiq_worker_spec.rb

@@ -8,17 +8,17 @@ RSpec.describe 'Every Sidekiq worker' do
   end
 
   it 'does not use the default queue' do
-    expect(workers_without_defaults.map(&:queue)).not_to include('default')
+    expect(workers_without_defaults.map(&:generated_queue_name)).not_to include('default')
   end
 
   it 'uses the cronjob queue when the worker runs as a cronjob' do
-    expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(start_with('cronjob:'))
+    expect(Gitlab::SidekiqConfig.cron_workers.map(&:generated_queue_name)).to all(start_with('cronjob:'))
   end
 
   it 'has its queue in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS', :aggregate_failures do
     file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set
 
-    worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set
+    worker_queues = Gitlab::SidekiqConfig.workers.map(&:generated_queue_name).to_set
     worker_queues << ActionMailer::MailDeliveryJob.new.queue_name
     worker_queues << 'default'
 
@@ -33,7 +33,7 @@ RSpec.describe 'Every Sidekiq worker' do
     config_queues = Gitlab::SidekiqConfig.config_queues.to_set
 
     Gitlab::SidekiqConfig.workers.each do |worker|
-      queue = worker.queue
+      queue = worker.generated_queue_name
       queue_namespace = queue.split(':').first
 
       expect(config_queues).to include(queue).or(include(queue_namespace))