Commit dcf09d11 authored by Shinya Maeda's avatar Shinya Maeda

Implement `failure_reason` on `ci_builds`

parent 597bc292
...@@ -103,7 +103,7 @@ module Ci ...@@ -103,7 +103,7 @@ module Ci
end end
end end
before_transition any => [:failed] do |build| before_transition any => [:failed] do |build, transition|
next if build.retries_max.zero? next if build.retries_max.zero?
if build.retries_count < build.retries_max if build.retries_count < build.retries_max
......
...@@ -38,6 +38,19 @@ class CommitStatus < ActiveRecord::Base ...@@ -38,6 +38,19 @@ class CommitStatus < ActiveRecord::Base
scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) } scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
scope :after_stage, -> (index) { where('stage_idx > ?', index) } scope :after_stage, -> (index) { where('stage_idx > ?', index) }
enum failure_reason: {
no_error: nil,
failed_by_script: 1, # TODO: Not used. Should we expand pipeline as well?
failed_by_missing_dependency: 2, # This will be done in the next MR.
failed_by_system: 3, # TODO: Not used. What's this state?
failed_by_failed_job_state: 4,
failed_by_out_of_quota: 5, # TODO: Only EE. How can we detect?
failed_by_stuck_and_timeout: 6,
failed_by_no_runner: 7, # TODO: Not used. How can we detect?
failed_by_api: 8,
failed_by_page: 9
}
state_machine :status do state_machine :status do
event :process do event :process do
transition [:skipped, :manual] => :created transition [:skipped, :manual] => :created
...@@ -79,6 +92,11 @@ class CommitStatus < ActiveRecord::Base ...@@ -79,6 +92,11 @@ class CommitStatus < ActiveRecord::Base
commit_status.finished_at = Time.now commit_status.finished_at = Time.now
end 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| after_transition do |commit_status, transition|
next if transition.loopback? next if transition.loopback?
......
...@@ -53,7 +53,7 @@ module Projects ...@@ -53,7 +53,7 @@ module Projects
log_error("Projects::UpdatePagesService: #{message}") log_error("Projects::UpdatePagesService: #{message}")
@status.allow_failure = !latest? @status.allow_failure = !latest?
@status.description = message @status.description = message
@status.drop @status.drop(:page)
super super
end end
......
...@@ -53,7 +53,7 @@ class StuckCiJobsWorker ...@@ -53,7 +53,7 @@ class StuckCiJobsWorker
def drop_build(type, build, 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})" 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| Gitlab::OptimisticLocking.retry_lock(build, 3) do |b|
b.drop b.drop(:stuck_and_timeout)
end end
end end
end end
class AddFailureReasonToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :ci_builds, :failure_reason, :integer
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -247,6 +247,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do ...@@ -247,6 +247,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do
t.boolean "retried" t.boolean "retried"
t.integer "stage_id" t.integer "stage_id"
t.boolean "protected" t.boolean "protected"
t.integer "failure_reason"
end end
add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree
......
...@@ -103,7 +103,7 @@ module API ...@@ -103,7 +103,7 @@ module API
when 'success' when 'success'
status.success! status.success!
when 'failed' when 'failed'
status.drop! status.drop!(:api)
when 'canceled' when 'canceled'
status.cancel! status.cancel!
else else
......
...@@ -127,7 +127,7 @@ module API ...@@ -127,7 +127,7 @@ module API
when 'success' when 'success'
job.success job.success
when 'failed' when 'failed'
job.drop job.drop(:failed_job_state)
end end
end end
......
...@@ -1710,4 +1710,24 @@ describe Ci::Build do ...@@ -1710,4 +1710,24 @@ describe Ci::Build do
end end
end end
end end
describe 'set failure_reason when drop' do
let(:build) { create(:ci_build, :created) }
before do
build.drop!(reason)
end
context 'when failure_reason is nil' do
let(:reason) { }
it { expect(build).to be_no_error }
end
context 'when failure_reason is script_error' do
let(:reason) { :script_error }
it { expect(build).to be_failed_by_missing_dependency }
end
end
end end
...@@ -142,6 +142,9 @@ describe API::CommitStatuses do ...@@ -142,6 +142,9 @@ describe API::CommitStatuses do
expect(json_response['ref']).not_to be_empty expect(json_response['ref']).not_to be_empty
expect(json_response['target_url']).to be_nil expect(json_response['target_url']).to be_nil
expect(json_response['description']).to be_nil expect(json_response['description']).to be_nil
if status == 'failed'
expect(CommitStatus.find(json_response['id'])).to be_failed_by_api
end
end end
end end
end end
......
...@@ -627,12 +627,14 @@ describe API::Runner do ...@@ -627,12 +627,14 @@ describe API::Runner do
update_job(state: 'success') update_job(state: 'success')
expect(job.reload.status).to eq 'success' expect(job.reload.status).to eq 'success'
expect(job).to be_no_error
end end
it 'mark job as failed' do it 'mark job as failed' do
update_job(state: 'failed') update_job(state: 'failed')
expect(job.reload.status).to eq 'failed' expect(job.reload.status).to eq 'failed'
expect(job).to be_failed_by_failed_job_state
end end
end end
......
...@@ -116,6 +116,7 @@ describe Projects::UpdatePagesService do ...@@ -116,6 +116,7 @@ describe Projects::UpdatePagesService do
expect(deploy_status.description) expect(deploy_status.description)
.to match(/artifacts for pages are too large/) .to match(/artifacts for pages are too large/)
expect(deploy_status).to be_failed_by_page
end end
end end
......
...@@ -20,6 +20,7 @@ describe StuckCiJobsWorker do ...@@ -20,6 +20,7 @@ describe StuckCiJobsWorker do
it 'changes status' do it 'changes status' do
worker.perform worker.perform
is_expected.to eq('failed') is_expected.to eq('failed')
expect(job).to be_failed_by_stuck_and_timeout
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment