Commit fef014d9 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'pks-drop-on-disk-remotes' into 'master'

Drop infrastructure for on-disk remotes [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!66513
parents db931a94 af514115
...@@ -25,11 +25,8 @@ class RemoteMirror < ApplicationRecord ...@@ -25,11 +25,8 @@ class RemoteMirror < ApplicationRecord
before_save :set_new_remote_name, if: :mirror_url_changed? before_save :set_new_remote_name, if: :mirror_url_changed?
after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available } after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available }
after_save :refresh_remote, if: :saved_change_to_mirror_url?
after_update :reset_fields, if: :saved_change_to_mirror_url? after_update :reset_fields, if: :saved_change_to_mirror_url?
after_commit :remove_remote, on: :destroy
before_validation :store_credentials before_validation :store_credentials
scope :enabled, -> { where(enabled: true) } scope :enabled, -> { where(enabled: true) }
...@@ -100,11 +97,11 @@ class RemoteMirror < ApplicationRecord ...@@ -100,11 +97,11 @@ class RemoteMirror < ApplicationRecord
update_status == 'started' update_status == 'started'
end end
def update_repository(inmemory_remote:) def update_repository
Gitlab::Git::RemoteMirror.new( Gitlab::Git::RemoteMirror.new(
project.repository.raw, project.repository.raw,
remote_name, remote_name,
inmemory_remote ? remote_url : nil, remote_url,
**options_for_update **options_for_update
).update ).update
end end
...@@ -227,15 +224,6 @@ class RemoteMirror < ApplicationRecord ...@@ -227,15 +224,6 @@ class RemoteMirror < ApplicationRecord
Gitlab::UrlSanitizer.new(read_attribute(:url)).full_url Gitlab::UrlSanitizer.new(read_attribute(:url)).full_url
end end
def ensure_remote!
return unless project
return unless remote_name && remote_url
# If this fails or the remote already exists, we won't know due to
# https://gitlab.com/gitlab-org/gitaly/issues/1317
project.repository.add_remote(remote_name, remote_url)
end
def after_sent_notification def after_sent_notification
update_column(:error_notification_sent, true) update_column(:error_notification_sent, true)
end end
...@@ -312,25 +300,6 @@ class RemoteMirror < ApplicationRecord ...@@ -312,25 +300,6 @@ class RemoteMirror < ApplicationRecord
self.remote_name = "remote_mirror_#{SecureRandom.hex}" self.remote_name = "remote_mirror_#{SecureRandom.hex}"
end end
def refresh_remote
return unless project
# Before adding a new remote we have to delete the data from
# the previous remote name
prev_remote_name = remote_name_before_last_save || fallback_remote_name
run_after_commit do
project.repository.async_remove_remote(prev_remote_name)
end
project.repository.add_remote(remote_name, remote_url)
end
def remove_remote
return unless project # could be pending to delete so don't need to touch the git repository
project.repository.async_remove_remote(remote_name)
end
def mirror_url_changed? def mirror_url_changed?
url_changed? || attribute_changed?(:credentials) url_changed? || attribute_changed?(:credentials)
end end
......
...@@ -939,32 +939,7 @@ class Repository ...@@ -939,32 +939,7 @@ class Repository
end end
def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true) def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true)
return fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune) if Feature.enabled?(:fetch_remote_params, project, default_enabled: :yaml) fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune)
unless remote_name
remote_name = "tmp-#{SecureRandom.hex}"
tmp_remote_name = true
end
add_remote(remote_name, url, mirror_refmap: refmap)
fetch_remote(remote_name, forced: forced, prune: prune)
ensure
async_remove_remote(remote_name) if tmp_remote_name
end
def async_remove_remote(remote_name)
return unless remote_name
return unless project
job_id = RepositoryRemoveRemoteWorker.perform_async(project.id, remote_name)
if job_id
Gitlab::AppLogger.info("Remove remote job scheduled for #{project.id} with remote name: #{remote_name} job ID #{job_id}.")
else
Gitlab::AppLogger.info("Remove remote job failed to create for #{project.id} with remote name #{remote_name}.")
end
job_id
end end
def fetch_source_branch!(source_repository, source_branch, local_ref) def fetch_source_branch!(source_repository, source_branch, local_ref)
......
...@@ -43,12 +43,7 @@ module Projects ...@@ -43,12 +43,7 @@ module Projects
# 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 = if Feature.enabled?(:update_remote_mirror_inmemory, project, default_enabled: :yaml) response = remote_mirror.update_repository
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:"
......
...@@ -16,22 +16,13 @@ class RepositoryRemoveRemoteWorker # rubocop:disable Scalability/IdempotentWorke ...@@ -16,22 +16,13 @@ class RepositoryRemoveRemoteWorker # rubocop:disable Scalability/IdempotentWorke
attr_reader :project, :remote_name attr_reader :project, :remote_name
def perform(project_id, remote_name) def perform(project_id, remote_name)
@remote_name = remote_name # On-disk remotes are slated for removal, and GitLab doesn't create any of
@project = Project.find_by_id(project_id) # them anymore. For backwards compatibility, we need to keep the worker
# though such that we can be sure to drain all jobs on an update. Making
return unless @project # this a no-op is fine though: the worst that can happen is that we still
# have old remotes lingering in the repository's config, but Gitaly will
logger.info("Removing remote #{remote_name} from project #{project.id}") # start to clean these up in repository maintenance.
# https://gitlab.com/gitlab-org/gitlab/-/issues/336745
try_obtain_lease do
remove_remote = @project.repository.remove_remote(remote_name)
if remove_remote
logger.info("Remote #{remote_name} was successfully removed from project #{project.id}")
else
logger.error("Could not remove remote #{remote_name} from project #{project.id}")
end
end
end end
def lease_timeout def lease_timeout
......
---
name: fetch_remote_params
introduced_by_url:
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325528
milestone: '13.12'
type: development
group: group::gitaly
default_enabled: true
---
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: true
...@@ -29,9 +29,6 @@ module EE ...@@ -29,9 +29,6 @@ module EE
before_save :set_override_pull_mirror_available, unless: -> { ::Gitlab::CurrentSettings.mirror_available } before_save :set_override_pull_mirror_available, unless: -> { ::Gitlab::CurrentSettings.mirror_available }
before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state } before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state }
after_update :remove_mirror_repository_reference,
if: ->(project) { project.mirror? && project.import_url_updated? }
after_create :create_security_setting, unless: :security_setting after_create :create_security_setting, unless: :security_setting
belongs_to :mirror_user, class_name: 'User' belongs_to :mirror_user, class_name: 'User'
...@@ -569,12 +566,6 @@ module EE ...@@ -569,12 +566,6 @@ module EE
saved_change_to_import_url? && saved_changes['import_url'].first saved_change_to_import_url? && saved_changes['import_url'].first
end end
def remove_mirror_repository_reference
run_after_commit do
repository.async_remove_remote(::Repository::MIRROR_REMOTE)
end
end
def username_only_import_url def username_only_import_url
bare_url = read_attribute(:import_url) bare_url = read_attribute(:import_url)
return bare_url unless ::Gitlab::UrlSanitizer.valid?(bare_url) return bare_url unless ::Gitlab::UrlSanitizer.valid?(bare_url)
......
...@@ -34,20 +34,14 @@ module EE ...@@ -34,20 +34,14 @@ module EE
end end
def fetch_upstream(url, forced: false, check_tags_changed: false) def fetch_upstream(url, forced: false, check_tags_changed: false)
if ::Feature.enabled?(:fetch_remote_params, project, default_enabled: :yaml) fetch_remote(
return fetch_remote( MIRROR_REMOTE,
MIRROR_REMOTE, url: url,
url: url, refmap: ["+refs/heads/*:refs/remotes/#{MIRROR_REMOTE}/*"],
refmap: ["+refs/heads/*:refs/remotes/#{MIRROR_REMOTE}/*"], ssh_auth: project&.import_data,
ssh_auth: project&.import_data, forced: forced,
forced: forced, check_tags_changed: check_tags_changed
check_tags_changed: check_tags_changed )
)
end
add_remote(MIRROR_REMOTE, url)
fetch_remote(MIRROR_REMOTE, ssh_auth: project&.import_data, forced: forced, check_tags_changed: check_tags_changed)
end end
def upstream_branches def upstream_branches
......
...@@ -1064,7 +1064,7 @@ RSpec.describe Project do ...@@ -1064,7 +1064,7 @@ RSpec.describe Project do
it 'removes previous remote' do it 'removes previous remote' do
project = create(:project, :repository, :mirror) project = create(:project, :repository, :mirror)
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(project.id, ::Repository::MIRROR_REMOTE).and_call_original expect(RepositoryRemoveRemoteWorker).not_to receive(:perform_async)
project.update(import_url: "http://test.com") project.update(import_url: "http://test.com")
end end
......
...@@ -52,35 +52,13 @@ RSpec.describe Repository do ...@@ -52,35 +52,13 @@ RSpec.describe Repository do
describe '#fetch_upstream' do describe '#fetch_upstream' do
let(:url) { "http://example.com" } let(:url) { "http://example.com" }
context 'when :fetch_remote_params is enabled' do it 'fetches the URL without creating a remote' do
it 'fetches the URL without creating a remote' do expect(repository)
expect(repository).not_to receive(:add_remote) .to receive(:fetch_remote)
expect(repository) .with(described_class::MIRROR_REMOTE, url: url, refmap: ['+refs/heads/*:refs/remotes/upstream/*'], ssh_auth: nil, forced: true, check_tags_changed: true)
.to receive(:fetch_remote) .and_return(nil)
.with(described_class::MIRROR_REMOTE, url: url, refmap: ['+refs/heads/*:refs/remotes/upstream/*'], ssh_auth: nil, forced: true, check_tags_changed: true)
.and_return(nil)
repository.fetch_upstream(url, forced: true, check_tags_changed: true)
end
end
context 'when :fetch_remote_params is disabled' do
before do
stub_feature_flags(fetch_remote_params: false)
end
it 'creates a remote and fetches it' do repository.fetch_upstream(url, forced: true, check_tags_changed: true)
expect(repository)
.to receive(:add_remote)
.with(described_class::MIRROR_REMOTE, url)
.and_return(nil)
expect(repository)
.to receive(:fetch_remote)
.with(described_class::MIRROR_REMOTE, ssh_auth: nil, forced: true, check_tags_changed: true)
.and_return(nil)
repository.fetch_upstream(url, forced: true, check_tags_changed: true)
end
end end
end end
......
...@@ -703,19 +703,6 @@ module Gitlab ...@@ -703,19 +703,6 @@ module Gitlab
write_ref(ref, start_point) write_ref(ref, start_point)
end end
# If `mirror_refmap` is present the remote is set as mirror with that mapping
def add_remote(remote_name, url, mirror_refmap: nil)
wrapped_gitaly_errors do
gitaly_remote_client.add_remote(remote_name, url, mirror_refmap)
end
end
def remove_remote(remote_name)
wrapped_gitaly_errors do
gitaly_remote_client.remove_remote(remote_name)
end
end
def find_remote_root_ref(remote_name, remote_url, authorization = nil) def find_remote_root_ref(remote_name, remote_url, authorization = nil)
return unless remote_name.present? && remote_url.present? return unless remote_name.present? && remote_url.present?
......
...@@ -26,23 +26,6 @@ module Gitlab ...@@ -26,23 +26,6 @@ module Gitlab
@storage = repository.storage @storage = repository.storage
end end
def add_remote(name, url, mirror_refmaps)
request = Gitaly::AddRemoteRequest.new(
repository: @gitaly_repo,
name: name,
url: url,
mirror_refmaps: Array.wrap(mirror_refmaps).map(&:to_s)
)
GitalyClient.call(@storage, :remote_service, :add_remote, request, timeout: GitalyClient.fast_timeout)
end
def remove_remote(name)
request = Gitaly::RemoveRemoteRequest.new(repository: @gitaly_repo, name: name)
GitalyClient.call(@storage, :remote_service, :remove_remote, request, timeout: GitalyClient.long_timeout).result
end
# The remote_name parameter is deprecated and will be removed soon. # The remote_name parameter is deprecated and will be removed soon.
def find_remote_root_ref(remote_name, remote_url, authorization) def find_remote_root_ref(remote_name, remote_url, authorization)
request = Gitaly::FindRemoteRootRefRequest.new(repository: @gitaly_repo, request = Gitaly::FindRemoteRootRefRequest.new(repository: @gitaly_repo,
......
...@@ -40,11 +40,7 @@ module Gitlab ...@@ -40,11 +40,7 @@ module Gitlab
# updating the timestamp. # updating the timestamp.
project.update_column(:last_repository_updated_at, Time.zone.now) project.update_column(:last_repository_updated_at, Time.zone.now)
if Feature.enabled?(:fetch_remote_params, project, default_enabled: :yaml) project.repository.fetch_remote('github', url: project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false)
project.repository.fetch_remote('github', url: project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false)
else
project.repository.fetch_remote('github', forced: false)
end
pname = project.path_with_namespace pname = project.path_with_namespace
......
...@@ -30,7 +30,6 @@ RSpec.describe 'factories' do ...@@ -30,7 +30,6 @@ RSpec.describe 'factories' do
[:pages_domain, :with_trusted_expired_chain], [:pages_domain, :with_trusted_expired_chain],
[:pages_domain, :explicit_ecdsa], [:pages_domain, :explicit_ecdsa],
[:project_member, :blocked], [:project_member, :blocked],
[:project, :remote_mirror],
[:remote_mirror, :ssh], [:remote_mirror, :ssh],
[:user_preference, :only_comments], [:user_preference, :only_comments],
[:ci_pipeline_artifact, :remote_store] [:ci_pipeline_artifact, :remote_store]
......
...@@ -2001,47 +2001,6 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do ...@@ -2001,47 +2001,6 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
end end
end end
describe 'remotes' do
let(:repository) { mutable_repository }
let(:remote_name) { 'my-remote' }
let(:url) { 'http://my-repo.git' }
after do
ensure_seeds
end
describe '#add_remote' do
let(:mirror_refmap) { '+refs/*:refs/*' }
it 'added the remote' do
begin
repository_rugged.remotes.delete(remote_name)
rescue Rugged::ConfigError
end
repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap)
expect(repository_rugged.remotes[remote_name]).not_to be_nil
expect(repository_rugged.config["remote.#{remote_name}.mirror"]).to eq('true')
expect(repository_rugged.config["remote.#{remote_name}.prune"]).to eq('true')
expect(repository_rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap)
end
end
describe '#remove_remote' do
it 'removes the remote' do
repository_rugged.remotes.create(remote_name, url)
expect(repository.remove_remote(remote_name)).to be true
# Since we deleted the remote via Gitaly, Rugged doesn't know
# this changed underneath it. Let's refresh the Rugged repo.
repository_rugged = Rugged::Repository.new(repository_path)
expect(repository_rugged.remotes[remote_name]).to be_nil
end
end
end
describe '#bundle_to_disk' do describe '#bundle_to_disk' do
let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") }
......
...@@ -9,31 +9,6 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do ...@@ -9,31 +9,6 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
let(:remote_name) { 'my-remote' } let(:remote_name) { 'my-remote' }
let(:client) { described_class.new(project.repository) } let(:client) { described_class.new(project.repository) }
describe '#add_remote' do
let(:url) { 'http://my-repo.git' }
let(:mirror_refmap) { :all_refs }
it 'sends an add_remote message' do
expect_any_instance_of(Gitaly::RemoteService::Stub)
.to receive(:add_remote)
.with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
.and_return(double(:add_remote_response))
client.add_remote(remote_name, url, mirror_refmap)
end
end
describe '#remove_remote' do
it 'sends an remove_remote message and returns the result value' do
expect_any_instance_of(Gitaly::RemoteService::Stub)
.to receive(:remove_remote)
.with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
.and_return(double(result: true))
expect(client.remove_remote(remote_name)).to be(true)
end
end
describe '#find_remote_root_ref' do describe '#find_remote_root_ref' do
let(:remote) { 'origin' } let(:remote) { 'origin' }
let(:url) { 'http://git.example.com/my-repo.git' } let(:url) { 'http://git.example.com/my-repo.git' }
......
...@@ -148,7 +148,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do ...@@ -148,7 +148,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do
end end
end end
shared_examples '#update_repository' do describe '#update_repository' do
it 'updates the repository' do it 'updates the repository' do
importer = described_class.new(project, client) importer = described_class.new(project, client)
...@@ -162,34 +162,16 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do ...@@ -162,34 +162,16 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do
.to receive(:increment) .to receive(:increment)
.and_call_original .and_call_original
freeze_time do
importer.update_repository
expect(project.last_repository_updated_at).to be_like_time(Time.zone.now)
end
end
end
describe '#update_repository with :fetch_remote_params enabled' do
before do
stub_feature_flags(fetch_remote_params: true)
expect(project.repository) expect(project.repository)
.to receive(:fetch_remote) .to receive(:fetch_remote)
.with('github', forced: false, url: url, refmap: Gitlab::GithubImport.refmap) .with('github', forced: false, url: url, refmap: Gitlab::GithubImport.refmap)
end
it_behaves_like '#update_repository' freeze_time do
end importer.update_repository
describe '#update_repository with :fetch_remote_params disabled' do expect(project.last_repository_updated_at).to be_like_time(Time.zone.now)
before do end
stub_feature_flags(fetch_remote_params: false)
expect(project.repository)
.to receive(:fetch_remote)
.with('github', forced: false)
end end
it_behaves_like '#update_repository'
end end
describe '#update_repository?' do describe '#update_repository?' do
......
...@@ -2914,10 +2914,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2914,10 +2914,6 @@ RSpec.describe Project, factory_default: :keep do
subject { project.has_remote_mirror? } subject { project.has_remote_mirror? }
before do
allow_any_instance_of(RemoteMirror).to receive(:refresh_remote)
end
it 'returns true when a remote mirror is enabled' do it 'returns true when a remote mirror is enabled' do
is_expected.to be_truthy is_expected.to be_truthy
end end
...@@ -2934,10 +2930,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2934,10 +2930,6 @@ RSpec.describe Project, factory_default: :keep do
delegate :update_remote_mirrors, to: :project delegate :update_remote_mirrors, to: :project
before do
allow_any_instance_of(RemoteMirror).to receive(:refresh_remote)
end
it 'syncs enabled remote mirror' do it 'syncs enabled remote mirror' do
expect_any_instance_of(RemoteMirror).to receive(:sync) expect_any_instance_of(RemoteMirror).to receive(:sync)
......
...@@ -93,22 +93,14 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -93,22 +93,14 @@ RSpec.describe RemoteMirror, :mailer do
expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' }) expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' })
end end
it 'updates the remote config if credentials changed' do it 'does not update the repository config if credentials changed' do
mirror = create_mirror(url: 'http://foo:bar@test.com') mirror = create_mirror(url: 'http://foo:bar@test.com')
repo = mirror.project.repository repo = mirror.project.repository
old_config = rugged_repo(repo).config
mirror.update_attribute(:url, 'http://foo:baz@test.com') mirror.update_attribute(:url, 'http://foo:baz@test.com')
config = rugged_repo(repo).config expect(rugged_repo(repo).config.to_hash).to eq(old_config.to_hash)
expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com')
end
it 'removes previous remote' do
mirror = create_mirror(url: 'http://foo:bar@test.com')
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original
mirror.update(url: 'http://test.com')
end end
end end
end end
...@@ -157,34 +149,20 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -157,34 +149,20 @@ RSpec.describe RemoteMirror, :mailer do
end end
describe '#update_repository' do describe '#update_repository' do
shared_examples 'an update' do it 'performs update including options' do
it 'performs update including options' do git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy)
git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) mirror = build(:remote_mirror)
mirror = build(:remote_mirror)
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,
mirror.remote_name,
inmemory ? mirror.url : nil,
keep_divergent_refs: true
)
expect(git_remote_mirror).to have_received(:update)
end
end
context 'with inmemory remote' do expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true)
let(:inmemory) { true } mirror.update_repository
it_behaves_like 'an update' expect(git_remote_mirror).to have_received(:new).with(
end mirror.project.repository.raw,
mirror.remote_name,
context 'with on-disk remote' do mirror.url,
let(:inmemory) { false } keep_divergent_refs: true
)
it_behaves_like 'an update' expect(git_remote_mirror).to have_received(:update)
end end
end end
...@@ -303,10 +281,10 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -303,10 +281,10 @@ RSpec.describe RemoteMirror, :mailer do
end end
context 'when remote mirror gets destroyed' do context 'when remote mirror gets destroyed' do
it 'removes remote' do it 'does not remove the remote' do
mirror = create_mirror(url: 'http://foo:bar@test.com') mirror = create_mirror(url: 'http://foo:bar@test.com')
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original expect(RepositoryRemoveRemoteWorker).not_to receive(:perform_async)
mirror.destroy! mirror.destroy!
end end
...@@ -402,30 +380,6 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -402,30 +380,6 @@ RSpec.describe RemoteMirror, :mailer do
end end
end end
describe '#ensure_remote!' do
let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first }
let(:project) { remote_mirror.project }
let(:repository) { project.repository }
it 'adds a remote multiple times with no errors' do
expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, remote_mirror.url).twice.and_call_original
2.times do
remote_mirror.ensure_remote!
end
end
context 'SSH public-key authentication' do
it 'omits the password from the URL' do
remote_mirror.update!(auth_method: 'ssh_public_key', url: 'ssh://git:pass@example.com')
expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, 'ssh://git@example.com')
remote_mirror.ensure_remote!
end
end
end
describe '#url=' do describe '#url=' do
let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first }
......
...@@ -1094,99 +1094,17 @@ RSpec.describe Repository do ...@@ -1094,99 +1094,17 @@ RSpec.describe Repository do
end end
end end
describe '#async_remove_remote' do
before do
masterrev = repository.find_branch('master').dereferenced_target
create_remote_branch('joe', 'remote_branch', masterrev)
end
context 'when worker is scheduled successfully' do
before do
masterrev = repository.find_branch('master').dereferenced_target
create_remote_branch('remote_name', 'remote_branch', masterrev)
allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return('1234')
end
it 'returns job_id' do
expect(repository.async_remove_remote('joe')).to eq('1234')
end
end
context 'when worker does not schedule successfully' do
before do
allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return(nil)
end
it 'returns nil' do
expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.")
expect(repository.async_remove_remote('joe')).to be_nil
end
end
end
describe '#fetch_as_mirror' do describe '#fetch_as_mirror' do
let(:url) { "http://example.com" } let(:url) { "http://example.com" }
let(:remote_name) { "remote-name" }
context 'when :fetch_remote_params is enabled' do it 'fetches the URL without creating a remote' do
let(:remote_name) { "remote-name" } expect(repository)
.to receive(:fetch_remote)
before do .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs)
stub_feature_flags(fetch_remote_params: true) .and_return(nil)
end
it 'fetches the URL without creating a remote' do
expect(repository).not_to receive(:add_remote)
expect(repository)
.to receive(:fetch_remote)
.with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs)
.and_return(nil)
repository.fetch_as_mirror(url, remote_name: remote_name)
end
end
context 'when :fetch_remote_params is disabled' do
before do
stub_feature_flags(fetch_remote_params: false)
end
shared_examples 'a fetch' do
it 'adds and fetches a remote' do
expect(repository)
.to receive(:add_remote)
.with(expected_remote, url, mirror_refmap: :all_refs)
.and_return(nil)
expect(repository)
.to receive(:fetch_remote)
.with(expected_remote, forced: false, prune: true)
.and_return(nil)
repository.fetch_as_mirror(url, remote_name: remote_name)
end
end
context 'with temporary remote' do
let(:remote_name) { nil }
let(:expected_remote_suffix) { "123456" }
let(:expected_remote) { "tmp-#{expected_remote_suffix}" }
before do
expect(repository)
.to receive(:async_remove_remote).with(expected_remote).and_return(nil)
allow(SecureRandom).to receive(:hex).and_return(expected_remote_suffix)
end
it_behaves_like 'a fetch'
end
context 'with remote name' do
let(:remote_name) { "foo" }
let(:expected_remote) { "foo" }
it_behaves_like 'a fetch' repository.fetch_as_mirror(url, remote_name: remote_name)
end
end end
end end
......
...@@ -3368,10 +3368,6 @@ RSpec.describe NotificationService, :mailer do ...@@ -3368,10 +3368,6 @@ RSpec.describe NotificationService, :mailer do
project.add_maintainer(u_maintainer2) project.add_maintainer(u_maintainer2)
project.add_developer(u_developer) project.add_developer(u_developer)
# Mock remote update
allow(project.repository).to receive(:async_remove_remote)
allow(project.repository).to receive(:add_remote)
reset_delivered_emails! reset_delivered_emails!
end end
......
...@@ -13,38 +13,17 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -13,38 +13,17 @@ 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
context 'with in-memory remote disabled' do
let(:inmemory) { false }
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
it 'does not fetch the remote repository' do it 'does not fetch the remote repository' do
# See https://gitlab.com/gitlab-org/gitaly/-/issues/2670 # See https://gitlab.com/gitlab-org/gitaly/-/issues/2670
expect(project.repository).not_to receive(:fetch_remote) expect(project.repository).not_to receive(:fetch_remote)
......
...@@ -24,37 +24,25 @@ RSpec.describe RepositoryRemoveRemoteWorker do ...@@ -24,37 +24,25 @@ RSpec.describe RepositoryRemoveRemoteWorker do
.and_return(project) .and_return(project)
end end
it 'does not remove remote when cannot obtain lease' do it 'does nothing when cannot obtain lease' do
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
expect(project.repository) expect(project.repository)
.not_to receive(:remove_remote) .not_to receive(:remove_remote)
expect(subject) expect(subject)
.to receive(:log_error) .not_to receive(:log_error)
.with("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.")
subject.perform(project.id, remote_name) subject.perform(project.id, remote_name)
end end
it 'removes remote from repository when obtain a lease' do it 'does nothing when obtain a lease' do
stub_exclusive_lease(lease_key, timeout: lease_timeout) stub_exclusive_lease(lease_key, timeout: lease_timeout)
masterrev = project.repository.find_branch('master').dereferenced_target
create_remote_branch(remote_name, 'remote_branch', masterrev)
expect(project.repository) expect(project.repository)
.to receive(:remove_remote) .not_to receive(:remove_remote)
.with(remote_name)
.and_call_original
subject.perform(project.id, remote_name) subject.perform(project.id, remote_name)
end end
end end
end end
def create_remote_branch(remote_name, branch_name, target)
rugged = rugged_repo(project.repository)
rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)
end
end end
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