diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-02 16:32:52 +0200 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-02 18:41:40 +0200 |
commit | 684751d3c2233ee1ac33cf623e8b7822c60209d3 (patch) | |
tree | 5696d3f3e6a99a806ff185f3d9c11c22e50db92c | |
parent | 8156e77c1a25bc6050e5036fa3bbfd29201a6d5c (diff) | |
download | gitlab-ce-684751d3c2233ee1ac33cf623e8b7822c60209d3.tar.gz |
Make needs: to require previous jobsmake-needs-strong-connection
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.
-rw-r--r-- | app/models/ci/pipeline.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/seed/build.rb | 31 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/seed/stage.rb | 19 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/seed/build_spec.rb | 38 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 56 |
7 files changed, 151 insertions, 20 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3515f0b83ee..ffab4e82f90 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -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?) diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index d8296940a04..ab0d4c38ab6 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -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? diff --git a/lib/gitlab/ci/pipeline/seed/stage.rb b/lib/gitlab/ci/pipeline/seed/stage.rb index 9c15064756a..7c737027445 100644 --- a/lib/gitlab/ci/pipeline/seed/stage.rb +++ b/lib/gitlab/ci/pipeline/seed/stage.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 417a2d119ff..9bccd5be4fe 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 46ea0d7554b..762025f9bd9 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index ad864d0d56e..6fba9f37d91 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -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! diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d9b61dfe503..7e2f311a065 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -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 |