From 08cdf65b9e1f5f6fc1f4063a8422c25005868e58 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 17 Oct 2018 09:31:35 +0200 Subject: [PATCH 1/2] Remove broken git storage for testing Broken storage used to be used to test situations where the Git storage wasn't being reached. These days we can just mock the Gitaly response. But given the broken storage is removed now, Gitaly can take over control of the storage being reachable. If it's not, Gitaly won't boot. That's nice for situations where a disk wasn't mounted for instance. Gitaly MR: https://gitlab.com/gitlab-org/gitaly/merge_requests/675 --- config/gitlab.yml.example | 3 --- lib/gitlab/setup_helper.rb | 5 ++++- spec/lib/gitaly/server_spec.rb | 8 ++------ spec/support/helpers/test_env.rb | 9 --------- spec/support/stored_repositories.rb | 4 ---- spec/workers/repository_check/batch_worker_spec.rb | 2 +- 6 files changed, 7 insertions(+), 24 deletions(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 749cdd0f869..a4db125f831 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -772,9 +772,6 @@ test: default: path: tmp/tests/repositories/ gitaly_address: unix:tmp/tests/gitaly/gitaly.socket - broken: - path: tmp/tests/non-existent-repositories - gitaly_address: unix:tmp/tests/gitaly/gitaly.socket gitaly: client_path: tmp/tests/gitaly diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index 4a745147858..2b7e12639be 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -32,7 +32,10 @@ module Gitlab end if Rails.env.test? - storages << { name: 'test_second_storage', path: Rails.root.join('tmp', 'tests', 'second_storage').to_s } + storage_path = Rails.root.join('tmp', 'tests', 'second_storage').to_s + + FileUtils.mkdir(storage_path) unless File.exist?(storage_path) + storages << { name: 'test_second_storage', path: storage_path } end config = { socket_path: address.sub(/\Aunix:/, ''), storage: storages } diff --git a/spec/lib/gitaly/server_spec.rb b/spec/lib/gitaly/server_spec.rb index 09bf21b5946..292ab870dad 100644 --- a/spec/lib/gitaly/server_spec.rb +++ b/spec/lib/gitaly/server_spec.rb @@ -26,9 +26,7 @@ describe Gitaly::Server do end end - context 'when the storage is not readable' do - let(:server) { described_class.new('broken') } - + context 'when the storage is not readable', :broken_storage do it 'returns false' do expect(server).not_to be_readable end @@ -42,9 +40,7 @@ describe Gitaly::Server do end end - context 'when the storage is not writeable' do - let(:server) { described_class.new('broken') } - + context 'when the storage is not writeable', :broken_storage do it 'returns false' do expect(server).not_to be_writeable end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 1a9aa252511..71287f28171 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -70,7 +70,6 @@ module TestEnv TMP_TEST_PATH = Rails.root.join('tmp', 'tests', '**') REPOS_STORAGE = 'default'.freeze - BROKEN_STORAGE = 'broken'.freeze # Test environment # @@ -159,10 +158,6 @@ module TestEnv version: Gitlab::GitalyClient.expected_server_version, task: "gitlab:gitaly:install[#{gitaly_dir},#{repos_path}]") do - # Re-create config, to specify the broken storage path - storage_paths = { 'default' => repos_path, 'broken' => broken_path } - Gitlab::SetupHelper.create_gitaly_configuration(gitaly_dir, storage_paths, force: true) - start_gitaly(gitaly_dir) end end @@ -257,10 +252,6 @@ module TestEnv @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end - def broken_path - @broken_path ||= Gitlab.config.repositories.storages[BROKEN_STORAGE].legacy_disk_path - end - def backup_path Gitlab.config.backup.path end diff --git a/spec/support/stored_repositories.rb b/spec/support/stored_repositories.rb index 6a9ad43941d..55212355daa 100644 --- a/spec/support/stored_repositories.rb +++ b/spec/support/stored_repositories.rb @@ -1,8 +1,4 @@ RSpec.configure do |config| - config.before(:all, :broken_storage) do - FileUtils.rm_rf Gitlab.config.repositories.storages.broken.legacy_disk_path - end - config.before(:each, :broken_storage) do allow(Gitlab::GitalyClient).to receive(:call) do raise GRPC::Unavailable.new('Gitaly broken in this spec') diff --git a/spec/workers/repository_check/batch_worker_spec.rb b/spec/workers/repository_check/batch_worker_spec.rb index ede271b2cdd..50b93fce2dc 100644 --- a/spec/workers/repository_check/batch_worker_spec.rb +++ b/spec/workers/repository_check/batch_worker_spec.rb @@ -51,7 +51,7 @@ describe RepositoryCheck::BatchWorker do it 'does nothing when shard is unhealthy' do shard_name = 'broken' - create(:project, created_at: 1.week.ago, repository_storage: shard_name) + create(:project, :broken_storage, created_at: 1.week.ago) expect(subject.perform(shard_name)).to eq(nil) end From c40b5aaa6a2b679dceaadde5a6340b11ea20fd46 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 23 Oct 2018 10:42:41 +0200 Subject: [PATCH 2/2] Bump Gitaly to v0.126.0 This will allow changes in Gitaly that will make it fail when running rspec, to make it to GitLab. Else Gitaly won't start as it can't reach the broken storage directory. --- GITALY_SERVER_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 33e061fe7a0..bcc9c2840a7 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.125.1 +0.126.0