diff options
-rw-r--r-- | app/models/ci/build.rb | 2 | ||||
-rw-r--r-- | app/models/commit_status.rb | 18 | ||||
-rw-r--r-- | app/services/projects/update_pages_service.rb | 2 | ||||
-rw-r--r-- | app/workers/stuck_ci_jobs_worker.rb | 2 | ||||
-rw-r--r-- | db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb | 9 | ||||
-rw-r--r-- | db/schema.rb | 3 | ||||
-rw-r--r-- | lib/api/commit_statuses.rb | 2 | ||||
-rw-r--r-- | lib/api/runner.rb | 2 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 20 | ||||
-rw-r--r-- | spec/requests/api/commit_statuses_spec.rb | 3 | ||||
-rw-r--r-- | spec/requests/api/runner_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/projects/update_pages_service_spec.rb | 1 | ||||
-rw-r--r-- | spec/workers/stuck_ci_jobs_worker_spec.rb | 1 |
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 |