Commit c0d91726 authored by Fabio Pitino's avatar Fabio Pitino

Merge branch 'lm-allow-need-jobs-same-stage' into 'master'

Allow jobs need to refer to a job in the same stage [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59668
parents 021ece83 87dc59be
......@@ -37,12 +37,20 @@ module Ci
next [] unless processable.pipeline_id # we don't have any dependency when creating the pipeline
deps = model_class.where(pipeline_id: processable.pipeline_id).latest
deps = from_previous_stages(deps)
deps = from_needs(deps)
deps = find_dependencies(processable, deps)
from_dependencies(deps).to_a
end
end
def find_dependencies(processable, deps)
if processable.scheduling_type_dag?
from_needs(deps)
else
from_previous_stages(deps)
end
end
# Dependencies from the same parent-pipeline hierarchy excluding
# the current job's pipeline
def cross_pipeline
......@@ -125,8 +133,6 @@ module Ci
end
def from_needs(scope)
return scope unless processable.scheduling_type_dag?
needs_names = processable.needs.artifacts.select(:name)
scope.where(name: needs_names)
end
......
......@@ -10,8 +10,16 @@ module Ci
private
def process_subsequent_jobs(processable)
processable.pipeline.processables.skipped.after_stage(processable.stage_idx).find_each do |processable|
process(processable)
if Feature.enabled?(:ci_same_stage_job_needs, processable.project, default_enabled: :yaml)
(stage_dependent_jobs(processable) | needs_dependent_jobs(processable))
.each do |processable|
process(processable)
end
else
skipped_jobs(processable).after_stage(processable.stage_idx)
.find_each do |job|
process(job)
end
end
end
......@@ -24,5 +32,17 @@ module Ci
processable.process(current_user)
end
end
def skipped_jobs(processable)
processable.pipeline.processables.skipped
end
def stage_dependent_jobs(processable)
skipped_jobs(processable).scheduling_type_stage.after_stage(processable.stage_idx)
end
def needs_dependent_jobs(processable)
skipped_jobs(processable).scheduling_type_dag.with_needs([processable.name])
end
end
end
---
name: ci_same_stage_job_needs
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59668
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/328253
milestone: '14.1'
type: development
group: group::pipeline authoring
default_enabled: false
......@@ -1563,6 +1563,14 @@ production:
#### Requirements and limitations
- In [GitLab 14.1 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/30632)
you can refer to jobs in the same stage as the job you are configuring. This feature
is [Deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
- Disabled on GitLab.com.
- Not recommended for production use.
- For GitLab self-managed instances, GitLab adminsitrators
can choose to [disable it](#enable-or-disable-needs-for-jobs-in-the-same-stage)
- In GitLab 14.0 and older, you can only refer to jobs in earlier stages.
- 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:
......@@ -1579,6 +1587,22 @@ production:
- Stages must be explicitly defined for all jobs
that have the keyword `needs:` or are referred to by one.
##### Enable or disable `needs` for jobs in the same stage **(FREE SELF)**
`needs` for jobs in the same stage is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails
console](../../administration/feature_flags.md)
can opt to disable it.
To enable it:
`Feature.enable(:ci_same_stage_job_needs)`
To disable it:
`Feature.disable(:ci_same_stage_job_needs)`
##### Changing the `needs:` job limit **(FREE SELF)**
The maximum number of jobs that can be defined in `needs:` defaults to 50.
......
......@@ -9,7 +9,7 @@ module EE
extend ::Gitlab::Utils::Override
override :attributes
def initialize(context, attributes, previous_stages)
def initialize(context, attributes, previous_stages, current_stage = nil)
super
@dast_configuration = attributes.dig(:options, :dast_configuration)
......
......@@ -11,11 +11,16 @@ module Gitlab
delegate :dig, to: :@seed_attributes
def initialize(context, attributes, previous_stages)
def initialize(context, attributes, previous_stages, current_stage)
@context = context
@pipeline = context.pipeline
@seed_attributes = attributes
@previous_stages = previous_stages
@stages_for_needs_lookup = if Feature.enabled?(:ci_same_stage_job_needs, @pipeline.project, default_enabled: :yaml)
(previous_stages + [current_stage]).compact
else
previous_stages
end
@needs_attributes = dig(:needs_attributes)
@resource_group_key = attributes.delete(:resource_group_key)
@job_variables = @seed_attributes.delete(:job_variables)
......@@ -148,14 +153,18 @@ module Gitlab
@needs_attributes.flat_map do |need|
next if need[:optional]
result = @previous_stages.any? do |stage|
stage.seeds_names.include?(need[:name])
end
result = need_present?(need)
"'#{name}' job needs '#{need[:name]}' job, but it was not added to the pipeline" unless result
"'#{name}' job needs '#{need[:name]}' job, but '#{need[:name]}' is not in any previous stage" unless result
end.compact
end
def need_present?(need)
@stages_for_needs_lookup.any? do |stage|
stage.seeds_names.include?(need[:name])
end
end
def max_needs_allowed
@pipeline.project.actual_limits.ci_needs_size_limit
end
......
......@@ -17,7 +17,7 @@ module Gitlab
@previous_stages = previous_stages
@builds = attributes.fetch(:builds).map do |attributes|
Seed::Build.new(context, attributes, previous_stages)
Seed::Build.new(context, attributes, previous_stages, self)
end
end
......
......@@ -46,6 +46,10 @@ module Gitlab
@jobs.each do |name, job|
validate_job!(name, job)
end
if ::Feature.enabled?(:ci_same_stage_job_needs, @opts[:project], default_enabled: :yaml)
YamlProcessor::Dag.check_circular_dependencies!(@jobs)
end
end
def validate_job!(name, job)
......@@ -99,10 +103,16 @@ module Gitlab
job_stage_index = stage_index(name)
dependency_stage_index = stage_index(dependency)
# A dependency might be defined later in the configuration
# with a stage that does not exist
unless dependency_stage_index.present? && dependency_stage_index < job_stage_index
error!("#{name} job: #{dependency_type} #{dependency} is not defined in prior stages")
if ::Feature.enabled?(:ci_same_stage_job_needs, @opts[:project], default_enabled: :yaml)
unless dependency_stage_index.present? && dependency_stage_index <= job_stage_index
error!("#{name} job: #{dependency_type} #{dependency} is not defined in current or prior stages")
end
else
# A dependency might be defined later in the configuration
# with a stage that does not exist
unless dependency_stage_index.present? && dependency_stage_index < job_stage_index
error!("#{name} job: #{dependency_type} #{dependency} is not defined in prior stages")
end
end
end
......
# frozen_string_literal: true
# Represents Dag pipeline
module Gitlab
module Ci
class YamlProcessor
class Dag
include TSort
MissingNodeError = Class.new(StandardError)
def initialize(nodes)
@nodes = nodes
end
def self.check_circular_dependencies!(jobs)
nodes = jobs.values.to_h do |job|
name = job[:name].to_s
needs = job.dig(:needs, :job).to_a
[name, needs.map { |need| need[:name].to_s }]
end
new(nodes).tsort
rescue TSort::Cyclic
raise ValidationError, 'The pipeline has circular dependencies.'
rescue MissingNodeError
end
def tsort_each_child(node, &block)
raise MissingNodeError, "node #{node} is missing" unless @nodes[node]
@nodes[node].each(&block)
end
def tsort_each_node(&block)
@nodes.each_key(&block)
end
end
end
end
end
......@@ -247,7 +247,7 @@ RSpec.describe Gitlab::Ci::Lint do
include_context 'advanced validations' do
it 'runs advanced logical validations' do
expect(subject).not_to be_valid
expect(subject.errors).to eq(["'test' job needs 'build' job, but it was not added to the pipeline"])
expect(subject.errors).to eq(["'test' job needs 'build' job, but 'build' is not in any previous stage"])
end
end
......
......@@ -11,8 +11,9 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
let(:seed_context) { double(pipeline: pipeline, root_variables: root_variables) }
let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } }
let(:previous_stages) { [] }
let(:current_stage) { double(seeds_names: [attributes[:name]]) }
let(:seed_build) { described_class.new(seed_context, attributes, previous_stages) }
let(:seed_build) { described_class.new(seed_context, attributes, previous_stages, current_stage) }
describe '#attributes' do
subject { seed_build.attributes }
......@@ -1079,7 +1080,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
it "returns an error" do
expect(subject.errors).to contain_exactly(
"'rspec' job needs 'build' job, but it was not added to the pipeline")
"'rspec' job needs 'build' job, but 'build' is not in any previous stage")
end
context 'when the needed job is optional' do
......@@ -1115,6 +1116,28 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
end
end
context 'when build job is part of the same stage' do
let(:current_stage) { double(seeds_names: [attributes[:name], 'build']) }
it 'is included' do
is_expected.to be_included
end
it 'does not have errors' do
expect(subject.errors).to be_empty
end
context 'when ci_same_stage_job_needs FF is disabled' do
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
it 'has errors' do
expect(subject.errors).to contain_exactly("'rspec' job needs 'build' job, but 'build' is not in any previous stage")
end
end
end
context 'when using 101 needs' do
let(:needs_count) { 101 }
......
......@@ -34,6 +34,10 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Pipeline do
described_class.new(seed_context, stages_attributes)
end
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
describe '#stages' do
it 'returns the stage resources' do
stages = seed.stages
......@@ -65,7 +69,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Pipeline do
}
expect(seed.errors).to contain_exactly(
"'invalid_job' job needs 'non-existent' job, but it was not added to the pipeline")
"'invalid_job' job needs 'non-existent' job, but 'non-existent' is not in any previous stage")
end
end
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
RSpec.describe Gitlab::Ci::YamlProcessor::Dag do
let(:nodes) { {} }
subject(:result) { described_class.new(nodes).tsort }
context 'when it is a regular pipeline' do
let(:nodes) do
{ 'job_c' => %w(job_b job_d), 'job_d' => %w(job_a), 'job_b' => %w(job_a), 'job_a' => %w() }
end
it 'returns ordered jobs' do
expect(result).to eq(%w(job_a job_b job_d job_c))
end
end
context 'when there is a circular dependency' do
let(:nodes) do
{ 'job_a' => %w(job_c), 'job_b' => %w(job_a), 'job_c' => %w(job_b) }
end
it 'raises TSort::Cyclic' do
expect { result }.to raise_error(TSort::Cyclic, /topological sort failed/)
end
end
context 'when there is a missing job' do
let(:nodes) do
{ 'job_a' => %w(job_d), 'job_b' => %w(job_a) }
end
it 'raises MissingNodeError' do
expect { result }.to raise_error(
Gitlab::Ci::YamlProcessor::Dag::MissingNodeError, 'node job_d is missing'
)
end
end
end
......@@ -595,7 +595,15 @@ module Gitlab
EOYML
end
it_behaves_like 'has warnings and expected error', /build job: need test is not defined in prior stages/
it_behaves_like 'has warnings and expected error', /build job: need test is not defined in current or prior stages/
context 'with ci_same_stage_job_needs FF disabled' do
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
it_behaves_like 'has warnings and expected error', /build job: need test is not defined in prior stages/
end
end
end
end
......@@ -1858,7 +1866,7 @@ module Gitlab
build2: { stage: 'build', script: 'test' },
test1: { stage: 'test', script: 'test', dependencies: dependencies },
test2: { stage: 'test', script: 'test' },
deploy: { stage: 'test', script: 'test' }
deploy: { stage: 'deploy', script: 'test' }
}
end
......@@ -1891,7 +1899,15 @@ module Gitlab
context 'dependencies to deploy' do
let(:dependencies) { ['deploy'] }
it_behaves_like 'returns errors', 'test1 job: dependency deploy is not defined in prior stages'
it_behaves_like 'returns errors', 'test1 job: dependency deploy is not defined in current or prior stages'
context 'with ci_same_stage_job_needs FF disabled' do
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
it_behaves_like 'returns errors', 'test1 job: dependency deploy is not defined in prior stages'
end
end
context 'when a job depends on another job that references a not-yet defined stage' do
......@@ -1916,7 +1932,7 @@ module Gitlab
}
end
it_behaves_like 'returns errors', /is not defined in prior stages/
it_behaves_like 'returns errors', /is not defined in current or prior stages/
end
end
......@@ -1931,7 +1947,7 @@ module Gitlab
parallel: { stage: 'build', script: 'test', parallel: 2 },
test1: { stage: 'test', script: 'test', needs: needs, dependencies: dependencies },
test2: { stage: 'test', script: 'test' },
deploy: { stage: 'test', script: 'test' }
deploy: { stage: 'deploy', script: 'test' }
}
end
......@@ -1941,6 +1957,45 @@ module Gitlab
it { is_expected.to be_valid }
end
context 'needs a job from the same stage' do
let(:needs) { %w(test2) }
it 'creates jobs with valid specifications' do
expect(subject.builds.size).to eq(7)
expect(subject.builds[0]).to eq(
stage: 'build',
stage_idx: 1,
name: 'build1',
only: { refs: %w[branches tags] },
options: {
script: ['test']
},
when: 'on_success',
allow_failure: false,
yaml_variables: [],
job_variables: [],
root_variables_inheritance: true,
scheduling_type: :stage
)
expect(subject.builds[4]).to eq(
stage: 'test',
stage_idx: 2,
name: 'test1',
only: { refs: %w[branches tags] },
options: { script: ['test'] },
needs_attributes: [
{ name: 'test2', artifacts: true, optional: false }
],
when: 'on_success',
allow_failure: false,
yaml_variables: [],
job_variables: [],
root_variables_inheritance: true,
scheduling_type: :dag
)
end
end
context 'needs two builds' do
let(:needs) { %w(build1 build2) }
......@@ -2096,7 +2151,15 @@ module Gitlab
context 'needs to deploy' do
let(:needs) { ['deploy'] }
it_behaves_like 'returns errors', 'test1 job: need deploy is not defined in prior stages'
it_behaves_like 'returns errors', 'test1 job: need deploy is not defined in current or prior stages'
context 'with ci_same_stage_job_needs FF disabled' do
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
it_behaves_like 'returns errors', 'test1 job: need deploy is not defined in prior stages'
end
end
context 'needs and dependencies that are mismatching' do
......@@ -2767,6 +2830,29 @@ module Gitlab
it_behaves_like 'returns errors', 'jobs:rspec:parallel should be an integer or a hash'
end
context 'when the pipeline has a circular dependency' do
let(:config) do
<<~YAML
job_a:
stage: test
script: build
needs: [job_c]
job_b:
stage: test
script: test
needs: [job_a]
job_c:
stage: test
script: deploy
needs: [job_b]
YAML
end
it_behaves_like 'returns errors', 'The pipeline has circular dependencies.'
end
end
describe '#execute' do
......
......@@ -55,6 +55,24 @@ RSpec.describe Ci::BuildDependencies do
end
end
end
context 'when needs refer to jobs from the same stage' do
let(:job) do
create(:ci_build,
pipeline: pipeline,
name: 'dag_job',
scheduling_type: :dag,
stage_idx: 2,
stage: 'deploy'
)
end
before do
create(:ci_build_need, build: job, name: 'staging', artifacts: true)
end
it { is_expected.to contain_exactly(staging) }
end
end
describe 'jobs from specified dependencies' do
......
......@@ -8,9 +8,9 @@ RSpec.describe Ci::AfterRequeueJobService do
let(:pipeline) { create(:ci_pipeline, project: project) }
let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 0) }
let!(:test1) { create(:ci_build, :success, pipeline: pipeline, stage_idx: 1) }
let!(:test2) { create(:ci_build, :skipped, pipeline: pipeline, stage_idx: 1) }
let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 0, name: 'build') }
subject(:execute_service) { described_class.new(project, user).execute(build) }
......@@ -24,6 +24,34 @@ RSpec.describe Ci::AfterRequeueJobService do
expect(test2.reload).to be_created
end
context 'when there is a job need from the same stage' do
let!(:test3) do
create(:ci_build,
:skipped,
pipeline: pipeline,
stage_idx: 0,
scheduling_type: :dag)
end
before do
create(:ci_build_need, build: test3, name: 'build')
end
it 'marks subsequent skipped jobs as processable' do
expect { execute_service }.to change { test3.reload.status }.from('skipped').to('created')
end
context 'with ci_same_stage_job_needs FF disabled' do
before do
stub_feature_flags(ci_same_stage_job_needs: false)
end
it 'does nothing with the build' do
expect { execute_service }.not_to change { test3.reload.status }
end
end
end
context 'when the pipeline is a downstream pipeline and the bridge is depended' do
let!(:trigger_job) { create(:ci_bridge, :strategy_depend, status: 'success') }
......
......@@ -69,7 +69,7 @@ RSpec.describe Ci::CreatePipelineService do
end
it 'contains both errors and warnings' do
error_message = 'build job: need test is not defined in prior stages'
error_message = 'build job: need test is not defined in current or prior stages'
warning_message = /jobs:test may allow multiple pipelines to run/
expect(pipeline.yaml_errors).to eq(error_message)
......
......@@ -84,7 +84,7 @@ RSpec.describe Ci::CreatePipelineService do
it_behaves_like 'returns a non persisted pipeline'
it 'returns a pipeline with errors', :aggregate_failures do
error_message = 'build job: need test is not defined in prior stages'
error_message = 'build job: need test is not defined in current or prior stages'
expect(subject.error_messages.map(&:content)).to eq([error_message])
expect(subject.errors).not_to be_empty
......@@ -109,7 +109,7 @@ RSpec.describe Ci::CreatePipelineService do
it_behaves_like 'returns a non persisted pipeline'
it 'returns a pipeline with errors', :aggregate_failures do
error_message = "'test' job needs 'build' job, but it was not added to the pipeline"
error_message = "'test' job needs 'build' job, but 'build' is not in any previous stage"
expect(subject.error_messages.map(&:content)).to eq([error_message])
expect(subject.errors).not_to be_empty
......
......@@ -257,7 +257,7 @@ RSpec.describe Ci::CreatePipelineService do
it 'returns error' do
expect(pipeline.yaml_errors)
.to eq("'test' job needs 'build' job, but it was not added to the pipeline")
.to eq("'test' job needs 'build' job, but 'build' is not in any previous stage")
end
context 'when need is optional' do
......
......@@ -252,7 +252,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
end
it_behaves_like 'creation failure' do
let(:expected_error) { /test job: dependency generator is not defined in prior stages/ }
let(:expected_error) { /test job: dependency generator is not defined in current or prior stages/ }
end
end
......
......@@ -1715,7 +1715,7 @@ RSpec.describe Ci::CreatePipelineService do
it 'contains the expected errors' do
expect(pipeline.builds).to be_empty
error_message = "'test_a' job needs 'build_a' job, but it was not added to the pipeline"
error_message = "'test_a' job needs 'build_a' job, but 'build_a' is not in any previous stage"
expect(pipeline.yaml_errors).to eq(error_message)
expect(pipeline.error_messages.map(&:content)).to contain_exactly(error_message)
expect(pipeline.errors[:base]).to contain_exactly(error_message)
......
config:
build:
stage: test
script: exit 0
test:
stage: test
script: exit 0
needs: [build]
deploy:
stage: test
script: exit 0
needs: [test]
init:
expect:
pipeline: pending
stages:
test: pending
jobs:
build: pending
test: created
deploy: created
transitions:
- event: success
jobs: [build]
expect:
pipeline: running
stages:
test: running
jobs:
build: success
test: pending
deploy: created
- event: success
jobs: [test]
expect:
pipeline: running
stages:
test: running
jobs:
build: success
test: success
deploy: pending
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