diff options
19 files changed, 148 insertions, 6 deletions
diff --git a/app/assets/javascripts/pipelines/components/pipeline_url.vue b/app/assets/javascripts/pipelines/components/pipeline_url.vue index 76b97af39f1..9da0aac50a1 100644 --- a/app/assets/javascripts/pipelines/components/pipeline_url.vue +++ b/app/assets/javascripts/pipelines/components/pipeline_url.vue @@ -72,6 +72,13 @@ :title="pipeline.yaml_errors"> yaml invalid </span> + <span + v-if="pipeline.flags.failure_reason" + v-tooltip + class="js-pipeline-url-failure label label-danger" + :title="pipeline.failure_reason"> + error + </span> <a v-if="pipeline.flags.auto_devops" tabindex="0" diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3d5acc00f8f..cf3ce3c9e54 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -5,6 +5,7 @@ module Ci include Importable include AfterCommitQueue include Presentable + include Gitlab::OptimisticLocking belongs_to :project belongs_to :user @@ -58,6 +59,11 @@ module Ci auto_devops_source: 2 } + enum failure_reason: { + unknown_failure: 0, + config_error: 1 + } + state_machine :status, initial: :created do event :enqueue do transition created: :pending @@ -109,6 +115,12 @@ module Ci pipeline.auto_canceled_by = nil end + before_transition any => :failed do |pipeline, transition| + transition.args.first.try do |reason| + pipeline.failure_reason = reason + end + end + after_transition [:created, :pending] => :running do |pipeline| pipeline.run_after_commit { PipelineMetricsWorker.perform_async(pipeline.id) } end @@ -263,7 +275,7 @@ module Ci end def cancel_running - Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable| + retry_optimistic_lock(cancelable_statuses) do |cancelable| cancelable.find_each do |job| yield(job) if block_given? job.cancel @@ -312,6 +324,10 @@ module Ci @stage_seeds ||= config_processor.stage_seeds(self) end + def seeds_size + @seeds_size ||= stage_seeds.sum(&:size) + end + def has_kubernetes_active? project.kubernetes_service&.active? end @@ -403,7 +419,7 @@ module Ci end def update_status - Gitlab::OptimisticLocking.retry_lock(self) do + retry_optimistic_lock(self) do case latest_builds_status when 'pending' then enqueue when 'running' then run diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 3803e18a96e..7c3ed96bc28 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -81,6 +81,7 @@ module HasStatus scope :canceled, -> { where(status: 'canceled') } scope :skipped, -> { where(status: 'skipped') } scope :manual, -> { where(status: 'manual') } + scope :alive, -> { where(status: [:created, :pending, :running]) } scope :created_or_pending, -> { where(status: [:created, :pending]) } scope :running_or_pending, -> { where(status: [:running, :pending]) } scope :finished, -> { where(status: [:success, :failed, :canceled]) } diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb index a542bdd8295..099b4720fb6 100644 --- a/app/presenters/ci/pipeline_presenter.rb +++ b/app/presenters/ci/pipeline_presenter.rb @@ -1,7 +1,18 @@ module Ci class PipelinePresenter < Gitlab::View::Presenter::Delegated + FAILURE_REASONS = { + config_error: 'CI/CD YAML configuration error!' + }.freeze + presents :pipeline + def failure_reason + return unless pipeline.failure_reason? + + FAILURE_REASONS[pipeline.failure_reason.to_sym] || + pipeline.failure_reason + end + def status_title if auto_canceled? "Pipeline is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}" diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index 357fc71f877..6457294b285 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -20,6 +20,7 @@ class PipelineEntity < Grape::Entity expose :has_yaml_errors?, as: :yaml_errors expose :can_retry?, as: :retryable expose :can_cancel?, as: :cancelable + expose :failure_reason?, as: :failure_reason end expose :details do @@ -44,6 +45,11 @@ class PipelineEntity < Grape::Entity end expose :commit, using: CommitEntity + expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } + + expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? } do |pipeline| + pipeline.present.failure_reason + end expose :retry_path, if: -> (*) { can_retry? } do |pipeline| retry_project_pipeline_path(pipeline.project, pipeline) @@ -53,8 +59,6 @@ class PipelineEntity < Grape::Entity cancel_project_pipeline_path(pipeline.project, pipeline) end - expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } - private alias_method :pipeline, :object diff --git a/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb b/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb new file mode 100644 index 00000000000..82adddbc1ec --- /dev/null +++ b/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb @@ -0,0 +1,9 @@ +class AddFailureReasonToPipelines < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_pipelines, :failure_reason, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 17be774e9de..7fc26519cd7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -342,6 +342,7 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.integer "source" t.integer "config_source" t.boolean "protected" + t.integer "failure_reason" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree diff --git a/lib/gitlab/ci/pipeline/chain/validate/config.rb b/lib/gitlab/ci/pipeline/chain/validate/config.rb index 489bcd79655..075504bcce5 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/config.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/config.rb @@ -13,7 +13,7 @@ module Gitlab end if @command.save_incompleted && @pipeline.has_yaml_errors? - @pipeline.drop + @pipeline.drop!(:config_error) end return error(@pipeline.yaml_errors) diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index e19aae35a81..bc97aa63b02 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -3,7 +3,9 @@ module Gitlab module Stage class Seed attr_reader :pipeline + delegate :project, to: :pipeline + delegate :size, to: :@jobs def initialize(pipeline, stage, jobs) @pipeline = pipeline diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index e5ea6b41ea3..f994c2df821 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -47,6 +47,7 @@ FactoryGirl.define do trait :invalid do config(rspec: nil) + failure_reason :config_error end trait :blocked do diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index f7b40cb1820..92486d2bc57 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -162,6 +162,16 @@ describe 'Pipelines', :js do expect(page).to have_selector( %Q{span[data-original-title="#{pipeline.yaml_errors}"]}) end + + it 'contains badge that indicates failure reason' do + expect(page).to have_content 'error' + end + + it 'contains badge with tooltip which contains failure reason' do + expect(pipeline.failure_reason?).to eq true + expect(page).to have_selector( + %Q{span[data-original-title="#{pipeline.present.failure_reason}"]}) + end end context 'with manual actions' do diff --git a/spec/javascripts/pipelines/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index 256fdbe743c..4a4f2259d23 100644 --- a/spec/javascripts/pipelines/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js @@ -125,4 +125,23 @@ describe('Pipeline Url Component', () => { component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim(), ).toEqual('Auto DevOps'); }); + + it('should render error badge when pipeline has a failure reason set', () => { + const component = new PipelineUrlComponent({ + propsData: { + pipeline: { + id: 1, + path: 'foo', + flags: { + failure_reason: true, + }, + failure_reason: 'some reason', + }, + autoDevopsHelpPath: 'foo', + }, + }).$mount(); + + expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error'); + expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title')).toContain('some reason'); + }); }); diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 3740df88f42..8357af38f92 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -55,6 +55,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do it 'fails the pipeline' do expect(pipeline.reload).to be_failed end + + it 'sets a config error failure reason' do + expect(pipeline.reload.config_error?).to eq true + end end context 'when saving incomplete pipeline is not allowed' do diff --git a/spec/lib/gitlab/ci/stage/seed_spec.rb b/spec/lib/gitlab/ci/stage/seed_spec.rb index 9ecd128faca..3fe8d50c49a 100644 --- a/spec/lib/gitlab/ci/stage/seed_spec.rb +++ b/spec/lib/gitlab/ci/stage/seed_spec.rb @@ -11,6 +11,12 @@ describe Gitlab::Ci::Stage::Seed do described_class.new(pipeline, 'test', builds) end + describe '#size' do + it 'returns a number of jobs in the stage' do + expect(subject.size).to eq 2 + end + end + describe '#stage' do it 'returns hash attributes of a stage' do expect(subject.stage).to be_a Hash diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 7268226112c..48938346577 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -224,6 +224,7 @@ Ci::Pipeline: - auto_canceled_by_id - pipeline_schedule_id - config_source +- failure_reason - protected Ci::Stage: - id diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 9c1e460ab20..2c9e7013b77 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -238,7 +238,7 @@ describe Ci::Pipeline, :mailer do describe '#stage_seeds' do let(:pipeline) do - create(:ci_pipeline, config: { rspec: { script: 'rake' } }) + build(:ci_pipeline, config: { rspec: { script: 'rake' } }) end it 'returns preseeded stage seeds object' do @@ -247,6 +247,14 @@ describe Ci::Pipeline, :mailer do end end + describe '#seeds_size' do + let(:pipeline) { build(:ci_pipeline_with_one_job) } + + it 'returns number of jobs in stage seeds' do + expect(pipeline.seeds_size).to eq 1 + end + end + describe '#legacy_stages' do subject { pipeline.legacy_stages } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index a38f2553eb1..6866b43432c 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.alive' do + subject { CommitStatus.alive } + + %i[running pending created].each do |status| + it_behaves_like 'containing the job', status + end + + %i[failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.created_or_pending' do subject { CommitStatus.created_or_pending } diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb index e4886a8f019..f7ceaf844be 100644 --- a/spec/presenters/ci/pipeline_presenter_spec.rb +++ b/spec/presenters/ci/pipeline_presenter_spec.rb @@ -51,4 +51,21 @@ describe Ci::PipelinePresenter do end end end + + context '#failure_reason' do + context 'when pipeline has failure reason' do + it 'represents a failure reason sentence' do + pipeline.failure_reason = :config_error + + expect(presenter.failure_reason) + .to eq 'CI/CD YAML configuration error!' + end + end + + context 'when pipeline does not have failure reason' do + it 'returns nil' do + expect(presenter.failure_reason).to be_nil + end + end + end end diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index f8df461bc81..248552d1858 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -108,5 +108,18 @@ describe PipelineEntity do expect(subject[:ref][:path]).to be_nil end end + + context 'when pipeline has a failure reason set' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + pipeline.drop!(:config_error) + end + + it 'has a correct failure reason' do + expect(subject[:failure_reason]) + .to eq 'CI/CD YAML configuration error!' + end + end end end |