Make needs: to require previous jobs
This changes `needs:` from weak reference to have a strong reference. This means that job will not be created unless all needs are present as part of a pipeline.
This commit is contained in:
		
							parent
							
								
									8156e77c1a
								
							
						
					
					
						commit
						684751d3c2
					
				|  | @ -504,8 +504,9 @@ module Ci | |||
|       return [] unless config_processor | ||||
| 
 | ||||
|       strong_memoize(:stage_seeds) do | ||||
|         seeds = config_processor.stages_attributes.map do |attributes| | ||||
|           Gitlab::Ci::Pipeline::Seed::Stage.new(self, attributes) | ||||
|         seeds = config_processor.stages_attributes.inject([]) do |previous_stages, attributes| | ||||
|           seed = Gitlab::Ci::Pipeline::Seed::Stage.new(self, attributes, previous_stages) | ||||
|           previous_stages + [seed] | ||||
|         end | ||||
| 
 | ||||
|         seeds.select(&:included?) | ||||
|  |  | |||
|  | @ -9,9 +9,10 @@ module Gitlab | |||
| 
 | ||||
|           delegate :dig, to: :@attributes | ||||
| 
 | ||||
|           def initialize(pipeline, attributes) | ||||
|           def initialize(pipeline, attributes, previous_stages) | ||||
|             @pipeline = pipeline | ||||
|             @attributes = attributes | ||||
|             @previous_stages = previous_stages | ||||
| 
 | ||||
|             @only = Gitlab::Ci::Build::Policy | ||||
|               .fabricate(attributes.delete(:only)) | ||||
|  | @ -19,10 +20,15 @@ module Gitlab | |||
|               .fabricate(attributes.delete(:except)) | ||||
|           end | ||||
| 
 | ||||
|           def name | ||||
|             dig(:name) | ||||
|           end | ||||
| 
 | ||||
|           def included? | ||||
|             strong_memoize(:inclusion) do | ||||
|               @only.all? { |spec| spec.satisfied_by?(@pipeline, self) } && | ||||
|                 @except.none? { |spec| spec.satisfied_by?(@pipeline, self) } | ||||
|               all_of_only? && | ||||
|                 none_of_except? && | ||||
|                 all_of_needs? | ||||
|             end | ||||
|           end | ||||
| 
 | ||||
|  | @ -42,6 +48,25 @@ module Gitlab | |||
|             @attributes.to_h.dig(:options, :trigger).present? | ||||
|           end | ||||
| 
 | ||||
|           def all_of_only? | ||||
|             @only.all? { |spec| spec.satisfied_by?(@pipeline, self) } | ||||
|           end | ||||
| 
 | ||||
|           def none_of_except? | ||||
|             @except.none? { |spec| spec.satisfied_by?(@pipeline, self) } | ||||
|           end | ||||
| 
 | ||||
|           def all_of_needs? | ||||
|             return true unless Feature.enabled?(:ci_dag_support, @pipeline.project) | ||||
|             return true if dig(:needs_attributes).nil? | ||||
| 
 | ||||
|             dig(:needs_attributes).all? do |need| | ||||
|               @previous_stages.any? do |stage| | ||||
|                 stage.seeds_names.include?(need[:name]) | ||||
|               end | ||||
|             end | ||||
|           end | ||||
| 
 | ||||
|           def to_resource | ||||
|             strong_memoize(:resource) do | ||||
|               if bridge? | ||||
|  |  | |||
|  | @ -10,12 +10,13 @@ module Gitlab | |||
|           delegate :size, to: :seeds | ||||
|           delegate :dig, to: :seeds | ||||
| 
 | ||||
|           def initialize(pipeline, attributes) | ||||
|           def initialize(pipeline, attributes, previous_stages) | ||||
|             @pipeline = pipeline | ||||
|             @attributes = attributes | ||||
|             @previous_stages = previous_stages | ||||
| 
 | ||||
|             @builds = attributes.fetch(:builds).map do |attributes| | ||||
|               Seed::Build.new(@pipeline, attributes) | ||||
|               Seed::Build.new(@pipeline, attributes, previous_stages) | ||||
|             end | ||||
|           end | ||||
| 
 | ||||
|  | @ -32,6 +33,12 @@ module Gitlab | |||
|             end | ||||
|           end | ||||
| 
 | ||||
|           def seeds_names | ||||
|             strong_memoize(:seeds_names) do | ||||
|               seeds.map(&:name).to_set | ||||
|             end | ||||
|           end | ||||
| 
 | ||||
|           def included? | ||||
|             seeds.any? | ||||
|           end | ||||
|  | @ -39,13 +46,7 @@ module Gitlab | |||
|           def to_resource | ||||
|             strong_memoize(:stage) do | ||||
|               ::Ci::Stage.new(attributes).tap do |stage| | ||||
|                 seeds.each do |seed| | ||||
|                   if seed.bridge? | ||||
|                     stage.bridges << seed.to_resource | ||||
|                   else | ||||
|                     stage.builds << seed.to_resource | ||||
|                   end | ||||
|                 end | ||||
|                 stage.statuses = seeds.map(&:to_resource) | ||||
|               end | ||||
|             end | ||||
|           end | ||||
|  |  | |||
|  | @ -38,8 +38,8 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do | |||
|     it 'populates pipeline with stages' do | ||||
|       expect(pipeline.stages).to be_one | ||||
|       expect(pipeline.stages.first).not_to be_persisted | ||||
|       expect(pipeline.stages.first.builds).to be_one | ||||
|       expect(pipeline.stages.first.builds.first).not_to be_persisted | ||||
|       expect(pipeline.stages.first.statuses).to be_one | ||||
|       expect(pipeline.stages.first.statuses.first).not_to be_persisted | ||||
|     end | ||||
| 
 | ||||
|     it 'correctly assigns user' do | ||||
|  | @ -191,8 +191,8 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do | |||
|         step.perform! | ||||
| 
 | ||||
|         expect(pipeline.stages.size).to eq 1 | ||||
|         expect(pipeline.stages.first.builds.size).to eq 1 | ||||
|         expect(pipeline.stages.first.builds.first.name).to eq 'rspec' | ||||
|         expect(pipeline.stages.first.statuses.size).to eq 1 | ||||
|         expect(pipeline.stages.first.statuses.first.name).to eq 'rspec' | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -6,8 +6,9 @@ describe Gitlab::Ci::Pipeline::Seed::Build do | |||
|   let(:project) { create(:project, :repository) } | ||||
|   let(:pipeline) { create(:ci_empty_pipeline, project: project) } | ||||
|   let(:attributes) { { name: 'rspec', ref: 'master' } } | ||||
|   let(:previous_stages) { [] } | ||||
| 
 | ||||
|   let(:seed_build) { described_class.new(pipeline, attributes) } | ||||
|   let(:seed_build) { described_class.new(pipeline, attributes, previous_stages) } | ||||
| 
 | ||||
|   describe '#attributes' do | ||||
|     subject { seed_build.attributes } | ||||
|  | @ -381,4 +382,39 @@ describe Gitlab::Ci::Pipeline::Seed::Build do | |||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe 'applying needs: dependency' do | ||||
|     subject { seed_build } | ||||
| 
 | ||||
|     let(:attributes) do | ||||
|       { | ||||
|         name: 'rspec', | ||||
|         needs_attributes: [{ | ||||
|           name: 'build' | ||||
|         }] | ||||
|       } | ||||
|     end | ||||
| 
 | ||||
|     context 'when build job is not present in prior stages' do | ||||
|       it { is_expected.not_to be_included } | ||||
|     end | ||||
| 
 | ||||
|     context 'when build job is part of prior stages' do | ||||
|       let(:stage_attributes) do | ||||
|         { | ||||
|           name: 'build', | ||||
|           index: 0, | ||||
|           builds: [{ name: 'build' }] | ||||
|         } | ||||
|       end | ||||
| 
 | ||||
|       let(:stage_seed) do | ||||
|         Gitlab::Ci::Pipeline::Seed::Stage.new(pipeline, stage_attributes, []) | ||||
|       end | ||||
| 
 | ||||
|       let(:previous_stages) { [stage_seed] } | ||||
| 
 | ||||
|       it { is_expected.to be_included } | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -5,6 +5,7 @@ require 'spec_helper' | |||
| describe Gitlab::Ci::Pipeline::Seed::Stage do | ||||
|   let(:project) { create(:project, :repository) } | ||||
|   let(:pipeline) { create(:ci_empty_pipeline, project: project) } | ||||
|   let(:previous_stages) { [] } | ||||
| 
 | ||||
|   let(:attributes) do | ||||
|     { name: 'test', | ||||
|  | @ -15,7 +16,7 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do | |||
|   end | ||||
| 
 | ||||
|   subject do | ||||
|     described_class.new(pipeline, attributes) | ||||
|     described_class.new(pipeline, attributes, previous_stages) | ||||
|   end | ||||
| 
 | ||||
|   describe '#size' do | ||||
|  | @ -109,6 +110,17 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '#seeds_names' do | ||||
|     it 'returns all job names' do | ||||
|       expect(subject.seeds_names).to contain_exactly( | ||||
|         'rspec', 'spinach') | ||||
|     end | ||||
| 
 | ||||
|     it 'returns a set' do | ||||
|       expect(subject.seeds_names).to be_a(Set) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '#to_resource' do | ||||
|     it 'builds a valid stage object with all builds' do | ||||
|       subject.to_resource.save! | ||||
|  |  | |||
|  | @ -1099,6 +1099,62 @@ describe Ci::CreatePipelineService do | |||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'when needs is used' do | ||||
|       let(:pipeline) { execute_service } | ||||
| 
 | ||||
|       let(:config) do | ||||
|         { | ||||
|           build_a: { | ||||
|             stage: "build", | ||||
|             script: "ls", | ||||
|             only: %w[master] | ||||
|           }, | ||||
|           test_a: { | ||||
|             stage: "test", | ||||
|             script: "ls", | ||||
|             only: %w[master feature tags], | ||||
|             needs: %w[build_a] | ||||
|           }, | ||||
|           deploy: { | ||||
|             stage: "deploy", | ||||
|             script: "ls", | ||||
|             only: %w[tags] | ||||
|           } | ||||
|         } | ||||
|       end | ||||
| 
 | ||||
|       before do | ||||
|         stub_ci_pipeline_yaml_file(YAML.dump(config)) | ||||
|       end | ||||
| 
 | ||||
|       context 'when pipeline on master is created' do | ||||
|         let(:ref_name) { 'refs/heads/master' } | ||||
| 
 | ||||
|         it 'creates a pipeline with build_a and test_a' do | ||||
|           expect(pipeline).to be_persisted | ||||
|           expect(pipeline.builds.map(&:name)).to contain_exactly("build_a", "test_a") | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       context 'when pipeline on feature is created' do | ||||
|         let(:ref_name) { 'refs/heads/feature' } | ||||
| 
 | ||||
|         it 'does not create a pipeline as test_a depends on build_a' do | ||||
|           expect(pipeline).not_to be_persisted | ||||
|           expect(pipeline.builds).to be_empty | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       context 'when pipeline on v1.0.0 is created' do | ||||
|         let(:ref_name) { 'refs/tags/v1.0.0' } | ||||
| 
 | ||||
|         it 'does create a pipeline only with deploy' do | ||||
|           expect(pipeline).to be_persisted | ||||
|           expect(pipeline.builds.map(&:name)).to contain_exactly("deploy") | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '#execute!' do | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue