From 22a34840b73aa322728e7d66db00b70d56fe87d6 Mon Sep 17 00:00:00 2001 From: Brett Walker <bwalker@gitlab.com> Date: Mon, 10 Jan 2022 23:34:03 +0000 Subject: [PATCH] Revert "Merge branch 'toon-gitaly-test-setup' into 'master'" This reverts merge request !76558 --- lib/tasks/gitlab/gitaly.rake | 38 ++++++++ scripts/gitaly-test-build | 2 + scripts/gitaly-test-spawn | 13 ++- spec/support/helpers/gitaly_setup.rb | 131 ++------------------------- spec/support/helpers/test_env.rb | 117 ++++++++++++++++++++---- spec/support/praefect.rb | 4 +- 6 files changed, 162 insertions(+), 143 deletions(-) diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 18c68615637a3..b01a7902bf24e 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/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 } diff --git a/scripts/gitaly-test-build b/scripts/gitaly-test-build index adc9b56ca4f83..e6afadccc7e1f 100755 --- a/scripts/gitaly-test-build +++ b/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" diff --git a/scripts/gitaly-test-spawn b/scripts/gitaly-test-spawn index 3bf2de7676085..e7e25a217b281 100755 --- a/scripts/gitaly-test-spawn +++ b/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 diff --git a/spec/support/helpers/gitaly_setup.rb b/spec/support/helpers/gitaly_setup.rb index cc0ddfdedac2b..923051a2e0438 100644 --- a/spec/support/helpers/gitaly_setup.rb +++ b/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 @@ def env { '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 @@ def env } 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 @@ def config_path(service) 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 @@ def try_connect!(service) 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 diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index ecd2baa0ca691..294e5bdd085df 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -1,7 +1,6 @@ # 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 @@ def init # Can be overriden def post_init - start_gitaly + start_gitaly(gitaly_dir) end # Clean /tmp/tests @@ -142,7 +142,7 @@ def clean_test_path 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 @@ def setup_gitlab_shell 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 @@ def copy_repo(subject, bare_repo:, refs:) 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 @@ def rm_storage_dir(storage, dir) 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 @@ def create_bare_repository(path) 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 @@ def set_repo_refs(repo_path, branch_sha) 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 @@ def component_timed_setup(component, install_dir:, version:, task:, fresh_instal 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 diff --git a/spec/support/praefect.rb b/spec/support/praefect.rb index 451b47cc83c54..3218275c2aa03 100644 --- a/spec/support/praefect.rb +++ b/spec/support/praefect.rb @@ -1,11 +1,11 @@ # 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 -- GitLab