Merge branch 'gitaly-socket-per-shard' into 'master'

One Gitaly connection per storage shard

See merge request !10150

app/workers/post_receive.rb

@@ -3,20 +3,16 @@ class PostReceive
   include DedicatedSidekiqQueue
 
   def perform(repo_path, identifier, changes)
-    if repository_storage = Gitlab.config.repositories.storages.find { |p| repo_path.start_with?(p[1]['path'].to_s) }
-      repo_path.gsub!(repository_storage[1]['path'].to_s, "")
-    else
-      log("Check gitlab.yml config for correct repositories.storages values. No repository storage path matches \"#{repo_path}\"")
-    end
+    repo_relative_path = Gitlab::RepoPath.strip_storage_path(repo_path)
 
     changes = Base64.decode64(changes) unless changes.include?(' ')
     # Use Sidekiq.logger so arguments can be correlated with execution
     # time and thread ID's.
     Sidekiq.logger.info "changes: #{changes.inspect}" if ENV['SIDEKIQ_LOG_ARGUMENTS']
-    post_received = Gitlab::GitPostReceive.new(repo_path, identifier, changes)
+    post_received = Gitlab::GitPostReceive.new(repo_relative_path, identifier, changes)
 
     if post_received.project.nil?
-      log("Triggered hook for non-existing project with full path \"#{repo_path}\"")
+      log("Triggered hook for non-existing project with full path \"#{repo_relative_path}\"")
       return false
     end
 
@@ -25,7 +21,7 @@ class PostReceive
     elsif post_received.regular_project?
       process_project_changes(post_received)
     else
-      log("Triggered hook for unidentifiable repository type with full path \"#{repo_path}\"")
+      log("Triggered hook for unidentifiable repository type with full path \"#{repo_relative_path}\"")
       false
     end
   end

config/gitlab.yml.example

@@ -443,14 +443,10 @@ production: &base
 
   # Gitaly settings
   gitaly:
-    # The socket_path setting is optional and obsolete. When this is set
-    # GitLab assumes it can reach a Gitaly services via a Unix socket at
-    # this path. When this is commented out GitLab will not use Gitaly.
-    # 
-    # This setting is obsolete because we expect it to be moved under
-    # repositories/storages in GitLab 9.1.
-    #
-    # socket_path: tmp/sockets/private/gitaly.socket
+    # This setting controls whether GitLab uses Gitaly (new component
+    # introduced in 9.0). Eventually Gitaly use will become mandatory and
+    # this option will disappear.
+    enabled: false
 
   #
   # 4. Advanced settings
@@ -465,6 +461,7 @@ production: &base
     storages: # You must have at least a `default` storage path.
       default:
         path: /home/git/repositories/
+        gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket
 
   ## Backup settings
   backup:
@@ -577,6 +574,9 @@ test:
     storages:
       default:
         path: tmp/tests/repositories/
+        gitaly_address: unix:<%= Rails.root.join('tmp/sockets/private/gitaly.socket') %>
+  gitaly:
+    enabled: false
   backup:
     path: tmp/tests/backups
   gitlab_shell:

config/initializers/1_settings.rb

@@ -79,6 +79,10 @@ class Settings < Settingslogic
       value
     end
 
+    def absolute(path)
+      File.expand_path(path, Rails.root)
+    end
+
     private
 
     def base_url(config)
@@ -178,7 +182,7 @@ if github_settings
 end
 
 Settings['shared'] ||= Settingslogic.new({})
-Settings.shared['path'] = File.expand_path(Settings.shared['path'] || "shared", Rails.root)
+Settings.shared['path'] = Settings.absolute(Settings.shared['path'] || "shared")
 
 Settings['issues_tracker'] ||= {}
 
@@ -237,7 +241,7 @@ Settings['gitlab_ci'] ||= Settingslogic.new({})
 Settings.gitlab_ci['shared_runners_enabled'] = true if Settings.gitlab_ci['shared_runners_enabled'].nil?
 Settings.gitlab_ci['all_broken_builds']     = true if Settings.gitlab_ci['all_broken_builds'].nil?
 Settings.gitlab_ci['add_pusher']            = false if Settings.gitlab_ci['add_pusher'].nil?
-Settings.gitlab_ci['builds_path']           = File.expand_path(Settings.gitlab_ci['builds_path'] || "builds/", Rails.root)
+Settings.gitlab_ci['builds_path']           = Settings.absolute(Settings.gitlab_ci['builds_path'] || "builds/")
 Settings.gitlab_ci['url']                 ||= Settings.send(:build_gitlab_ci_url)
 
 #
@@ -251,7 +255,7 @@ Settings.incoming_email['enabled'] = false if Settings.incoming_email['enabled']
 #
 Settings['artifacts'] ||= Settingslogic.new({})
 Settings.artifacts['enabled']      = true if Settings.artifacts['enabled'].nil?
-Settings.artifacts['path']         = File.expand_path(Settings.artifacts['path'] || File.join(Settings.shared['path'], "artifacts"), Rails.root)
+Settings.artifacts['path']         = Settings.absolute(Settings.artifacts['path'] || File.join(Settings.shared['path'], "artifacts"))
 Settings.artifacts['max_size']   ||= 100 # in megabytes
 
 #
@@ -265,14 +269,14 @@ Settings.registry['api_url']       ||= "http://localhost:5000/"
 Settings.registry['key']           ||= nil
 Settings.registry['issuer']        ||= nil
 Settings.registry['host_port']     ||= [Settings.registry['host'], Settings.registry['port']].compact.join(':')
-Settings.registry['path']            = File.expand_path(Settings.registry['path'] || File.join(Settings.shared['path'], 'registry'), Rails.root)
+Settings.registry['path']            = Settings.absolute(Settings.registry['path'] || File.join(Settings.shared['path'], 'registry'))
 
 #
 # Pages
 #
 Settings['pages'] ||= Settingslogic.new({})
 Settings.pages['enabled']         = false if Settings.pages['enabled'].nil?
-Settings.pages['path']            = File.expand_path(Settings.pages['path'] || File.join(Settings.shared['path'], "pages"), Rails.root)
+Settings.pages['path']            = Settings.absolute(Settings.pages['path'] || File.join(Settings.shared['path'], "pages"))
 Settings.pages['https']           = false if Settings.pages['https'].nil?
 Settings.pages['host']            ||= "example.com"
 Settings.pages['port']            ||= Settings.pages.https ? 443 : 80
@@ -286,7 +290,7 @@ Settings.pages['external_https']  ||= false unless Settings.pages['external_http
 #
 Settings['lfs'] ||= Settingslogic.new({})
 Settings.lfs['enabled']      = true if Settings.lfs['enabled'].nil?
-Settings.lfs['storage_path'] = File.expand_path(Settings.lfs['storage_path'] || File.join(Settings.shared['path'], "lfs-objects"), Rails.root)
+Settings.lfs['storage_path'] = Settings.absolute(Settings.lfs['storage_path'] || File.join(Settings.shared['path'], "lfs-objects"))
 
 #
 # Mattermost
@@ -350,8 +354,8 @@ Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['job_class'] = 'Rem
 # GitLab Shell
 #
 Settings['gitlab_shell'] ||= Settingslogic.new({})
-Settings.gitlab_shell['path']         ||= Settings.gitlab['user_home'] + '/gitlab-shell/'
-Settings.gitlab_shell['hooks_path']   ||= Settings.gitlab['user_home'] + '/gitlab-shell/hooks/'
+Settings.gitlab_shell['path']           = Settings.absolute(Settings.gitlab_shell['path'] || Settings.gitlab['user_home'] + '/gitlab-shell/')
+Settings.gitlab_shell['hooks_path']     = Settings.absolute(Settings.gitlab_shell['hooks_path'] || Settings.gitlab['user_home'] + '/gitlab-shell/hooks/')
 Settings.gitlab_shell['secret_file'] ||= Rails.root.join('.gitlab_shell_secret')
 Settings.gitlab_shell['receive_pack']   = true if Settings.gitlab_shell['receive_pack'].nil?
 Settings.gitlab_shell['upload_pack']    = true if Settings.gitlab_shell['upload_pack'].nil?
@@ -374,6 +378,11 @@ unless Settings.repositories.storages['default']
   Settings.repositories.storages['default']['path'] ||= Settings.gitlab['user_home'] + '/repositories/'
 end
 
+Settings.repositories.storages.values.each do |storage|
+  # Expand relative paths
+  storage['path'] = Settings.absolute(storage['path'])
+end
+
 #
 # The repository_downloads_path is used to remove outdated repository
 # archives, if someone has it configured incorrectly, and it points
@@ -395,7 +404,7 @@ end
 Settings['backup'] ||= Settingslogic.new({})
 Settings.backup['keep_time']  ||= 0
 Settings.backup['pg_schema']    = nil
-Settings.backup['path']         = File.expand_path(Settings.backup['path'] || "tmp/backups/", Rails.root)
+Settings.backup['path']         = Settings.absolute(Settings.backup['path'] || "tmp/backups/")
 Settings.backup['archive_permissions'] ||= 0600
 Settings.backup['upload'] ||= Settingslogic.new({ 'remote_directory' => nil, 'connection' => nil })
 # Convert upload connection settings to use symbol keys, to make Fog happy
@@ -418,7 +427,7 @@ Settings.git['timeout']   ||= 10
 # least. This setting is fed to 'rm -rf' in
 # db/migrate/20151023144219_remove_satellites.rb
 Settings['satellites'] ||= Settingslogic.new({})
-Settings.satellites['path'] = File.expand_path(Settings.satellites['path'] || "tmp/repo_satellites/", Rails.root)
+Settings.satellites['path'] = Settings.absolute(Settings.satellites['path'] || "tmp/repo_satellites/")
 
 #
 # Extra customization
@@ -440,7 +449,7 @@ Settings.rack_attack.git_basic_auth['bantime'] ||= 1.hour
 # Gitaly
 #
 Settings['gitaly'] ||= Settingslogic.new({})
-Settings.gitaly['socket_path'] ||= ENV['GITALY_SOCKET_PATH']
+Settings.gitaly['enabled'] ||= false
 
 #
 # Webpack settings

config/initializers/8_gitaly.rb

-# Make sure we initialize a Gitaly channel before Sidekiq starts multi-threaded execution.
-Gitlab::GitalyClient.channel unless Rails.env.test?
+require 'uri'
+
+# Make sure we initialize our Gitaly channels before Sidekiq starts multi-threaded execution.
+if Gitlab.config.gitaly.enabled || Rails.env.test?
+  Gitlab.config.repositories.storages.each do |name, params|
+    address = params['gitaly_address']
+
+    unless address.present?
+      raise "storage #{name.inspect} is missing a gitaly_address"
+    end
+
+    unless URI(address).scheme == 'unix'
+      raise "Unsupported Gitaly address: #{address.inspect}"
+    end
+
+    Gitlab::GitalyClient.configure_channel(name, address)
+  end
+end

doc/install/installation.md

@@ -477,12 +477,12 @@ with setting up Gitaly until you upgrade to GitLab 9.1 or later.
     # Enable Gitaly in the init script
     echo 'gitaly_enabled=true' | sudo tee -a /etc/default/gitlab
 
-Next, edit `/home/git/gitlab/config/gitlab.yml` and make sure `socket_path` in
+Next, edit `/home/git/gitlab/config/gitlab.yml` and make sure `enabled: true` in
 the `gitaly:` section is uncommented.
 
     # <- gitlab.yml indentation starts here
       gitaly:
-        socket_path: tmp/sockets/private/gitaly.socket
+        enabled: true
 
 For more information about configuring Gitaly see
 [doc/administration/gitaly](../administration/gitaly).

lib/api/internal.rb

@@ -139,7 +139,7 @@ module API
         return unless Gitlab::GitalyClient.enabled?
 
         begin
-          Gitlab::GitalyClient::Notifications.new.post_receive(params[:repo_path])
+          Gitlab::GitalyClient::Notifications.new(params[:repo_path]).post_receive
         rescue GRPC::Unavailable => e
           render_api_error(e, 500)
         end

lib/backup/repository.rb

@@ -182,7 +182,9 @@ module Backup
 
       dir_entries = Dir.entries(path)
 
-      yield('custom_hooks') if dir_entries.include?('custom_hooks')
+      if dir_entries.include?('custom_hooks') || dir_entries.include?('custom_hooks.tar')
+        yield('custom_hooks')
+      end
     end
 
     def prepare

lib/gitlab/gitaly_client.rb

@@ -4,28 +4,30 @@ module Gitlab
   module GitalyClient
     SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze
 
-    def self.gitaly_address
-      if Gitlab.config.gitaly.socket_path
-        "unix://#{Gitlab.config.gitaly.socket_path}"
-      end
+    def self.configure_channel(storage, address)
+      @addresses ||= {}
+      @addresses[storage] = address
+      @channels ||= {}
+      @channels[storage] = new_channel(address)
+    end
+
+    def self.new_channel(address)
+      # NOTE: Gitaly currently runs on a Unix socket, so permissions are
+      # handled using the file system and no additional authentication is
+      # required (therefore the :this_channel_is_insecure flag)
+      GRPC::Core::Channel.new(address, {}, :this_channel_is_insecure)
     end
 
-    def self.channel
-      return @channel if defined?(@channel)
+    def self.get_channel(storage)
+      @channels[storage]
+    end
 
-      @channel =
-        if enabled?
-          # NOTE: Gitaly currently runs on a Unix socket, so permissions are
-          # handled using the file system and no additional authentication is
-          # required (therefore the :this_channel_is_insecure flag)
-          GRPC::Core::Channel.new(gitaly_address, {}, :this_channel_is_insecure)
-        else
-          nil
-        end
+    def self.get_address(storage)
+      @addresses[storage]
     end
 
     def self.enabled?
-      gitaly_address.present?
+      Gitlab.config.gitaly.enabled
     end
 
     def self.feature_enabled?(feature)

lib/gitlab/gitaly_client/commit.rb

@@ -7,8 +7,10 @@ module Gitlab
 
       class << self
         def diff_from_parent(commit, options = {})
-          stub      = Gitaly::Diff::Stub.new(nil, nil, channel_override: GitalyClient.channel)
-          repo      = Gitaly::Repository.new(path: commit.project.repository.path_to_repo)
+          project   = commit.project
+          channel   = GitalyClient.get_channel(project.repository_storage)
+          stub      = Gitaly::Diff::Stub.new(nil, nil, channel_override: channel)
+          repo      = Gitaly::Repository.new(path: project.repository.path_to_repo)
           parent    = commit.parents[0]
           parent_id = parent ? parent.id : EMPTY_TREE_ID
           request   = Gitaly::CommitDiffRequest.new(

lib/gitlab/gitaly_client/notifications.rb

@@ -3,14 +3,19 @@ module Gitlab
     class Notifications
       attr_accessor :stub
 
-      def initialize
-        @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: GitalyClient.channel)
+      def initialize(repo_path)
+        full_path = Gitlab::RepoPath.strip_storage_path(repo_path).
+          sub(/\.git\z/, '').sub(/\.wiki\z/, '')
+        @project = Project.find_by_full_path(full_path)
+
+        channel = GitalyClient.get_channel(@project.repository_storage)
+        @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: channel)
       end
 
-      def post_receive(repo_path)
-        repository = Gitaly::Repository.new(path: repo_path)
+      def post_receive
+        repository = Gitaly::Repository.new(path: @project.repository.path_to_repo)
         request = Gitaly::PostReceiveRequest.new(repository: repository)
-        stub.post_receive(request)
+        @stub.post_receive(request)
       end
     end
   end

lib/gitlab/repo_path.rb

+module Gitlab
+  module RepoPath
+    NotFoundError = Class.new(StandardError)
+
+    def self.strip_storage_path(repo_path)
+      result = nil
+
+      Gitlab.config.repositories.storages.values.each do |params|
+        storage_path = params['path']
+        if repo_path.start_with?(storage_path)
+          result = repo_path.sub(storage_path, '')
+          break
+        end
+      end
+
+      if result.nil?
+        raise NotFoundError.new("No known storage path matches #{repo_path.inspect}")
+      end
+
+      result.sub(/\A\/*/, '')
+    end
+  end
+end

lib/gitlab/workhorse.rb

 require 'base64'
 require 'json'
 require 'securerandom'
+require 'uri'
 
 module Gitlab
   class Workhorse
@@ -21,10 +22,10 @@ module Gitlab
           RepoPath: repository.path_to_repo,
         }
 
-        params.merge!(
-          GitalySocketPath: Gitlab.config.gitaly.socket_path,
-          GitalyResourcePath: "/projects/#{repository.project.id}/git-http/info-refs",
-        ) if Gitlab.config.gitaly.socket_path.present?
+        if Gitlab.config.gitaly.enabled
+          address = Gitlab::GitalyClient.get_address(repository.project.repository_storage)
+          params[:GitalySocketPath] = URI(address).path
+        end
 
         params
       end

spec/lib/gitlab/gitaly_client/notifications_spec.rb

 require 'spec_helper'
 
 describe Gitlab::GitalyClient::Notifications do
-  let(:client) { Gitlab::GitalyClient::Notifications.new }
-
-  before do
-    allow(Gitlab.config.gitaly).to receive(:socket_path).and_return('path/to/gitaly.socket')
-  end
-
   describe '#post_receive' do
-    let(:repo_path) { '/path/to/my_repo.git' }
-
     it 'sends a post_receive message' do
+      repo_path = create(:empty_project).repository.path_to_repo
       expect_any_instance_of(Gitaly::Notifications::Stub).
         to receive(:post_receive).with(post_receive_request_with_repo_path(repo_path))
 
-      client.post_receive(repo_path)
+      described_class.new(repo_path).post_receive
     end
   end
 end

spec/lib/gitlab/repo_path_spec.rb

+require 'spec_helper'
+
+describe ::Gitlab::RepoPath do
+  describe '.strip_storage_path' do
+    before do
+      allow(Gitlab.config.repositories).to receive(:storages).and_return({
+        'storage1' => { 'path' => '/foo' },
+        'storage2' => { 'path' => '/bar' },
+      })
+    end
+
+    it 'strips the storage path' do
+      expect(described_class.strip_storage_path('/bar/foo/qux/baz.git')).to eq('foo/qux/baz.git')
+    end
+
+    it 'raises NotFoundError if no storage matches the path' do
+      expect { described_class.strip_storage_path('/doesnotexist/foo.git') }.to raise_error(
+        described_class::NotFoundError
+      )
+    end
+  end
+end

spec/lib/gitlab/workhorse_spec.rb

@@ -184,18 +184,14 @@ describe Gitlab::Workhorse, lib: true do
 
     it { expect(subject).to eq({ GL_ID: "user-#{user.id}", RepoPath: repository.path_to_repo }) }
 
-    context 'when Gitaly socket path is present' do
-      let(:gitaly_socket_path) { '/tmp/gitaly.sock' }
-
+    context 'when Gitaly is enabled' do
       before do
-        allow(Gitlab.config.gitaly).to receive(:socket_path).and_return(gitaly_socket_path)
+        allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)
       end
 
       it 'includes Gitaly params in the returned value' do
-        expect(subject).to include({
-          GitalyResourcePath: "/projects/#{repository.project.id}/git-http/info-refs",
-          GitalySocketPath: gitaly_socket_path,
-        })
+        gitaly_socket_path = URI(Gitlab::GitalyClient.get_address('default')).path
+        expect(subject).to include({ GitalySocketPath: gitaly_socket_path })
       end
     end
   end

spec/requests/api/internal_spec.rb

@@ -424,12 +424,12 @@ describe API::Internal, api: true  do
     end
 
     before do
-      allow(Gitlab.config.gitaly).to receive(:socket_path).and_return('path/to/gitaly.socket')
+      allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)
     end
 
     it "calls the Gitaly client if it's enabled" do
       expect_any_instance_of(Gitlab::GitalyClient::Notifications).
-        to receive(:post_receive).with(project.repository.path)
+        to receive(:post_receive)
 
       post api("/internal/notify_post_receive"), valid_params
 
@@ -438,7 +438,7 @@ describe API::Internal, api: true  do
 
     it "returns 500 if the gitaly call fails" do
       expect_any_instance_of(Gitlab::GitalyClient::Notifications).
-        to receive(:post_receive).with(project.repository.path).and_raise(GRPC::Unavailable)
+        to receive(:post_receive).and_raise(GRPC::Unavailable)
 
       post api("/internal/notify_post_receive"), valid_params
 

spec/tasks/gitlab/backup_rake_spec.rb

@@ -81,6 +81,10 @@ describe 'gitlab:app namespace rake task' do
   end # backup_restore task
 
   describe 'backup' do
+    before(:all) do
+      ENV['force'] = 'yes'
+    end
+
     def tars_glob
       Dir.glob(File.join(Gitlab.config.backup.path, '*_gitlab_backup.tar'))
     end
@@ -88,6 +92,9 @@ describe 'gitlab:app namespace rake task' do
     def create_backup
       FileUtils.rm tars_glob
 
+      # This reconnect makes our project fixture disappear, breaking the restore. Stub it out.
+      allow(ActiveRecord::Base.connection).to receive(:reconnect!)
+
       # Redirect STDOUT and run the rake task
       orig_stdout = $stdout
       $stdout = StringIO.new
@@ -119,9 +126,6 @@ describe 'gitlab:app namespace rake task' do
         FileUtils.mkdir_p(path)
         FileUtils.touch(File.join(path, "dummy.txt"))
 
-        # We need to use the full path instead of the relative one
-        allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(File.expand_path(Gitlab.config.gitlab_shell.path, Rails.root.to_s))
-
         ENV["SKIP"] = "db"
         create_backup
       end
@@ -227,8 +231,8 @@ describe 'gitlab:app namespace rake task' do
         FileUtils.mkdir('tmp/tests/default_storage')
         FileUtils.mkdir('tmp/tests/custom_storage')
         storages = {
-          'default' => { 'path' => 'tmp/tests/default_storage' },
-          'custom' => { 'path' => 'tmp/tests/custom_storage' }
+          'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') },
+          'custom' => { 'path' => Settings.absolute('tmp/tests/custom_storage') }
         }
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)