diff options
author | John Cai <jcai@gitlab.com> | 2019-07-09 20:23:11 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2019-07-10 02:00:25 +0300 |
commit | 117b0564b5a35751240b9f437c9d1810f8ca42b2 (patch) | |
tree | 4803b75e6819fa96230042b0ae1a94be8a344362 /spec/lib/gitlab | |
parent | 6e106f66ad9d692b88e90506c0b35c4281f2b600 (diff) |
Allow disk access if we can access storage directlyjc-allow-disk-access
Move the methods to detect if we can access disk directly into
StorageSettings. This reduces complexity by no longer needing
synchronization, as well as being able to bypass the disk access check.
Diffstat (limited to 'spec/lib/gitlab')
-rw-r--r-- | spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb | 25 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/storage_settings_spec.rb | 27 |
2 files changed, 35 insertions, 17 deletions
diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb index e7ef9d08f80..8d18eb7073a 100644 --- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -7,6 +7,8 @@ require 'tempfile' describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do let(:project) { create(:project, :repository) } let(:repository) { project.repository } + let(:storage_name) { repository.storage } + let(:storage) { Gitlab.config.repositories.storages[storage_name] } let(:feature_flag_name) { 'feature-flag-name' } let(:feature_flag) { Feature.get(feature_flag_name) } let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } @@ -21,7 +23,8 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do end before do - Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {}) + # reset the cached value before each spec + storage.instance_variable_set(:@can_use_disk, nil) end context 'when feature flag is not persisted' do @@ -30,31 +33,29 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do end it 'returns true when gitaly matches disk' do - pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338') - expect(subject.use_rugged?(repository, feature_flag_name)).to be true + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be true end it 'returns false when disk access fails' do - allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return("/fake/path/doesnt/exist") + allow(storage).to receive(:storage_metadata_file_path).and_return("/fake/path/doesnt/exist") - expect(subject.use_rugged?(repository, feature_flag_name)).to be false + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be false end it "returns false when gitaly doesn't match disk" do - allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file) + allow(storage).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file) - expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be_falsey File.delete(temp_gitaly_metadata_file) end it "doesn't lead to a second rpc call because gitaly client should use the cached value" do - pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338') - expect(subject.use_rugged?(repository, feature_flag_name)).to be true + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be true expect(Gitlab::GitalyClient).not_to receive(:filesystem_id) - subject.use_rugged?(repository, feature_flag_name) + subject.use_rugged?(storage_name, feature_flag_name) end end @@ -66,14 +67,14 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do it 'returns false when the feature flag is off' do allow(feature_flag).to receive(:enabled?).and_return(false) - expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be_falsey end it "returns true when feature flag is on" do allow(feature_flag).to receive(:enabled?).and_return(true) allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(false) - expect(subject.use_rugged?(repository, feature_flag_name)).to be true + expect(subject.use_rugged?(storage_name, feature_flag_name)).to be true end end diff --git a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb index f2f53982b09..bc774532c09 100644 --- a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb +++ b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::GitalyClient::StorageSettings do context 'when the storage contains no path' do it 'raises an error' do expect do - described_class.new("foo" => {}) + described_class.new("default", "foo" => {}) end.to raise_error(described_class::InvalidConfigurationError) end end @@ -13,7 +13,7 @@ describe Gitlab::GitalyClient::StorageSettings do context "when the argument isn't a hash" do it 'raises an error' do expect do - described_class.new("test") + described_class.new("default", "test") end.to raise_error("expected a Hash, got a String") end end @@ -21,22 +21,39 @@ describe Gitlab::GitalyClient::StorageSettings do context 'when the storage is valid' do it 'raises no error' do expect do - described_class.new("path" => Rails.root) + described_class.new("default", "path" => Rails.root) end.not_to raise_error end end end describe '.disk_access_denied?' do + let(:storage_name) { "default" } + subject { described_class.new(storage_name, "path" => Rails.root) } + + before do + allow(subject).to receive(:can_use_disk?).and_return(false) + end + context 'when Rugged is enabled', :enable_rugged do it 'returns false' do - expect(described_class.disk_access_denied?).to be_falsey + expect(subject.disk_access_denied?).to be_falsey end end context 'when Rugged is disabled' do it 'returns true' do - expect(described_class.disk_access_denied?).to be_truthy + expect(subject.disk_access_denied?).to be_truthy + end + end + + context 'when disk is accessible' do + before do + allow(subject).to receive(:can_use_disk?).and_return(true) + end + + it 'returns false' do + expect(subject.disk_access_denied?).to be_falsey end end end |