diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 842c6e5cb50d79bfa7c2b7626eb330a758aea188..1e8f28b646636bab046767445ee1965ffb924dfc 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -38,6 +38,13 @@ class CommitStatus < ActiveRecord::Base scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) } scope :after_stage, -> (index) { where('stage_idx > ?', index) } + enum failure_reason: { + unknown_failure: nil, + job_failure: 1, + api_failure: 2, + stuck_or_timeout_failure: 3 + } + state_machine :status do event :process do transition [:skipped, :manual] => :created @@ -79,6 +86,11 @@ class CommitStatus < ActiveRecord::Base commit_status.finished_at = Time.now end + before_transition any => :failed do |commit_status, transition| + failure_reason = transition.args.first + commit_status.failure_reason = failure_reason + end + after_transition do |commit_status, transition| next if transition.loopback? diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 0c131afca315d1eb8baa9c3bffc403a18f677539..ac7f4df14d73ac098a98c81bee2cb5da4868e24c 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -51,7 +51,7 @@ def error(message, http_status = nil) log_error("Projects::UpdatePagesService: #{message}") @status.allow_failure = !latest? @status.description = message - @status.drop + @status.drop(:job_failure) super end diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 8b0cfcc8af8057878753b1ac2fbf2fd31009dc10..269776a1f626e0b3dca2801aa207c030fb390ab7 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -53,7 +53,7 @@ def search(status, timeout) def drop_build(type, build, status, timeout) Rails.logger.info "#{self.class}: Dropping #{type} build #{build.id} for runner #{build.runner_id} (status: #{status}, timeout: #{timeout})" Gitlab::OptimisticLocking.retry_lock(build, 3) do |b| - b.drop + b.drop(:stuck_or_timeout_failure) end end end diff --git a/changelogs/unreleased/feature-sm-37239-implement-failure_reason-on-ci_builds.yml b/changelogs/unreleased/feature-sm-37239-implement-failure_reason-on-ci_builds.yml new file mode 100644 index 0000000000000000000000000000000000000000..006b0b458446d47d1f96d84c489af43ec0ebf923 --- /dev/null +++ b/changelogs/unreleased/feature-sm-37239-implement-failure_reason-on-ci_builds.yml @@ -0,0 +1,5 @@ +--- +title: Implement `failure_reason` on `ci_builds` +merge_request: 13937 +author: +type: added diff --git a/db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb b/db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..5a7487b9227cd6564b3a94073b278c47654d7b9c --- /dev/null +++ b/db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb @@ -0,0 +1,9 @@ +class AddFailureReasonToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_builds, :failure_reason, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 26dddd3d74c181ada6184ae307abace4676cc3b2..4d3be96031dc45a997927ba24cc650f3958eedbf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170824162758) do +ActiveRecord::Schema.define(version: 20170830125940) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -301,6 +301,7 @@ t.integer "stage_id" t.integer "artifacts_file_store" t.integer "artifacts_metadata_store" + t.integer "failure_reason" end add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 485b680cd5fddbdf73e5bc004fe5ee90b6ed120e..892a5e80a1b28d698580d748f78ff1aa111037c2 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -101,7 +101,7 @@ class CommitStatuses < Grape::API when 'success' status.success! when 'failed' - status.drop! + status.drop!(:api_failure) when 'canceled' status.cancel! else diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 119993545947fdd3cf00cca6d05e342457b4853b..44ca42fef684ddc27e178ae684db7b15fbd0deb7 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -114,6 +114,8 @@ class Runner < Grape::API requires :id, type: Integer, desc: %q(Job's ID) optional :trace, type: String, desc: %q(Job's full trace) optional :state, type: String, desc: %q(Job's status: success, failed) + optional :failure_reason, type: String, values: CommitStatus.failure_reasons.keys, + desc: %q(Job's failure_reason) end put '/:id' do job = authenticate_job! @@ -127,7 +129,11 @@ class Runner < Grape::API when 'success' job.success when 'failed' - job.drop + if params[:failure_reason] + job.drop(params[:failure_reason].to_sym) + else + job.drop(:job_failure) + end end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index a4ef0344ce35b11a108aa9405b01860db76efa63..30a7d9a154f3d9fb0c56b850673112ab883cb877 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -282,6 +282,7 @@ CommitStatus: - coverage_regex - auto_canceled_by_id - retried +- failure_reason Ci::Variable: - id - project_id diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index f7583645e690b28254670a3257553c62ff3fc5b5..0a6b0023dea3ab652c8acc0fff1f3b890c16cd33 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -443,4 +443,25 @@ def create_status(**opts) end end end + + describe 'set failure_reason when drop' do + let(:commit_status) { create(:commit_status, :created) } + + subject do + commit_status.drop!(reason) + commit_status + end + + context 'when failure_reason is nil' do + let(:reason) { } + + it { is_expected.to be_unknown_failure } + end + + context 'when failure_reason is job_failure' do + let(:reason) { :job_failure } + + it { is_expected.to be_job_failure } + end + end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 3c02e6302b4e15fc61be8378d527556e3a091e53..fcdcf461c52d2174b84b461b32bffb047501750f 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -142,6 +142,9 @@ def create_status(commit, opts = {}) expect(json_response['ref']).not_to be_empty expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil + if status == 'failed' + expect(CommitStatus.find(json_response['id'])).to be_api_failure + end end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 4ff981330490efe495bb9b33d5d3785f45388127..5aa5f08b1829a9450ea403c2b2a18836b76d1105 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -629,12 +629,23 @@ def request_job(token = runner.token, **params) update_job(state: 'success') expect(job.reload.status).to eq 'success' + expect(job).to be_unknown_failure end it 'mark job as failed' do update_job(state: 'failed') expect(job.reload.status).to eq 'failed' + expect(job).to be_job_failure + end + + context 'when failure_reason is given' do + it 'mark job as failed' do + update_job(state: 'failed', failure_reason: 'stuck_or_timeout_failure') + + expect(job.reload.status).to eq 'failed' + expect(job).to be_stuck_or_timeout_failure + end end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 7e7a68c947b214a0674a230d4efea065ca694a64..383bd73c32daa15c6cd872b636508ea20b85a2af 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -22,8 +22,8 @@ %i[type lock_version target_url base_tags 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 sourced_pipelines - artifacts_file_store artifacts_metadata_store].freeze + user_id auto_canceled_by_id retried failure_reason + sourced_pipelines artifacts_file_store artifacts_metadata_store].freeze # EE shared_examples 'build duplication' do let(:stage) do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index aa6ad6340f5c0eacff5fe0776c59b52c9a5d03a0..5fc69c7e4e56c90cd0360556d3884012d65e5357 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -116,6 +116,7 @@ expect(deploy_status.description) .to match(/artifacts for pages are too large/) + expect(deploy_status).to be_job_failure end end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 549635f7f334dba64ca5ef4f2a7dd9f8e3f629c3..ac6f4fefb4ef951d3f59036eea3dfec84442b58d 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -6,27 +6,31 @@ let(:worker) { described_class.new } let(:exclusive_lease_uuid) { SecureRandom.uuid } - subject do - job.reload - job.status - end - before do job.update!(status: status, updated_at: updated_at) allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(exclusive_lease_uuid) end shared_examples 'job is dropped' do - it 'changes status' do + before do worker.perform - is_expected.to eq('failed') + job.reload + end + + it "changes status" do + expect(job).to be_failed + expect(job).to be_stuck_or_timeout_failure end end shared_examples 'job is unchanged' do - it "doesn't change status" do + before do worker.perform - is_expected.to eq(status) + job.reload + end + + it "doesn't change status" do + expect(job.status).to eq(status) end end