Merge branch '295665-fix-sidekiq-check' into 'master'

Fix Sidekiq system check for cluster mode

See merge request gitlab-org/gitlab!55530

changelogs/unreleased/295665-fix-sidekiq-check.yml

+---
+title: Fix Sidekiq system check for cluster mode
+merge_request: 55530
+author: Horst Prote
+type: other

lib/system_check/sidekiq_check.rb

@@ -15,7 +15,7 @@ module SystemCheck
     def check_sidekiq_running
       $stdout.print "Running? ... "
 
-      if sidekiq_process_count > 0
+      if sidekiq_worker_process_count > 0
         $stdout.puts "yes".color(:green)
       else
         $stdout.puts "no".color(:red)
@@ -31,15 +31,16 @@ module SystemCheck
     end
 
     def only_one_sidekiq_running
-      process_count = sidekiq_process_count
-      return if process_count == 0
+      worker_count = sidekiq_worker_process_count
+      cluster_count = sidekiq_cluster_process_count
+      return if worker_count == 0
 
-      $stdout.print 'Number of Sidekiq processes ... '
+      $stdout.print 'Number of Sidekiq processes (cluster/worker) ... '
 
-      if process_count == 1
-        $stdout.puts '1'.color(:green)
+      if (cluster_count == 1 && worker_count > 0) || (cluster_count == 0 && worker_count == 1)
+        $stdout.puts "#{cluster_count}/#{worker_count}".color(:green)
       else
-        $stdout.puts "#{process_count}".color(:red)
+        $stdout.puts "#{cluster_count}/#{worker_count}".color(:red)
         try_fixing_it(
           'sudo service gitlab stop',
           "sudo pkill -u #{gitlab_user} -f sidekiq",
@@ -50,9 +51,14 @@ module SystemCheck
       end
     end
 
-    def sidekiq_process_count
+    def sidekiq_worker_process_count
       ps_ux, _ = Gitlab::Popen.popen(%w(ps uxww))
-      ps_ux.scan(/sidekiq \d+\.\d+\.\d+/).count
+      ps_ux.lines.grep(/sidekiq \d+\.\d+\.\d+/).count
+    end
+
+    def sidekiq_cluster_process_count
+      ps_ux, _ = Gitlab::Popen.popen(%w(ps uxww))
+      ps_ux.lines.grep(/sidekiq-cluster/).count
     end
   end
 end

spec/lib/system_check/sidekiq_check_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SystemCheck::SidekiqCheck do
+  describe '#multi_check' do
+    def stub_ps_output(output)
+      allow(Gitlab::Popen).to receive(:popen).with(%w(ps uxww)).and_return([output, nil])
+    end
+
+    def expect_check_output(matcher)
+      expect { subject.multi_check }.to output(matcher).to_stdout
+    end
+
+    it 'fails when no worker processes are running' do
+      stub_ps_output <<~PS
+        root 2193947  0.9  0.1 146564 18104 ?        Ssl  17:34   0:00 ruby bin/sidekiq-cluster * -P ...
+      PS
+
+      expect_check_output include(
+        'Running? ... no',
+        'Please fix the error above and rerun the checks.'
+      )
+    end
+
+    it 'fails when more than one cluster process is running' do
+      stub_ps_output <<~PS
+        root 2193947  0.9  0.1 146564 18104 ?        Ssl  17:34   0:00 ruby bin/sidekiq-cluster * -P ...
+        root 2193948  0.9  0.1 146564 18104 ?        Ssl  17:34   0:00 ruby bin/sidekiq-cluster * -P ...
+        root 2193955 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+      PS
+
+      expect_check_output include(
+        'Running? ... yes',
+        'Number of Sidekiq processes (cluster/worker) ... 2/1',
+        'Please fix the error above and rerun the checks.'
+      )
+    end
+
+    it 'succeeds when one cluster process and one or more worker processes are running' do
+      stub_ps_output <<~PS
+        root 2193947  0.9  0.1 146564 18104 ?        Ssl  17:34   0:00 ruby bin/sidekiq-cluster * -P ...
+        root 2193955 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+        root 2193956 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+      PS
+
+      expect_check_output <<~OUTPUT
+        Running? ... yes
+        Number of Sidekiq processes (cluster/worker) ... 1/2
+      OUTPUT
+    end
+
+    # TODO: Running without a cluster is deprecated and will be removed in GitLab 14.0
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/323225
+    context 'when running without a cluster' do
+      it 'fails when more than one worker process is running' do
+        stub_ps_output <<~PS
+          root 2193955 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+          root 2193956 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+        PS
+
+        expect_check_output include(
+          'Running? ... yes',
+          'Number of Sidekiq processes (cluster/worker) ... 0/2',
+          'Please fix the error above and rerun the checks.'
+        )
+      end
+
+      it 'succeeds when one worker process is running' do
+        stub_ps_output <<~PS
+          root 2193955 92.2  3.1 4675972 515516 ?      Sl   17:34   0:13 sidekiq 5.2.9 ...
+        PS
+
+        expect_check_output <<~OUTPUT
+          Running? ... yes
+          Number of Sidekiq processes (cluster/worker) ... 0/1
+        OUTPUT
+      end
+    end
+  end
+end