Retry push mirror with accepted refsrs-mirror-with-retry-retry

When the `gitaly-feature-ruby-push-mirror-retry` flag is enabled, a
failed push mirror will attempt a second push with only refs that were
accepted.

4 files changed, 83 insertions, 12 deletions

diff --git a/changelogs/unreleased/rs-mirror-with-retry-retry.yml b/changelogs/unreleased/rs-mirror-with-retry-retry.yml
new file mode 100644
index 000000000..20f5669b4
--- /dev/null
+++ b/changelogs/unreleased/rs-mirror-with-retry-retry.yml
@@ -0,0 +1,5 @@
+---
+title: Retry push mirror with accepted refs
+merge_request: 1899
+author:
+type: added
diff --git a/ruby/lib/gitlab/git/repository_mirroring.rb b/ruby/lib/gitlab/git/repository_mirroring.rb
index 4b226fef9..35fb0ea81 100644
--- a/ruby/lib/gitlab/git/repository_mirroring.rb
+++ b/ruby/lib/gitlab/git/repository_mirroring.rb
@@ -37,16 +37,20 @@ module Gitlab
         return true if success
 
         results = PushResults.new(@gitlab_projects.output)
+        accepted_refs = results.accepted_refs
+        rejected_refs = results.rejected_refs
+
+        if @gitlab_projects.feature_flags.enabled?(:push_mirror_retry) && accepted_refs.any?
+          @gitlab_projects.push_branches(remote_name, GITLAB_PROJECTS_TIMEOUT, forced, accepted_refs, env: env)
+        else
+          @gitlab_projects.logger.info(
+            "Failed to push to remote #{remote_name}. " \
+            "Accepted: #{accepted_refs.join(', ')} / " \
+            "Rejected: #{rejected_refs.join(', ')}"
+          )
+        end
 
-        accepted_refs = results.accepted_refs.join(', ')
-        rejected_refs = results.rejected_refs.join(', ')
-
-        @gitlab_projects.logger.info(
-          "Failed to push to remote #{remote_name}. " \
-          "Accepted: #{accepted_refs} / " \
-          "Rejected: #{rejected_refs}"
-        )
-
+        # NOTE: Even with retry, the initial push still failed, so error out
         gitlab_projects_error
       end
 
diff --git a/ruby/spec/lib/gitlab/git/push_results_spec.rb b/ruby/spec/lib/gitlab/git/push_results_spec.rb
index 4a2cd2e02..62205fff9 100644
--- a/ruby/spec/lib/gitlab/git/push_results_spec.rb
+++ b/ruby/spec/lib/gitlab/git/push_results_spec.rb
@@ -73,7 +73,7 @@ describe Gitlab::Git::PushResults do
   describe Gitlab::Git::PushResults::Result do
     describe '#ref_name' do
       it 'deletes only one prefix' do
-        # It's  valid (albeit insane) for a branch to be named `refs/tags/foo`
+        # It's valid (albeit insane) for a branch to be named `refs/tags/foo`
         ref_name = 'refs/heads/refs/tags/branch'
         result = described_class.new('!', ref_name, ref_name, '')
 
diff --git a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
index 927cb863e..37578bb13 100644
--- a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
@@ -33,7 +33,13 @@ describe Gitlab::Git::RepositoryMirroring do
       end
     end
 
-    context 'with a failed push' do
+    context 'with a failed push and disabled retries' do
+      before do
+        allow(projects_stub)
+          .to receive(:feature_flags)
+          .and_return(double(enabled?: false))
+      end
+
       it 'logs a list of branch results and raises CommandError' do
         output = "Oh no, push mirroring failed!"
         logger = spy
@@ -53,7 +59,7 @@ describe Gitlab::Git::RepositoryMirroring do
         # Push fails
         expect(projects_stub).to receive(:push_branches).and_return(false)
 
-        # The CommandError gets re-raised, matching existing behavior
+        # The CommandError gets raised, matching existing behavior
         expect { repository.push_remote_branches('remote_a', %w[master develop]) }
           .to raise_error(Gitlab::Git::CommandError, output)
 
@@ -61,5 +67,61 @@ describe Gitlab::Git::RepositoryMirroring do
         expect(logger).to have_received(:info).with(%r{Accepted: develop / Rejected: master})
       end
     end
+
+    context 'with a failed push and enabled retries' do
+      let(:feature_flags) do
+        GitalyServer::FeatureFlags.new('gitaly-feature-ruby-push-mirror-retry' => 'true')
+      end
+
+      before do
+        expect(projects_stub).to receive(:feature_flags).and_return(feature_flags)
+      end
+
+      it 'retries with accepted refs' do
+        output = "Oh no, push mirroring failed!"
+
+        # Once for parsing, once for the exception
+        allow(projects_stub).to receive(:output).and_return(output)
+
+        push_results = double(
+          accepted_refs: %w[develop],
+          rejected_refs: %w[master]
+        )
+        expect(Gitlab::Git::PushResults).to receive(:new)
+          .with(output)
+          .and_return(push_results)
+
+        # First push fails
+        expect(projects_stub)
+          .to receive(:push_branches)
+          .with('remote_a', anything, true, %w[master develop], env: {})
+          .and_return(false)
+
+        # Second push still fails, but we tried!
+        expect(projects_stub)
+          .to receive(:push_branches)
+          .with('remote_a', anything, true, %w[develop], env: {})
+          .and_return(false)
+
+        # The CommandError gets raised, matching existing behavior
+        expect { repository.push_remote_branches('remote_a', %w[master develop]) }
+          .to raise_error(Gitlab::Git::CommandError, output)
+      end
+
+      it 'does nothing with no accepted refs' do
+        push_results = double(
+          accepted_refs: [],
+          rejected_refs: %w[master develop]
+        )
+        expect(Gitlab::Git::PushResults).to receive(:new).and_return(push_results)
+
+        # Nothing was accepted, so we only push once
+        expect(projects_stub).to receive(:push_branches).once.and_return(false)
+
+        # The CommandError gets raised, matching existing behavior
+        expect { repository.push_remote_branches('remote_a', %w[master develop]) }
+          .to raise_error(Gitlab::Git::CommandError)
+      end
+    end
   end
 end