Merge branch 'add-per-runner-job-timeout' into 'master'

Add per runner job timeout

Closes #43426

See merge request gitlab-org/gitlab-ce!17221

11 files changed, 353 insertions, 12 deletions

diff --git a/spec/factories/ci/build_metadata.rb b/spec/factories/ci/build_metadata.rb
new file mode 100644
index 00000000000..66bbd977b88
--- /dev/null
+++ b/spec/factories/ci/build_metadata.rb
@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :ci_build_metadata, class: Ci::BuildMetadata do
+    build factory: :ci_build
+
+    after(:build) do |build_metadata, _|
+      build_metadata.project ||= build_metadata.build.project
+    end
+  end
+end
diff --git a/spec/javascripts/jobs/mock_data.js b/spec/javascripts/jobs/mock_data.js
index 43589d54be4..25ca8eb6c0b 100644
--- a/spec/javascripts/jobs/mock_data.js
+++ b/spec/javascripts/jobs/mock_data.js
@@ -115,6 +115,10 @@ export default {
       commit_path: '/root/ci-mock/commit/c58647773a6b5faf066d4ad6ff2c9fbba5f180f6',
     },
   },
+  metadata: {
+    timeout_human_readable: '1m 40s',
+    timeout_source: 'runner',
+  },
   merge_request: {
     iid: 2,
     path: '/root/ci-mock/merge_requests/2',
diff --git a/spec/javascripts/jobs/sidebar_detail_row_spec.js b/spec/javascripts/jobs/sidebar_detail_row_spec.js
index 3ac65709c4a..e6bfb0c4adc 100644
--- a/spec/javascripts/jobs/sidebar_detail_row_spec.js
+++ b/spec/javascripts/jobs/sidebar_detail_row_spec.js
@@ -37,4 +37,25 @@ describe('Sidebar detail row', () => {
       vm.$el.textContent.replace(/\s+/g, ' ').trim(),
     ).toEqual('this is the title: this is the value');
   });
+
+  describe('when helpUrl not provided', () => {
+    it('should not render help', () => {
+      expect(vm.$el.querySelector('.help-button')).toBeNull();
+    });
+  });
+
+  describe('when helpUrl provided', () => {
+    beforeEach(() => {
+      vm = new SidebarDetailRow({
+        propsData: {
+          helpUrl: 'help url',
+          value: 'foo',
+        },
+      }).$mount();
+    });
+
+    it('should render help', () => {
+      expect(vm.$el.querySelector('.help-button a').getAttribute('href')).toEqual('help url');
+    });
+  });
 });
diff --git a/spec/javascripts/jobs/sidebar_details_block_spec.js b/spec/javascripts/jobs/sidebar_details_block_spec.js
index 95532ef5382..602dae514b1 100644
--- a/spec/javascripts/jobs/sidebar_details_block_spec.js
+++ b/spec/javascripts/jobs/sidebar_details_block_spec.js
@@ -96,6 +96,12 @@ describe('Sidebar details block', () => {
       ).toEqual('Runner: #1');
     });
 
+    it('should render timeout information', () => {
+      expect(
+        trimWhitespace(vm.$el.querySelector('.js-job-timeout')),
+      ).toEqual('Timeout: 1m 40s (from runner)');
+    });
+
     it('should render coverage', () => {
       expect(
         trimWhitespace(vm.$el.querySelector('.js-job-coverage')),
diff --git a/spec/lib/gitlab/ci/build/step_spec.rb b/spec/lib/gitlab/ci/build/step_spec.rb
index 5a21282712a..cce4efaa069 100644
--- a/spec/lib/gitlab/ci/build/step_spec.rb
+++ b/spec/lib/gitlab/ci/build/step_spec.rb
@@ -5,10 +5,14 @@ describe Gitlab::Ci::Build::Step do
     shared_examples 'has correct script' do
       subject { described_class.from_commands(job) }
 
+      before do
+        job.run!
+      end
+
       it 'fabricates an object' do
         expect(subject.name).to eq(:script)
         expect(subject.script).to eq(script)
-        expect(subject.timeout).to eq(job.timeout)
+        expect(subject.timeout).to eq(job.metadata_timeout)
         expect(subject.when).to eq('on_success')
         expect(subject.allow_failure).to be_falsey
       end
@@ -47,6 +51,10 @@ describe Gitlab::Ci::Build::Step do
 
     subject { described_class.from_after_script(job) }
 
+    before do
+      job.run!
+    end
+
     context 'when after_script is empty' do
       it 'doesn not fabricate an object' do
         is_expected.to be_nil
@@ -59,7 +67,7 @@ describe Gitlab::Ci::Build::Step do
       it 'fabricates an object' do
         expect(subject.name).to eq(:after_script)
         expect(subject.script).to eq(['ls -la', 'date'])
-        expect(subject.timeout).to eq(job.timeout)
+        expect(subject.timeout).to eq(job.metadata_timeout)
         expect(subject.when).to eq('always')
         expect(subject.allow_failure).to be_truthy
       end
diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb
new file mode 100644
index 00000000000..268561ee941
--- /dev/null
+++ b/spec/models/ci/build_metadata_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe Ci::BuildMetadata do
+  set(:user) { create(:user) }
+  set(:group) { create(:group, :access_requestable) }
+  set(:project) { create(:project, :repository, group: group, build_timeout: 2000) }
+
+  set(:pipeline) do
+    create(:ci_pipeline, project: project,
+                         sha: project.commit.id,
+                         ref: project.default_branch,
+                         status: 'success')
+  end
+
+  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:build_metadata) { create(:ci_build_metadata, build: build) }
+
+  describe '#update_timeout_state' do
+    subject { build_metadata }
+
+    context 'when runner is not assigned to the job' do
+      it "doesn't change timeout value" do
+        expect { subject.update_timeout_state }.not_to change { subject.reload.timeout }
+      end
+
+      it "doesn't change timeout_source value" do
+        expect { subject.update_timeout_state }.not_to change { subject.reload.timeout_source }
+      end
+    end
+
+    context 'when runner is assigned to the job' do
+      before do
+        build.update_attributes(runner: runner)
+      end
+
+      context 'when runner timeout is lower than project timeout' do
+        let(:runner) { create(:ci_runner, maximum_timeout: 1900) }
+
+        it 'sets runner timeout' do
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(1900)
+        end
+
+        it 'sets runner_timeout_source' do
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('runner_timeout_source')
+        end
+      end
+
+      context 'when runner timeout is higher than project timeout' do
+        let(:runner) { create(:ci_runner, maximum_timeout: 2100) }
+
+        it 'sets project timeout' do
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(2000)
+        end
+
+        it 'sets project_timeout_source' do
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('project_timeout_source')
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 7d935cf8d76..f5534d22a54 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1271,12 +1271,6 @@ describe Ci::Build do
   end
 
   describe 'project settings' do
-    describe '#timeout' do
-      it 'returns project timeout configuration' do
-        expect(build.timeout).to eq(project.build_timeout)
-      end
-    end
-
     describe '#allow_git_fetch' do
       it 'return project allow_git_fetch configuration' do
         expect(build.allow_git_fetch).to eq(project.build_allow_git_fetch)
@@ -2011,6 +2005,70 @@ describe Ci::Build do
     end
   end
 
+  describe 'state transition: pending: :running' do
+    let(:runner) { create(:ci_runner) }
+    let(:job) { create(:ci_build, :pending, runner: runner) }
+
+    before do
+      job.project.update_attribute(:build_timeout, 1800)
+    end
+
+    def run_job_without_exception
+      job.run!
+    rescue StateMachines::InvalidTransition
+    end
+
+    shared_examples 'saves data on transition' do
+      it 'saves timeout' do
+        expect { job.run! }.to change { job.reload.ensure_metadata.timeout }.from(nil).to(expected_timeout)
+      end
+
+      it 'saves timeout_source' do
+        expect { job.run! }.to change { job.reload.ensure_metadata.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source)
+      end
+
+      context 'when Ci::BuildMetadata#update_timeout_state fails update' do
+        before do
+          allow_any_instance_of(Ci::BuildMetadata).to receive(:update_timeout_state).and_return(false)
+        end
+
+        it "doesn't save timeout" do
+          expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout_source }
+        end
+
+        it "doesn't save timeout_source" do
+          expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout_source }
+        end
+
+        it 'raises an exception' do
+          expect { job.run! }.to raise_error(StateMachines::InvalidTransition)
+        end
+      end
+    end
+
+    context 'when runner timeout overrides project timeout' do
+      let(:expected_timeout) { 900 }
+      let(:expected_timeout_source) { 'runner_timeout_source' }
+
+      before do
+        runner.update_attribute(:maximum_timeout, 900)
+      end
+
+      it_behaves_like 'saves data on transition'
+    end
+
+    context "when runner timeout doesn't override project timeout" do
+      let(:expected_timeout) { 1800 }
+      let(:expected_timeout_source) { 'project_timeout_source' }
+
+      before do
+        runner.update_attribute(:maximum_timeout, 3600)
+      end
+
+      it_behaves_like 'saves data on transition'
+    end
+  end
+
   describe 'state transition: any => [:running]' do
     shared_examples 'validation is active' do
       context 'when depended job has not been completed yet' do
diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb
new file mode 100644
index 00000000000..27c86e60e60
--- /dev/null
+++ b/spec/models/concerns/chronic_duration_attribute_spec.rb
@@ -0,0 +1,115 @@
+require 'spec_helper'
+
+shared_examples 'ChronicDurationAttribute reader' do
+  it 'contains dynamically created reader method' do
+    expect(subject.class).to be_public_method_defined(virtual_field)
+  end
+
+  it 'outputs chronic duration formatted value' do
+    subject.send("#{source_field}=", 120)
+
+    expect(subject.send(virtual_field)).to eq('2m')
+  end
+
+  context 'when value is set to nil' do
+    it 'outputs nil' do
+      subject.send("#{source_field}=", nil)
+
+      expect(subject.send(virtual_field)).to be_nil
+    end
+  end
+end
+
+shared_examples 'ChronicDurationAttribute writer' do
+  it 'contains dynamically created writer method' do
+    expect(subject.class).to be_public_method_defined("#{virtual_field}=")
+  end
+
+  before do
+    subject.send("#{virtual_field}=", '10m')
+  end
+
+  it 'parses chronic duration input' do
+    expect(subject.send(source_field)).to eq(600)
+  end
+
+  it 'passes validation' do
+    expect(subject.valid?).to be_truthy
+  end
+
+  context 'when negative input is used' do
+    before do
+      subject.send("#{source_field}=", 3600)
+    end
+
+    it "doesn't raise exception" do
+      expect { subject.send("#{virtual_field}=", '-10m') }.not_to raise_error(ChronicDuration::DurationParseError)
+    end
+
+    it "doesn't change value" do
+      expect { subject.send("#{virtual_field}=", '-10m') }.not_to change { subject.send(source_field) }
+    end
+
+    it "doesn't pass validation" do
+      subject.send("#{virtual_field}=", '-10m')
+
+      expect(subject.valid?).to be_falsey
+      expect(subject.errors&.messages).to include(virtual_field => ['is not a correct duration'])
+    end
+  end
+
+  context 'when empty input is used' do
+    before do
+      subject.send("#{virtual_field}=", '')
+    end
+
+    it 'writes nil' do
+      expect(subject.send(source_field)).to be_nil
+    end
+
+    it 'passes validation' do
+      expect(subject.valid?).to be_truthy
+    end
+  end
+
+  context 'when nil input is used' do
+    before do
+      subject.send("#{virtual_field}=", nil)
+    end
+
+    it 'writes nil' do
+      expect(subject.send(source_field)).to be_nil
+    end
+
+    it 'passes validation' do
+      expect(subject.valid?).to be_truthy
+    end
+
+    it "doesn't raise exception" do
+      expect { subject.send("#{virtual_field}=", nil) }.not_to raise_error(NoMethodError)
+    end
+  end
+end
+
+describe 'ChronicDurationAttribute' do
+  let(:source_field) {:maximum_timeout}
+  let(:virtual_field) {:maximum_timeout_human_readable}
+
+  subject { Ci::Runner.new }
+
+  it_behaves_like 'ChronicDurationAttribute reader'
+  it_behaves_like 'ChronicDurationAttribute writer'
+end
+
+describe 'ChronicDurationAttribute - reader' do
+  let(:source_field) {:timeout}
+  let(:virtual_field) {:timeout_human_readable}
+
+  subject {Ci::BuildMetadata.new}
+
+  it "doesn't contain dynamically created writer method" do
+    expect(subject.class).not_to be_public_method_defined("#{virtual_field}=")
+  end
+
+  it_behaves_like 'ChronicDurationAttribute reader'
+end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index f3dd121faa9..5084b36c761 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -109,6 +109,26 @@ describe API::Runner do
         end
       end
 
+      context 'when maximum job timeout is specified' do
+        it 'creates runner' do
+          post api('/runners'), token: registration_token,
+                                maximum_timeout: 9000
+
+          expect(response).to have_gitlab_http_status 201
+          expect(Ci::Runner.first.maximum_timeout).to eq(9000)
+        end
+
+        context 'when maximum job timeout is empty' do
+          it 'creates runner' do
+            post api('/runners'), token: registration_token,
+                                  maximum_timeout: ''
+
+            expect(response).to have_gitlab_http_status 201
+            expect(Ci::Runner.first.maximum_timeout).to be_nil
+          end
+        end
+      end
+
       %w(name version revision platform architecture).each do |param|
         context "when info parameter '#{param}' info is present" do
           let(:value) { "#{param}_value" }
@@ -340,12 +360,12 @@ describe API::Runner do
           let(:expected_steps) do
             [{ 'name' => 'script',
                'script' => %w(ls date),
-               'timeout' => job.timeout,
+               'timeout' => job.metadata_timeout,
                'when' => 'on_success',
                'allow_failure' => false },
              { 'name' => 'after_script',
                'script' => %w(ls date),
-               'timeout' => job.timeout,
+               'timeout' => job.metadata_timeout,
                'when' => 'always',
                'allow_failure' => true }]
           end
@@ -648,6 +668,41 @@ describe API::Runner do
               end
             end
           end
+
+          describe 'timeout support' do
+            context 'when project specifies job timeout' do
+              let(:project) { create(:project, shared_runners_enabled: false, build_timeout: 1234) }
+
+              it 'contains info about timeout taken from project' do
+                request_job
+
+                expect(response).to have_gitlab_http_status(201)
+                expect(json_response['runner_info']).to include({ 'timeout' => 1234 })
+              end
+
+              context 'when runner specifies lower timeout' do
+                let(:runner) { create(:ci_runner, maximum_timeout: 1000) }
+
+                it 'contains info about timeout overridden by runner' do
+                  request_job
+
+                  expect(response).to have_gitlab_http_status(201)
+                  expect(json_response['runner_info']).to include({ 'timeout' => 1000 })
+                end
+              end
+
+              context 'when runner specifies bigger timeout' do
+                let(:runner) { create(:ci_runner, maximum_timeout: 2000) }
+
+                it 'contains info about timeout not overridden by runner' do
+                  request_job
+
+                  expect(response).to have_gitlab_http_status(201)
+                  expect(json_response['runner_info']).to include({ 'timeout' => 1234 })
+                end
+              end
+            end
+          end
         end
 
         def request_job(token = runner.token, **params)
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index ec5cad4f4fd..d30f0cf36e2 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -123,6 +123,7 @@ describe API::Runners do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response['description']).to eq(shared_runner.description)
+          expect(json_response['maximum_timeout']).to be_nil
         end
       end
 
@@ -192,7 +193,8 @@ describe API::Runners do
                                                  tag_list: ['ruby2.1', 'pgsql', 'mysql'],
                                                  run_untagged: 'false',
                                                  locked: 'true',
-                                                 access_level: 'ref_protected')
+                                                 access_level: 'ref_protected',
+                                                 maximum_timeout: 1234)
           shared_runner.reload
 
           expect(response).to have_gitlab_http_status(200)
@@ -204,6 +206,7 @@ describe API::Runners do
           expect(shared_runner.ref_protected?).to be_truthy
           expect(shared_runner.ensure_runner_queue_value)
             .not_to eq(runner_queue_value)
+          expect(shared_runner.maximum_timeout).to eq(1234)
         end
       end
 
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index b86a3d72bb4..8de0bdf92e2 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -29,7 +29,8 @@ describe Ci::RetryBuildService do
        commit_id deployments erased_by_id last_deployment project_id
        runner_id tag_taggings taggings tags trigger_request_id
        user_id auto_canceled_by_id retried failure_reason
-       artifacts_file_store artifacts_metadata_store].freeze
+       artifacts_file_store artifacts_metadata_store
+       metadata].freeze
 
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }