From 007db85dd4e4c92e060160427429c4fb2ad5cb32 Mon Sep 17 00:00:00 2001 From: Markus Doits Date: Sat, 15 Sep 2018 14:02:43 +0200 Subject: [PATCH] add `retry_failure?` option and use it to decide if to retry a build failure --- app/models/ci/build.rb | 17 +++- spec/models/ci/build_spec.rb | 150 +++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 3 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b133c756fcf..592b784807c 100644 --- a/app/models/ci/build.rb +++ b/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 diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index ba1de3612ad..3085cf7b271 100644 --- a/spec/models/ci/build_spec.rb +++ b/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