Commit 8be71768 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'rs-remove-ls-remote-flag' into 'master'

Remove gitaly_ruby_remote_branches_ls_remote flag

See merge request gitlab-org/gitlab!37760
parents 539d69fc e0fbc005
...@@ -25,14 +25,8 @@ module Projects ...@@ -25,14 +25,8 @@ module Projects
def update_mirror(remote_mirror) def update_mirror(remote_mirror)
remote_mirror.update_start! remote_mirror.update_start!
remote_mirror.ensure_remote! remote_mirror.ensure_remote!
# https://gitlab.com/gitlab-org/gitaly/-/issues/2670
if Feature.disabled?(:gitaly_ruby_remote_branches_ls_remote, default_enabled: true)
repository.fetch_remote(remote_mirror.remote_name, ssh_auth: remote_mirror, no_tags: true)
end
response = remote_mirror.update_repository response = remote_mirror.update_repository
if response.divergent_refs.any? if response.divergent_refs.any?
......
...@@ -10,10 +10,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -10,10 +10,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
subject(:service) { described_class.new(project, project.creator) } subject(:service) { described_class.new(project, project.creator) }
before do
stub_feature_flags(gitaly_ruby_remote_branches_ls_remote: false)
end
describe '#execute' do describe '#execute' do
subject(:execute!) { service.execute(remote_mirror, 0) } subject(:execute!) { service.execute(remote_mirror, 0) }
...@@ -26,17 +22,14 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -26,17 +22,14 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
end end
it 'ensures the remote exists' do it 'ensures the remote exists' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
expect(remote_mirror).to receive(:ensure_remote!) expect(remote_mirror).to receive(:ensure_remote!)
execute! execute!
end end
it 'fetches the remote repository' do it 'does not fetch the remote repository' do
expect(project.repository) # See https://gitlab.com/gitlab-org/gitaly/-/issues/2670
.to receive(:fetch_remote) expect(project.repository).not_to receive(:fetch_remote)
.with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror)
execute! execute!
end end
...@@ -48,8 +41,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -48,8 +41,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
end end
it 'marks the mirror as successfully finished' do it 'marks the mirror as successfully finished' do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
result = execute! result = execute!
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
...@@ -57,7 +48,7 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -57,7 +48,7 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
end end
it 'marks the mirror as failed and raises the error when an unexpected error occurs' do it 'marks the mirror as failed and raises the error when an unexpected error occurs' do
allow(project.repository).to receive(:fetch_remote).and_raise('Badly broken') allow(remote_mirror).to receive(:update_repository).and_raise('Badly broken')
expect { execute! }.to raise_error(/Badly broken/) expect { execute! }.to raise_error(/Badly broken/)
...@@ -67,33 +58,30 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -67,33 +58,30 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
context 'when the update fails because of a `Gitlab::Git::CommandError`' do context 'when the update fails because of a `Gitlab::Git::CommandError`' do
before do before do
allow(project.repository).to receive(:fetch_remote).and_raise(Gitlab::Git::CommandError.new('fetch failed')) allow(remote_mirror).to receive(:update_repository)
.and_raise(Gitlab::Git::CommandError.new('update failed'))
end end
it 'wraps `Gitlab::Git::CommandError`s in a service error' do it 'wraps `Gitlab::Git::CommandError`s in a service error' do
expect(execute!).to eq(status: :error, message: 'fetch failed') expect(execute!).to eq(status: :error, message: 'update failed')
end end
it 'marks the mirror as to be retried' do it 'marks the mirror as to be retried' do
execute! execute!
expect(remote_mirror).to be_to_retry expect(remote_mirror).to be_to_retry
expect(remote_mirror.last_error).to include('fetch failed') expect(remote_mirror.last_error).to include('update failed')
end end
it "marks the mirror as failed after #{described_class::MAX_TRIES} tries" do it "marks the mirror as failed after #{described_class::MAX_TRIES} tries" do
service.execute(remote_mirror, described_class::MAX_TRIES) service.execute(remote_mirror, described_class::MAX_TRIES)
expect(remote_mirror).to be_failed expect(remote_mirror).to be_failed
expect(remote_mirror.last_error).to include('fetch failed') expect(remote_mirror.last_error).to include('update failed')
end end
end end
context 'when there are divergent refs' do context 'when there are divergent refs' do
before do
stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror)
end
it 'marks the mirror as failed and sets an error message' do it 'marks the mirror as failed and sets an error message' do
response = double(divergent_refs: %w[refs/heads/master refs/heads/develop]) response = double(divergent_refs: %w[refs/heads/master refs/heads/develop])
expect(remote_mirror).to receive(:update_repository).and_return(response) expect(remote_mirror).to receive(:update_repository).and_return(response)
...@@ -106,37 +94,5 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -106,37 +94,5 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
expect(remote_mirror.last_error).to include("refs/heads/develop") expect(remote_mirror.last_error).to include("refs/heads/develop")
end end
end end
# https://gitlab.com/gitlab-org/gitaly/-/issues/2670
context 'when `gitaly_ruby_remote_branches_ls_remote` is enabled' do
before do
stub_feature_flags(gitaly_ruby_remote_branches_ls_remote: true)
end
it 'does not perform a fetch' do
expect(project.repository).not_to receive(:fetch_remote)
execute!
end
end
end
def stub_fetch_remote(project, remote_name:, ssh_auth:)
allow(project.repository)
.to receive(:fetch_remote)
.with(remote_name, no_tags: true, ssh_auth: ssh_auth) { fetch_remote(project.repository, remote_name) }
end
def fetch_remote(repository, remote_name)
local_branch_names(repository).each do |branch|
commit = repository.commit(branch)
repository.write_ref("refs/remotes/#{remote_name}/#{branch}", commit.id) if commit
end
end
def local_branch_names(repository)
branch_names = repository.branches.map(&:name)
# we want the protected branch to be pushed first
branch_names.unshift(branch_names.delete('master'))
end 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