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.

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