Merge branch '345794-sidekiq-cluster-leader' into 'master'

Run SidekiqExporter only on first worker

See merge request gitlab-org/gitlab!74625

config/initializers/7_prometheus_metrics.rb

@@ -28,11 +28,15 @@ Gitlab::Application.configure do |config|
   config.middleware.insert_after(Labkit::Middleware::Rack, Gitlab::Metrics::RequestsRackMiddleware)
 end
 
-Sidekiq.configure_server do |config|
-  config.on(:startup) do
-    # Do not clean the metrics directory here - the supervisor script should
-    # have already taken care of that
-    Gitlab::Metrics::Exporter::SidekiqExporter.instance.start
+if Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER_ID'] == '0')
+  # The single worker outside of a sidekiq-cluster, or the first worker (sidekiq_0)
+  # in a cluster of processes, is responsible for serving health checks.
+  Sidekiq.configure_server do |config|
+    config.on(:startup) do
+      # Do not clean the metrics directory here - the supervisor script should
+      # have already taken care of that
+      Gitlab::Metrics::Exporter::SidekiqExporter.instance.start
+    end
   end
 end
 

lib/gitlab/metrics/exporter/sidekiq_exporter.rb

@@ -15,29 +15,6 @@ module Gitlab
             File::NULL
           end
         end
-
-        private
-
-        # Sidekiq Exporter does not work properly in sidekiq-cluster
-        # mode. It tries to start the service on the same port for
-        # each of the cluster workers, this results in failure
-        # due to duplicate binding.
-        #
-        # For now we ignore this error, as metrics are still "kind of"
-        # valid as they are rendered from shared directory.
-        #
-        # Issue: https://gitlab.com/gitlab-org/gitlab/issues/5714
-        def start_working
-          super
-        rescue Errno::EADDRINUSE => e
-          Sidekiq.logger.error(
-            class: self.class.to_s,
-            message: 'Cannot start sidekiq_exporter',
-            'exception.message' => e.message
-          )
-
-          false
-        end
       end
     end
   end

spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb

@@ -50,40 +50,4 @@ RSpec.describe Gitlab::Metrics::Exporter::SidekiqExporter do
       expect(exporter.log_filename).to end_with('sidekiq_exporter.log')
     end
   end
-
-  context 'when port is already taken' do
-    let(:first_exporter) { described_class.new }
-
-    before do
-      stub_config(
-        monitoring: {
-          sidekiq_exporter: {
-            enabled: true,
-            port: 9992,
-            address: '127.0.0.1'
-          }
-        }
-      )
-
-      first_exporter.start
-    end
-
-    after do
-      first_exporter.stop
-    end
-
-    it 'does print error message' do
-      expect(Sidekiq.logger).to receive(:error)
-        .with(
-          class: described_class.to_s,
-          message: 'Cannot start sidekiq_exporter',
-          'exception.message' => anything)
-
-      exporter.start
-    end
-
-    it 'does not start thread' do
-      expect(exporter.start).to be_nil
-    end
-  end
 end

spec/sidekiq_cluster/sidekiq_cluster_spec.rb

@@ -7,19 +7,26 @@ require_relative '../../sidekiq_cluster/sidekiq_cluster'
 RSpec.describe Gitlab::SidekiqCluster do # rubocop:disable RSpec/FilePath
   describe '.start' do
     it 'starts Sidekiq with the given queues, environment and options' do
-      expected_options = {
-        env: :production,
-        directory: 'foo/bar',
-        max_concurrency: 20,
-        min_concurrency: 10,
-        timeout: 25,
-        dryrun: true
+      process_options = {
+        pgroup: true,
+        err: $stderr,
+        out: $stdout
       }
 
-      expect(described_class).to receive(:start_sidekiq).ordered.with(%w(foo), expected_options.merge(worker_id: 0))
-      expect(described_class).to receive(:start_sidekiq).ordered.with(%w(bar baz), expected_options.merge(worker_id: 1))
-
-      described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10, dryrun: true)
+      expect(Process).to receive(:spawn).ordered.with({
+          "ENABLE_SIDEKIQ_CLUSTER" => "1",
+          "SIDEKIQ_WORKER_ID" => "0"
+        },
+        "bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:foo", "-rfoo/bar", "-qfoo,1", process_options
+      )
+      expect(Process).to receive(:spawn).ordered.with({
+        "ENABLE_SIDEKIQ_CLUSTER" => "1",
+        "SIDEKIQ_WORKER_ID" => "1"
+        },
+        "bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:bar,baz", "-rfoo/bar", "-qbar,1", "-qbaz,1", process_options
+      )
+
+      described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10)
     end
 
     it 'starts Sidekiq with the given queues and sensible default options' do