diff --git a/CHANGELOG b/CHANGELOG
index 39532e88138f5d13c8d48d8eca8afedd6ed90dc8..66ed3f4afb99988114c122d347fbc7273b60c268 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
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.
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 4bbfb4cc8067beec300592fe2528ae0e910b3402..a5f2ac59001876e655123f1f1d38d147e31db366 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -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
diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb
index 3a74ae094e81e5061df6040e9263f9bbdbe7d963..2dcb052d274e25513203989c509582900a7ff18f 100644
--- a/app/services/ci/create_builds_service.rb
+++ b/app/services/ci/create_builds_service.rb
@@ -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,34 +20,37 @@ module Ci
end
end
+ # don't create the same build twice
+ 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|
- # 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])
- build_attrs.slice!(:name,
- :commands,
- :tag_list,
- :options,
- :allow_failure,
- :stage,
- :stage_idx,
- :environment)
+ build_attrs.slice!(:name,
+ :commands,
+ :tag_list,
+ :options,
+ :allow_failure,
+ :stage,
+ :stage_idx,
+ :environment)
- build_attrs.merge!(ref: @pipeline.ref,
- tag: @pipeline.tag,
- trigger_request: trigger_request,
- user: user,
- project: @pipeline.project)
+ 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
+ ##
+ # We do not persist new builds here.
+ # Those will be persisted when @pipeline is saved.
+ #
+ @pipeline.builds.new(build_attrs)
end
end
-
- private
-
- def config_processor
- @config_processor ||= @pipeline.config_processor
- end
end
end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index a7751b8effcb9b9e7271b7fe8ae54fb944ae49ac..b1ee68741902327a40c346f436d197147b7945d7 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -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')
+ return pipeline
+ end
- unless pipeline.config_processor
- pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file')
- raise ActiveRecord::Rollback
- end
+ pipeline.save!
- 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
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 418f5cf809145518bbc54c999b1cb5a07f61107b..f947e8f452eafb64cd82380fd7249ab30c8298c6 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -1,15 +1,11 @@
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
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 68246497e90af470424c61dcfdbe59277b3aee2c..a66602f919490f64fda7860dd8b7b2443cb56a23 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -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
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 0d769ed7324c03ce26b2a4389e2522370a92f310..34507cf508323b9ce13b95cbfd5bbf14b026d31b 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -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
diff --git a/spec/services/ci/create_builds_service_spec.rb b/spec/services/ci/create_builds_service_spec.rb
index 984b78487d410d652ea880e9f8b8beddbd2b4fb2..8b0becd83d3805ceb6a47ae87cea2c26fb576455 100644
--- a/spec/services/ci/create_builds_service_spec.rb
+++ b/spec/services/ci/create_builds_service_spec.rb
@@ -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
diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb
index a5b4d9f05de8c6b378189c66b4e5ca76506c87dd..deab242f45a577b63eaf69508876cb9f1b54b23e 100644
--- a/spec/services/create_commit_builds_service_spec.rb
+++ b/spec/services/create_commit_builds_service_spec.rb
@@ -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