summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/commit_status.rb13
-rw-r--r--app/services/projects/update_pages_service.rb2
-rw-r--r--app/workers/stuck_ci_jobs_worker.rb2
-rw-r--r--changelogs/unreleased/feature-sm-37239-implement-failure_reason-on-ci_builds.yml5
-rw-r--r--db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb9
-rw-r--r--db/schema.rb3
-rw-r--r--lib/api/commit_statuses.rb2
-rw-r--r--lib/api/runner.rb4
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/commit_status_spec.rb21
-rw-r--r--spec/requests/api/commit_statuses_spec.rb3
-rw-r--r--spec/requests/api/runner_spec.rb25
-rw-r--r--spec/services/ci/retry_build_service_spec.rb2
-rw-r--r--spec/services/projects/update_pages_service_spec.rb1
-rw-r--r--spec/workers/stuck_ci_jobs_worker_spec.rb22
15 files changed, 98 insertions, 17 deletions
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 842c6e5cb50..f3888528940 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -38,6 +38,14 @@ 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,
+ script_failure: 1,
+ api_failure: 2,
+ stuck_or_timeout_failure: 3,
+ runner_system_failure: 4
+ }
+
state_machine :status do
event :process do
transition [:skipped, :manual] => :created
@@ -79,6 +87,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 f6b83a2f621..d34903c9989 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -53,7 +53,7 @@ module Projects
log_error("Projects::UpdatePagesService: #{message}")
@status.allow_failure = !latest?
@status.description = message
- @status.drop
+ @status.drop(:script_failure)
super
end
diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb
index 8b0cfcc8af8..269776a1f62 100644
--- a/app/workers/stuck_ci_jobs_worker.rb
+++ b/app/workers/stuck_ci_jobs_worker.rb
@@ -53,7 +53,7 @@ class StuckCiJobsWorker
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 00000000000..006b0b45844
--- /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 00000000000..5a7487b9227
--- /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 a5f867df9ae..216b3cddab0 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"
@@ -247,6 +247,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do
t.boolean "retried"
t.integer "stage_id"
t.boolean "protected"
+ 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 6314ea63197..829eef18795 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -103,7 +103,7 @@ module 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 11999354594..a3987c560dd 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -114,6 +114,8 @@ module 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,7 @@ module API
when 'success'
job.success
when 'failed'
- job.drop
+ job.drop(params[:failure_reason] || :unknown_failure)
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 27f2ce60084..b852ac570a3 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -278,6 +278,7 @@ CommitStatus:
- auto_canceled_by_id
- retried
- protected
+- failure_reason
Ci::Variable:
- id
- project_id
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index f7583645e69..858ec831200 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -443,4 +443,25 @@ describe CommitStatus do
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 script_failure' do
+ let(:reason) { :script_failure }
+
+ it { is_expected.to be_script_failure }
+ end
+ end
end
diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index cc71865e1f3..e4c73583545 100644
--- a/spec/requests/api/commit_statuses_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -142,6 +142,9 @@ describe API::CommitStatuses do
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 993164aa8fe..419a65c69b5 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -626,13 +626,34 @@ describe API::Runner do
it 'mark job as succeeded' do
update_job(state: 'success')
- expect(job.reload.status).to eq 'success'
+ job.reload
+ expect(job).to be_success
end
it 'mark job as failed' do
update_job(state: 'failed')
- expect(job.reload.status).to eq 'failed'
+ job.reload
+ expect(job).to be_failed
+ expect(job).to be_unknown_failure
+ end
+
+ context 'when failure_reason is script_failure' do
+ before do
+ update_job(state: 'failed', failure_reason: 'script_failure')
+ job.reload
+ end
+
+ it { expect(job).to be_script_failure }
+ end
+
+ context 'when failure_reason is runner_system_failure' do
+ before do
+ update_job(state: 'failed', failure_reason: 'runner_system_failure')
+ job.reload
+ end
+
+ it { expect(job).to be_runner_system_failure }
end
end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index 7c9c117bf71..f5ed9ff608f 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -22,7 +22,7 @@ describe Ci::RetryBuildService do
%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].freeze
+ user_id auto_canceled_by_id retried failure_reason].freeze
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 aa6ad6340f5..031366d1825 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -116,6 +116,7 @@ describe Projects::UpdatePagesService do
expect(deploy_status.description)
.to match(/artifacts for pages are too large/)
+ expect(deploy_status).to be_script_failure
end
end
diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb
index 549635f7f33..ac6f4fefb4e 100644
--- a/spec/workers/stuck_ci_jobs_worker_spec.rb
+++ b/spec/workers/stuck_ci_jobs_worker_spec.rb
@@ -6,27 +6,31 @@ describe StuckCiJobsWorker do
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