Commit 96ecbe62 authored by Patrick Steinhardt's avatar Patrick Steinhardt

remote_mirror: Drop refreshing and removal of on-disk remotes

We don't use on-disk remotes anymore, and as a result we don't need to
update any on-disk remotes either. Drop refreshes and removal of such
remotes in the remote mirror model.

This change also lets us un-skip the remote_mirror trait.
parent 670708b7
...@@ -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) }
...@@ -303,25 +300,6 @@ class RemoteMirror < ApplicationRecord ...@@ -303,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
......
...@@ -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]
......
...@@ -2912,10 +2912,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2912,10 +2912,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
...@@ -2932,10 +2928,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2932,10 +2928,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
...@@ -289,10 +281,10 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -289,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
......
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