Merge branch 'gitaly-testing' into 'master'

Setup and run a Gitaly server for testing if GitalyClient is enabled

See merge request !10298

.gitlab-ci.yml

@@ -1,4 +1,4 @@
-image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1"
+image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-golang-1.8-git-2.7-phantomjs-2.1-node-7.1"
 
 cache:
   key: "ruby-233"

app/models/commit.rb

@@ -326,14 +326,13 @@ class Commit
   end
 
   def raw_diffs(*args)
-    use_gitaly = Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
+    # NOTE: This feature is intentionally disabled until
-    deltas_only = args.last.is_a?(Hash) && args.last[:deltas_only]
+    # https://gitlab.com/gitlab-org/gitaly/issues/178 is resolved
-
+    # if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
-    if use_gitaly && !deltas_only
+    #   Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
-      Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
+    # else
-    else
+    raw.diffs(*args)
-      raw.diffs(*args)
+    # end
-    end
   end
 
   def diffs(diff_options = nil)

app/models/repository.rb

@@ -963,13 +963,15 @@ class Repository
   end
 
   def is_ancestor?(ancestor_id, descendant_id)
-    Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
+    # NOTE: This feature is intentionally disabled until
-      if is_enabled
+    # https://gitlab.com/gitlab-org/gitlab-ce/issues/30586 is resolved
-        raw_repository.is_ancestor?(ancestor_id, descendant_id)
+    # Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
-      else
+    #   if is_enabled
-        merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+    #     raw_repository.is_ancestor?(ancestor_id, descendant_id)
-      end
+    #   else
-    end
+    merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+    #   end
+    # end
   end
 
   def empty_repo?

config/gitlab.yml.example

@@ -579,9 +579,9 @@ test:
     storages:
       default:
         path: tmp/tests/repositories/
-        gitaly_address: unix:<%= Rails.root.join('tmp/sockets/private/gitaly.socket') %>
+        gitaly_address: unix:tmp/tests/gitaly/gitaly.socket
   gitaly:
-    enabled: false
+    enabled: true
   backup:
     path: tmp/tests/backups
   gitlab_shell:

lib/gitlab/git/repository.rb

@@ -45,13 +45,15 @@ module Gitlab
 
       # Default branch in the repository
       def root_ref
-        @root_ref ||= Gitlab::GitalyClient.migrate(:root_ref) do |is_enabled|
+        # NOTE: This feature is intentionally disabled until
-          if is_enabled
+        # https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-            gitaly_ref_client.default_branch_name
+        # @root_ref ||= Gitlab::GitalyClient.migrate(:root_ref) do |is_enabled|
-          else
+        #   if is_enabled
-            discover_default_branch
+        #     gitaly_ref_client.default_branch_name
-          end
+        #   else
-        end
+        @root_ref ||= discover_default_branch
+        #   end
+        # end
       rescue GRPC::BadStatus => e
         raise CommandError.new(e)
       end
@@ -70,13 +72,15 @@ module Gitlab
       # Returns an Array of branch names
       # sorted by name ASC
       def branch_names
-        Gitlab::GitalyClient.migrate(:branch_names) do |is_enabled|
+        # Gitlab::GitalyClient.migrate(:branch_names) do |is_enabled|
-          if is_enabled
+        #   NOTE: This feature is intentionally disabled until
-            gitaly_ref_client.branch_names
+        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-          else
+        #   if is_enabled
-            branches.map(&:name)
+        #     gitaly_ref_client.branch_names
-          end
+        #   else
-        end
+        branches.map(&:name)
+        #   end
+        # end
       rescue GRPC::BadStatus => e
         raise CommandError.new(e)
       end
@@ -131,13 +135,15 @@ module Gitlab
 
       # Returns an Array of tag names
       def tag_names
-        Gitlab::GitalyClient.migrate(:tag_names) do |is_enabled|
+        # Gitlab::GitalyClient.migrate(:tag_names) do |is_enabled|
-          if is_enabled
+        #   NOTE: This feature is intentionally disabled until
-            gitaly_ref_client.tag_names
+        #   https://gitlab.com/gitlab-org/gitaly/issues/179 is resolved
-          else
+        #   if is_enabled
-            rugged.tags.map { |t| t.name }
+        #     gitaly_ref_client.tag_names
-          end
+        #   else
-        end
+        rugged.tags.map { |t| t.name }
+        #   end
+        # end
       rescue GRPC::BadStatus => e
         raise CommandError.new(e)
       end
@@ -458,17 +464,19 @@ module Gitlab
 
       # Returns a RefName for a given SHA
       def ref_name_for_sha(ref_path, sha)
-        Gitlab::GitalyClient.migrate(:find_ref_name) do |is_enabled|
+        # NOTE: This feature is intentionally disabled until
-          if is_enabled
+        # https://gitlab.com/gitlab-org/gitaly/issues/180 is resolved
-            gitaly_ref_client.find_ref_name(sha, ref_path)
+        # Gitlab::GitalyClient.migrate(:find_ref_name) do |is_enabled|
-          else
+        #   if is_enabled
-            args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha})
+        #     gitaly_ref_client.find_ref_name(sha, ref_path)
+        #   else
+        args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha})
 
-            # Not found -> ["", 0]
+        # Not found -> ["", 0]
-            # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]
+        # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]
-            Gitlab::Popen.popen(args, @path).first.split.last
+        Gitlab::Popen.popen(args, @path).first.split.last
-          end
+        #   end
-        end
+        # end
       end
 
       # Returns commits collection

spec/lib/gitlab/git/repository_spec.rb

@@ -24,20 +24,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
       end
     end
 
-    context 'with gitaly enabled' do
+    # TODO: Uncomment when feature is reenabled
-      before { stub_gitaly }
+    # context 'with gitaly enabled' do
-
+    #   before { stub_gitaly }
-      it 'gets the branch name from GitalyClient' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+    #   it 'gets the branch name from GitalyClient' do
-        repository.root_ref
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-      end
+    #     repository.root_ref
-
+    #   end
-      it 'wraps GRPC exceptions' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
+    #   it 'wraps GRPC exceptions' do
-          and_raise(GRPC::Unknown)
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name).
-        expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
+    #       and_raise(GRPC::Unknown)
-      end
+    #     expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
-    end
+    #   end
+    # end
   end
 
   describe "#rugged" do
@@ -112,20 +113,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("master") }
     it { is_expected.not_to include("branch-from-space") }
 
-    context 'with gitaly enabled' do
+    # TODO: Uncomment when feature is reenabled
-      before { stub_gitaly }
+    # context 'with gitaly enabled' do
-
+    #   before { stub_gitaly }
-      it 'gets the branch names from GitalyClient' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+    #   it 'gets the branch names from GitalyClient' do
-        subject
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-      end
+    #     subject
-
+    #   end
-      it 'wraps GRPC exceptions' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
+    #   it 'wraps GRPC exceptions' do
-          and_raise(GRPC::Unknown)
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names).
-        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+    #       and_raise(GRPC::Unknown)
-      end
+    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    end
+    #   end
+    # end
   end
 
   describe '#tag_names' do
@@ -143,20 +145,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("v1.0.0") }
     it { is_expected.not_to include("v5.0.0") }
 
-    context 'with gitaly enabled' do
+    # TODO: Uncomment when feature is reenabled
-      before { stub_gitaly }
+    # context 'with gitaly enabled' do
-
+    #   before { stub_gitaly }
-      it 'gets the tag names from GitalyClient' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+    #   it 'gets the tag names from GitalyClient' do
-        subject
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-      end
+    #     subject
-
+    #   end
-      it 'wraps GRPC exceptions' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
+    #   it 'wraps GRPC exceptions' do
-          and_raise(GRPC::Unknown)
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names).
-        expect { subject }.to raise_error(Gitlab::Git::CommandError)
+    #       and_raise(GRPC::Unknown)
-      end
+    #     expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    end
+    #   end
+    # end
   end
 
   shared_examples 'archive check' do |extenstion|

spec/models/commit_spec.rb

@@ -389,31 +389,32 @@ eos
     end
   end
 
-  describe '#raw_diffs' do
+  # describe '#raw_diffs' do
-    context 'Gitaly commit_raw_diffs feature enabled' do
+  # TODO: Uncomment when feature is reenabled
-      before do
+  #   context 'Gitaly commit_raw_diffs feature enabled' do
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true)
+  #     before do
-      end
+  #       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true)
-
+  #     end
-      context 'when a truthy deltas_only is not passed to args' do
+  #
-        it 'fetches diffs from Gitaly server' do
+  #     context 'when a truthy deltas_only is not passed to args' do
-          expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent).
+  #       it 'fetches diffs from Gitaly server' do
-            with(commit)
+  #         expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent).
-
+  #           with(commit)
-          commit.raw_diffs
+  #
-        end
+  #         commit.raw_diffs
-      end
+  #       end
-
+  #     end
-      context 'when a truthy deltas_only is passed to args' do
+  #
-        it 'fetches diffs using Rugged' do
+  #     context 'when a truthy deltas_only is passed to args' do
-          opts = { deltas_only: true }
+  #       it 'fetches diffs using Rugged' do
-
+  #         opts = { deltas_only: true }
-          expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent)
+  #
-          expect(commit.raw).to receive(:diffs).with(opts)
+  #         expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent)
-
+  #         expect(commit.raw).to receive(:diffs).with(opts)
-          commit.raw_diffs(opts)
+  #
-        end
+  #         commit.raw_diffs(opts)
-      end
+  #       end
-    end
+  #     end
-  end
+  #   end
+  # end
 end

spec/models/environment_spec.rb

@@ -110,17 +110,18 @@ describe Environment, models: true do
       end
     end
 
-    context 'Gitaly find_ref_name feature enabled' do
+    # TODO: Uncomment when feature is reenabled
-      before do
+    # context 'Gitaly find_ref_name feature enabled' do
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true)
+    #   before do
-      end
+    #     allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true)
-
+    #   end
-      it 'calls GitalyClient' do
+    #
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name)
+    #   it 'calls GitalyClient' do
-
+    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name)
-        environment.first_deployment_for(commit)
+    #
-      end
+    #     environment.first_deployment_for(commit)
-    end
+    #   end
+    # end
   end
 
   describe '#environment_type' do

spec/models/repository_spec.rb

@@ -1829,16 +1829,17 @@ describe Repository, models: true do
     end
   end
 
-  describe '#is_ancestor?' do
+  # TODO: Uncomment when feature is reenabled
-    context 'Gitaly is_ancestor feature enabled' do
+  # describe '#is_ancestor?' do
-      it 'asks Gitaly server if it\'s an ancestor' do
+  #   context 'Gitaly is_ancestor feature enabled' do
-        commit = repository.commit
+  #     it 'asks Gitaly server if it\'s an ancestor' do
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
+  #       commit = repository.commit
-        expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor).
+  #       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
-          with(repository.raw_repository, commit.id, commit.id).and_return(true)
+  #       expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor).
-
+  #         with(repository.raw_repository, commit.id, commit.id).and_return(true)
-        expect(repository.is_ancestor?(commit.id, commit.id)).to be true
+  #
-      end
+  #       expect(repository.is_ancestor?(commit.id, commit.id)).to be true
-    end
+  #     end
-  end
+  #   end
+  # end
 end

spec/spec_helper.rb

@@ -59,6 +59,10 @@ RSpec.configure do |config|
     TestEnv.init
   end
 
+  config.after(:suite) do
+    TestEnv.cleanup
+  end
+
   if ENV['CI']
     # Retry only on feature specs that use JS
     config.around :each, :js do |ex|

spec/support/gitaly.rb

@@ -0,0 +1,7 @@
+if Gitlab::GitalyClient.enabled?
+  RSpec.configure do |config|
+    config.before(:each) do
+      allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(true)
+    end
+  end
+end

spec/support/test_env.rb

@@ -64,6 +64,8 @@ module TestEnv
     # Setup GitLab shell for test instance
     setup_gitlab_shell
 
+    setup_gitaly if Gitlab::GitalyClient.enabled?
+
     # Create repository for FactoryGirl.create(:project)
     setup_factory_repo
 
@@ -71,6 +73,10 @@ module TestEnv
     setup_forked_repo
   end
 
+  def cleanup
+    stop_gitaly
+  end
+
   def disable_mailer
     allow_any_instance_of(NotificationService).to receive(:mailer).
       and_return(double.as_null_object)
@@ -92,7 +98,7 @@ module TestEnv
     tmp_test_path = Rails.root.join('tmp', 'tests', '**')
 
     Dir[tmp_test_path].each do |entry|
-      unless File.basename(entry) =~ /\Agitlab-(shell|test|test_bare|test-fork|test-fork_bare)\z/
+      unless File.basename(entry) =~ /\A(gitaly|gitlab-(shell|test|test_bare|test-fork|test-fork_bare))\z/
         FileUtils.rm_rf(entry)
       end
     end
@@ -110,6 +116,28 @@ module TestEnv
     end
   end
 
+  def setup_gitaly
+    socket_path = Gitlab::GitalyClient.get_address('default').sub(/\Aunix:/, '')
+    gitaly_dir = File.dirname(socket_path)
+
+    unless File.directory?(gitaly_dir) || system('rake', "gitlab:gitaly:install[#{gitaly_dir}]")
+      raise "Can't clone gitaly"
+    end
+
+    start_gitaly(gitaly_dir, socket_path)
+  end
+
+  def start_gitaly(gitaly_dir, socket_path)
+    gitaly_exec = File.join(gitaly_dir, 'gitaly')
+    @gitaly_pid = spawn({ "GITALY_SOCKET_PATH" => socket_path }, gitaly_exec, [:out, :err] => '/dev/null')
+  end
+
+  def stop_gitaly
+    return unless @gitaly_pid
+
+    Process.kill('KILL', @gitaly_pid)
+  end
+
   def setup_factory_repo
     setup_repo(factory_repo_path, factory_repo_path_bare, factory_repo_name,
                BRANCH_SHA)