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.

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.
8379bf63 · Fabio Pitino · 7c9014c6 · 8379bf63 · 8379bf63 · 8379bf63
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -425,7 +425,9 @@ def depends_on_builds
    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)

--- 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 @@ class BuildMetadata < ApplicationRecord
    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 @@ def set_build_project
      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


--- 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

--- a/spec/lib/gitlab/ci/build/step_spec.rb
+++ b/spec/lib/gitlab/ci/build/step_spec.rb
@@ -39,7 +39,7 @@
      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 @@
        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

--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -2027,12 +2027,34 @@ def create_mr(build, pipeline, factory: :merge_request, created_at: Time.now)
    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