Store worker queues as hashes

This allows us to include the annotations such as a worker's resource
boundaries, so in future we can use those annotations in
sidekiq-cluster.

ee/spec/lib/gitlab/sidekiq_config_spec.rb

@@ -50,12 +50,14 @@ describe Gitlab::SidekiqConfig do
   end
 
   describe '.all_queues_yml_outdated?' do
-    before do
-      workers = [
+    let(:workers) do
+      [
         LdapGroupSyncWorker,
         RepositoryUpdateMirrorWorker
       ].map { |worker| described_class::Worker.new(worker, ee: true) }
+    end
 
+    before do
       allow(described_class).to receive(:workers).and_return(workers)
 
       allow(YAML).to receive(:load_file)
@@ -66,7 +68,7 @@ describe Gitlab::SidekiqConfig do
     it 'returns true if the YAML file does not match the application code' do
       allow(YAML).to receive(:load_file)
                        .with(described_class::EE_QUEUE_CONFIG_PATH)
-                       .and_return(['ldap_group_sync'])
+                       .and_return([workers.first.to_yaml])
 
       expect(described_class.all_queues_yml_outdated?).to be(true)
     end
@@ -74,7 +76,7 @@ describe Gitlab::SidekiqConfig do
     it 'returns false if the YAML file matches the application code' do
       allow(YAML).to receive(:load_file)
                        .with(described_class::EE_QUEUE_CONFIG_PATH)
-                       .and_return(%w[ldap_group_sync repository_update_mirror])
+                       .and_return(workers.map(&:to_yaml))
 
       expect(described_class.all_queues_yml_outdated?).to be(false)
     end

lib/gitlab/sidekiq_config.rb

@@ -13,21 +13,10 @@ module Gitlab
       (EE_QUEUE_CONFIG_PATH if Gitlab.ee?)
     ].compact.freeze
 
-    # For queues that don't have explicit workers - default and mailers
-    DummyWorker = Struct.new(:queue, :weight) do
-      def queue_namespace
-        nil
-      end
-
-      def get_weight
-        weight
-      end
-    end
-
     DEFAULT_WORKERS = [
-      Gitlab::SidekiqConfig::Worker.new(DummyWorker.new('default', 1), ee: false),
-      Gitlab::SidekiqConfig::Worker.new(DummyWorker.new('mailers', 2), ee: false)
-    ].freeze
+      DummyWorker.new('default', weight: 1),
+      DummyWorker.new('mailers', weight: 2)
+    ].map { |worker| Gitlab::SidekiqConfig::Worker.new(worker, ee: false) }.freeze
 
     class << self
       include Gitlab::SidekiqConfig::CliMethods

lib/gitlab/sidekiq_config/cli_methods.rb

@@ -25,6 +25,7 @@ module Gitlab
           full_path = File.join(rails_path, path)
           queues = File.exist?(full_path) ? YAML.load_file(full_path) : []
 
+          # https://gitlab.com/gitlab-org/gitlab/issues/199230
           queues.map { |queue| queue.is_a?(Hash) ? queue[:name] : queue }
         end
       end

lib/gitlab/sidekiq_config/dummy_worker.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module SidekiqConfig
+    # For queues that don't have explicit workers - default and mailers
+    class DummyWorker
+      attr_accessor :queue
+
+      ATTRIBUTE_METHODS = {
+        feature_category: :get_feature_category,
+        has_external_dependencies: :worker_has_external_dependencies?,
+        latency_sensitive: :latency_sensitive_worker?,
+        resource_boundary: :get_worker_resource_boundary,
+        weight: :get_weight
+      }.freeze
+
+      def initialize(queue, attributes = {})
+        @queue = queue
+        @attributes = attributes
+      end
+
+      def queue_namespace
+        nil
+      end
+
+      ATTRIBUTE_METHODS.each do |attribute, meth|
+        define_method meth do
+          @attributes[attribute]
+        end
+      end
+    end
+  end
+end

lib/gitlab/sidekiq_config/worker.rb

@@ -41,11 +41,18 @@ module Gitlab
 
       # YAML representation
       def encode_with(coder)
-        coder.represent_scalar(nil, to_yaml)
+        coder.represent_map(nil, to_yaml)
       end
 
       def to_yaml
-        queue
+        {
+          name: queue,
+          feature_category: get_feature_category,
+          has_external_dependencies: worker_has_external_dependencies?,
+          latency_sensitive: latency_sensitive_worker?,
+          resource_boundary: get_worker_resource_boundary,
+          weight: get_weight
+        }
       end
 
       def namespace_and_weight

spec/lib/gitlab/sidekiq_config/cli_methods_spec.rb

@@ -35,11 +35,7 @@ describe Gitlab::SidekiqConfig::CliMethods do
         stub_exists(exists: true)
       end
 
-      context 'when the file contains an array of strings' do
-        before do
-          stub_contents(['queue_a'], ['queue_b'])
-        end
-
+      shared_examples 'valid file contents' do
         it 'memoizes the result' do
           result = described_class.worker_queues(dummy_root)
 
@@ -49,28 +45,28 @@ describe Gitlab::SidekiqConfig::CliMethods do
         end
 
         it 'flattens and joins the contents' do
+          expected_queues = %w[queue_a queue_b]
+          expected_queues = expected_queues.first(1) unless Gitlab.ee?
+
           expect(described_class.worker_queues(dummy_root))
-            .to contain_exactly('queue_a', 'queue_b')
+            .to match_array(expected_queues)
         end
       end
 
-      context 'when the file contains an array of hashes' do
+      context 'when the file contains an array of strings' do
         before do
-          stub_contents([{ name: 'queue_a' }], [{ name: 'queue_b' }])
+          stub_contents(['queue_a'], ['queue_b'])
         end
 
-        it 'memoizes the result' do
-          result = described_class.worker_queues(dummy_root)
-
-          stub_exists(exists: false)
-
-          expect(described_class.worker_queues(dummy_root)).to eq(result)
+        include_examples 'valid file contents'
       end
 
-        it 'flattens and joins the values of the name field' do
-          expect(described_class.worker_queues(dummy_root))
-            .to contain_exactly('queue_a', 'queue_b')
+      context 'when the file contains an array of hashes' do
+        before do
+          stub_contents([{ name: 'queue_a' }], [{ name: 'queue_b' }])
         end
+
+        include_examples 'valid file contents'
       end
     end
 

spec/lib/gitlab/sidekiq_config/worker_spec.rb

@@ -3,9 +3,17 @@
 require 'fast_spec_helper'
 
 describe Gitlab::SidekiqConfig::Worker do
-  def create_worker(queue:, weight: 0)
+  def create_worker(queue:, **attributes)
     namespace = queue.include?(':') && queue.split(':').first
-    inner_worker = double(queue: queue, queue_namespace: namespace, get_weight: weight)
+    inner_worker = double(
+      queue: queue,
+      queue_namespace: namespace,
+      get_feature_category: attributes[:feature_category],
+      get_weight: attributes[:weight],
+      get_worker_resource_boundary: attributes[:resource_boundary],
+      latency_sensitive_worker?: attributes[:latency_sensitive],
+      worker_has_external_dependencies?: attributes[:has_external_dependencies]
+    )
 
     described_class.new(inner_worker, ee: false)
   end
@@ -75,13 +83,32 @@ describe Gitlab::SidekiqConfig::Worker do
   end
 
   describe 'YAML encoding' do
-    it 'encodes the worker in YAML as a string of the queue' do
-      worker_a = create_worker(queue: 'a')
-      worker_b = create_worker(queue: 'b')
+    it 'encodes the worker in YAML as a hash of the queue' do
+      attributes_a = {
+        feature_category: :source_code_management,
+        has_external_dependencies: false,
+        latency_sensitive: false,
+        resource_boundary: :memory,
+        weight: 2
+      }
+
+      attributes_b = {
+        feature_category: :not_owned,
+        has_external_dependencies: true,
+        latency_sensitive: true,
+        resource_boundary: :unknown,
+        weight: 1
+      }
+
+      worker_a = create_worker(queue: 'a', **attributes_a)
+      worker_b = create_worker(queue: 'b', **attributes_b)
+
+      expect(YAML.dump(worker_a))
+        .to eq(YAML.dump(attributes_a.reverse_merge(name: 'a')))
 
-      expect(YAML.dump(worker_a)).to eq(YAML.dump('a'))
       expect(YAML.dump([worker_a, worker_b]))
-        .to eq(YAML.dump(%w[a b]))
+        .to eq(YAML.dump([attributes_a.reverse_merge(name: 'a'),
+                          attributes_b.reverse_merge(name: 'b')]))
     end
   end
 

spec/lib/gitlab/sidekiq_config_spec.rb

@@ -32,21 +32,23 @@ describe Gitlab::SidekiqConfig do
   end
 
   describe '.all_queues_yml_outdated?' do
-    before do
-      workers = [
-        PostReceive,
+    let(:workers) do
+      [
         MergeWorker,
+        PostReceive,
         ProcessCommitWorker
       ].map { |worker| described_class::Worker.new(worker, ee: false) }
+    end
 
+    before do
       allow(described_class).to receive(:workers).and_return(workers)
       allow(Gitlab).to receive(:ee?).and_return(false)
     end
 
-    it 'returns true if the YAML file does not match the application code' do
+    it 'returns true if the YAML file does not matcph the application code' do
       allow(YAML).to receive(:load_file)
                        .with(described_class::FOSS_QUEUE_CONFIG_PATH)
-                       .and_return(%w[post_receive merge])
+                       .and_return(workers.first(2).map(&:to_yaml))
 
       expect(described_class.all_queues_yml_outdated?).to be(true)
     end
@@ -54,7 +56,7 @@ describe Gitlab::SidekiqConfig do
     it 'returns false if the YAML file matches the application code' do
       allow(YAML).to receive(:load_file)
                        .with(described_class::FOSS_QUEUE_CONFIG_PATH)
-                       .and_return(%w[merge post_receive process_commit])
+                       .and_return(workers.map(&:to_yaml))
 
       expect(described_class.all_queues_yml_outdated?).to be(false)
     end