Wrap rugged calls with access disk blockjc-wrap-rugged-calls-with-disk-access

Whenever we use the rugged implementation, we are going straight to disk
so we want to bypass the disk access check.

9 files changed, 27 insertions, 22 deletions

diff --git a/lib/gitlab/git/rugged_impl/blob.rb b/lib/gitlab/git/rugged_impl/blob.rb
index 86c9f33d82a..9aea736527b 100644
--- a/lib/gitlab/git/rugged_impl/blob.rb
+++ b/lib/gitlab/git/rugged_impl/blob.rb
@@ -16,7 +16,7 @@ module Gitlab
           override :tree_entry
           def tree_entry(repository, sha, path, limit)
             if use_rugged?(repository, :rugged_tree_entry)
-              rugged_tree_entry(repository, sha, path, limit)
+              wrap_rugged_call { rugged_tree_entry(repository, sha, path, limit) }
             else
               super
             end
diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb
index 971a33b2e99..29ae9bdd851 100644
--- a/lib/gitlab/git/rugged_impl/commit.rb
+++ b/lib/gitlab/git/rugged_impl/commit.rb
@@ -36,7 +36,7 @@ module Gitlab
           override :find_commit
           def find_commit(repo, commit_id)
             if use_rugged?(repo, :rugged_find_commit)
-              rugged_find(repo, commit_id)
+              wrap_rugged_call { rugged_find(repo, commit_id) }
             else
               super
             end
@@ -45,7 +45,7 @@ module Gitlab
           override :batch_by_oid
           def batch_by_oid(repo, oids)
             if use_rugged?(repo, :rugged_list_commits_by_oid)
-              rugged_batch_by_oid(repo, oids)
+              wrap_rugged_call { rugged_batch_by_oid(repo, oids) }
             else
               super
             end
@@ -68,7 +68,7 @@ module Gitlab
         override :commit_tree_entry
         def commit_tree_entry(path)
           if use_rugged?(@repository, :rugged_commit_tree_entry)
-            rugged_tree_entry(path)
+            wrap_rugged_call { rugged_tree_entry(path) }
           else
             super
           end
diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb
index 9268abdfed9..7bed553393c 100644
--- a/lib/gitlab/git/rugged_impl/repository.rb
+++ b/lib/gitlab/git/rugged_impl/repository.rb
@@ -48,7 +48,7 @@ module Gitlab
         override :ancestor?
         def ancestor?(from, to)
           if use_rugged?(self, :rugged_commit_is_ancestor)
-            rugged_is_ancestor?(from, to)
+            wrap_rugged_call { rugged_is_ancestor?(from, to) }
           else
             super
           end
diff --git a/lib/gitlab/git/rugged_impl/tree.rb b/lib/gitlab/git/rugged_impl/tree.rb
index f3721a3f1b7..479c5f9d8b7 100644
--- a/lib/gitlab/git/rugged_impl/tree.rb
+++ b/lib/gitlab/git/rugged_impl/tree.rb
@@ -16,7 +16,7 @@ module Gitlab
           override :tree_entries
           def tree_entries(repository, sha, path, recursive)
             if use_rugged?(repository, :rugged_tree_entries)
-              tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive)
+              wrap_rugged_call { tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive) }
             else
               super
             end
diff --git a/lib/gitlab/git/rugged_impl/use_rugged.rb b/lib/gitlab/git/rugged_impl/use_rugged.rb
index 99091b03cd1..902fa3c7822 100644
--- a/lib/gitlab/git/rugged_impl/use_rugged.rb
+++ b/lib/gitlab/git/rugged_impl/use_rugged.rb
@@ -10,6 +10,12 @@ module Gitlab
 
           Gitlab::GitalyClient.can_use_disk?(repo.storage)
         end
+
+        def wrap_rugged_call(&block)
+          Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            yield
+          end
+        end
       end
     end
   end
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index 091351e5cb2..3cafa49ec51 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -388,21 +388,20 @@ module Gitlab
     end
 
     def self.can_use_disk?(storage)
-      false
-      # cached_value = MUTEX.synchronize do
-      #   @can_use_disk ||= {}
-      #   @can_use_disk[storage]
-      # end
+      cached_value = MUTEX.synchronize do
+        @can_use_disk ||= {}
+        @can_use_disk[storage]
+      end
 
-      # return cached_value unless cached_value.nil?
+      return cached_value if cached_value.present?
 
-      # gitaly_filesystem_id = filesystem_id(storage)
-      # direct_filesystem_id = filesystem_id_from_disk(storage)
+      gitaly_filesystem_id = filesystem_id(storage)
+      direct_filesystem_id = filesystem_id_from_disk(storage)
 
-      # MUTEX.synchronize do
-      #   @can_use_disk[storage] = gitaly_filesystem_id.present? &&
-      #     gitaly_filesystem_id == direct_filesystem_id
-      # end
+      MUTEX.synchronize do
+        @can_use_disk[storage] = gitaly_filesystem_id.present? &&
+          gitaly_filesystem_id == direct_filesystem_id
+      end
     end
 
     def self.filesystem_id(storage)
@@ -415,7 +414,7 @@ module Gitlab
       metadata_file = File.read(storage_metadata_file_path(storage))
       metadata_hash = JSON.parse(metadata_file)
       metadata_hash['gitaly_filesystem_id']
-    rescue Errno::ENOENT, JSON::ParserError
+    rescue Errno::ENOENT, Errno::ACCESS, JSON::ParserError
       nil
     end
 
diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
index 972dd7e0d2b..483c5ea9cff 100644
--- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
+++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
@@ -26,7 +26,6 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do
       end
 
       it 'loads 10 projects without hitting Gitaly call limit', :request_store do
-        allow(Gitlab::GitalyClient).to receive(:can_access_disk?).and_return(false)
         projects = Gitlab::GitalyClient.allow_n_plus_1_calls do
           (1..10).map { create(:project, :repository) }
         end
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..e437647c258 100644
--- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb
+++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb
@@ -21,6 +21,7 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
   end
 
   before do
+    allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_call_original
     Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {})
   end
 
@@ -30,7 +31,6 @@ 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
     end
 
@@ -49,7 +49,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
     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(Gitlab::GitalyClient).not_to receive(:filesystem_id)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 95e0d8858b9..978158192d9 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -134,6 +134,8 @@ RSpec.configure do |config|
       allow(Feature).to receive(:enabled?).with(flag).and_return(enabled)
     end
 
+    allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enabled)
+
     # The following can be removed when we remove the staged rollout strategy
     # and we can just enable it using instance wide settings
     # (ie. ApplicationSetting#auto_devops_enabled)