Commit 700a88a8 authored by Rémy Coutable's avatar Rémy Coutable Committed by Robert Speicher

Merge branch 'fix/status-of-pipeline-without-builds' into 'master'

Improve pipeline status in case that pipeline has no jobs

## What does this MR do?

This MR resolves problem with pipeline status when there are no build in pipeline.

This can happen when builds were skipped - for example - by using `only`/`except` keyword in `.gitlab-ci.yml`.

## What are the relevant issue numbers?

Closes #17977

See merge request !4403
parent aafbfb00
Please view this file on the master branch, on stable branches it's out of date.
v 8.9.0 (unreleased)
- Fix pipeline status when there are no builds in pipeline
- Fix Error 500 when using closes_issues API with an external issue tracker
- Add more information into RSS feed for issues (Alexander Matyushentsev)
- Bulk assign/unassign labels to issues.
......
......@@ -94,10 +94,13 @@ module Ci
end
def create_builds(user, trigger_request = nil)
##
# We persist pipeline only if there are builds available
#
return unless config_processor
config_processor.stages.any? do |stage|
CreateBuildsService.new(self).execute(stage, user, 'success', trigger_request).present?
end
build_builds_for_stages(config_processor.stages, user,
'success', trigger_request) && save
end
def create_next_builds(build)
......@@ -115,10 +118,10 @@ module Ci
prior_builds = latest_builds.where.not(stage: next_stages)
prior_status = prior_builds.status
# create builds for next stages based
next_stages.any? do |stage|
CreateBuildsService.new(self).execute(stage, build.user, prior_status, build.trigger_request).present?
end
# build builds for next stage that has builds available
# and save pipeline if we have builds
build_builds_for_stages(next_stages, build.user, prior_status,
build.trigger_request) && save
end
def retried
......@@ -139,10 +142,10 @@ module Ci
@config_processor ||= begin
Ci::GitlabCiYamlProcessor.new(ci_yaml_file, project.path_with_namespace)
rescue Ci::GitlabCiYamlProcessor::ValidationError, Psych::SyntaxError => e
save_yaml_error(e.message)
self.yaml_errors = e.message
nil
rescue
save_yaml_error("Undefined error")
self.yaml_errors = 'Undefined error'
nil
end
end
......@@ -169,6 +172,17 @@ module Ci
private
def build_builds_for_stages(stages, user, status, trigger_request)
##
# Note that `Array#any?` implements a short circuit evaluation, so we
# build builds only for the first stage that has builds available.
#
stages.any? do |stage|
CreateBuildsService.new(self)
.execute(stage, user, status, trigger_request).present?
end
end
def update_state
statuses.reload
self.status = if yaml_errors.blank?
......@@ -181,11 +195,5 @@ module Ci
self.duration = statuses.latest.duration
save
end
def save_yaml_error(error)
return if self.yaml_errors?
self.yaml_errors = error
update_state
end
end
end
......@@ -2,10 +2,11 @@ module Ci
class CreateBuildsService
def initialize(pipeline)
@pipeline = pipeline
@config = pipeline.config_processor
end
def execute(stage, user, status, trigger_request = nil)
builds_attrs = config_processor.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request)
builds_attrs = @config.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request)
# check when to create next build
builds_attrs = builds_attrs.select do |build_attrs|
......@@ -19,10 +20,15 @@ module Ci
end
end
builds_attrs.map do |build_attrs|
# don't create the same build twice
unless @pipeline.builds.find_by(ref: @pipeline.ref, tag: @pipeline.tag,
trigger_request: trigger_request, name: build_attrs[:name])
builds_attrs.reject! do |build_attrs|
@pipeline.builds.find_by(ref: @pipeline.ref,
tag: @pipeline.tag,
trigger_request: trigger_request,
name: build_attrs[:name])
end
builds_attrs.map do |build_attrs|
build_attrs.slice!(:name,
:commands,
:tag_list,
......@@ -32,21 +38,19 @@ module Ci
:stage_idx,
:environment)
build_attrs.merge!(ref: @pipeline.ref,
build_attrs.merge!(pipeline: @pipeline,
ref: @pipeline.ref,
tag: @pipeline.tag,
trigger_request: trigger_request,
user: user,
project: @pipeline.project)
@pipeline.builds.create!(build_attrs)
end
end
##
# We do not persist new builds here.
# Those will be persisted when @pipeline is saved.
#
@pipeline.builds.new(build_attrs)
end
private
def config_processor
@config_processor ||= @pipeline.config_processor
end
end
end
......@@ -8,7 +8,9 @@ module Ci
return pipeline
end
unless commit
if commit
pipeline.sha = commit.id
else
pipeline.errors.add(:base, 'Commit not found')
return pipeline
end
......@@ -18,22 +20,18 @@ module Ci
return pipeline
end
begin
Ci::Pipeline.transaction do
pipeline.sha = commit.id
unless pipeline.config_processor
pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file')
raise ActiveRecord::Rollback
return pipeline
end
pipeline.save!
pipeline.create_builds(current_user)
end
rescue
pipeline.errors.add(:base, 'The pipeline could not be created. Please try again.')
unless pipeline.create_builds(current_user)
pipeline.errors.add(:base, 'No builds for this pipeline.')
end
pipeline.save
pipeline
end
......
class CreateCommitBuildsService
def execute(project, user, params)
return false unless project.builds_enabled?
return unless project.builds_enabled?
before_sha = params[:checkout_sha] || params[:before]
sha = params[:checkout_sha] || params[:after]
origin_ref = params[:ref]
unless origin_ref && sha.present?
return false
end
ref = Gitlab::Git.ref_name(origin_ref)
tag = Gitlab::Git.tag_ref?(origin_ref)
......@@ -18,23 +14,50 @@ class CreateCommitBuildsService
return false
end
pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
@pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
# Skip creating pipeline when no gitlab-ci.yml is found
unless pipeline.ci_yaml_file
##
# Skip creating pipeline if no gitlab-ci.yml is found
#
unless @pipeline.ci_yaml_file
return false
end
# Create a new pipeline
pipeline.save!
##
# Skip creating builds for commits that have [ci skip]
unless pipeline.skip_ci?
# Create builds for commit
pipeline.create_builds(user)
# but save pipeline object
#
if @pipeline.skip_ci?
return save_pipeline!
end
##
# Skip creating builds when CI config is invalid
# but save pipeline object
#
unless @pipeline.config_processor
return save_pipeline!
end
pipeline.touch
pipeline
##
# Skip creating pipeline object if there are no builds for it.
#
unless @pipeline.create_builds(user)
@pipeline.errors.add(:base, 'No builds created')
return false
end
save_pipeline!
end
private
##
# Create a new pipeline and touch object to calculate status
#
def save_pipeline!
@pipeline.save!
@pipeline.touch
@pipeline
end
end
......@@ -30,7 +30,10 @@ module Ci
end
def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
builds.select{|build| build[:stage] == stage && process?(build[:only], build[:except], ref, tag, trigger_request)}
builds.select do |build|
build[:stage] == stage &&
process?(build[:only], build[:except], ref, tag, trigger_request)
end
end
def builds
......
......@@ -258,6 +258,19 @@ describe Ci::Pipeline, models: true do
end
end
end
context 'when no builds created' do
let(:pipeline) { build(:ci_pipeline) }
before do
stub_ci_pipeline_yaml_file(YAML.dump(before_script: ['ls']))
end
it 'returns false' do
expect(pipeline.create_builds(nil)).to be_falsey
expect(pipeline).not_to be_persisted
end
end
end
describe "#finished_at" do
......
......@@ -9,7 +9,7 @@ describe Ci::CreateBuildsService, services: true do
#
subject do
described_class.new(pipeline).execute('test', nil, user, status)
described_class.new(pipeline).execute('test', user, status, nil)
end
context 'next builds available' do
......@@ -17,6 +17,10 @@ describe Ci::CreateBuildsService, services: true do
it { is_expected.to be_an_instance_of Array }
it { is_expected.to all(be_an_instance_of Ci::Build) }
it 'does not persist created builds' do
expect(subject.first).not_to be_persisted
end
end
context 'builds skipped' do
......
......@@ -39,7 +39,7 @@ describe CreateCommitBuildsService, services: true do
end
it "creates commit if there is no appropriate job but deploy job has right ref setting" do
config = YAML.dump({ deploy: { deploy: "ls", only: ["0_1"] } })
config = YAML.dump({ deploy: { script: "ls", only: ["0_1"] } })
stub_ci_pipeline_yaml_file(config)
result = service.execute(project, user,
......@@ -81,7 +81,7 @@ describe CreateCommitBuildsService, services: true do
expect(pipeline.yaml_errors).not_to be_nil
end
describe :ci_skip? do
context 'when commit contains a [ci skip] directive' do
let(:message) { "some message[ci skip]" }
before do
......@@ -171,5 +171,24 @@ describe CreateCommitBuildsService, services: true do
expect(pipeline.status).to eq("failed")
expect(pipeline.builds.any?).to be false
end
context 'when there are no jobs for this pipeline' do
before do
config = YAML.dump({ test: { script: 'ls', only: ['feature'] } })
stub_ci_pipeline_yaml_file(config)
end
it 'does not create a new pipeline' do
result = service.execute(project, user,
ref: 'refs/heads/master',
before: '00000000',
after: '31das312',
commits: [{ message: 'some msg' }])
expect(result).to be_falsey
expect(Ci::Build.all).to be_empty
expect(Ci::Pipeline.count).to eq(0)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment