Commit 6b6f8fc4 authored by Patrick Steinhardt's avatar Patrick Steinhardt

Implement in-memory remotes for `update_remote_mirror`

The `update_remote_mirror` functions is currently using on-disk remotes
by first calling `remote_mirror.ensure_remote!` followed by the call to
`update_repository`. Gitaly is phasing out support for on-disk remotes
though in favor of in-memory remotes, where the remote is specified as
part of the request itself.

Implement the ability to use in-memory remotes for this RPC. For now,
this is implemented behind a feature flag.
parent 4b6338a7
...@@ -100,10 +100,11 @@ class RemoteMirror < ApplicationRecord ...@@ -100,10 +100,11 @@ class RemoteMirror < ApplicationRecord
update_status == 'started' update_status == 'started'
end end
def update_repository def update_repository(inmemory_remote:)
Gitlab::Git::RemoteMirror.new( Gitlab::Git::RemoteMirror.new(
project.repository.raw, project.repository.raw,
remote_name, remote_name,
inmemory_remote ? remote_url : nil,
**options_for_update **options_for_update
).update ).update
end end
......
...@@ -39,12 +39,16 @@ module Projects ...@@ -39,12 +39,16 @@ module Projects
def update_mirror(remote_mirror) def update_mirror(remote_mirror)
remote_mirror.update_start! remote_mirror.update_start!
remote_mirror.ensure_remote!
# LFS objects must be sent first, or the push has dangling pointers # LFS objects must be sent first, or the push has dangling pointers
send_lfs_objects!(remote_mirror) send_lfs_objects!(remote_mirror)
response = remote_mirror.update_repository response = if Feature.enabled?(:update_remote_mirror_inmemory, project, default_enabled: :yaml)
remote_mirror.update_repository(inmemory_remote: true)
else
remote_mirror.ensure_remote!
remote_mirror.update_repository(inmemory_remote: false)
end
if response.divergent_refs.any? if response.divergent_refs.any?
message = "Some refs have diverged and have not been updated on the remote:" message = "Some refs have diverged and have not been updated on the remote:"
......
---
name: update_remote_mirror_inmemory
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63962
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333517
milestone: '14.0'
type: development
group: group::gitaly
default_enabled: false
...@@ -5,11 +5,12 @@ module Gitlab ...@@ -5,11 +5,12 @@ module Gitlab
class RemoteMirror class RemoteMirror
include Gitlab::Git::WrapsGitalyErrors include Gitlab::Git::WrapsGitalyErrors
attr_reader :repository, :ref_name, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs attr_reader :repository, :ref_name, :remote_url, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs
def initialize(repository, ref_name, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false) def initialize(repository, ref_name, remote_url, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
@repository = repository @repository = repository
@ref_name = ref_name @ref_name = ref_name
@remote_url = remote_url
@only_branches_matching = only_branches_matching @only_branches_matching = only_branches_matching
@ssh_key = ssh_key @ssh_key = ssh_key
@known_hosts = known_hosts @known_hosts = known_hosts
...@@ -20,6 +21,7 @@ module Gitlab ...@@ -20,6 +21,7 @@ module Gitlab
wrapped_gitaly_errors do wrapped_gitaly_errors do
repository.gitaly_remote_client.update_remote_mirror( repository.gitaly_remote_client.update_remote_mirror(
ref_name, ref_name,
remote_url,
only_branches_matching, only_branches_matching,
ssh_key: ssh_key, ssh_key: ssh_key,
known_hosts: known_hosts, known_hosts: known_hosts,
......
...@@ -55,13 +55,18 @@ module Gitlab ...@@ -55,13 +55,18 @@ module Gitlab
encode_utf8(response.ref) encode_utf8(response.ref)
end end
def update_remote_mirror(ref_name, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false) def update_remote_mirror(ref_name, remote_url, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
req_enum = Enumerator.new do |y| req_enum = Enumerator.new do |y|
first_request = Gitaly::UpdateRemoteMirrorRequest.new( first_request = Gitaly::UpdateRemoteMirrorRequest.new(
repository: @gitaly_repo, repository: @gitaly_repo
ref_name: ref_name
) )
if remote_url
first_request.remote = Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: remote_url)
else
first_request.ref_name = ref_name
end
first_request.ssh_key = ssh_key if ssh_key.present? first_request.ssh_key = ssh_key if ssh_key.present?
first_request.known_hosts = known_hosts if known_hosts.present? first_request.known_hosts = known_hosts if known_hosts.present?
first_request.keep_divergent_refs = keep_divergent_refs first_request.keep_divergent_refs = keep_divergent_refs
......
...@@ -7,16 +7,29 @@ RSpec.describe Gitlab::Git::RemoteMirror do ...@@ -7,16 +7,29 @@ RSpec.describe Gitlab::Git::RemoteMirror do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:ref_name) { 'foo' } let(:ref_name) { 'foo' }
let(:url) { 'https://example.com' }
let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true } } let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true } }
subject(:remote_mirror) { described_class.new(repository, ref_name, **options) } subject(:remote_mirror) { described_class.new(repository, ref_name, url, **options) }
it 'delegates to the Gitaly client' do shared_examples 'an update' do
expect(repository.gitaly_remote_client) it 'delegates to the Gitaly client' do
.to receive(:update_remote_mirror) expect(repository.gitaly_remote_client)
.with(ref_name, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true) .to receive(:update_remote_mirror)
.with(ref_name, url, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true)
remote_mirror.update # rubocop:disable Rails/SaveBang
end
end
context 'with url' do
it_behaves_like 'an update'
end
context 'without url' do
let(:url) { nil }
remote_mirror.update # rubocop:disable Rails/SaveBang it_behaves_like 'an update'
end end
it 'wraps gitaly errors' do it 'wraps gitaly errors' do
......
...@@ -67,13 +67,29 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do ...@@ -67,13 +67,29 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
let(:ssh_key) { 'KEY' } let(:ssh_key) { 'KEY' }
let(:known_hosts) { 'KNOWN HOSTS' } let(:known_hosts) { 'KNOWN HOSTS' }
it 'sends an update_remote_mirror message' do shared_examples 'an update' do
expect_any_instance_of(Gitaly::RemoteService::Stub) it 'sends an update_remote_mirror message' do
.to receive(:update_remote_mirror) expect_any_instance_of(Gitaly::RemoteService::Stub)
.with(kind_of(Enumerator), kind_of(Hash)) .to receive(:update_remote_mirror)
.and_return(double(:update_remote_mirror_response)) .with(array_including(gitaly_request_with_params(expected_params)), kind_of(Hash))
.and_return(double(:update_remote_mirror_response))
client.update_remote_mirror(ref_name, url, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true)
end
end
context 'with remote name' do
let(:url) { nil }
let(:expected_params) { { ref_name: ref_name } }
it_behaves_like 'an update'
end
context 'with remote URL' do
let(:url) { 'http:://git.example.com/my-repo.git' }
let(:expected_params) { { remote: Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: url) } }
client.update_remote_mirror(ref_name, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true) it_behaves_like 'an update'
end end
end end
......
...@@ -157,19 +157,34 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -157,19 +157,34 @@ RSpec.describe RemoteMirror, :mailer do
end end
describe '#update_repository' do describe '#update_repository' do
it 'performs update including options' do shared_examples 'an update' do
git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) it 'performs update including options' do
mirror = build(:remote_mirror) git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy)
mirror = build(:remote_mirror)
expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true)
mirror.update_repository expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true)
mirror.update_repository(inmemory_remote: inmemory)
expect(git_remote_mirror).to have_received(:new).with(
mirror.project.repository.raw, expect(git_remote_mirror).to have_received(:new).with(
mirror.remote_name, mirror.project.repository.raw,
keep_divergent_refs: true mirror.remote_name,
) inmemory ? mirror.url : nil,
expect(git_remote_mirror).to have_received(:update) keep_divergent_refs: true
)
expect(git_remote_mirror).to have_received(:update)
end
end
context 'with inmemory remote' do
let(:inmemory) { true }
it_behaves_like 'an update'
end
context 'with on-disk remote' do
let(:inmemory) { false }
it_behaves_like 'an update'
end end
end end
......
...@@ -13,21 +13,36 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -13,21 +13,36 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
describe '#execute' do describe '#execute' do
let(:retries) { 0 } let(:retries) { 0 }
let(:inmemory) { true }
subject(:execute!) { service.execute(remote_mirror, retries) } subject(:execute!) { service.execute(remote_mirror, retries) }
before do before do
stub_feature_flags(update_remote_mirror_inmemory: inmemory)
project.repository.add_branch(project.owner, 'existing-branch', 'master') project.repository.add_branch(project.owner, 'existing-branch', 'master')
allow(remote_mirror) allow(remote_mirror)
.to receive(:update_repository) .to receive(:update_repository)
.with(inmemory_remote: inmemory)
.and_return(double(divergent_refs: [])) .and_return(double(divergent_refs: []))
end end
it 'ensures the remote exists' do context 'with in-memory remote disabled' do
expect(remote_mirror).to receive(:ensure_remote!) let(:inmemory) { false }
execute! it 'ensures the remote exists' do
expect(remote_mirror).to receive(:ensure_remote!)
execute!
end
end
context 'with in-memory remote enabled' do
it 'does not ensure the remote exists' do
expect(remote_mirror).not_to receive(:ensure_remote!)
execute!
end
end end
it 'does not fetch the remote repository' do it 'does not fetch the remote repository' do
......
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