Merge branch 'feature/sm/37239-implement-failure_reason-on-ci_builds' into 'master'

Implement `failure_reason` on `ci_builds`

Closes #37239

See merge request !13937

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?
 

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
 

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

changelogs/unreleased/feature-sm-37239-implement-failure_reason-on-ci_builds.yml

+---
+title: Implement `failure_reason` on `ci_builds`
+merge_request: 13937
+author:
+type: added

db/migrate/20170830125940_add_failure_reason_to_ci_builds.rb

+class AddFailureReasonToCiBuilds < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :failure_reason, :integer
+  end
+end

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

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

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
 

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

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

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

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
 

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

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
 

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