From 8379bf636e1f6f7dc86791d1b51428021ac9a0d9 Mon Sep 17 00:00:00 2001 From: Fabio Pitino Date: Thu, 29 Aug 2019 17:19:35 +0200 Subject: Allow timeout to be specified as chronic duration It can either be specified as integer (seconds) or as human readable format such as 1m 30s. --- app/models/ci/build.rb | 4 +++- app/models/ci/build_metadata.rb | 16 +++++++++++----- doc/ci/yaml/README.md | 7 ++++++- spec/lib/gitlab/ci/build/step_spec.rb | 13 ++++++++++++- spec/models/ci/build_spec.rb | 30 ++++++++++++++++++++++++++---- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index aaecd571616..71c500ee011 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -425,7 +425,9 @@ module Ci end def timeout - options[:job_timeout] + strong_memoize(:timeout) do + ChronicDuration.parse(options[:job_timeout].to_s) if options[:job_timeout] + end end def triggered_by?(current_user) diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 2266d665994..244b67f7d4c 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -4,6 +4,8 @@ module Ci # The purpose of this class is to store Build related data that can be disposed. # Data that should be persisted forever, should be stored with Ci::Build model. class BuildMetadata < ApplicationRecord + BuildTimeout = Struct.new(:value, :source) + extend Gitlab::Ci::Model include Presentable include ChronicDurationAttribute @@ -33,9 +35,9 @@ module Ci def update_timeout_state return unless build.runner.present? || build.timeout.present? - timeout = [(job_timeout || project_timeout), runner_timeout].compact.min_by { |timeout| timeout[:value] } + timeout = timeout_with_highest_precedence - update(timeout: timeout[:value], timeout_source: timeout[:source]) + update(timeout: timeout.value, timeout_source: timeout.source) end private @@ -44,21 +46,25 @@ module Ci self.project_id ||= self.build.project_id end + def timeout_with_highest_precedence + [(job_timeout || project_timeout), runner_timeout].compact.min_by { |timeout| timeout.value } + end + def project_timeout strong_memoize(:project_timeout) do - { value: project&.build_timeout, source: :project_timeout_source } + BuildTimeout.new(project&.build_timeout, :project_timeout_source) end end def job_timeout strong_memoize(:job_timeout) do - { value: build.timeout, source: :job_timeout_source } if build.timeout + BuildTimeout.new(build.timeout, :job_timeout_source) if build.timeout end end def runner_timeout strong_memoize(:runner_timeout) do - { value: build.runner.maximum_timeout, source: :runner_timeout_source } if runner_timeout_set? + BuildTimeout.new(build.runner.maximum_timeout, :runner_timeout_source) if runner_timeout_set? end end diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 3908a31f142..aa982db0014 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1997,11 +1997,16 @@ Possible values for `when` are: ### timeout -`timeout` allows you to configure a timeout (in seconds) for a specific job. +`timeout` allows you to configure a timeout for a specific job. The value can be either +numeric (seconds) or human readable. A simple example: ```yaml +build: + script: build.sh + timeout: 3h 30m + test: script: rspec timeout: 1800 diff --git a/spec/lib/gitlab/ci/build/step_spec.rb b/spec/lib/gitlab/ci/build/step_spec.rb index 2ca47041860..07959732c4d 100644 --- a/spec/lib/gitlab/ci/build/step_spec.rb +++ b/spec/lib/gitlab/ci/build/step_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::Ci::Build::Step do it_behaves_like 'has correct script' end - context 'when timeout option is specified' do + context 'when timeout option is specified in seconds' do let(:job) { create(:ci_build, options: { job_timeout: 3, script: ["ls -la\necho aaa", 'date'] }) } let(:script) { ["ls -la\necho aaa", 'date'] } @@ -49,6 +49,17 @@ describe Gitlab::Ci::Build::Step do expect(subject.timeout).to eq(3) end end + + context 'when timeout option is specified as human readable format' do + let(:job) { create(:ci_build, options: { job_timeout: '2m 3s', script: ["ls -la\necho aaa", 'date'] }) } + let(:script) { ["ls -la\necho aaa", 'date'] } + + it_behaves_like 'has correct script' + + it 'has job level timeout' do + expect(subject.timeout).to eq(123) + end + end end describe '#from_after_script' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index df94451c3e9..8123fc8dcbe 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2027,12 +2027,34 @@ describe Ci::Build do end describe '#timeout' do - let(:build) do - create(:ci_build, pipeline: pipeline, options: { job_timeout: 3 }) + context 'when specified in seconds' do + let(:build) do + create(:ci_build, pipeline: pipeline, options: { job_timeout: 3 }) + end + + it 'delegates to serialized options as is' do + expect(build.timeout).to eq(3) + end end - it 'delegates to serialized options' do - expect(build.timeout).to eq(3) + context 'when specified as human readable time' do + let(:build) do + create(:ci_build, pipeline: pipeline, options: { job_timeout: '2m 3s' }) + end + + it 'converts the value in chronic duration' do + expect(build.timeout).to eq(123) + end + end + + context 'when not specified' do + let(:build) do + create(:ci_build, pipeline: pipeline, options: {}) + end + + it 'returns nil' do + expect(build.timeout).to be_nil + end end end -- cgit v1.2.3