add `retry_failure?` option and use it to decide if to retry a build failure

app/models/ci/build.rb

@@ -220,9 +220,7 @@ module Ci
 
         build.deployment&.drop
 
-        next if build.retries_max.zero?
-
-        if build.retries_count < build.retries_max
+        if build.retry_failure?
           begin
             Ci::Build.retry(build, build.user)
           rescue Gitlab::Access::AccessDeniedError => ex
@@ -329,6 +327,19 @@ module Ci
       retries.is_a?(Hash) && retries.fetch(:when, 'always').to_s || 'always'
     end
 
+    def retry_failure?
+      return false if retries_max.zero? || retries_count >= retries_max
+
+      case failure_reason.to_s
+      when 'runner_system_failure'
+        %['always', 'system failure'].include?(retry_when)
+      when 'script_failure'
+        %['always', 'script failure'].include?(retry_when)
+      else
+        retry_when == 'always'
+      end
+    end
+
     def latest?
       !retried?
     end

spec/models/ci/build_spec.rb

@@ -1538,6 +1538,156 @@ describe Ci::Build do
         end
       end
     end
+
+    describe '#retry_failure?' do
+      subject { create(:ci_build) }
+
+      context 'when retries max is zero' do
+        before do
+          expect(subject).to receive(:retries_max).at_least(:once).and_return(0)
+        end
+
+        it 'returns false' do
+          expect(subject.retry_failure?).to eq false
+        end
+      end
+
+      context 'when retries max equals retries count' do
+        before do
+          expect(subject).to receive(:retries_max).at_least(:once).and_return(1)
+          expect(subject).to receive(:retries_count).at_least(:once).and_return(1)
+        end
+
+        it 'returns false' do
+          expect(subject.retry_failure?).to eq false
+        end
+      end
+
+      context 'when retries max is higher than retries count' do
+        before do
+          expect(subject).to receive(:retries_max).at_least(:once).and_return(2)
+          expect(subject).to receive(:retries_count).at_least(:once).and_return(1)
+        end
+
+        context 'and retry when is always' do
+          before do
+            expect(subject).to receive(:retry_when).at_least(:once).and_return('always')
+          end
+
+          it 'returns true' do
+            expect(subject.retry_failure?).to eq true
+          end
+        end
+
+        context 'and failure was a system runner failure' do
+          before do
+            expect(subject).to receive(:failure_reason).at_least(:once).and_return('runner_system_failure')
+          end
+
+          context 'and retry when is always' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('always')
+            end
+
+            it 'returns true' do
+              expect(subject.retry_failure?).to eq true
+            end
+          end
+
+          context 'and retry when is system failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
+            end
+
+            it 'returns true' do
+              expect(subject.retry_failure?).to eq true
+            end
+          end
+
+          context 'and retry when is script failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
+            end
+
+            it 'returns false' do
+              expect(subject.retry_failure?).to eq false
+            end
+          end
+        end
+
+        context 'and failure was a script failure' do
+          before do
+            expect(subject).to receive(:failure_reason).at_least(:once).and_return('script_failure')
+          end
+
+          context 'and retry when is always' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('always')
+            end
+
+            it 'returns true' do
+              expect(subject.retry_failure?).to eq true
+            end
+          end
+
+          context 'and retry when is system failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
+            end
+
+            it 'returns false' do
+              expect(subject.retry_failure?).to eq false
+            end
+          end
+
+          context 'and retry when is script failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
+            end
+
+            it 'returns true' do
+              expect(subject.retry_failure?).to eq true
+            end
+          end
+        end
+
+        context 'and failure was some other failure' do
+          before do
+            expect(subject).to receive(:failure_reason).at_least(:once).and_return('some other reason')
+          end
+
+          context 'and retry when is always' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('always')
+            end
+
+            it 'returns true' do
+              expect(subject.retry_failure?).to eq true
+            end
+          end
+
+          context 'and retry when is system failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
+            end
+
+            it 'returns false' do
+              expect(subject.retry_failure?).to eq false
+            end
+          end
+
+          context 'and retry when is script failure' do
+            before do
+              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
+            end
+
+            it 'returns false' do
+              expect(subject.retry_failure?).to eq false
+            end
+          end
+        end
+      end
+    end
   end
 
   describe '#keep_artifacts!' do