Merge branch 'an-multithreading' into 'master'

Puma in GDK and rack server lifecycle event abstractions

Closes #52762

See merge request gitlab-org/gitlab-ce!22372

4 files changed, 140 insertions, 21 deletions

diff --git a/spec/rack_servers/configs/config.ru b/spec/rack_servers/configs/config.ru
new file mode 100644
index 00000000000..63daeb9eec5
--- /dev/null
+++ b/spec/rack_servers/configs/config.ru
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+app = proc do |env|
+  if env['REQUEST_METHOD'] == 'GET'
+    [200, {}, ["#{Process.pid}"]]
+  else
+    Process.kill(env['QUERY_STRING'], Process.pid)
+    [200, {}, ['Bye!']]
+  end
+end
+
+run app
diff --git a/spec/rack_servers/configs/puma.rb b/spec/rack_servers/configs/puma.rb
new file mode 100644
index 00000000000..d6b6d83d648
--- /dev/null
+++ b/spec/rack_servers/configs/puma.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+# Note: this file is used for testing puma in `spec/rack_servers/puma_spec.rb` only
+# Note: as per the convention in `config/puma.example.development.rb`,
+# this file will replace `/home/git` with the actual working directory
+
+directory '/home/git'
+threads 1, 10
+queue_requests false
+pidfile '/home/git/gitlab/tmp/pids/puma.pid'
+bind 'unix:///home/git/gitlab/tmp/tests/puma.socket'
+workers 1
+preload_app!
+worker_timeout 60
+
+require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events"
+require_relative "/home/git/gitlab/lib/gitlab/cluster/puma_worker_killer_initializer"
+
+before_fork do
+  Gitlab::Cluster::PumaWorkerKillerInitializer.start @config.options
+  Gitlab::Cluster::LifecycleEvents.do_before_fork
+end
+
+Gitlab::Cluster::LifecycleEvents.set_puma_options @config.options
+on_worker_boot do
+  Gitlab::Cluster::LifecycleEvents.do_worker_start
+  File.write('/home/git/gitlab/tmp/tests/puma-worker-ready', Process.pid)
+end
+
+on_restart do
+  Gitlab::Cluster::LifecycleEvents.do_master_restart
+end
diff --git a/spec/rack_servers/puma_spec.rb b/spec/rack_servers/puma_spec.rb
new file mode 100644
index 00000000000..431fab87857
--- /dev/null
+++ b/spec/rack_servers/puma_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'fileutils'
+
+require 'excon'
+
+require 'spec_helper'
+
+describe 'Puma' do
+  before(:all) do
+    project_root = File.expand_path('../..', __dir__)
+
+    config_lines = File.read('spec/rack_servers/configs/puma.rb')
+      .gsub('/home/git/gitlab', project_root)
+      .gsub('/home/git', project_root)
+
+    config_path = File.join(project_root, "tmp/tests/puma.rb")
+    @socket_path = File.join(project_root, 'tmp/tests/puma.socket')
+
+    File.write(config_path, config_lines)
+
+    cmd = %W[puma -e test -C #{config_path} #{File.join(__dir__, 'configs/config.ru')}]
+    @puma_master_pid = spawn(*cmd)
+    wait_puma_boot!(@puma_master_pid, File.join(project_root, 'tmp/tests/puma-worker-ready'))
+    WebMock.allow_net_connect!
+  end
+
+  %w[SIGQUIT SIGTERM SIGKILL].each do |signal|
+    it "has a worker that self-terminates on signal #{signal}" do
+      response = Excon.get('unix://', socket: @socket_path)
+      expect(response.status).to eq(200)
+
+      worker_pid = response.body.to_i
+      expect(worker_pid).to be > 0
+
+      begin
+        Excon.post("unix://?#{signal}", socket: @socket_path)
+      rescue Excon::Error::Socket
+        # The connection may be closed abruptly
+      end
+
+      expect(pid_gone?(worker_pid)).to eq(true)
+    end
+  end
+
+  after(:all) do
+    begin
+      WebMock.disable_net_connect!(allow_localhost: true)
+      Process.kill('TERM', @puma_master_pid)
+    rescue Errno::ESRCH
+    end
+  end
+
+  def wait_puma_boot!(master_pid, ready_file)
+    # We have seen the boot timeout after 2 minutes in CI so let's set it to 5 minutes.
+    timeout = 5 * 60
+    timeout.times do
+      return if File.exist?(ready_file)
+
+      pid = Process.waitpid(master_pid, Process::WNOHANG)
+      raise "puma failed to boot: #{$?}" unless pid.nil?
+
+      sleep 1
+    end
+
+    raise "puma boot timed out after #{timeout} seconds"
+  end
+
+  def pid_gone?(pid)
+    # Worker termination should take less than a second. That makes 10
+    # seconds a generous timeout.
+    10.times do
+      begin
+        Process.kill(0, pid)
+      rescue Errno::ESRCH
+        return true
+      end
+
+      sleep 1
+    end
+
+    false
+  end
+end
diff --git a/spec/unicorn/unicorn_spec.rb b/spec/rack_servers/unicorn_spec.rb
index a4cf479a339..6a02ebcd048 100644
--- a/spec/unicorn/unicorn_spec.rb
+++ b/spec/rack_servers/unicorn_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'fileutils'
 
 require 'excon'
@@ -6,12 +8,16 @@ require 'spec_helper'
 
 describe 'Unicorn' do
   before(:all) do
-    config_lines = File.read('config/unicorn.rb.example').split("\n")
+    project_root = File.expand_path('../..', __dir__)
+
+    config_lines = File.read('config/unicorn.rb.example')
+      .gsub('/home/git/gitlab', project_root)
+      .gsub('/home/git', project_root)
+      .split("\n")
 
     # Remove these because they make setup harder.
     config_lines = config_lines.reject do |line|
       %w[
-        working_directory
         worker_processes
         listen
         pid
@@ -26,33 +32,18 @@ describe 'Unicorn' do
     # predictable which process will handle our requests.
     config_lines << 'worker_processes 1'
 
-    @socket_path = File.join(Dir.pwd, 'tmp/tests/unicorn.socket')
+    @socket_path = File.join(project_root, 'tmp/tests/unicorn.socket')
     config_lines << "listen '#{@socket_path}'"
 
-    ready_file = 'tmp/tests/unicorn-worker-ready'
+    ready_file = File.join(project_root, 'tmp/tests/unicorn-worker-ready')
     FileUtils.rm_f(ready_file)
     after_fork_index = config_lines.index { |l| l.start_with?('after_fork') }
     config_lines.insert(after_fork_index + 1, "File.write('#{ready_file}', Process.pid)")
 
-    config_path = 'tmp/tests/unicorn.rb'
+    config_path = File.join(project_root, 'tmp/tests/unicorn.rb')
     File.write(config_path, config_lines.join("\n") + "\n")
 
-    rackup_path = 'tmp/tests/config.ru'
-    File.write(rackup_path, <<~EOS)
-      app =
-        proc do |env|
-          if env['REQUEST_METHOD'] == 'GET'
-            [200, {}, [Process.pid]]
-          else
-            Process.kill(env['QUERY_STRING'], Process.pid)
-            [200, {}, ['Bye!']]
-          end
-        end
-
-      run app
-    EOS
-
-    cmd = %W[unicorn -E test -c #{config_path} #{rackup_path}]
+    cmd = %W[unicorn -E test -c #{config_path} spec/rack_servers/configs/config.ru]
     @unicorn_master_pid = spawn(*cmd)
     wait_unicorn_boot!(@unicorn_master_pid, ready_file)
     WebMock.allow_net_connect!