Avoid race condition when expiring artifacts
It may happen that job meant to remove expired artifacts will be executed asynchronously when, in the meantime, project associated with given build gets removed by another asynchronous job. In that case we should not remove artifacts because such build will be removed anyway, when project removal is complete.
This commit is contained in:
parent
11485c5acc
commit
22ef066862
2 changed files with 39 additions and 15 deletions
|
@ -2,10 +2,14 @@ class ExpireBuildInstanceArtifactsWorker
|
|||
include Sidekiq::Worker
|
||||
|
||||
def perform(build_id)
|
||||
build = Ci::Build.with_expired_artifacts.reorder(nil).find_by(id: build_id)
|
||||
return unless build
|
||||
build = Ci::Build
|
||||
.with_expired_artifacts
|
||||
.reorder(nil)
|
||||
.find_by(id: build_id)
|
||||
|
||||
Rails.logger.info "Removing artifacts build #{build.id}..."
|
||||
return unless build.try(:project)
|
||||
|
||||
Rails.logger.info "Removing artifacts for build #{build.id}..."
|
||||
build.erase_artifacts!
|
||||
end
|
||||
end
|
||||
|
|
|
@ -6,28 +6,48 @@ describe ExpireBuildInstanceArtifactsWorker do
|
|||
let(:worker) { described_class.new }
|
||||
|
||||
describe '#perform' do
|
||||
before { build }
|
||||
|
||||
subject! { worker.perform(build.id) }
|
||||
before do
|
||||
worker.perform(build.id)
|
||||
end
|
||||
|
||||
context 'with expired artifacts' do
|
||||
let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) }
|
||||
let(:artifacts_expiry) { { artifacts_expire_at: Time.now - 7.days } }
|
||||
|
||||
it 'does expire' do
|
||||
expect(build.reload.artifacts_expired?).to be_truthy
|
||||
context 'when associated project is valid' do
|
||||
let(:build) do
|
||||
create(:ci_build, :artifacts, artifacts_expiry)
|
||||
end
|
||||
|
||||
it 'does expire' do
|
||||
expect(build.reload.artifacts_expired?).to be_truthy
|
||||
end
|
||||
|
||||
it 'does remove files' do
|
||||
expect(build.reload.artifacts_file.exists?).to be_falsey
|
||||
end
|
||||
|
||||
it 'does nullify artifacts_file column' do
|
||||
expect(build.reload.artifacts_file_identifier).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
it 'does remove files' do
|
||||
expect(build.reload.artifacts_file.exists?).to be_falsey
|
||||
end
|
||||
context 'when associated project was removed' do
|
||||
let(:build) do
|
||||
create(:ci_build, :artifacts, artifacts_expiry) do |build|
|
||||
build.project.delete
|
||||
end
|
||||
end
|
||||
|
||||
it 'does nullify artifacts_file column' do
|
||||
expect(build.reload.artifacts_file_identifier).to be_nil
|
||||
it 'does not remove artifacts' do
|
||||
expect(build.reload.artifacts_file.exists?).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with not yet expired artifacts' do
|
||||
let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) }
|
||||
let(:build) do
|
||||
create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days)
|
||||
end
|
||||
|
||||
it 'does not expire' do
|
||||
expect(build.reload.artifacts_expired?).to be_falsey
|
||||
|
|
Loading…
Reference in a new issue