Merge branch '30680-job-needs-optional' into 'master'

Implement needs:job:optional for CI pipelines [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55468

app/models/ci/build_need.rb

@@ -10,6 +10,7 @@ module Ci
 
     validates :build, presence: true
     validates :name, presence: true, length: { maximum: 128 }
+    validates :optional, inclusion: { in: [true, false] }
 
     scope :scoped_build, -> { where('ci_builds.id=ci_build_needs.build_id') }
     scope :artifacts, -> { where(artifacts: true) }

changelogs/unreleased/30680-job-needs-optional.yml

+---
+title: Implement needs:job:optional for CI pipelines
+merge_request: 55468
+author:
+type: added

config/feature_flags/development/ci_needs_optional.yml

+---
+name: ci_needs_optional
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55468
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323891
+milestone: '13.10'
+type: development
+group: group::pipeline authoring
+default_enabled: false

db/migrate/20210301193412_add_optional_to_ci_build_needs.rb

+# frozen_string_literal: true
+
+class AddOptionalToCiBuildNeeds < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :ci_build_needs, :optional, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :ci_build_needs, :optional
+    end
+  end
+end

db/schema_migrations/20210301193412

+2929e4796e85fa6cf8b5950fb57295ae87c48c914d0a71123a29d579d797d636
\ No newline at end of file

db/structure.sql

@@ -10299,7 +10299,8 @@ CREATE TABLE ci_build_needs (
     id integer NOT NULL,
     build_id integer NOT NULL,
     name text NOT NULL,
-    artifacts boolean DEFAULT true NOT NULL
+    artifacts boolean DEFAULT true NOT NULL,
+    optional boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE ci_build_needs_id_seq

doc/ci/yaml/README.md

@@ -1950,9 +1950,8 @@ production:
 
 #### Requirements and limitations
 
-- If `needs:` is set to point to a job that is not instantiated
-  because of `only/except` rules or otherwise does not exist, the
-  pipeline is not created and a YAML error is shown.
+- In GitLab 13.9 and older, if `needs:` refers to a job that might not be added to
+  a pipeline because of `only`, `except`, or `rules`, the pipeline might fail to create.
 - The maximum number of jobs that a single job can need in the `needs:` array is limited:
   - For GitLab.com, the limit is 50. For more information, see our
     [infrastructure issue](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/7541).
@@ -2143,6 +2142,68 @@ in the same parent-child pipeline hierarchy of the given pipeline.
 The `pipeline` attribute does not accept the current pipeline ID (`$CI_PIPELINE_ID`).
 To download artifacts from a job in the current pipeline, use the basic form of [`needs`](#artifact-downloads-with-needs).
 
+#### Optional `needs`
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/30680) in GitLab 13.10.
+> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
+> - It's disabled on GitLab.com.
+> - It's not recommended for production use.
+> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-optional-needs). **(FREE SELF)**
+
+WARNING:
+This feature might not be available to you. Check the **version history** note above for details.
+
+To need a job that sometimes does not exist in the pipeline, add `optional: true`
+to the `needs` configuration. If not defined, `optional: false` is the default.
+
+Jobs that use [`rules`](#rules), [`only`, or `except`](#onlyexcept-basic), might
+not always exist in a pipeline. When the pipeline starts, it checks the `needs`
+relationships before running. Without `optional: true`, needs relationships that
+point to a job that does not exist stops the pipeline from starting and causes a pipeline
+error similar to:
+
+- `'job1' job needs 'job2' job, but it was not added to the pipeline`
+
+In this example:
+
+- When the branch is `master`, the `build` job exists in the pipeline, and the `rspec`
+  job waits for it to complete before starting.
+- When the branch is not `master`, the `build` job does not exist in the pipeline.
+  The `rspec` job runs immediately (similar to `needs: []`) because its `needs`
+  relationship to the `build` job is optional.
+
+```yaml
+build:
+  stage: build
+  rules:
+    - if: $CI_COMMIT_REF_NAME == "master"
+
+rspec:
+  stage: test
+  needs:
+    - job: build
+      optional: true
+```
+
+#### Enable or disable optional needs **(FREE SELF)**
+
+Optional needs is under development and not ready for production use. It is
+deployed behind a feature flag that is **disabled by default**.
+[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
+can enable it.
+
+To enable it:
+
+```ruby
+Feature.enable(:ci_needs_optional)
+```
+
+To disable it:
+
+```ruby
+Feature.disable(:ci_needs_optional)
+```
+
 ### `tags`
 
 Use `tags` to select a specific runner from the list of all runners that are

ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb

@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do
         it 'is returns a bridge job configuration' do
           expect(subject.value).to eq(name: :my_bridge,
                                       trigger: { project: 'some/project' },
-                                      needs: { job: [{ name: 'some_job', artifacts: true }] },
+                                      needs: { job: [{ name: 'some_job', artifacts: true, optional: false }] },
                                       ignore: false,
                                       stage: 'test',
                                       only: { refs: %w[branches tags] },

ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb

@@ -141,8 +141,8 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do
         it 'returns key value' do
           expect(needs.value).to eq(
             job: [
-              { name: 'first_job_name',  artifacts: true },
-              { name: 'second_job_name', artifacts: false }
+              { name: 'first_job_name',  artifacts: true, optional: false },
+              { name: 'second_job_name', artifacts: false, optional: false }
             ],
             bridge: [{ pipeline: 'some/project' }],
             cross_dependency: [

ee/spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -74,7 +74,7 @@ RSpec.describe Gitlab::Ci::YamlProcessor do
             bridge_needs: { pipeline: 'some/project' }
           },
           needs_attributes: [
-            { name: "build", artifacts: true }
+            { name: "build", artifacts: true, optional: false }
           ],
           when: "on_success",
           allow_failure: false,
@@ -147,7 +147,7 @@ RSpec.describe Gitlab::Ci::YamlProcessor do
             ]
           },
           needs_attributes: [
-            { name: 'build', artifacts: true }
+            { name: 'build', artifacts: true, optional: false }
           ],
           only: { refs: %w[branches tags] },
           when: 'on_success',

lib/gitlab/ci/config/entry/need.rb

@@ -35,7 +35,14 @@ module Gitlab
             end
 
             def value
-              { name: @config, artifacts: true }
+              if ::Feature.enabled?(:ci_needs_optional, default_enabled: :yaml)
+                { name: @config,
+                  artifacts: true,
+                  optional: false }
+              else
+                { name: @config,
+                  artifacts: true }
+              end
             end
           end
 
@@ -43,14 +50,15 @@ module Gitlab
             include ::Gitlab::Config::Entry::Validatable
             include ::Gitlab::Config::Entry::Attributable
 
-            ALLOWED_KEYS = %i[job artifacts].freeze
-            attributes :job, :artifacts
+            ALLOWED_KEYS = %i[job artifacts optional].freeze
+            attributes :job, :artifacts, :optional
 
             validations do
               validates :config, presence: true
               validates :config, allowed_keys: ALLOWED_KEYS
               validates :job, type: String, presence: true
               validates :artifacts, boolean: true, allow_nil: true
+              validates :optional, boolean: true, allow_nil: true
             end
 
             def type
@@ -58,7 +66,14 @@ module Gitlab
             end
 
             def value
-              { name: job, artifacts: artifacts || artifacts.nil? }
+              if ::Feature.enabled?(:ci_needs_optional, default_enabled: :yaml)
+                { name: job,
+                  artifacts: artifacts || artifacts.nil?,
+                  optional: !!optional }
+              else
+                { name: job,
+                  artifacts: artifacts || artifacts.nil? }
+              end
             end
           end
 

lib/gitlab/ci/pipeline/seed/build.rb

@@ -141,6 +141,8 @@ module Gitlab
             end
 
             @needs_attributes.flat_map do |need|
+              next if ::Feature.enabled?(:ci_needs_optional, default_enabled: :yaml) && need[:optional]
+
               result = @previous_stages.any? do |stage|
                 stage.seeds_names.include?(need[:name])
               end

spec/lib/gitlab/ci/config/entry/need_spec.rb

@@ -23,9 +23,19 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do
 
       describe '#value' do
         it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
+        end
+
+        context 'when the FF ci_needs_optional is disabled' do
+          before do
+            stub_feature_flags(ci_needs_optional: false)
+          end
+
+          it 'returns job needs configuration without `optional`' do
             expect(need.value).to eq(name: 'job_name', artifacts: true)
           end
         end
+      end
 
       it_behaves_like 'job type'
     end
@@ -58,7 +68,7 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do
 
       describe '#value' do
         it 'returns job needs configuration' do
-          expect(need.value).to eq(name: 'job_name', artifacts: true)
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
         end
       end
 
@@ -74,7 +84,7 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do
 
       describe '#value' do
         it 'returns job needs configuration' do
-          expect(need.value).to eq(name: 'job_name', artifacts: false)
+          expect(need.value).to eq(name: 'job_name', artifacts: false, optional: false)
         end
       end
 
@@ -90,7 +100,7 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do
 
       describe '#value' do
         it 'returns job needs configuration' do
-          expect(need.value).to eq(name: 'job_name', artifacts: true)
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
         end
       end
 
@@ -106,11 +116,77 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do
 
       describe '#value' do
         it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
+        end
+      end
+
+      it_behaves_like 'job type'
+    end
+
+    context 'with job name and optional true' do
+      let(:config) { { job: 'job_name', optional: true } }
+
+      it { is_expected.to be_valid }
+
+      it_behaves_like 'job type'
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: true)
+        end
+
+        context 'when the FF ci_needs_optional is disabled' do
+          before do
+            stub_feature_flags(ci_needs_optional: false)
+          end
+
+          it 'returns job needs configuration without `optional`' do
             expect(need.value).to eq(name: 'job_name', artifacts: true)
           end
         end
+      end
+    end
+
+    context 'with job name and optional false' do
+      let(:config) { { job: 'job_name', optional: false } }
+
+      it { is_expected.to be_valid }
 
       it_behaves_like 'job type'
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
+        end
+      end
+    end
+
+    context 'with job name and optional nil' do
+      let(:config) { { job: 'job_name', optional: nil } }
+
+      it { is_expected.to be_valid }
+
+      it_behaves_like 'job type'
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
+        end
+      end
+    end
+
+    context 'without optional key' do
+      let(:config) { { job: 'job_name' } }
+
+      it { is_expected.to be_valid }
+
+      it_behaves_like 'job type'
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true, optional: false)
+        end
+      end
     end
 
     context 'when job name is empty' do

spec/lib/gitlab/ci/config/entry/needs_spec.rb

@@ -111,8 +111,8 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do
         it 'returns key value' do
           expect(needs.value).to eq(
             job: [
-              { name: 'first_job_name',  artifacts: true },
-              { name: 'second_job_name', artifacts: true }
+              { name: 'first_job_name',  artifacts: true, optional: false },
+              { name: 'second_job_name', artifacts: true, optional: false }
             ]
           )
         end
@@ -124,8 +124,8 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do
     context 'with complex job entries composed' do
       let(:config) do
         [
-          { job: 'first_job_name',  artifacts: true },
-          { job: 'second_job_name', artifacts: false }
+          { job: 'first_job_name',  artifacts: true, optional: false },
+          { job: 'second_job_name', artifacts: false, optional: false }
         ]
       end
 
@@ -137,8 +137,8 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do
         it 'returns key value' do
           expect(needs.value).to eq(
             job: [
-              { name: 'first_job_name',  artifacts: true },
-              { name: 'second_job_name', artifacts: false }
+              { name: 'first_job_name',  artifacts: true, optional: false },
+              { name: 'second_job_name', artifacts: false, optional: false }
             ]
           )
         end
@@ -163,8 +163,8 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do
         it 'returns key value' do
           expect(needs.value).to eq(
             job: [
-              { name: 'first_job_name',  artifacts: true },
-              { name: 'second_job_name', artifacts: false }
+              { name: 'first_job_name',  artifacts: true, optional: false },
+              { name: 'second_job_name', artifacts: false, optional: false }
             ]
           )
         end

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -1049,6 +1049,25 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
         expect(subject.errors).to contain_exactly(
           "'rspec' job needs 'build' job, but it was not added to the pipeline")
       end
+
+      context 'when the needed job is optional' do
+        let(:needs_attributes) { [{ name: 'build', optional: true }] }
+
+        it "does not return an error" do
+          expect(subject.errors).to be_empty
+        end
+
+        context 'when the FF ci_needs_optional is disabled' do
+          before do
+            stub_feature_flags(ci_needs_optional: false)
+          end
+
+          it "returns an error" do
+            expect(subject.errors).to contain_exactly(
+              "'rspec' job needs 'build' job, but it was not added to the pipeline")
+          end
+        end
+      end
     end
 
     context 'when build job is part of prior stages' do

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -2089,8 +2089,8 @@ module Gitlab
               only: { refs: %w[branches tags] },
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "build1", artifacts: true },
-                { name: "build2", artifacts: true }
+                { name: "build1", artifacts: true, optional: false },
+                { name: "build2", artifacts: true, optional: false }
               ],
               when: "on_success",
               allow_failure: false,
@@ -2104,7 +2104,7 @@ module Gitlab
           let(:needs) do
             [
               { job: 'parallel', artifacts: false },
-              { job: 'build1',   artifacts: true },
+              { job: 'build1',   artifacts: true, optional: true },
               'build2'
             ]
           end
@@ -2131,10 +2131,10 @@ module Gitlab
               only: { refs: %w[branches tags] },
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "parallel 1/2", artifacts: false },
-                { name: "parallel 2/2", artifacts: false },
-                { name: "build1", artifacts: true },
-                { name: "build2", artifacts: true }
+                { name: "parallel 1/2", artifacts: false, optional: false },
+                { name: "parallel 2/2", artifacts: false, optional: false },
+                { name: "build1", artifacts: true, optional: true },
+                { name: "build2", artifacts: true, optional: false }
               ],
               when: "on_success",
               allow_failure: false,
@@ -2156,8 +2156,8 @@ module Gitlab
               only: { refs: %w[branches tags] },
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "parallel 1/2", artifacts: true },
-                { name: "parallel 2/2", artifacts: true }
+                { name: "parallel 1/2", artifacts: true, optional: false },
+                { name: "parallel 2/2", artifacts: true, optional: false }
               ],
               when: "on_success",
               allow_failure: false,
@@ -2185,10 +2185,10 @@ module Gitlab
               only: { refs: %w[branches tags] },
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "build1", artifacts: true },
-                { name: "build2", artifacts: true },
-                { name: "parallel 1/2", artifacts: true },
-                { name: "parallel 2/2", artifacts: true }
+                { name: "build1", artifacts: true, optional: false },
+                { name: "build2", artifacts: true, optional: false },
+                { name: "parallel 1/2", artifacts: true, optional: false },
+                { name: "parallel 2/2", artifacts: true, optional: false }
               ],
               when: "on_success",
               allow_failure: false,

spec/models/ci/processable_spec.rb

@@ -112,8 +112,8 @@ RSpec.describe Ci::Processable do
 
       it 'returns all needs attributes' do
         is_expected.to contain_exactly(
-          { 'artifacts' => true, 'name' => 'test1' },
-          { 'artifacts' => true, 'name' => 'test2' }
+          { 'artifacts' => true, 'name' => 'test1', 'optional' => false },
+          { 'artifacts' => true, 'name' => 'test2', 'optional' => false }
         )
       end
     end

spec/services/ci/create_pipeline_service/needs_spec.rb

@@ -238,5 +238,51 @@ RSpec.describe Ci::CreatePipelineService do
           .to eq('jobs:invalid_dag_job:needs config can not be an empty hash')
       end
     end
+
+    context 'when the needed job has rules' do
+      let(:config) do
+        <<~YAML
+          build:
+            stage: build
+            script: exit 0
+            rules:
+              - if: $CI_COMMIT_REF_NAME == "invalid"
+
+          test:
+            stage: test
+            script: exit 0
+            needs: [build]
+        YAML
+      end
+
+      it 'returns error' do
+        expect(pipeline.yaml_errors)
+          .to eq("'test' job needs 'build' job, but it was not added to the pipeline")
+      end
+
+      context 'when need is optional' do
+        let(:config) do
+          <<~YAML
+            build:
+              stage: build
+              script: exit 0
+              rules:
+                - if: $CI_COMMIT_REF_NAME == "invalid"
+
+            test:
+              stage: test
+              script: exit 0
+              needs:
+                - job: build
+                  optional: true
+          YAML
+        end
+
+        it 'creates the pipeline without an error' do
+          expect(pipeline).to be_persisted
+          expect(pipeline.builds.pluck(:name)).to contain_exactly('test')
+        end
+      end
+    end
   end
 end

spec/services/ci/pipeline_processing/test_cases/dag_build2_build1_rules_out_test_needs_build1_with_optional.yml

+config:
+  build1:
+    stage: build
+    script: exit 0
+    rules:
+      - if: $CI_COMMIT_REF_NAME == "invalid"
+
+  build2:
+    stage: build
+    script: exit 0
+
+  test:
+    stage: test
+    script: exit 0
+    needs:
+      - job: build1
+        optional: true
+
+init:
+  expect:
+    pipeline: pending
+    stages:
+      build: pending
+      test: pending
+    jobs:
+      build2: pending
+      test: pending
+
+transitions:
+  - event: success
+    jobs: [test]
+    expect:
+      pipeline: running
+      stages:
+        build: pending
+        test: success
+      jobs:
+        build2: pending
+        test: success
+
+  - event: success
+    jobs: [build2]
+    expect:
+      pipeline: success
+      stages:
+        build: success
+        test: success
+      jobs:
+        build2: success
+        test: success

spec/services/ci/pipeline_processing/test_cases/dag_build_rules_out_test_needs_build_with_optional.yml

+config:
+  build:
+    stage: build
+    script: exit 0
+    rules:
+      - if: $CI_COMMIT_REF_NAME == "invalid"
+
+  test:
+    stage: test
+    script: exit 0
+    needs:
+      - job: build
+        optional: true
+
+init:
+  expect:
+    pipeline: pending
+    stages:
+      test: pending
+    jobs:
+      test: pending
+
+transitions:
+  - event: success
+    jobs: [test]
+    expect:
+      pipeline: success
+      stages:
+        test: success
+      jobs:
+        test: success