Commit c1b8cab3 authored by John Cai's avatar John Cai

Allow gitaly create_repository to take a default branch argument

Currently when rails wants to ensure a default branch name, it will call
the Gitaly WriteRef RPC to manually change it. If this call happens in a
racy way eg: it gets called when another RPC is operating on the same
repository, it will fail. Rather than do this, the Gitaly
CreateRepository RPC has a default_branch parameter.

Allow rails callers to set this value. If it is not provided, then the
default branch in Gitaly will be used, which is main.
parent 574514aa
...@@ -476,7 +476,7 @@ gem 'ssh_data', '~> 1.2' ...@@ -476,7 +476,7 @@ gem 'ssh_data', '~> 1.2'
gem 'spamcheck', '~> 0.1.0' gem 'spamcheck', '~> 0.1.0'
# Gitaly GRPC protocol definitions # Gitaly GRPC protocol definitions
gem 'gitaly', '~> 14.9.0.pre.rc2' gem 'gitaly', '~> 14.9.0.pre.rc3'
# KAS GRPC protocol definitions # KAS GRPC protocol definitions
gem 'kas-grpc', '~> 0.0.2' gem 'kas-grpc', '~> 0.0.2'
......
...@@ -455,7 +455,7 @@ GEM ...@@ -455,7 +455,7 @@ GEM
rails (>= 3.2.0) rails (>= 3.2.0)
git (1.7.0) git (1.7.0)
rchardet (~> 1.8) rchardet (~> 1.8)
gitaly (14.9.0.pre.rc2) gitaly (14.9.0.pre.rc3)
grpc (~> 1.0) grpc (~> 1.0)
github-markup (1.7.0) github-markup (1.7.0)
gitlab (4.16.1) gitlab (4.16.1)
...@@ -1475,7 +1475,7 @@ DEPENDENCIES ...@@ -1475,7 +1475,7 @@ DEPENDENCIES
gettext (~> 3.3) gettext (~> 3.3)
gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3) gettext_i18n_rails_js (~> 1.3)
gitaly (~> 14.9.0.pre.rc2) gitaly (~> 14.9.0.pre.rc3)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5) gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 2.10.2) gitlab-dangerfiles (~> 2.10.2)
......
...@@ -1085,10 +1085,10 @@ class Repository ...@@ -1085,10 +1085,10 @@ class Repository
blob.data blob.data
end end
def create_if_not_exists def create_if_not_exists(default_branch = nil)
return if exists? return if exists?
raw.create_repository raw.create_repository(default_branch)
after_create after_create
true true
......
...@@ -367,7 +367,7 @@ class Snippet < ApplicationRecord ...@@ -367,7 +367,7 @@ class Snippet < ApplicationRecord
def create_repository def create_repository
return if repository_exists? && snippet_repository return if repository_exists? && snippet_repository
repository.create_if_not_exists repository.create_if_not_exists(default_branch)
track_snippet_repository(repository.storage) track_snippet_repository(repository.storage)
end end
......
...@@ -99,9 +99,9 @@ module Gitlab ...@@ -99,9 +99,9 @@ module Gitlab
gitaly_repository_client.exists? gitaly_repository_client.exists?
end end
def create_repository def create_repository(default_branch = nil)
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_repository_client.create_repository gitaly_repository_client.create_repository(default_branch)
rescue GRPC::AlreadyExists => e rescue GRPC::AlreadyExists => e
raise RepositoryExists, e.message raise RepositoryExists, e.message
end end
......
...@@ -107,8 +107,8 @@ module Gitlab ...@@ -107,8 +107,8 @@ module Gitlab
end end
# rubocop: enable Metrics/ParameterLists # rubocop: enable Metrics/ParameterLists
def create_repository def create_repository(default_branch = nil)
request = Gitaly::CreateRepositoryRequest.new(repository: @gitaly_repo) request = Gitaly::CreateRepositoryRequest.new(repository: @gitaly_repo, default_branch: default_branch)
GitalyClient.call(@storage, :repository_service, :create_repository, request, timeout: GitalyClient.fast_timeout) GitalyClient.call(@storage, :repository_service, :create_repository, request, timeout: GitalyClient.fast_timeout)
end end
......
...@@ -78,6 +78,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat ...@@ -78,6 +78,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat
end end
shared_examples 'migration_bot user commits files' do shared_examples 'migration_bot user commits files' do
before do
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main')
end
it do it do
subject subject
...@@ -89,6 +93,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat ...@@ -89,6 +93,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat
end end
shared_examples 'commits the file to the repository' do shared_examples 'commits the file to the repository' do
before do
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main')
end
context 'when author can update snippet and use git' do context 'when author can update snippet and use git' do
it 'creates the repository and commit the file' do it 'creates the repository and commit the file' do
subject subject
...@@ -269,6 +277,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat ...@@ -269,6 +277,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat
let!(:snippet) { snippets.create!(id: 5, type: 'PersonalSnippet', author_id: other_user.id, file_name: file_name, content: content) } let!(:snippet) { snippets.create!(id: 5, type: 'PersonalSnippet', author_id: other_user.id, file_name: file_name, content: content) }
let(:ids) { [4, 5] } let(:ids) { [4, 5] }
before do
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main')
end
after do after do
raw_repository(snippet).remove raw_repository(snippet).remove
raw_repository(invalid_snippet).remove raw_repository(invalid_snippet).remove
......
...@@ -218,6 +218,26 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do ...@@ -218,6 +218,26 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
end end
end end
describe '#create_repository' do
it 'sends a create_repository message without arguments' do
expect_any_instance_of(Gitaly::RepositoryService::Stub)
.to receive(:create_repository)
.with(gitaly_request_with_path(storage_name, relative_path).and(gitaly_request_with_params(default_branch: '')), kind_of(Hash))
.and_return(double)
client.create_repository
end
it 'sends a create_repository message with default branch' do
expect_any_instance_of(Gitaly::RepositoryService::Stub)
.to receive(:create_repository)
.with(gitaly_request_with_path(storage_name, relative_path).and(gitaly_request_with_params(default_branch: 'default-branch-name')), kind_of(Hash))
.and_return(double)
client.create_repository('default-branch-name')
end
end
describe '#create_from_snapshot' do describe '#create_from_snapshot' do
it 'sends a create_repository_from_snapshot message' do it 'sends a create_repository_from_snapshot message' do
expect_any_instance_of(Gitaly::RepositoryService::Stub) expect_any_instance_of(Gitaly::RepositoryService::Stub)
......
...@@ -3062,6 +3062,14 @@ RSpec.describe Repository do ...@@ -3062,6 +3062,14 @@ RSpec.describe Repository do
repository.create_if_not_exists repository.create_if_not_exists
end end
it 'creates a repository with a default branch name' do
default_branch_name = 'branch-a'
repository.create_if_not_exists(default_branch_name)
repository.create_file(user, 'file', 'content', message: 'initial commit', branch_name: default_branch_name)
expect(repository.root_ref).to eq(default_branch_name)
end
context 'it does nothing if the repository already existed' do context 'it does nothing if the repository already existed' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
......
...@@ -667,6 +667,16 @@ RSpec.describe Snippet do ...@@ -667,6 +667,16 @@ RSpec.describe Snippet do
expect(snippet.repository.exists?).to be_truthy expect(snippet.repository.exists?).to be_truthy
end end
it 'sets the default branch' do
expect(snippet).to receive(:default_branch).and_return('default-branch-1')
expect(subject).to be_truthy
snippet.repository.create_file(snippet.author, 'file', 'content', message: 'initial commit', branch_name: 'default-branch-1')
expect(snippet.repository.exists?).to be_truthy
expect(snippet.repository.root_ref).to eq('default-branch-1')
end
it 'tracks snippet repository' do it 'tracks snippet repository' do
expect do expect do
subject subject
...@@ -677,6 +687,7 @@ RSpec.describe Snippet do ...@@ -677,6 +687,7 @@ RSpec.describe Snippet do
expect(snippet).to receive(:repository_storage).and_return('picked') expect(snippet).to receive(:repository_storage).and_return('picked')
expect(snippet).to receive(:repository_exists?).and_return(false) expect(snippet).to receive(:repository_exists?).and_return(false)
expect(snippet.repository).to receive(:create_if_not_exists) expect(snippet.repository).to receive(:create_if_not_exists)
allow(snippet).to receive(:default_branch).and_return('picked')
subject subject
...@@ -899,22 +910,6 @@ RSpec.describe Snippet do ...@@ -899,22 +910,6 @@ RSpec.describe Snippet do
end end
end end
context 'when repository is empty' do
let(:snippet) { create(:snippet, :empty_repo) }
before do
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return(default_branch)
end
context 'when default branch in settings is different from "master"' do
let(:default_branch) { 'custom-branch' }
it 'changes the HEAD reference to the default branch' do
expect { subject }.to change { File.read(head_path).squish }.to("ref: refs/heads/#{default_branch}")
end
end
end
context 'when repository is not empty' do context 'when repository is not empty' do
let(:snippet) { create(:snippet, :empty_repo) } let(:snippet) { create(:snippet, :empty_repo) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment