Add push_remote_branches retryrs-mirror-with-retry

If the first push fails, we parse the output and try again with only the
branches that would have been successful, which should resolve the "one
failure blocks all mirroring" behavior we currently see.

This functionality is behind a `gitaly_ruby_push_mirror_retry` feature
flag.

3 files changed, 96 insertions, 7 deletions

diff --git a/changelogs/unreleased/rs-mirror-with-retry.yml b/changelogs/unreleased/rs-mirror-with-retry.yml
new file mode 100644
index 000000000..b15af9a81
--- /dev/null
+++ b/changelogs/unreleased/rs-mirror-with-retry.yml
@@ -0,0 +1,5 @@
+---
+title: If a push mirror fails, try again with the refs that would have succeeded
+merge_request: 1823
+author:
+type: added
diff --git a/ruby/lib/gitlab/git/repository_mirroring.rb b/ruby/lib/gitlab/git/repository_mirroring.rb
index 31d36f27a..a1d75251e 100644
--- a/ruby/lib/gitlab/git/repository_mirroring.rb
+++ b/ruby/lib/gitlab/git/repository_mirroring.rb
@@ -32,23 +32,43 @@ module Gitlab
       end
 
       def push_remote_branches(remote_name, branch_names, forced: true, env: {})
-        success = @gitlab_projects.push_branches(remote_name, GITLAB_PROJECTS_TIMEOUT, forced, branch_names, env: env)
+        success = @gitlab_projects.push_branches(
+          remote_name,
+          GITLAB_PROJECTS_TIMEOUT,
+          forced,
+          branch_names,
+          env: env
+        )
 
         begin
           success || gitlab_projects_error
         rescue CommandError => ex
           results = PushResults.new(ex.message)
 
-          accepted_branches = results.accepted_branches.join(', ')
-          rejected_branches = results.rejected_branches.join(', ')
+          accepted_branches = results.accepted_branches
+          rejected_branches = results.rejected_branches
 
           @gitlab_projects.logger.info(
             "Failed to push to remote #{remote_name}. " \
-            "Accepted: #{accepted_branches} / " \
-            "Rejected: #{rejected_branches}"
+            "Accepted: #{accepted_branches.join(', ')} / " \
+            "Rejected: #{rejected_branches.join(', ')}"
           )
 
-          raise ex
+          raise ex unless accepted_branches.any? &&
+            @gitlab_projects.features.enabled?(:push_mirror_retry)
+
+          @gitlab_projects.logger.info("Retrying failed push to #{remote_name} with limited branches: #{accepted_branches}")
+
+          # Try one more push without branches that failed
+          success = @gitlab_projects.push_branches(
+            remote_name,
+            GITLAB_PROJECTS_TIMEOUT,
+            forced,
+            accepted_branches,
+            env: env
+          )
+
+          success || gitlab_projects_error
         end
       end
 
diff --git a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
index af303975f..f958570bf 100644
--- a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
@@ -17,6 +17,11 @@ describe Gitlab::Git::RepositoryMirroring do
 
   describe '#push_remote_branches' do
     let(:projects_stub) { double.as_null_object }
+    let(:features) do
+      GitalyServer::FeatureFlags.new(
+        'gitaly-feature-ruby-push-mirror-retry' => 'true'
+      )
+    end
 
     subject(:repository) { FakeRepository.new(projects_stub) }
 
@@ -33,7 +38,12 @@ describe Gitlab::Git::RepositoryMirroring do
       end
     end
 
-    context 'with a failed push' do
+    context 'with a failed push and no retry' do
+      before do
+        allow(projects_stub).to receive(:features)
+          .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
@@ -60,5 +70,59 @@ describe Gitlab::Git::RepositoryMirroring do
         expect(logger).to have_received(:info).with(%r{Accepted: develop / Rejected: master})
       end
     end
+
+    context 'with a failed first push and failed retry push' do
+      before do
+        allow(projects_stub).to receive(:features).and_return(features)
+      end
+
+      it 'raises a `CommandError`' do
+        # First push with two branches fails
+        expect(projects_stub).to receive(:push_branches)
+          .with('remote_a', anything, true, %w[master develop], env: {})
+          .once
+          .and_return(false)
+
+        # Only one branch was accepted
+        push_results = double(accepted_branches: %w[develop]).as_null_object
+        stub_const('Gitlab::Git::PushResults', push_results)
+
+        # Retry push also fails, but we tried!
+        expect(projects_stub).to receive(:push_branches)
+          .with('remote_a', anything, true, %w[develop], env: {})
+          .once
+          .and_return(false)
+
+        allow(projects_stub).to receive(:output).and_return('output')
+
+        expect { repository.push_remote_branches('remote_a', %w[master develop]) }
+          .to raise_error(Gitlab::Git::CommandError, 'output')
+      end
+    end
+
+    context 'with a failed first push and a successful retry push' do
+      it 'returns true' do
+        # First push with two branches fails
+        expect(projects_stub).to receive(:push_branches)
+          .with('remote_a', anything, true, %w[master develop], env: {})
+          .once
+          .and_return(false)
+
+        # Only one branch was accepted
+        push_results = double(accepted_branches: %w[develop]).as_null_object
+        stub_const('Gitlab::Git::PushResults', push_results)
+
+        # Retry push succeeds
+        expect(projects_stub).to receive(:push_branches)
+          .with('remote_a', anything, true, %w[develop], env: {})
+          .once
+          .and_return(true)
+
+        expect(projects_stub).to receive(:output).once.and_return('first_output')
+
+        expect(repository.push_remote_branches('remote_a', %w[master develop]))
+          .to eq(true)
+      end
+    end
   end
 end