Commit a96ba32b authored by Vitali Tatarintev's avatar Vitali Tatarintev

Merge branch '293554-expired-artifacts-locked-pipelines' into 'master'

Fix expired artifacts causing dependency failures on locked pipelines

See merge request gitlab-org/gitlab!67764
parents 7b276cb7 a9e9bfc8
...@@ -891,7 +891,7 @@ module Ci ...@@ -891,7 +891,7 @@ module Ci
end end
def valid_dependency? def valid_dependency?
return false if artifacts_expired? return false if artifacts_expired? && !pipeline.artifacts_locked?
return false if erased? return false if erased?
true true
......
...@@ -6,7 +6,7 @@ module API ...@@ -6,7 +6,7 @@ module API
module JobRequest module JobRequest
class Dependency < Grape::Entity class Dependency < Grape::Entity
expose :id, :name, :token expose :id, :name, :token
expose :artifacts_file, using: Entities::Ci::JobArtifactFile, if: ->(job, _) { job.artifacts? } expose :artifacts_file, using: Entities::Ci::JobArtifactFile, if: ->(job, _) { job.available_artifacts? }
end end
end end
end end
......
...@@ -3743,9 +3743,23 @@ RSpec.describe Ci::Build do ...@@ -3743,9 +3743,23 @@ RSpec.describe Ci::Build do
context 'when artifacts of depended job has been expired' do context 'when artifacts of depended job has been expired' do
let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) }
context 'when pipeline is not locked' do
before do
build.pipeline.unlocked!
end
it { expect(job).not_to have_valid_build_dependencies } it { expect(job).not_to have_valid_build_dependencies }
end end
context 'when pipeline is locked' do
before do
build.pipeline.artifacts_locked!
end
it { expect(job).to have_valid_build_dependencies }
end
end
context 'when artifacts of depended job has been erased' do context 'when artifacts of depended job has been erased' do
let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) }
...@@ -4763,11 +4777,27 @@ RSpec.describe Ci::Build do ...@@ -4763,11 +4777,27 @@ RSpec.describe Ci::Build do
let!(:pre_stage_job_invalid) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test2', stage_idx: 1) } let!(:pre_stage_job_invalid) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test2', stage_idx: 1) }
let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w(test1 test2) }) } let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w(test1 test2) }) }
it 'returns invalid dependencies' do context 'when pipeline is locked' do
before do
build.pipeline.unlocked!
end
it 'returns invalid dependencies when expired' do
expect(job.invalid_dependencies).to eq([pre_stage_job_invalid]) expect(job.invalid_dependencies).to eq([pre_stage_job_invalid])
end end
end end
context 'when pipeline is not locked' do
before do
build.pipeline.artifacts_locked!
end
it 'returns no invalid dependencies when expired' do
expect(job.invalid_dependencies).to eq([])
end
end
end
describe '#execute_hooks' do describe '#execute_hooks' do
before do before do
build.clear_memoization(:build_data) build.clear_memoization(:build_data)
......
...@@ -133,6 +133,7 @@ RSpec.describe BuildDetailsEntity do ...@@ -133,6 +133,7 @@ RSpec.describe BuildDetailsEntity do
let(:message) { subject[:callout_message] } let(:message) { subject[:callout_message] }
before do before do
build.pipeline.unlocked!
build.drop!(:missing_dependency_failure) build.drop!(:missing_dependency_failure)
end end
......
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
module Ci module Ci
RSpec.describe RegisterJobService do RSpec.describe RegisterJobService do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:project, reload: true) { create(:project, group: group, shared_runners_enabled: false, group_runners_enabled: false) } let_it_be_with_reload(:project) { create(:project, group: group, shared_runners_enabled: false, group_runners_enabled: false) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project) } let_it_be_with_reload(:pipeline) { create(:ci_pipeline, project: project) }
let!(:shared_runner) { create(:ci_runner, :instance) } let!(:shared_runner) { create(:ci_runner, :instance) }
let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
...@@ -467,14 +467,28 @@ module Ci ...@@ -467,14 +467,28 @@ module Ci
context 'when depended job has not been completed yet' do context 'when depended job has not been completed yet' do
let!(:pre_stage_job) { create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } let!(:pre_stage_job) { create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) }
it { expect(subject).to eq(pending_job) } it { is_expected.to eq(pending_job) }
end end
context 'when artifacts of depended job has been expired' do context 'when artifacts of depended job has been expired' do
let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) }
context 'when the pipeline is locked' do
before do
pipeline.artifacts_locked!
end
it { is_expected.to eq(pending_job) }
end
context 'when the pipeline is unlocked' do
before do
pipeline.unlocked!
end
it_behaves_like 'not pick' it_behaves_like 'not pick'
end end
end
context 'when artifacts of depended job has been erased' do context 'when artifacts of depended job has been erased' do
let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) }
...@@ -490,9 +504,13 @@ module Ci ...@@ -490,9 +504,13 @@ module Ci
let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) }
before do before do
allow_any_instance_of(Ci::Build).to receive(:drop!) pipeline.unlocked!
allow_next_instance_of(Ci::Build) do |build|
expect(build).to receive(:drop!)
.and_raise(ActiveRecord::StaleObjectError.new(pending_job, :drop!)) .and_raise(ActiveRecord::StaleObjectError.new(pending_job, :drop!))
end end
end
it 'does not drop nor pick' do it 'does not drop nor pick' do
expect(subject).to be_nil expect(subject).to be_nil
......
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