Merge branch 'revert-2e85fc86' into 'master'

Revert "Merge branch 'toon-gitaly-test-setup' into 'master'"

See merge request gitlab-org/gitlab!77915

lib/tasks/gitlab/gitaly.rake

@@ -2,6 +2,41 @@
 
 namespace :gitlab do
   namespace :gitaly do
+    desc 'Installs gitaly for running tests within gitlab-development-kit'
+    task :test_install, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args|
+      inside_gdk = Rails.env.test? && File.exist?(Rails.root.join('../GDK_ROOT'))
+
+      if ENV['FORCE_GITALY_INSTALL'] || !inside_gdk
+        Rake::Task["gitlab:gitaly:install"].invoke(*args)
+
+        next
+      end
+
+      gdk_gitaly_dir = ENV.fetch('GDK_GITALY', Rails.root.join('../gitaly'))
+
+      # Our test setup expects a git repo, so clone rather than copy
+      clone_repo(gdk_gitaly_dir, args.dir, clone_opts: %w[--depth 1]) unless Dir.exist?(args.dir)
+
+      # We assume the GDK gitaly already compiled binaries
+      build_dir = File.join(gdk_gitaly_dir, '_build')
+      FileUtils.cp_r(build_dir, args.dir)
+
+      # We assume the GDK gitaly already ran bundle install
+      bundle_dir = File.join(gdk_gitaly_dir, 'ruby', '.bundle')
+      FileUtils.cp_r(bundle_dir, File.join(args.dir, 'ruby'))
+
+      # For completeness we copy this for gitaly's make target
+      ruby_bundle_file = File.join(gdk_gitaly_dir, '.ruby-bundle')
+      FileUtils.cp_r(ruby_bundle_file, args.dir)
+
+      gitaly_binary = File.join(build_dir, 'bin', 'gitaly')
+      warn_gitaly_out_of_date!(gitaly_binary, Gitlab::GitalyClient.expected_server_version)
+    rescue Errno::ENOENT => e
+      puts "Could not copy files, did you run `gdk update`? Error: #{e.message}"
+
+      raise
+    end
+
     desc 'GitLab | Gitaly | Clone and checkout gitaly'
     task :clone, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args|
       warn_user_is_not_gitlab
@@ -25,6 +60,9 @@ Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]")
       storage_paths = { 'default' => args.storage_path }
       Gitlab::SetupHelper::Gitaly.create_configuration(args.dir, storage_paths)
 
+      # In CI we run scripts/gitaly-test-build
+      next if ENV['CI'].present?
+
       Dir.chdir(args.dir) do
         Bundler.with_original_env do
           env = { "RUBYOPT" => nil, "BUNDLE_GEMFILE" => nil }

scripts/gitaly-test-build

@@ -13,6 +13,8 @@ class GitalyTestBuild
   include GitalySetup
 
   def run
+    set_bundler_config
+
     # If we have the binaries from the cache, we can skip building them again
     if File.exist?(tmp_tests_gitaly_bin_dir)
       GitalySetup::LOGGER.debug "Gitaly binary already built. Skip building...\n"

scripts/gitaly-test-spawn

@@ -9,8 +9,17 @@ class GitalyTestSpawn
   include GitalySetup
 
   def run
-    install_gitaly_gems
-    spawn_gitaly
+    set_bundler_config
+    install_gitaly_gems if ENV['CI']
+    check_gitaly_config!
+
+    # # Uncomment line below to see all gitaly logs merged into CI trace
+    # spawn('sleep 1; tail -f log/gitaly-test.log')
+
+    # 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
 

spec/support/helpers/gitaly_setup.rb

@@ -9,13 +9,8 @@
 require 'securerandom'
 require 'socket'
 require 'logger'
-require 'bundler'
 
 module GitalySetup
-  extend self
-
-  REPOS_STORAGE = 'default'
-
   LOGGER = begin
     default_name = ENV['CI'] ? 'DEBUG' : 'WARN'
     level_name = ENV['GITLAB_TESTING_LOG_LEVEL']&.upcase
@@ -59,12 +54,9 @@ module GitalySetup
     {
       'HOME' => expand_path('tmp/tests'),
       'GEM_PATH' => Gem.path.join(':'),
+      'BUNDLE_APP_CONFIG' => File.join(gemfile_dir, '.bundle'),
       'BUNDLE_INSTALL_FLAGS' => nil,
-      'BUNDLE_IGNORE_CONFIG' => '1',
-      'BUNDLE_PATH' => bundle_path,
       'BUNDLE_GEMFILE' => gemfile,
-      'BUNDLE_JOBS' => '4',
-      'BUNDLE_RETRY' => '3',
       'RUBYOPT' => nil,
 
       # Git hooks can't run during tests as the internal API is not running.
@@ -73,20 +65,17 @@ module GitalySetup
     }
   end
 
-  def bundle_path
-    # Allow the user to override BUNDLE_PATH if they need to
-    return ENV['GITALY_TEST_BUNDLE_PATH'] if ENV['GITALY_TEST_BUNDLE_PATH']
+  # rubocop:disable GitlabSecurity/SystemCommandInjection
+  def set_bundler_config
+    system('bundle config set --local jobs 4', chdir: gemfile_dir)
+    system('bundle config set --local retry 3', chdir: gemfile_dir)
 
     if ENV['CI']
-      expand_path('vendor/gitaly-ruby')
-    else
-      explicit_path = Bundler.configured_bundle_path.explicit_path
-
-      return unless explicit_path
-
-      expand_path(explicit_path)
+      bundle_path = expand_path('vendor/gitaly-ruby')
+      system('bundle', 'config', 'set', '--local', 'path', bundle_path, chdir: gemfile_dir)
     end
   end
+  # rubocop:enable GitlabSecurity/SystemCommandInjection
 
   def config_path(service)
     case service
@@ -99,10 +88,6 @@ module GitalySetup
     end
   end
 
-  def repos_path(storage = REPOS_STORAGE)
-    Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path
-  end
-
   def service_binary(service)
     case service
     when :gitaly, :gitaly2
@@ -211,104 +196,4 @@ module GitalySetup
 
     raise "could not connect to #{socket}"
   end
-
-  def gitaly_socket_path
-    Gitlab::GitalyClient.address(REPOS_STORAGE).delete_prefix('unix:')
-  end
-
-  def gitaly_dir
-    socket_path = gitaly_socket_path
-    socket_path = File.expand_path(gitaly_socket_path) if expand_path_for_socket?
-
-    File.dirname(socket_path)
-  end
-
-  # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters:
-  # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure
-  # that changes in the current working directory don't affect GRPC reconnections.
-  def expand_path_for_socket?
-    !!ENV['CI']
-  end
-
-  def setup_gitaly
-    unless ENV['CI']
-      # In CI Gitaly is built in the setup-test-env job and saved in the
-      # artifacts. So when tests are started, there's no need to build Gitaly.
-      build_gitaly
-    end
-
-    Gitlab::SetupHelper::Gitaly.create_configuration(
-      gitaly_dir,
-      { 'default' => repos_path },
-      force: true,
-      options: {
-        prometheus_listen_addr: 'localhost:9236'
-      }
-    )
-    Gitlab::SetupHelper::Gitaly.create_configuration(
-      gitaly_dir,
-      { 'default' => repos_path },
-      force: true,
-      options: {
-        internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"),
-        gitaly_socket: "gitaly2.socket",
-        config_filename: "gitaly2.config.toml"
-      }
-    )
-    Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true)
-  end
-
-  def socket_path(service)
-    File.join(tmp_tests_gitaly_dir, "#{service}.socket")
-  end
-
-  def praefect_socket_path
-    "unix:" + socket_path(:praefect)
-  end
-
-  def wait(service)
-    sleep_time = 10
-    sleep_interval = 0.1
-    socket = socket_path(service)
-
-    Integer(sleep_time / sleep_interval).times do
-      Socket.unix(socket)
-      return
-    rescue StandardError
-      sleep sleep_interval
-    end
-
-    raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds"
-  end
-
-  def stop(pid)
-    Process.kill('KILL', pid)
-  rescue Errno::ESRCH
-    # The process can already be gone if the test run was INTerrupted.
-  end
-
-  def spawn_gitaly
-    check_gitaly_config!
-
-    gitaly_pid = start_gitaly
-    gitaly2_pid = start_gitaly2
-    praefect_pid = start_praefect
-
-    Kernel.at_exit do
-      # In CI this function is called by scripts/gitaly-test-spawn, triggered a
-      # before_script. Gitaly needs to remain running until the container is
-      # stopped.
-      next if ENV['CI']
-
-      pids = [gitaly_pid, gitaly2_pid, praefect_pid]
-      pids.each { |pid| stop(pid) }
-    end
-
-    wait('gitaly')
-    wait('praefect')
-  rescue StandardError
-    message = 'gitaly spawn failed'
-    message += " (try `rm -rf #{gitaly_dir}` ?)" unless ENV['CI']
-    raise message
-  end
 end

spec/support/helpers/test_env.rb

 # frozen_string_literal: true
 
 require 'parallel'
-require_relative 'gitaly_setup'
 
 module TestEnv
   extend self
@@ -94,6 +93,7 @@ module TestEnv
   }.freeze
 
   TMP_TEST_PATH = Rails.root.join('tmp', 'tests').freeze
+  REPOS_STORAGE = 'default'
   SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage')
   SETUP_METHODS = %i[setup_gitaly setup_gitlab_shell setup_workhorse setup_factory_repo setup_forked_repo].freeze
 
@@ -128,7 +128,7 @@ module TestEnv
 
   # Can be overriden
   def post_init
-    start_gitaly
+    start_gitaly(gitaly_dir)
   end
 
   # Clean /tmp/tests
@@ -142,7 +142,7 @@ module TestEnv
     end
 
     FileUtils.mkdir_p(
-      Gitlab::GitalyClient::StorageSettings.allow_disk_access { GitalySetup.repos_path }
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access { TestEnv.repos_path }
     )
     FileUtils.mkdir_p(SECOND_STORAGE_PATH)
     FileUtils.mkdir_p(backup_path)
@@ -158,28 +158,111 @@ module TestEnv
 
   def setup_gitaly
     component_timed_setup('Gitaly',
-      install_dir: GitalySetup.gitaly_dir,
+      install_dir: gitaly_dir,
       version: Gitlab::GitalyClient.expected_server_version,
-      task: "gitlab:gitaly:clone",
-      fresh_install: ENV.key?('FORCE_GITALY_INSTALL'),
-      task_args: [GitalySetup.gitaly_dir, GitalySetup.repos_path, gitaly_url].compact) do
-      GitalySetup.setup_gitaly
-    end
+      task: "gitlab:gitaly:test_install",
+      task_args: [gitaly_dir, repos_path, gitaly_url].compact) do
+        Gitlab::SetupHelper::Gitaly.create_configuration(
+          gitaly_dir,
+          { 'default' => repos_path },
+          force: true,
+          options: {
+            prometheus_listen_addr: 'localhost:9236'
+          }
+        )
+        Gitlab::SetupHelper::Gitaly.create_configuration(
+          gitaly_dir,
+          { 'default' => repos_path },
+          force: true,
+          options: {
+            internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"),
+            gitaly_socket: "gitaly2.socket",
+            config_filename: "gitaly2.config.toml"
+          }
+        )
+        Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true)
+      end
+  end
+
+  def gitaly_socket_path
+    Gitlab::GitalyClient.address('default').sub(/\Aunix:/, '')
+  end
+
+  def gitaly_dir
+    socket_path = gitaly_socket_path
+    socket_path = File.expand_path(gitaly_socket_path) if expand_path?
+
+    File.dirname(socket_path)
   end
 
-  def start_gitaly
+  # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters:
+  # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure
+  # that changes in the current working directory don't affect GRPC reconnections.
+  def expand_path?
+    !!ENV['CI']
+  end
+
+  def start_gitaly(gitaly_dir)
     if ci?
       # Gitaly has been spawned outside this process already
       return
     end
 
-    GitalySetup.spawn_gitaly
+    spawn_script = Rails.root.join('scripts/gitaly-test-spawn').to_s
+    Bundler.with_original_env do
+      unless system(spawn_script)
+        message = 'gitaly spawn failed'
+        message += " (try `rm -rf #{gitaly_dir}` ?)" unless ci?
+        raise message
+      end
+    end
+
+    gitaly_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly.pid')))
+    gitaly2_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly2.pid')))
+    praefect_pid = Integer(File.read(TMP_TEST_PATH.join('praefect.pid')))
+
+    Kernel.at_exit do
+      pids = [gitaly_pid, gitaly2_pid, praefect_pid]
+      pids.each { |pid| stop(pid) }
+    end
+
+    wait('gitaly')
+    wait('praefect')
+  end
+
+  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 = socket_path(service)
+
+    Integer(sleep_time / sleep_interval).times do
+      Socket.unix(socket)
+      return
+    rescue StandardError
+      sleep sleep_interval
+    end
+
+    raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds"
+  end
+
   # Feature specs are run through Workhorse
   def setup_workhorse
     # Always rebuild the config file
@@ -295,7 +378,8 @@ module TestEnv
 
   def rm_storage_dir(storage, dir)
     Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      target_repo_refs_path = File.join(GitalySetup.repos_path(storage), dir)
+      repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path
+      target_repo_refs_path = File.join(repos_path, dir)
       FileUtils.remove_dir(target_repo_refs_path)
     end
   rescue Errno::ENOENT
@@ -303,7 +387,8 @@ module TestEnv
 
   def storage_dir_exists?(storage, dir)
     Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      File.exist?(File.join(GitalySetup.repos_path(storage), dir))
+      repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path
+      File.exist?(File.join(repos_path, dir))
     end
   end
 
@@ -316,7 +401,7 @@ module TestEnv
   end
 
   def repos_path
-    @repos_path ||= GitalySetup.repos_path
+    @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path
   end
 
   def backup_path
@@ -437,7 +522,7 @@ module TestEnv
     end
   end
 
-  def component_timed_setup(component, install_dir:, version:, task:, fresh_install: true, task_args: [])
+  def component_timed_setup(component, install_dir:, version:, task:, task_args: [])
     start = Time.now
 
     ensure_component_dir_name_is_correct!(component, install_dir)
@@ -447,7 +532,7 @@ module TestEnv
 
     if component_needs_update?(install_dir, version)
       # Cleanup the component entirely to ensure we start fresh
-      FileUtils.rm_rf(install_dir) if fresh_install
+      FileUtils.rm_rf(install_dir)
 
       if ENV['SKIP_RAILS_ENV_IN_RAKE']
         # When we run `scripts/setup-test-env`, we take care of loading the necessary dependencies

spec/support/praefect.rb

 # frozen_string_literal: true
 
-require_relative 'helpers/gitaly_setup'
+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(GitalySetup.praefect_socket_path)
+      .and_return(TestEnv.praefect_socket_path)
   end
 end