Merge branch 'sh-use-praefect-in-testing-loop' into 'master'

[RUN ALL RSPEC] Use Praefect instead of Gitaly in testing loop

See merge request gitlab-org/gitlab!49922

config/gitlab.yml.example

@@ -1386,7 +1386,7 @@ test:
     storages:
       default:
         path: tmp/tests/repositories/
-        gitaly_address: unix:tmp/tests/gitaly/gitaly.socket
+        gitaly_address: unix:tmp/tests/gitaly/praefect.socket
 
   gitaly:
     client_path: tmp/tests/gitaly

lib/gitlab/setup_helper.rb

@@ -4,10 +4,10 @@ require 'toml-rb'
 
 module Gitlab
   module SetupHelper
-    def create_configuration(dir, storage_paths, force: false)
+    def create_configuration(dir, storage_paths, force: false, options: {})
       generate_configuration(
-        configuration_toml(dir, storage_paths),
-        get_config_path(dir),
+        configuration_toml(dir, storage_paths, options),
+        get_config_path(dir, options),
         force: force
       )
     end
@@ -31,7 +31,7 @@ module Gitlab
     module Workhorse
       extend Gitlab::SetupHelper
       class << self
-        def configuration_toml(dir, _)
+        def configuration_toml(dir, _, _)
           config = { redis: { URL: redis_url } }
 
           TomlRB.dump(config)
@@ -41,8 +41,8 @@ module Gitlab
           Gitlab::Redis::SharedState.url
         end
 
-        def get_config_path(dir)
-          File.join(dir, 'config.toml')
+        def get_config_path(dir, _)
+          File.join(dir, 'config_path')
         end
 
         def compile_into(dir)
@@ -76,7 +76,7 @@ module Gitlab
         # 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)
+        def configuration_toml(gitaly_dir, storage_paths, options, gitaly_ruby: true)
           storages = []
           address = nil
 
@@ -97,14 +97,20 @@ module Gitlab
           config = { socket_path: address.sub(/\Aunix:/, '') }
 
           if Rails.env.test?
+            socket_filename = options[:gitaly_socket] || "gitaly.socket"
+
+            config = {
+              # Override the set gitaly_address since Praefect is in the loop
+              socket_path: File.join(gitaly_dir, socket_filename),
+              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.
+              git: { catfile_cache_size: 5 }
+            }
+
             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[:storage] = storages
@@ -124,8 +130,9 @@ module Gitlab
 
         private
 
-        def get_config_path(dir)
-          File.join(dir, 'config.toml')
+        def get_config_path(dir, options)
+          config_filename = options[:config_filename] || 'config.toml'
+          File.join(dir, config_filename)
         end
       end
     end
@@ -133,9 +140,11 @@ module Gitlab
     module Praefect
       extend Gitlab::SetupHelper
       class << self
-        def configuration_toml(gitaly_dir, storage_paths)
+        def configuration_toml(gitaly_dir, _, _)
           nodes = [{ storage: 'default', address: "unix:#{gitaly_dir}/gitaly.socket", primary: true, token: 'secret' }]
-          storages = [{ name: 'default', node: nodes }]
+          second_storage_nodes = [{ storage: 'test_second_storage', address: "unix:#{gitaly_dir}/gitaly2.socket", primary: true, token: 'secret' }]
+
+          storages = [{ name: 'default', node: nodes }, { name: 'test_second_storage', node: second_storage_nodes }]
           failover = { enabled: false }
           config = { socket_path: "#{gitaly_dir}/praefect.socket", memory_queue_enabled: true, virtual_storage: storages, failover: failover }
           config[:token] = 'secret' if Rails.env.test?
@@ -145,7 +154,7 @@ module Gitlab
 
         private
 
-        def get_config_path(dir)
+        def get_config_path(dir, _)
           File.join(dir, 'praefect.config.toml')
         end
       end

scripts/gitaly-test-build

@@ -19,8 +19,10 @@ class GitalyTestBuild
 
     # Starting gitaly further validates its configuration
     gitaly_pid = start_gitaly
+    gitaly2_pid = start_gitaly2
     praefect_pid = start_praefect
     Process.kill('TERM', gitaly_pid)
+    Process.kill('TERM', gitaly2_pid)
     Process.kill('TERM', praefect_pid)
 
     # Make the 'gitaly' executable look newer than 'GITALY_SERVER_VERSION'.

scripts/gitaly-test-spawn

@@ -15,6 +15,7 @@ class GitalyTestSpawn
 
     # In local development this pid file is used by rspec.
     IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), start_gitaly)
+    IO.write(File.expand_path('../tmp/tests/gitaly2.pid', __dir__), start_gitaly2)
     IO.write(File.expand_path('../tmp/tests/praefect.pid', __dir__), start_praefect)
   end
 end

scripts/gitaly_test.rb

@@ -62,21 +62,36 @@ module GitalyTest
     case service
     when :gitaly
       File.join(tmp_tests_gitaly_dir, 'config.toml')
+    when :gitaly2
+      File.join(tmp_tests_gitaly_dir, 'gitaly2.config.toml')
     when :praefect
       File.join(tmp_tests_gitaly_dir, 'praefect.config.toml')
     end
   end
 
+  def service_binary(service)
+    case service
+    when :gitaly, :gitaly2
+      'gitaly'
+    when :praefect
+      'praefect'
+    end
+  end
+
   def start_gitaly
     start(:gitaly)
   end
 
+  def start_gitaly2
+    start(:gitaly2)
+  end
+
   def start_praefect
     start(:praefect)
   end
 
   def start(service)
-    args = ["#{tmp_tests_gitaly_dir}/#{service}"]
+    args = ["#{tmp_tests_gitaly_dir}/#{service_binary(service)}"]
     args.push("-config") if service == :praefect
     args.push(config_path(service))
     pid = spawn(env, *args, [:out, :err] => "log/#{service}-test.log")

spec/support/helpers/test_env.rb

@@ -168,6 +168,11 @@ module TestEnv
       version: Gitlab::GitalyClient.expected_server_version,
       task: "gitlab:gitaly:install[#{install_gitaly_args}]") do
         Gitlab::SetupHelper::Gitaly.create_configuration(gitaly_dir, { 'default' => repos_path }, force: true)
+        Gitlab::SetupHelper::Gitaly.create_configuration(
+          gitaly_dir,
+          { 'default' => repos_path }, force: true,
+          options: { gitaly_socket: "gitaly2.socket", config_filename: "gitaly2.config.toml" }
+        )
         Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true)
       end
 
@@ -283,7 +288,7 @@ module TestEnv
     host = "[#{host}]" if host.include?(':')
     listen_addr = [host, port].join(':')
 
-    config_path = Gitlab::SetupHelper::Workhorse.get_config_path(workhorse_dir)
+    config_path = Gitlab::SetupHelper::Workhorse.get_config_path(workhorse_dir, {})
 
     # This should be set up in setup_workhorse, but since
     # component_needs_update? only checks that versions are consistent,