summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2017-08-31 03:20:54 +0900
committerShinya Maeda <shinya@gitlab.com>2017-09-05 14:30:28 +0900
commitdcf09d11447c264f4b4028ea80eea2be913c2f5b (patch)
treed992feb77bb5246ba05ebf18610b05bce8859c63
parent597bc29260c4be3a1527a1c5307bec40004bac4d (diff)
downloadgitlab-ce-dcf09d11447c264f4b4028ea80eea2be913c2f5b.tar.gz
Implement `failure_reason` on `ci_builds`
-rw-r--r--app/models/ci/build.rb2
-rw-r--r--app/models/commit_status.rb18
-rw-r--r--app/services/projects/update_pages_service.rb2
-rw-r--r--app/workers/stuck_ci_jobs_worker.rb2
-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.rb2
-rw-r--r--spec/models/ci/build_spec.rb20
-rw-r--r--spec/requests/api/commit_statuses_spec.rb3
-rw-r--r--spec/requests/api/runner_spec.rb2
-rw-r--r--spec/services/projects/update_pages_service_spec.rb1
-rw-r--r--spec/workers/stuck_ci_jobs_worker_spec.rb1
13 files changed, 61 insertions, 6 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index ba3156154ac..9c50d521880 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -103,7 +103,7 @@ module Ci
end
end
- before_transition any => [:failed] do |build|
+ before_transition any => [:failed] do |build, transition|
next if build.retries_max.zero?
if build.retries_count < build.retries_max
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 842c6e5cb50..dae3174ba9b 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -38,6 +38,19 @@ class CommitStatus < ActiveRecord::Base
scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
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
event :process do
transition [:skipped, :manual] => :created
@@ -79,6 +92,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..c731789ce9b 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(:page)
super
end
diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb
index 8b0cfcc8af8..3e635f140fd 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_and_timeout)
end
end
end
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..c129cc9171d 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)
when 'canceled'
status.cancel!
else
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 11999354594..604bfd53296 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -127,7 +127,7 @@ module API
when 'success'
job.success
when 'failed'
- job.drop
+ job.drop(:failed_job_state)
end
end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 3fe3ec17d36..2f39a9b4b0f 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1710,4 +1710,24 @@ describe Ci::Build do
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
diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index cc71865e1f3..ce54a5702e3 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_failed_by_api
+ end
end
end
end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 993164aa8fe..23c2e7aa978 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -627,12 +627,14 @@ describe API::Runner do
update_job(state: 'success')
expect(job.reload.status).to eq 'success'
+ expect(job).to be_no_error
end
it 'mark job as failed' do
update_job(state: 'failed')
expect(job.reload.status).to eq 'failed'
+ expect(job).to be_failed_by_failed_job_state
end
end
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index aa6ad6340f5..0cc7dde3e4d 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_failed_by_page
end
end
diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb
index 549635f7f33..f0691813e77 100644
--- a/spec/workers/stuck_ci_jobs_worker_spec.rb
+++ b/spec/workers/stuck_ci_jobs_worker_spec.rb
@@ -20,6 +20,7 @@ describe StuckCiJobsWorker do
it 'changes status' do
worker.perform
is_expected.to eq('failed')
+ expect(job).to be_failed_by_stuck_and_timeout
end
end