diff options
Diffstat (limited to 'spec/services/ci/job_artifacts')
3 files changed, 184 insertions, 19 deletions
diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index e95a449d615..1c6963e4a31 100644 --- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -19,8 +19,23 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } context 'with preloaded relationships' do + let(:second_artifact) { create(:ci_job_artifact, :expired, :junit, job: job) } + + let(:more_artifacts) do + [ + create(:ci_job_artifact, :expired, :sast, job: job), + create(:ci_job_artifact, :expired, :metadata, job: job), + create(:ci_job_artifact, :expired, :codequality, job: job), + create(:ci_job_artifact, :expired, :accessibility, job: job) + ] + end + before do - stub_const("#{described_class}::LARGE_LOOP_LIMIT", 1) + stub_const("#{described_class}::LOOP_LIMIT", 1) + + # This artifact-with-file is created before the control execution to ensure + # that the DeletedObject operations are accounted for in the query count. + second_artifact end context 'with ci_destroy_unlocked_job_artifacts feature flag disabled' do @@ -28,19 +43,12 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s stub_feature_flags(ci_destroy_unlocked_job_artifacts: false) end - it 'performs the smallest number of queries for job_artifacts' do - log = ActiveRecord::QueryRecorder.new { subject } + it 'performs a consistent number of queries' do + control = ActiveRecord::QueryRecorder.new { service.execute } - # SELECT expired ci_job_artifacts - 3 queries from each_batch - # PRELOAD projects, routes, project_statistics - # BEGIN - # INSERT into ci_deleted_objects - # DELETE loaded ci_job_artifacts - # DELETE security_findings -- for EE - # COMMIT - # SELECT next expired ci_job_artifacts + more_artifacts - expect(log.count).to be_within(1).of(10) + expect { subject }.not_to exceed_query_limit(control.count) end end @@ -49,9 +57,12 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s stub_feature_flags(ci_destroy_unlocked_job_artifacts: true) end - it 'performs the smallest number of queries for job_artifacts' do - log = ActiveRecord::QueryRecorder.new { subject } - expect(log.count).to be_within(1).of(8) + it 'performs a consistent number of queries' do + control = ActiveRecord::QueryRecorder.new { service.execute } + + more_artifacts + + expect { subject }.not_to exceed_query_limit(control.count) end end end @@ -119,7 +130,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } before do - stub_const("#{described_class}::LARGE_LOOP_LIMIT", 10) + stub_const("#{described_class}::LOOP_LIMIT", 10) end context 'when the import fails' do @@ -189,8 +200,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s context 'when loop reached loop limit' do before do - stub_feature_flags(ci_artifact_fast_removal_large_loop_limit: false) - stub_const("#{described_class}::SMALL_LOOP_LIMIT", 1) + stub_const("#{described_class}::LOOP_LIMIT", 1) end it 'destroys one artifact' do diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index 67d664a617b..5e77041a632 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::JobArtifacts::DestroyBatchService do - let(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id]) } + let(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id, trace_artifact.id]) } let(:service) { described_class.new(artifacts, pick_up_at: Time.current) } let_it_be(:artifact_with_file, refind: true) do @@ -18,6 +18,10 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do create(:ci_job_artifact) end + let_it_be(:trace_artifact, refind: true) do + create(:ci_job_artifact, :trace, :expired) + end + describe '.execute' do subject(:execute) { service.execute } @@ -42,6 +46,12 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do execute end + it 'preserves trace artifacts and removes any timestamp' do + expect { subject } + .to change { trace_artifact.reload.expire_at }.from(trace_artifact.expire_at).to(nil) + .and not_change { Ci::JobArtifact.exists?(trace_artifact.id) } + end + context 'ProjectStatistics' do it 'resets project statistics' do expect(ProjectStatistics).to receive(:increment_statistic).once diff --git a/spec/services/ci/job_artifacts/update_unknown_locked_status_service_spec.rb b/spec/services/ci/job_artifacts/update_unknown_locked_status_service_spec.rb new file mode 100644 index 00000000000..67412e41fb8 --- /dev/null +++ b/spec/services/ci/job_artifacts/update_unknown_locked_status_service_spec.rb @@ -0,0 +1,145 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobArtifacts::UpdateUnknownLockedStatusService, :clean_gitlab_redis_shared_state do + include ExclusiveLeaseHelpers + + let(:service) { described_class.new } + + describe '.execute' do + subject { service.execute } + + let_it_be(:locked_pipeline) { create(:ci_pipeline, :artifacts_locked) } + let_it_be(:pipeline) { create(:ci_pipeline, :unlocked) } + let_it_be(:locked_job) { create(:ci_build, :success, pipeline: locked_pipeline) } + let_it_be(:job) { create(:ci_build, :success, pipeline: pipeline) } + + let!(:unknown_unlocked_artifact) do + create(:ci_job_artifact, :junit, expire_at: 1.hour.ago, job: job, locked: Ci::JobArtifact.lockeds[:unknown]) + end + + let!(:unknown_locked_artifact) do + create(:ci_job_artifact, :lsif, + expire_at: 1.day.ago, + job: locked_job, + locked: Ci::JobArtifact.lockeds[:unknown] + ) + end + + let!(:unlocked_artifact) do + create(:ci_job_artifact, :archive, expire_at: 1.hour.ago, job: job, locked: Ci::JobArtifact.lockeds[:unlocked]) + end + + let!(:locked_artifact) do + create(:ci_job_artifact, :sast, :raw, + expire_at: 1.day.ago, + job: locked_job, + locked: Ci::JobArtifact.lockeds[:artifacts_locked] + ) + end + + context 'when artifacts are expired' do + it 'sets artifact_locked when the pipeline is locked' do + expect { service.execute } + .to change { unknown_locked_artifact.reload.locked }.from('unknown').to('artifacts_locked') + .and not_change { Ci::JobArtifact.exists?(locked_artifact.id) } + end + + it 'destroys the artifact when the pipeline is unlocked' do + expect { subject }.to change { Ci::JobArtifact.exists?(unknown_unlocked_artifact.id) }.from(true).to(false) + end + + it 'does not update ci_job_artifact rows with known locked values' do + expect { service.execute } + .to not_change(locked_artifact, :attributes) + .and not_change { Ci::JobArtifact.exists?(locked_artifact.id) } + .and not_change(unlocked_artifact, :attributes) + .and not_change { Ci::JobArtifact.exists?(unlocked_artifact.id) } + end + + it 'logs the counts of affected artifacts' do + expect(subject).to eq({ removed: 1, locked: 1 }) + end + end + + context 'in a single iteration' do + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + end + + context 'due to the LOOP_TIMEOUT' do + before do + stub_const("#{described_class}::LOOP_TIMEOUT", 0.seconds) + end + + it 'affects the earliest expired artifact first' do + subject + + expect(unknown_locked_artifact.reload.locked).to eq('artifacts_locked') + expect(unknown_unlocked_artifact.reload.locked).to eq('unknown') + end + + it 'reports the number of destroyed artifacts' do + is_expected.to eq({ removed: 0, locked: 1 }) + end + end + + context 'due to @loop_limit' do + before do + stub_const("#{described_class}::LARGE_LOOP_LIMIT", 1) + end + + it 'affects the most recently expired artifact first' do + subject + + expect(unknown_locked_artifact.reload.locked).to eq('artifacts_locked') + expect(unknown_unlocked_artifact.reload.locked).to eq('unknown') + end + + it 'reports the number of destroyed artifacts' do + is_expected.to eq({ removed: 0, locked: 1 }) + end + end + end + + context 'when artifact is not expired' do + let!(:unknown_unlocked_artifact) do + create(:ci_job_artifact, :junit, + expire_at: 1.year.from_now, + job: job, + locked: Ci::JobArtifact.lockeds[:unknown] + ) + end + + it 'does not change the locked status' do + expect { service.execute }.not_to change { unknown_unlocked_artifact.locked } + expect(Ci::JobArtifact.exists?(unknown_unlocked_artifact.id)).to eq(true) + end + end + + context 'when exclusive lease has already been taken by the other instance' do + before do + stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY, timeout: described_class::LOCK_TIMEOUT) + end + + it 'raises an error and' do + expect { subject }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + end + end + + context 'when there are no unknown status artifacts' do + before do + Ci::JobArtifact.update_all(locked: :unlocked) + end + + it 'does not raise error' do + expect { subject }.not_to raise_error + end + + it 'reports the number of destroyed artifacts' do + is_expected.to eq({ removed: 0, locked: 0 }) + end + end + end +end |