diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 2f01a098b0e..cbd2cc6c58f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -93,6 +93,12 @@ module Ci chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout + enum timeout_source: { + unknown_timeout_source: nil, + project_timeout_source: 1, + runner_timeout_source: 2 + } + class << self # This is needed for url_for to work, # as the controller is JobsController @@ -123,10 +129,6 @@ module Ci end after_transition pending: :running do |build| - build.used_timeout = build.timeout - build.timeout_source = build.timeout < build.project.build_timeout ? 'runner' : 'project' - build.save! - build.run_after_commit do BuildHooksWorker.perform_async(id) end @@ -160,6 +162,11 @@ module Ci before_transition any => [:running] do |build| build.validates_dependencies! unless Feature.enabled?('ci_disable_validates_dependencies') end + + before_transition pending: :running do |build| + build.used_timeout = build.timeout + build.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source + end end def detailed_status(current_user) diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb index 255475e1fe6..be6cc2e70b1 100644 --- a/app/presenters/ci/build_presenter.rb +++ b/app/presenters/ci/build_presenter.rb @@ -1,5 +1,12 @@ module Ci class BuildPresenter < Gitlab::View::Presenter::Delegated + + TIMEOUT_SOURCES = { + unknown_timeout_source: nil, + project_timeout_source: 'project', + runner_timeout_source: 'runner' + }.freeze + presents :build def erased_by_user? @@ -18,6 +25,13 @@ module Ci end end + def timeout_source + return unless build.timeout_source? + + TIMEOUT_SOURCES[build.timeout_source.to_sym] || + build.timeout_source + end + def trigger_variables return [] unless trigger_request diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 17769790371..0ffc537dfd8 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -7,7 +7,7 @@ class BuildDetailsEntity < JobEntity expose :timeout, if: -> (*) { !build.used_timeout.nil? } do |build| { value: build.used_timeout_human_readable, - source: build.timeout_source } + source: build.present.timeout_source } end expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity diff --git a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb b/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb index 435fba712aa..18c4fd5bae4 100644 --- a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb +++ b/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb @@ -5,6 +5,6 @@ class AddUsedTimeoutAndTimeoutSourceColumnsToCiBuilds < ActiveRecord::Migration def change add_column :ci_builds, :used_timeout, :integer - add_column :ci_builds, :timeout_source, :string + add_column :ci_builds, :timeout_source, :integer end end diff --git a/db/schema.rb b/db/schema.rb index 61f15944f5a..9b126385045 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -312,7 +312,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do t.boolean "protected" t.integer "failure_reason" t.integer "used_timeout" - t.string "timeout_source" + t.integer "timeout_source" end add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 78842bd8a92..c3624158b76 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2046,20 +2046,18 @@ describe Ci::Build do end shared_examples 'saves data on transition' do - it 'saves used_timeout and timeout_source on transition' do - expect(job.used_timeout).to be_nil - expect(job.timeout_source).to be_nil + it 'saves used_timeout' do + expect { job.run! }.to change { job.reload.used_timeout }.from(nil).to(expected_timeout) + end - job.run! - - expect(job.used_timeout).to eq(expected_timeout) - expect(job.timeout_source).to eq(expected_timeout_source) + it 'saves timeout_source' do + expect { job.run! }.to change { job.reload.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source) end end context 'when runner timeout overrides project timeout' do let(:expected_timeout) { 900 } - let(:expected_timeout_source) { 'Runner' } + let(:expected_timeout_source) { 'runner_timeout_source' } before do runner.maximum_job_timeout = 900 @@ -2071,7 +2069,7 @@ describe Ci::Build do context "when runner timeout doesn't override project timeout" do let(:expected_timeout) { 1800 } - let(:expected_timeout_source) { 'Project' } + let(:expected_timeout_source) { 'project_timeout_source' } before do runner.maximum_job_timeout = 3600