diff options
| author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-09-12 12:26:59 +0200 |
|---|---|---|
| committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-10-05 14:11:32 +0200 |
| commit | 4656283c5cd10116d79812db24c37ff79b0e0273 (patch) | |
| tree | bc66bc0e89178eb36c2d6bbcf80c2f0c4fb6170e | |
| parent | 905d3ed4292e6b274ac51b26b3b3577087c5f737 (diff) | |
| download | gitlab-ce-4656283c5cd10116d79812db24c37ff79b0e0273.tar.gz | |
Gitaly namespace service enabled for GitLab
| -rw-r--r-- | app/models/project.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/namespace_service.rb | 39 | ||||
| -rw-r--r-- | lib/gitlab/shell.rb | 48 | ||||
| -rw-r--r-- | lib/tasks/gitlab/gitaly.rake | 7 | ||||
| -rw-r--r-- | spec/controllers/projects_controller_spec.rb | 3 | ||||
| -rw-r--r-- | spec/lib/gitlab/shell_spec.rb | 52 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 14 | ||||
| -rw-r--r-- | spec/models/repository_spec.rb | 4 | ||||
| -rw-r--r-- | spec/tasks/gitlab/backup_rake_spec.rb | 17 |
10 files changed, 158 insertions, 30 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 59b5a5b3cd7..5f80195028a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1034,6 +1034,8 @@ class Project < ActiveRecord::Base end true + rescue GRPC::Internal # if the path is too long + false end def create_repository(force: false) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e42bbb659b4..0f059bef808 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1112,6 +1112,8 @@ module Gitlab raise NoRepository.new(e) rescue GRPC::BadStatus => e raise CommandError.new(e) + rescue GRPC::InvalidArgument => e + raise ArgumentError.new(e) end private diff --git a/lib/gitlab/gitaly_client/namespace_service.rb b/lib/gitlab/gitaly_client/namespace_service.rb new file mode 100644 index 00000000000..bd7c345ac01 --- /dev/null +++ b/lib/gitlab/gitaly_client/namespace_service.rb @@ -0,0 +1,39 @@ +module Gitlab + module GitalyClient + class NamespaceService + def initialize(storage) + @storage = storage + end + + def exists?(name) + request = Gitaly::NamespaceExistsRequest.new(storage_name: @storage, name: name) + + gitaly_client_call(:namespace_exists, request).exists + end + + def add(name) + request = Gitaly::AddNamespaceRequest.new(storage_name: @storage, name: name) + + gitaly_client_call(:add_namespace, request) + end + + def remove(name) + request = Gitaly::RemoveNamespaceRequest.new(storage_name: @storage, name: name) + + gitaly_client_call(:remove_namespace, request) + end + + def rename(from, to) + request = Gitaly::RenameNamespaceRequest.new(storage_name: @storage, from: from, to: to) + + gitaly_client_call(:rename_namespace, request) + end + + private + + def gitaly_client_call(type, request) + GitalyClient.call(@storage, :namespace_service, type, request) + end + end + end +end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index a99f8e2b5f8..3fdf0ce1ce0 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -222,10 +222,18 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def add_namespace(storage, name) - path = full_path(storage, name) - FileUtils.mkdir_p(path, mode: 0770) unless exists?(storage, name) + Gitlab::GitalyClient.migrate(:add_namespace) do |enabled| + if enabled + gitaly_namespace_client(storage).add(name) + else + path = full_path(storage, name) + FileUtils.mkdir_p(path, mode: 0770) unless exists?(storage, name) + end + end rescue Errno::EEXIST => e Rails.logger.warn("Directory exists as a file: #{e} at: #{path}") + rescue GRPC::InvalidArgument => e + raise ArgumentError, e.message end # Remove directory from repositories storage @@ -236,7 +244,15 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def rm_namespace(storage, name) - FileUtils.rm_r(full_path(storage, name), force: true) + Gitlab::GitalyClient.migrate(:remove_namespace) do |enabled| + if enabled + gitaly_namespace_client(storage).remove(name) + else + FileUtils.rm_r(full_path(storage, name), force: true) + end + end + rescue GRPC::InvalidArgument => e + raise ArgumentError, e.message end # Move namespace directory inside repositories storage @@ -246,9 +262,15 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def mv_namespace(storage, old_name, new_name) - return false if exists?(storage, new_name) || !exists?(storage, old_name) + Gitlab::GitalyClient.migrate(:rename_namespace) do |enabled| + if enabled + gitaly_namespace_client(storage).rename(old_name, new_name) + else + return false if exists?(storage, new_name) || !exists?(storage, old_name) - FileUtils.mv(full_path(storage, old_name), full_path(storage, new_name)) + FileUtils.mv(full_path(storage, old_name), full_path(storage, new_name)) + end + end end def url_to_repo(path) @@ -272,7 +294,13 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def exists?(storage, dir_name) - File.exist?(full_path(storage, dir_name)) + Gitlab::GitalyClient.migrate(:namespace_exists) do |enabled| + if enabled + gitaly_namespace_client(storage).exists?(dir_name) + else + File.exist?(full_path(storage, dir_name)) + end + end end protected @@ -349,6 +377,14 @@ module Gitlab Bundler.with_original_env { Popen.popen(cmd, nil, vars) } end + def gitaly_namespace_client(storage_path) + storage, _value = Gitlab.config.repositories.storages.find do |storage, value| + value['path'] == storage_path + end + + Gitlab::GitalyClient::NamespaceService.new(storage) + end + def gitaly_migrate(method, &block) Gitlab::GitalyClient.migrate(method, &block) rescue GRPC::NotFound, GRPC::BadStatus => e diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 08677a98fc1..8377fe3269d 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -50,6 +50,8 @@ namespace :gitlab do # only generate a configuration for the most common and simplest case: when # we have exactly one Gitaly process and we are sure it is running locally # because it uses a Unix socket. + # For development and testing purposes, an extra storage is added to gitaly, + # which is not known to Rails, but must be explicitly stubbed. def gitaly_configuration_toml(gitaly_ruby: true) storages = [] address = nil @@ -67,6 +69,11 @@ namespace :gitlab do storages << { name: key, path: val['path'] } end + + if Rails.env.test? + storages << { name: 'test_second_storage', path: Rails.root.join('tmp', 'tests', 'second_storage').to_s } + end + config = { socket_path: address.sub(%r{\Aunix:}, ''), storage: storages } config[:auth] = { token: 'secret' } if Rails.env.test? config[:'gitaly-ruby'] = { dir: File.join(Dir.pwd, 'ruby') } if gitaly_ruby diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 2a91a6613e6..491f35d0fb6 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -140,7 +140,8 @@ describe ProjectsController do end context 'when the storage is not available', broken_storage: true do - let(:project) { create(:project, :broken_storage) } + set(:project) { create(:project, :broken_storage) } + before do project.add_developer(user) sign_in(user) diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 9efdd7940ca..139afa22d01 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -15,10 +15,6 @@ describe Gitlab::Shell do it { is_expected.to respond_to :add_repository } it { is_expected.to respond_to :remove_repository } it { is_expected.to respond_to :fork_repository } - it { is_expected.to respond_to :add_namespace } - it { is_expected.to respond_to :rm_namespace } - it { is_expected.to respond_to :mv_namespace } - it { is_expected.to respond_to :exists? } it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } @@ -363,4 +359,52 @@ describe Gitlab::Shell do end end end + + describe 'namespace actions' do + subject { described_class.new } + let(:storage_path) { Gitlab.config.repositories.storages.default.path } + + describe '#add_namespace' do + it 'creates a namespace' do + subject.add_namespace(storage_path, "mepmep") + + expect(subject.exists?(storage_path, "mepmep")).to be(true) + end + end + + describe '#exists?' do + context 'when the namespace does not exist' do + it 'returns false' do + expect(subject.exists?(storage_path, "non-existing")).to be(false) + end + end + + context 'when the namespace exists' do + it 'returns true' do + subject.add_namespace(storage_path, "mepmep") + + expect(subject.exists?(storage_path, "mepmep")).to be(true) + end + end + end + + describe '#remove' do + it 'removes the namespace' do + subject.add_namespace(storage_path, "mepmep") + subject.rm_namespace(storage_path, "mepmep") + + expect(subject.exists?(storage_path, "mepmep")).to be(false) + end + end + + describe '#mv_namespace' do + it 'renames the namespace' do + subject.add_namespace(storage_path, "mepmep") + subject.mv_namespace(storage_path, "mepmep", "2mep") + + expect(subject.exists?(storage_path, "mepmep")).to be(false) + expect(subject.exists?(storage_path, "2mep")).to be(true) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 176bb568cbe..3d6a79e0649 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -421,20 +421,10 @@ describe Project do end describe '#repository_storage_path' do - let(:project) { create(:project, repository_storage: 'custom') } - - before do - FileUtils.mkdir('tmp/tests/custom_repositories') - storages = { 'custom' => { 'path' => 'tmp/tests/custom_repositories' } } - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - end - - after do - FileUtils.rm_rf('tmp/tests/custom_repositories') - end + let(:project) { create(:project) } it 'returns the repository storage path' do - expect(project.repository_storage_path).to eq('tmp/tests/custom_repositories') + expect(Dir.exist?(project.repository_storage_path)).to be(true) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7156c1b7aa8..5764c451b67 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -634,6 +634,10 @@ describe Repository do end describe '#fetch_ref' do + # Setting the var here, sidesteps the stub that makes gitaly raise an error + # before the actual test call + set(:broken_repository) { create(:project, :broken_storage).repository } + describe 'when storage is broken', broken_storage: true do it 'should raise a storage error' do expect_to_raise_storage_error do diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 0c8c8a2ab05..886052d7848 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -224,17 +224,20 @@ describe 'gitlab:app namespace rake task' do end context 'multiple repository storages' do + let(:gitaly_address) { Gitlab.config.repositories.storages.default.gitaly_address } + let(:storages) do + { + 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage'), 'gitaly_address' => gitaly_address }, + 'test_second_storage' => { 'path' => Settings.absolute('tmp/tests/custom_storage'), 'gitaly_address' => gitaly_address } + } + end + let(:project_a) { create(:project, :repository, repository_storage: 'default') } - let(:project_b) { create(:project, :repository, repository_storage: 'custom') } + let(:project_b) { create(:project, :repository, repository_storage: 'test_second_storage') } before do FileUtils.mkdir('tmp/tests/default_storage') FileUtils.mkdir('tmp/tests/custom_storage') - gitaly_address = Gitlab.config.repositories.storages.default.gitaly_address - storages = { - 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage'), 'gitaly_address' => gitaly_address }, - 'custom' => { 'path' => Settings.absolute('tmp/tests/custom_storage'), 'gitaly_address' => gitaly_address } - } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) # Create the projects now, after mocking the settings but before doing the backup @@ -253,7 +256,7 @@ describe 'gitlab:app namespace rake task' do after do FileUtils.rm_rf('tmp/tests/default_storage') FileUtils.rm_rf('tmp/tests/custom_storage') - FileUtils.rm(@backup_tar) + FileUtils.rm(@backup_tar) if @backup_tar end it 'includes repositories in all repository storages' do |
