Add praefect as a socket for tests

Praefect functions as a proxy, but there are a few RPCs that are only in
praefect and do not get proxied to gitaly. To test functionality that
hits these RPCs, we need a way to hit a praefect instance that talks to
a gitaly instance.

This MR sets up praefect in front of the existing gitaly that is used in
tests.

lib/gitlab/setup_helper.rb

@@ -4,75 +4,110 @@ require 'toml-rb'
 
 module Gitlab
   module SetupHelper
-    class << self
-      # We cannot create config.toml files for all possible Gitaly configuations.
-      # For instance, if Gitaly is running on another machine then it makes no
-      # sense to write a config.toml file on the current machine. This method will
-      # only generate a configuration for the most common and simplest case: when
-      # we have exactly one Gitaly process and we are sure it is running locally
-      # because it uses a Unix socket.
-      # For development and testing purposes, an extra storage is added to gitaly,
-      # which is not known to Rails, but must be explicitly stubbed.
-      def gitaly_configuration_toml(gitaly_dir, storage_paths, gitaly_ruby: true)
-        storages = []
-        address = nil
-
-        Gitlab.config.repositories.storages.each do |key, val|
-          if address
-            if address != val['gitaly_address']
-              raise ArgumentError, "Your gitlab.yml contains more than one gitaly_address."
+    def create_configuration(dir, storage_paths, force: false)
+      generate_configuration(
+        configuration_toml(dir, storage_paths),
+        get_config_path(dir),
+        force: force
+      )
+    end
+
+    # rubocop:disable Rails/Output
+    def generate_configuration(toml_data, config_path, force: false)
+      FileUtils.rm_f(config_path) if force
+
+      File.open(config_path, File::WRONLY | File::CREAT | File::EXCL) do |f|
+        f.puts toml_data
+      end
+    rescue Errno::EEXIST
+      puts 'Skipping config.toml generation:'
+      puts 'A configuration file already exists.'
+    rescue ArgumentError => e
+      puts 'Skipping config.toml generation:'
+      puts e.message
+    end
+    # rubocop:enable Rails/Output
+
+    module Gitaly
+      extend Gitlab::SetupHelper
+      class << self
+        # We cannot create config.toml files for all possible Gitaly configuations.
+        # For instance, if Gitaly is running on another machine then it makes no
+        # sense to write a config.toml file on the current machine. This method will
+        # only generate a configuration for the most common and simplest case: when
+        # we have exactly one Gitaly process and we are sure it is running locally
+        # because it uses a Unix socket.
+        # For development and testing purposes, an extra storage is added to gitaly,
+        # which is not known to Rails, but must be explicitly stubbed.
+        def configuration_toml(gitaly_dir, storage_paths, gitaly_ruby: true)
+          storages = []
+          address = nil
+
+          Gitlab.config.repositories.storages.each do |key, val|
+            if address
+              if address != val['gitaly_address']
+                raise ArgumentError, "Your gitlab.yml contains more than one gitaly_address."
+              end
+            elsif URI(val['gitaly_address']).scheme != 'unix'
+              raise ArgumentError, "Automatic config.toml generation only supports 'unix:' addresses."
+            else
+              address = val['gitaly_address']
             end
-          elsif URI(val['gitaly_address']).scheme != 'unix'
-            raise ArgumentError, "Automatic config.toml generation only supports 'unix:' addresses."
-          else
-            address = val['gitaly_address']
+
+            storages << { name: key, path: storage_paths[key] }
           end
 
-          storages << { name: key, path: storage_paths[key] }
-        end
+          config = { socket_path: address.sub(/\Aunix:/, '') }
 
-        if Rails.env.test?
-          storage_path = Rails.root.join('tmp', 'tests', 'second_storage').to_s
-          storages << { name: 'test_second_storage', path: storage_path }
-        end
+          if Rails.env.test?
+            storage_path = Rails.root.join('tmp', 'tests', 'second_storage').to_s
+            storages << { name: 'test_second_storage', path: storage_path }
+
+            config[:auth] = { token: 'secret' }
+            # Compared to production, tests run in constrained environments. This
+            # number is meant to grow with the number of concurrent rails requests /
+            # sidekiq jobs, and concurrency will be low anyway in test.
+            config[:git] = { catfile_cache_size: 5 }
+          end
 
-        config = { socket_path: address.sub(/\Aunix:/, ''), storage: storages }
-        config[:auth] = { token: 'secret' } if Rails.env.test?
+          config[:storage] = storages
 
-        internal_socket_dir = File.join(gitaly_dir, 'internal_sockets')
-        FileUtils.mkdir(internal_socket_dir) unless File.exist?(internal_socket_dir)
-        config[:internal_socket_dir] = internal_socket_dir
+          internal_socket_dir = File.join(gitaly_dir, 'internal_sockets')
+          FileUtils.mkdir(internal_socket_dir) unless File.exist?(internal_socket_dir)
+          config[:internal_socket_dir] = internal_socket_dir
 
-        config[:'gitaly-ruby'] = { dir: File.join(gitaly_dir, 'ruby') } if gitaly_ruby
-        config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path }
-        config[:bin_dir] = Gitlab.config.gitaly.client_path
+          config[:'gitaly-ruby'] = { dir: File.join(gitaly_dir, 'ruby') } if gitaly_ruby
+          config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path }
+          config[:bin_dir] = Gitlab.config.gitaly.client_path
 
-        if Rails.env.test?
-          # Compared to production, tests run in constrained environments. This
-          # number is meant to grow with the number of concurrent rails requests /
-          # sidekiq jobs, and concurrency will be low anyway in test.
-          config[:git] = { catfile_cache_size: 5 }
+          TomlRB.dump(config)
         end
 
-        TomlRB.dump(config)
+        private
+
+        def get_config_path(dir)
+          File.join(dir, 'config.toml')
+        end
       end
+    end
+
+    module Praefect
+      extend Gitlab::SetupHelper
+      class << self
+        def configuration_toml(gitaly_dir, storage_paths)
+          nodes = [{ storage: 'default', address: "unix:#{gitaly_dir}/gitaly.socket", primary: true, token: 'secret' }]
+          config = { socket_path: "#{gitaly_dir}/praefect.socket", virtual_storage_name: 'default', token: 'secret', node: nodes }
+          config[:token] = 'secret' if Rails.env.test?
+
+          TomlRB.dump(config)
+        end
 
-      # rubocop:disable Rails/Output
-      def create_gitaly_configuration(dir, storage_paths, force: false)
-        config_path = File.join(dir, 'config.toml')
-        FileUtils.rm_f(config_path) if force
+        private
 
-        File.open(config_path, File::WRONLY | File::CREAT | File::EXCL) do |f|
-          f.puts gitaly_configuration_toml(dir, storage_paths)
+        def get_config_path(dir)
+          File.join(dir, 'praefect.config.toml')
         end
-      rescue Errno::EEXIST
-        puts "Skipping config.toml generation:"
-        puts "A configuration file already exists."
-      rescue ArgumentError => e
-        puts "Skipping config.toml generation:"
-        puts e.message
       end
-      # rubocop:enable Rails/Output
     end
   end
 end

lib/tasks/gitlab/gitaly.rake

@@ -27,7 +27,7 @@ Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]")
       end
 
       storage_paths = { 'default' => args.storage_path }
-      Gitlab::SetupHelper.create_gitaly_configuration(args.dir, storage_paths)
+      Gitlab::SetupHelper::Gitaly.create_configuration(args.dir, storage_paths)
       Dir.chdir(args.dir) do
         # In CI we run scripts/gitaly-test-build instead of this command
         unless ENV['CI'].present?

scripts/gitaly-test-build

@@ -17,13 +17,16 @@ class GitalyTestBuild
     check_gitaly_config!
 
     # Starting gitaly further validates its configuration
-    pid = start_gitaly
-    Process.kill('TERM', pid)
+    gitaly_pid = start_gitaly
+    praefect_pid = start_praefect
+    Process.kill('TERM', gitaly_pid)
+    Process.kill('TERM', praefect_pid)
 
     # Make the 'gitaly' executable look newer than 'GITALY_SERVER_VERSION'.
     # Without this a gitaly executable created in the setup-test-env job
     # will look stale compared to GITALY_SERVER_VERSION.
     FileUtils.touch(File.join(tmp_tests_gitaly_dir, 'gitaly'), mtime: Time.now + (1 << 24))
+    FileUtils.touch(File.join(tmp_tests_gitaly_dir, 'praefect'), mtime: Time.now + (1 << 24))
   end
 end
 

scripts/gitaly-test-spawn

@@ -13,10 +13,9 @@ class GitalyTestSpawn
     # # Uncomment line below to see all gitaly logs merged into CI trace
     # spawn('sleep 1; tail -f log/gitaly-test.log')
 
-    pid = start_gitaly
-
     # In local development this pid file is used by rspec.
-    IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), pid)
+    IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), start_gitaly)
+    IO.write(File.expand_path('../tmp/tests/praefect.pid', __dir__), start_praefect)
   end
 end
 

scripts/gitaly_test.rb

@@ -37,16 +37,31 @@ module GitalyTest
     env_hash
   end
 
-  def config_path
-    File.join(tmp_tests_gitaly_dir, 'config.toml')
+  def config_path(service)
+    case service
+    when :gitaly
+      File.join(tmp_tests_gitaly_dir, 'config.toml')
+    when :praefect
+      File.join(tmp_tests_gitaly_dir, 'praefect.config.toml')
+    end
   end
 
   def start_gitaly
-    args = %W[#{tmp_tests_gitaly_dir}/gitaly #{config_path}]
-    pid = spawn(env, *args, [:out, :err] => 'log/gitaly-test.log')
+    start(:gitaly)
+  end
+
+  def start_praefect
+    start(:praefect)
+  end
+
+  def start(service)
+    args = ["#{tmp_tests_gitaly_dir}/#{service}"]
+    args.push("-config") if service == :praefect
+    args.push(config_path(service))
+    pid = spawn(env, *args, [:out, :err] => "log/#{service}-test.log")
 
     begin
-      try_connect!
+      try_connect!(service)
     rescue
       Process.kill('TERM', pid)
       raise
@@ -68,11 +83,11 @@ module GitalyTest
     abort 'bundle check failed' unless system(env, 'bundle', 'check', chdir: File.dirname(gemfile))
   end
 
-  def read_socket_path
+  def read_socket_path(service)
     # This code needs to work in an environment where we cannot use bundler,
     # so we cannot easily use the toml-rb gem. This ad-hoc parser should be
     # good enough.
-    config_text = IO.read(config_path)
+    config_text = IO.read(config_path(service))
 
     config_text.lines.each do |line|
       match_data = line.match(/^\s*socket_path\s*=\s*"([^"]*)"$/)
@@ -80,14 +95,14 @@ module GitalyTest
       return match_data[1] if match_data
     end
 
-    raise "failed to find socket_path in #{config_path}"
+    raise "failed to find socket_path in #{config_path(service)}"
   end
 
-  def try_connect!
-    print "Trying to connect to gitaly: "
+  def try_connect!(service)
+    print "Trying to connect to #{service}: "
     timeout = 20
     delay = 0.1
-    socket = read_socket_path
+    socket = read_socket_path(service)
 
     Integer(timeout / delay).times do
       UNIXSocket.new(socket)

spec/support/helpers/test_env.rb

 # frozen_string_literal: true
 
 require 'rspec/mocks'
-require 'toml-rb'
 
 module TestEnv
   extend ActiveSupport::Concern
@@ -87,7 +86,7 @@ module TestEnv
     'conflict-resolvable-fork'   => '404fa3f'
   }.freeze
 
-  TMP_TEST_PATH = Rails.root.join('tmp', 'tests', '**')
+  TMP_TEST_PATH = Rails.root.join('tmp', 'tests').freeze
   REPOS_STORAGE = 'default'.freeze
   SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage')
 
@@ -140,7 +139,7 @@ module TestEnv
   #
   # Keeps gitlab-shell and gitlab-test
   def clean_test_path
-    Dir[TMP_TEST_PATH].each do |entry|
+    Dir[File.join(TMP_TEST_PATH, '**')].each do |entry|
       unless test_dirs.include?(File.basename(entry))
         FileUtils.rm_rf(entry)
       end
@@ -164,7 +163,8 @@ module TestEnv
       install_dir: gitaly_dir,
       version: Gitlab::GitalyClient.expected_server_version,
       task: "gitlab:gitaly:install[#{install_gitaly_args}]") do
-        Gitlab::SetupHelper.create_gitaly_configuration(gitaly_dir, { 'default' => repos_path }, force: true)
+        Gitlab::SetupHelper::Gitaly.create_configuration(gitaly_dir, { 'default' => repos_path }, force: true)
+        Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true)
         start_gitaly(gitaly_dir)
       end
   end
@@ -192,17 +192,38 @@ module TestEnv
       end
     end
 
-    @gitaly_pid = Integer(File.read('tmp/tests/gitaly.pid'))
+    gitaly_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly.pid')))
+    praefect_pid = Integer(File.read(TMP_TEST_PATH.join('praefect.pid')))
 
-    Kernel.at_exit { stop_gitaly }
+    Kernel.at_exit { stop(gitaly_pid) }
+    Kernel.at_exit { stop(praefect_pid) }
 
-    wait_gitaly
+    wait('gitaly')
+    wait('praefect')
   end
 
-  def wait_gitaly
+  def stop(pid)
+    Process.kill('KILL', pid)
+  rescue Errno::ESRCH
+    # The process can already be gone if the test run was INTerrupted.
+  end
+
+  def gitaly_url
+    ENV.fetch('GITALY_REPO_URL', nil)
+  end
+
+  def socket_path(service)
+    TMP_TEST_PATH.join('gitaly', "#{service}.socket").to_s
+  end
+
+  def praefect_socket_path
+    "unix:" + socket_path(:praefect)
+  end
+
+  def wait(service)
     sleep_time = 10
     sleep_interval = 0.1
-    socket = Gitlab::GitalyClient.address('default').sub('unix:', '')
+    socket = socket_path(service)
 
     Integer(sleep_time / sleep_interval).times do
       Socket.unix(socket)
@@ -211,19 +232,7 @@ module TestEnv
       sleep sleep_interval
     end
 
-    raise "could not connect to gitaly at #{socket.inspect} after #{sleep_time} seconds"
-  end
-
-  def stop_gitaly
-    return unless @gitaly_pid
-
-    Process.kill('KILL', @gitaly_pid)
-  rescue Errno::ESRCH
-    # The process can already be gone if the test run was INTerrupted.
-  end
-
-  def gitaly_url
-    ENV.fetch('GITALY_REPO_URL', nil)
+    raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds"
   end
 
   def setup_workhorse

spec/support/praefect.rb

+# frozen_string_literal: true
+
+require_relative 'helpers/test_env'
+
+RSpec.configure do |config|
+  config.before(:each, :praefect) do
+    allow(Gitlab.config.repositories.storages['default']).to receive(:[]).and_call_original
+    allow(Gitlab.config.repositories.storages['default']).to receive(:[]).with('gitaly_address')
+      .and_return(TestEnv.praefect_socket_path)
+  end
+end