Commit 903713ef authored by Marius Bobin's avatar Marius Bobin Committed by Kamil Trzciński

Pass artifacts in CI DAG needs

Implementation idea for adding the artifacts keyword
into CI DAG needs.
parent 42dc26c5
......@@ -764,7 +764,7 @@ module Ci
# find all jobs that are needed
if Feature.enabled?(:ci_dag_support, project, default_enabled: true) && needs.exists?
depended_jobs = depended_jobs.where(name: needs.select(:name))
depended_jobs = depended_jobs.where(name: needs.artifacts.select(:name))
end
# find all jobs that are dependent on
......@@ -772,6 +772,8 @@ module Ci
depended_jobs = depended_jobs.where(name: options[:dependencies])
end
# if both needs and dependencies are used,
# the end result will be an intersection between them
depended_jobs
end
......
......@@ -10,5 +10,6 @@ module Ci
validates :name, presence: true, length: { maximum: 128 }
scope :scoped_build, -> { where('ci_builds.id=ci_build_needs.build_id') }
scope :artifacts, -> { where(artifacts: true) }
end
end
---
title: Control passing artifacts from CI DAG needs
merge_request: 19943
author:
type: added
# frozen_string_literal: true
class AddArtifactsToCiBuildNeed < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:ci_build_needs, :artifacts,
:boolean,
default: true,
allow_null: false)
end
def down
remove_column(:ci_build_needs, :artifacts)
end
end
......@@ -595,6 +595,7 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
create_table "ci_build_needs", id: :serial, force: :cascade do |t|
t.integer "build_id", null: false
t.text "name", null: false
t.boolean "artifacts", default: true, null: false
t.index ["build_id", "name"], name: "index_ci_build_needs_on_build_id_and_name", unique: true
end
......
......@@ -2233,6 +2233,49 @@ This example creates three paths of execution:
- Related to the above, stages must be explicitly defined for all jobs
that have the keyword `needs:` or are referred to by one.
#### Artifact downloads with `needs`
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
When using `needs`, artifact downloads are controlled with `artifacts: true` or `artifacts: false`.
The `dependencies` keyword should not be used with `needs`, as this is deprecated since GitLab 12.6.
In the example below, the `rspec` job will download the `build_job` artifacts, while the
`rubocop` job will not:
```yaml
build_job:
stage: build
artifacts:
paths:
- binaries/
rspec:
stage: test
needs:
- job: build_job
artifacts: true
rubocop:
stage: test
needs:
- job: build_job
artifacts: false
```
Additionally, in the three syntax examples below, the `rspec` job will download the artifacts
from all three `build_jobs`, as `artifacts` is true for `build_job_1`, and will
**default** to true for both `build_job_2` and `build_job_3`.
```yaml
rspec:
needs:
- job: build_job_1
artifacts: true
- job: build_job_2
- build_job_3
```
### `coverage`
> [Introduced][ce-7447] in GitLab 8.17.
......
......@@ -9,10 +9,12 @@ module EE
extend ActiveSupport::Concern
prepended do
strategy :Bridge, class: EE::Gitlab::Ci::Config::Entry::Need::Bridge, if: -> (config) { config.is_a?(Hash) }
strategy :BridgeHash,
class: EE::Gitlab::Ci::Config::Entry::Need::BridgeHash,
if: -> (config) { config.is_a?(Hash) && !config.key?(:job) }
end
class Bridge < ::Gitlab::Config::Entry::Node
class BridgeHash < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
include ::Gitlab::Config::Entry::Attributable
......
......@@ -29,7 +29,7 @@ describe ::Gitlab::Ci::Config::Entry::Need do
describe '#errors' do
it 'is returns an error about an empty config' do
expect(subject.errors)
.to include("bridge config can't be blank")
.to include("bridge hash config can't be blank")
end
end
end
......
......@@ -48,7 +48,13 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
describe '.compose!' do
context 'when valid job entries composed' do
let(:config) { ['first_job_name', pipeline: 'some/project'] }
let(:config) do
[
'first_job_name',
{ job: 'second_job_name', artifacts: false },
{ pipeline: 'some/project' }
]
end
before do
needs.compose!
......@@ -61,7 +67,10 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
describe '#value' do
it 'returns key value' do
expect(needs.value).to eq(
job: [{ name: 'first_job_name' }],
job: [
{ name: 'first_job_name', artifacts: true },
{ name: 'second_job_name', artifacts: false }
],
bridge: [{ pipeline: 'some/project' }]
)
end
......@@ -69,7 +78,7 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
describe '#descendants' do
it 'creates valid descendant nodes' do
expect(needs.descendants.count).to eq 2
expect(needs.descendants.count).to eq(3)
expect(needs.descendants)
.to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
end
......
......@@ -67,7 +67,7 @@ describe Gitlab::Ci::YamlProcessor do
bridge_needs: { pipeline: 'some/project' }
},
needs_attributes: [
{ name: "build" }
{ name: "build", artifacts: true }
],
when: "on_success",
allow_failure: false,
......
......@@ -5,9 +5,10 @@ module Gitlab
class Config
module Entry
class Need < ::Gitlab::Config::Entry::Simplifiable
strategy :Job, if: -> (config) { config.is_a?(String) }
strategy :JobString, if: -> (config) { config.is_a?(String) }
strategy :JobHash, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
class Job < ::Gitlab::Config::Entry::Node
class JobString < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
validations do
......@@ -20,7 +21,30 @@ module Gitlab
end
def value
{ name: @config }
{ name: @config, artifacts: true }
end
end
class JobHash < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
include ::Gitlab::Config::Entry::Attributable
ALLOWED_KEYS = %i[job artifacts].freeze
attributes :job, :artifacts
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
end
def type
:job
end
def value
{ name: job, artifacts: artifacts || artifacts.nil? }
end
end
......
......@@ -44,7 +44,7 @@ module Gitlab
if all_job_names = parallelized_jobs[job_need_name]
all_job_names.map do |job_name|
{ name: job_name }
job_need.merge(name: job_name)
end
else
job_need
......
......@@ -5,31 +5,177 @@ require 'spec_helper'
describe ::Gitlab::Ci::Config::Entry::Need do
subject(:need) { described_class.new(config) }
context 'when job is specified' do
let(:config) { 'job_name' }
shared_examples 'job type' do
describe '#type' do
subject(:need_type) { need.type }
describe '#valid?' do
it { is_expected.to be_valid }
it { is_expected.to eq(:job) }
end
end
context 'with simple config' do
context 'when job is specified' do
let(:config) { 'job_name' }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name', artifacts: true)
end
end
it_behaves_like 'job type'
end
context 'when need is empty' do
let(:config) { '' }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about an empty config' do
expect(need.errors)
.to contain_exactly("job string config can't be blank")
end
end
it_behaves_like 'job type'
end
end
context 'with complex config' do
context 'with job name and artifacts true' do
let(:config) { { job: 'job_name', artifacts: true } }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name', artifacts: true)
end
end
it_behaves_like 'job type'
end
context 'with job name and artifacts false' do
let(:config) { { job: 'job_name', artifacts: false } }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name', artifacts: false)
end
end
it_behaves_like 'job type'
end
context 'with job name and artifacts nil' do
let(:config) { { job: 'job_name', artifacts: nil } }
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name')
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name', artifacts: true)
end
end
it_behaves_like 'job type'
end
context 'without artifacts key' do
let(:config) { { job: 'job_name' } }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'returns job needs configuration' do
expect(need.value).to eq(name: 'job_name', artifacts: true)
end
end
it_behaves_like 'job type'
end
context 'when job name is empty' do
let(:config) { { job: '', artifacts: true } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about an empty config' do
expect(need.errors)
.to contain_exactly("job hash job can't be blank")
end
end
it_behaves_like 'job type'
end
context 'when job name is not a string' do
let(:config) { { job: :job_name, artifacts: false } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about job type' do
expect(need.errors)
.to contain_exactly('job hash job should be a string')
end
end
it_behaves_like 'job type'
end
context 'when job has unknown keys' do
let(:config) { { job: 'job_name', artifacts: false, some: :key } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about job type' do
expect(need.errors)
.to contain_exactly('job hash config contains unknown keys: some')
end
end
it_behaves_like 'job type'
end
end
context 'when need is empty' do
let(:config) { '' }
context 'when need config is not a string or a hash' do
let(:config) { :job_name }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about an empty config' do
it 'is returns an error about job type' do
expect(need.errors)
.to contain_exactly("job config can't be blank")
.to contain_exactly('unknown strategy has an unsupported type')
end
end
end
......
......@@ -51,9 +51,34 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
end
end
end
context 'when wrong needs type is used' do
let(:config) { [{ job: 'job_name', artifacts: true, some: :key }] }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'returns error about incorrect type' do
expect(needs.errors).to contain_exactly(
'need config contains unknown keys: some')
end
end
end
end
describe '.compose!' do
shared_examples 'entry with descendant nodes' do
describe '#descendants' do
it 'creates valid descendant nodes' do
expect(needs.descendants.count).to eq 2
expect(needs.descendants)
.to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
end
end
end
context 'when valid job entries composed' do
let(:config) { %w[first_job_name second_job_name] }
......@@ -65,18 +90,80 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
it 'returns key value' do
expect(needs.value).to eq(
job: [
{ name: 'first_job_name' },
{ name: 'second_job_name' }
{ name: 'first_job_name', artifacts: true },
{ name: 'second_job_name', artifacts: true }
]
)
end
end
describe '#descendants' do
it 'creates valid descendant nodes' do
expect(needs.descendants.count).to eq 2
expect(needs.descendants)
.to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
it_behaves_like 'entry with descendant nodes'
end
context 'with complex job entries composed' do
let(:config) do
[
{ job: 'first_job_name', artifacts: true },
{ job: 'second_job_name', artifacts: false }
]
end
before do
needs.compose!
end
describe '#value' do
it 'returns key value' do
expect(needs.value).to eq(
job: [
{ name: 'first_job_name', artifacts: true },
{ name: 'second_job_name', artifacts: false }
]
)
end
end
it_behaves_like 'entry with descendant nodes'
end
context 'with mixed job entries composed' do
let(:config) do
[
'first_job_name',
{ job: 'second_job_name', artifacts: false }
]
end
before do
needs.compose!
end
describe '#value' do
it 'returns key value' do
expect(needs.value).to eq(
job: [
{ name: 'first_job_name', artifacts: true },
{ name: 'second_job_name', artifacts: false }
]
)
end
end
it_behaves_like 'entry with descendant nodes'
end
context 'with empty config' do
let(:config) do
[]
end
before do
needs.compose!
end
describe '#value' do
it 'returns empty value' do
expect(needs.value).to eq({})
end
end
end
......
......@@ -105,7 +105,7 @@ describe Gitlab::Ci::Config::Normalizer do
context 'for needs' do
let(:expanded_job_attributes) do
expanded_job_names.map do |job_name|
{ name: job_name }
{ name: job_name, extra: :key }
end
end
......@@ -117,7 +117,7 @@ describe Gitlab::Ci::Config::Normalizer do
script: 'echo 1',
needs: {
job: [
{ name: job_name.to_s }
{ name: job_name.to_s, extra: :key }
]
}
}
......@@ -140,8 +140,8 @@ describe Gitlab::Ci::Config::Normalizer do
script: 'echo 1',
needs: {
job: [
{ name: job_name.to_s },
{ name: "other_job" }
{ name: job_name.to_s, extra: :key },
{ name: "other_job", extra: :key }
]
}
}
......@@ -153,7 +153,7 @@ describe Gitlab::Ci::Config::Normalizer do
end
it "includes the regular job in dependencies" do
expect(subject.dig(:final_job, :needs, :job)).to include(name: 'other_job')
expect(subject.dig(:final_job, :needs, :job)).to include(name: 'other_job', extra: :key)
end
end
end
......
......@@ -1525,8 +1525,48 @@ module Gitlab
name: "test1",
options: { script: ["test"] },
needs_attributes: [
{ name: "build1" },
{ name: "build2" }
{ name: "build1", artifacts: true },
{ name: "build2", artifacts: true }
],
when: "on_success",
allow_failure: false,
yaml_variables: []
)
end
end
context 'needs two builds' do
let(:needs) do
[
{ job: 'parallel', artifacts: false },
{ job: 'build1', artifacts: true },
'build2'
]
end
it "does create jobs with valid specification" do
expect(subject.builds.size).to eq(7)
expect(subject.builds[0]).to eq(
stage: "build",
stage_idx: 1,
name: "build1",
options: {
script: ["test"]
},
when: "on_success",
allow_failure: false,
yaml_variables: []
)
expect(subject.builds[4]).to eq(
stage: "test",
stage_idx: 2,
name: "test1",
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 }
],
when: "on_success",
allow_failure: false,
......@@ -1546,8 +1586,37 @@ module Gitlab
name: "test1",
options: { script: ["test"] },
needs_attributes: [
{ name: "parallel 1/2" },
{ name: "parallel 2/2" }
{ name: "parallel 1/2", artifacts: true },
{ name: "parallel 2/2", artifacts: true }
],
when: "on_success",
allow_failure: false,
yaml_variables: []
)
end
end
context 'needs dependencies artifacts' do
let(:needs) do
[
"build1",
{ job: "build2" },
{ job: "parallel", artifacts: true }
]
end
it "does create jobs with valid specification" do
expect(subject.builds.size).to eq(7)
expect(subject.builds[4]).to eq(
stage: "test",
stage_idx: 2,
name: "test1",
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 }
],
when: "on_success",
allow_failure: false,
......
......@@ -10,4 +10,11 @@ describe Ci::BuildNeed, model: true do
it { is_expected.to validate_presence_of(:build) }
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_length_of(:name).is_at_most(128) }
describe '.artifacts' do
let_it_be(:with_artifacts) { create(:ci_build_need, artifacts: true) }
let_it_be(:without_artifacts) { create(:ci_build_need, artifacts: false) }
it { expect(described_class.artifacts).to contain_exactly(with_artifacts) }
end
end
......@@ -741,20 +741,26 @@ describe Ci::Build do
before do
needs.to_a.each do |need|
create(:ci_build_need, build: final, name: need)
create(:ci_build_need, build: final, **need)
end
end
subject { final.dependencies }
context 'when depedencies are defined' do
context 'when dependencies are defined' do
let(:dependencies) { %w(rspec staging) }
it { is_expected.to contain_exactly(rspec_test, staging) }
end
context 'when needs are defined' do
let(:needs) { %w(build rspec staging) }
let(:needs) do
[
{ name: 'build', artifacts: true },
{ name: 'rspec', artifacts: true },
{ name: 'staging', artifacts: true }
]
end
it { is_expected.to contain_exactly(build, rspec_test, staging) }
......@@ -767,13 +773,44 @@ describe Ci::Build do
end
end
context 'when need artifacts are defined' do
let(:needs) do
[
{ name: 'build', artifacts: true },
{ name: 'rspec', artifacts: false },
{ name: 'staging', artifacts: true }
]
end
it { is_expected.to contain_exactly(build, staging) }
end
context 'when needs and dependencies are defined' do
let(:dependencies) { %w(rspec staging) }
let(:needs) { %w(build rspec staging) }
let(:needs) do
[
{ name: 'build', artifacts: true },
{ name: 'rspec', artifacts: true },
{ name: 'staging', artifacts: true }
]
end
it { is_expected.to contain_exactly(rspec_test, staging) }
end
context 'when needs and dependencies contradict' do
let(:dependencies) { %w(rspec staging) }
let(:needs) do
[
{ name: 'build', artifacts: true },
{ name: 'rspec', artifacts: false },
{ name: 'staging', artifacts: true }
]
end
it { is_expected.to contain_exactly(staging) }
end
context 'when nor dependencies or needs are defined' do
it { is_expected.to contain_exactly(build, rspec_test, rubocop_test, staging) }
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Ci::CreatePipelineService do
context 'needs' do
let_it_be(:user) { create(:admin) }
let_it_be(:project) { create(:project, :repository, creator: user) }
let(:ref) { 'refs/heads/master' }
let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) }
before do
stub_ci_pipeline_yaml_file(config)
end
context 'with a valid config' do
let(:config) do
<<~YAML
build_a:
stage: build
script:
- make
artifacts:
paths:
- binaries/
build_b:
stage: build
script:
- make
artifacts:
paths:
- other_binaries/
build_c:
stage: build
script:
- make
build_d:
stage: build
script:
- make
parallel: 3
test_a:
stage: test
script:
- ls
needs:
- build_a
- job: build_b
artifacts: true
- job: build_c
artifacts: false
dependencies:
- build_a
test_b:
stage: test
script:
- ls
parallel: 2
needs:
- build_a
- job: build_b
artifacts: true
- job: build_d
artifacts: false
test_c:
stage: test
script:
- ls
needs:
- build_a
- job: build_b
- job: build_c
artifacts: true
YAML
end
let(:test_a_build) { pipeline.builds.find_by!(name: 'test_a') }
it 'creates a pipeline with builds' do
expected_builds = [
'build_a', 'build_b', 'build_c', 'build_d 1/3', 'build_d 2/3',
'build_d 3/3', 'test_a', 'test_b 1/2', 'test_b 2/2', 'test_c'
]
expect(pipeline).to be_persisted
expect(pipeline.builds.pluck(:name)).to contain_exactly(*expected_builds)
end
it 'saves needs' do
expect(test_a_build.needs.map(&:attributes))
.to contain_exactly(
a_hash_including('name' => 'build_a', 'artifacts' => true),
a_hash_including('name' => 'build_b', 'artifacts' => true),
a_hash_including('name' => 'build_c', 'artifacts' => false)
)
end
it 'saves dependencies' do
expect(test_a_build.options)
.to match(a_hash_including('dependencies' => ['build_a']))
end
it 'artifacts default to true' do
test_job = pipeline.builds.find_by!(name: 'test_c')
expect(test_job.needs.map(&:attributes))
.to contain_exactly(
a_hash_including('name' => 'build_a', 'artifacts' => true),
a_hash_including('name' => 'build_b', 'artifacts' => true),
a_hash_including('name' => 'build_c', 'artifacts' => true)
)
end
it 'saves parallel jobs' do
['1/2', '2/2'].each do |part|
test_job = pipeline.builds.find_by(name: "test_b #{part}")
expect(test_job.needs.map(&:attributes))
.to contain_exactly(
a_hash_including('name' => 'build_a', 'artifacts' => true),
a_hash_including('name' => 'build_b', 'artifacts' => true),
a_hash_including('name' => 'build_d 1/3', 'artifacts' => false),
a_hash_including('name' => 'build_d 2/3', 'artifacts' => false),
a_hash_including('name' => 'build_d 3/3', 'artifacts' => false)
)
end
end
end
context 'with an invalid config' do
let(:config) do
<<~YAML
build_a:
stage: build
script:
- make
artifacts:
paths:
- binaries/
build_b:
stage: build
script:
- make
artifacts:
paths:
- other_binaries/
test_a:
stage: test
script:
- ls
needs:
- build_a
- job: build_b
artifacts: string
YAML
end
it { expect(pipeline).to be_persisted }
it { expect(pipeline.builds.any?).to be_falsey }
it 'assigns an error to the pipeline' do
expect(pipeline.yaml_errors)
.to eq('jobs:test_a:needs:need artifacts should be a boolean value')
end
end
end
end
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