Commit 93e95182 authored by Kamil Trzciński's avatar Kamil Trzciński

Require `needs:` to be present

This changes the `needs:` logic to require
that all jobs to be present. Instead of skipping
do fail the pipeline creation if `needs:` dependency
is not found.
parent 583544d0
...@@ -23,12 +23,17 @@ module Gitlab ...@@ -23,12 +23,17 @@ module Gitlab
@command.seeds_block&.call(pipeline) @command.seeds_block&.call(pipeline)
## ##
# Populate pipeline with all stages, and stages with builds. # Gather all runtime build/stage errors
# #
pipeline.stage_seeds.each do |stage| if seeds_errors = pipeline.stage_seeds.flat_map(&:errors).compact.presence
pipeline.stages << stage.to_resource return error(seeds_errors.join("\n"))
end end
##
# Populate pipeline with all stages, and stages with builds.
#
pipeline.stages = pipeline.stage_seeds.map(&:to_resource)
if pipeline.stages.none? if pipeline.stages.none?
return error('No stages / jobs for this pipeline.') return error('No stages / jobs for this pipeline.')
end end
......
...@@ -13,6 +13,10 @@ module Gitlab ...@@ -13,6 +13,10 @@ module Gitlab
raise NotImplementedError raise NotImplementedError
end end
def errors
raise NotImplementedError
end
def to_resource def to_resource
raise NotImplementedError raise NotImplementedError
end end
......
...@@ -13,6 +13,7 @@ module Gitlab ...@@ -13,6 +13,7 @@ module Gitlab
@pipeline = pipeline @pipeline = pipeline
@attributes = attributes @attributes = attributes
@previous_stages = previous_stages @previous_stages = previous_stages
@needs_attributes = dig(:needs_attributes)
@only = Gitlab::Ci::Build::Policy @only = Gitlab::Ci::Build::Policy
.fabricate(attributes.delete(:only)) .fabricate(attributes.delete(:only))
...@@ -27,8 +28,15 @@ module Gitlab ...@@ -27,8 +28,15 @@ module Gitlab
def included? def included?
strong_memoize(:inclusion) do strong_memoize(:inclusion) do
all_of_only? && all_of_only? &&
none_of_except? && none_of_except?
all_of_needs? end
end
def errors
return unless included?
strong_memoize(:errors) do
needs_errors
end end
end end
...@@ -48,6 +56,18 @@ module Gitlab ...@@ -48,6 +56,18 @@ module Gitlab
@attributes.to_h.dig(:options, :trigger).present? @attributes.to_h.dig(:options, :trigger).present?
end end
def to_resource
strong_memoize(:resource) do
if bridge?
::Ci::Bridge.new(attributes)
else
::Ci::Build.new(attributes)
end
end
end
private
def all_of_only? def all_of_only?
@only.all? { |spec| spec.satisfied_by?(@pipeline, self) } @only.all? { |spec| spec.satisfied_by?(@pipeline, self) }
end end
...@@ -56,25 +76,17 @@ module Gitlab ...@@ -56,25 +76,17 @@ module Gitlab
@except.none? { |spec| spec.satisfied_by?(@pipeline, self) } @except.none? { |spec| spec.satisfied_by?(@pipeline, self) }
end end
def all_of_needs? def needs_errors
return true unless Feature.enabled?(:ci_dag_support, @pipeline.project) return unless Feature.enabled?(:ci_dag_support, @pipeline.project)
return true if dig(:needs_attributes).nil? return if @needs_attributes.nil?
dig(:needs_attributes).all? do |need| @needs_attributes.flat_map do |need|
@previous_stages.any? do |stage| result = @previous_stages.any? do |stage|
stage.seeds_names.include?(need[:name]) stage.seeds_names.include?(need[:name])
end end
end
end
def to_resource "#{name}: needs '#{need[:name]}'" unless result
strong_memoize(:resource) do end.compact
if bridge?
::Ci::Bridge.new(attributes)
else
::Ci::Build.new(attributes)
end
end
end end
end end
end end
......
...@@ -33,6 +33,12 @@ module Gitlab ...@@ -33,6 +33,12 @@ module Gitlab
end end
end end
def errors
strong_memoize(:errors) do
seeds.flat_map(&:errors).compact
end
end
def seeds_names def seeds_names
strong_memoize(:seeds_names) do strong_memoize(:seeds_names) do
seeds.map(&:name).to_set seeds.map(&:name).to_set
......
...@@ -396,7 +396,14 @@ describe Gitlab::Ci::Pipeline::Seed::Build do ...@@ -396,7 +396,14 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
end end
context 'when build job is not present in prior stages' do context 'when build job is not present in prior stages' do
it { is_expected.not_to be_included } it "is included" do
is_expected.to be_included
end
it "returns an error" do
expect(subject.errors).to contain_exactly(
"rspec: needs 'build'")
end
end end
context 'when build job is part of prior stages' do context 'when build job is part of prior stages' do
...@@ -414,7 +421,13 @@ describe Gitlab::Ci::Pipeline::Seed::Build do ...@@ -414,7 +421,13 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
let(:previous_stages) { [stage_seed] } let(:previous_stages) { [stage_seed] }
it { is_expected.to be_included } it "is included" do
is_expected.to be_included
end
it "does not have errors" do
expect(subject.errors).to be_empty
end
end end
end end
end end
...@@ -121,6 +121,16 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do ...@@ -121,6 +121,16 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do
end end
end end
describe '#seeds_errors' do
it 'returns all errors from seeds' do
expect(subject.seeds.first)
.to receive(:errors) { ["build error"] }
expect(subject.errors).to contain_exactly(
"build error")
end
end
describe '#to_resource' do describe '#to_resource' do
it 'builds a valid stage object with all builds' do it 'builds a valid stage object with all builds' do
subject.to_resource.save! subject.to_resource.save!
......
...@@ -1113,7 +1113,7 @@ describe Ci::CreatePipelineService do ...@@ -1113,7 +1113,7 @@ describe Ci::CreatePipelineService do
test_a: { test_a: {
stage: "test", stage: "test",
script: "ls", script: "ls",
only: %w[master feature tags], only: %w[master feature],
needs: %w[build_a] needs: %w[build_a]
}, },
deploy: { deploy: {
...@@ -1143,6 +1143,7 @@ describe Ci::CreatePipelineService do ...@@ -1143,6 +1143,7 @@ describe Ci::CreatePipelineService do
it 'does not create a pipeline as test_a depends on build_a' do it 'does not create a pipeline as test_a depends on build_a' do
expect(pipeline).not_to be_persisted expect(pipeline).not_to be_persisted
expect(pipeline.builds).to be_empty expect(pipeline.builds).to be_empty
expect(pipeline.errors[:base]).to contain_exactly("test_a: needs 'build_a'")
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