Merge branch 'mo-keep-latest-pipeline-artifact-forever' into 'master'

Keep latest pipeline artifact forever

See merge request gitlab-org/gitlab!60400

app/models/ci/pipeline_artifact.rb

@@ -40,6 +40,8 @@ module Ci
       code_quality_mr_diff: 2
     }
 
+    scope :unlocked, -> { joins(:pipeline).merge(::Ci::Pipeline.unlocked) }
+
     class << self
       def report_exists?(file_type)
         return false unless REPORT_TYPES.key?(file_type)

app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb

@@ -25,7 +25,7 @@ module Ci
       private
 
       def destroy_artifacts_batch
-        artifacts = ::Ci::PipelineArtifact.expired(BATCH_SIZE).to_a
+        artifacts = ::Ci::PipelineArtifact.unlocked.expired(BATCH_SIZE).to_a
         return false if artifacts.empty?
 
         artifacts.each(&:destroy!)

changelogs/unreleased/mo-keep-latest-pipeline-artifact-forever.yml

+---
+title: Keep latest pipeline artifact forever
+merge_request: 60400
+author:
+type: added

doc/ci/pipelines/pipeline_artifacts.md

@@ -15,4 +15,6 @@ Pipeline artifacts are used by the [test coverage visualization feature](../../u
 
 Pipeline artifacts are saved to disk or object storage. They count towards a project's [storage usage quota](../../user/usage_quotas.md#storage-usage-quota). The **Artifacts** on the Usage Quotas page is the sum of all job artifacts and pipeline artifacts.
 
-Pipeline artifacts are erased after one week.
+## When pipeline artifacts are deleted
+
+The latest artifacts for refs are locked against deletion, and kept regardless of the expiry time.

spec/factories/ci/pipeline_artifacts.rb

@@ -13,6 +13,10 @@ FactoryBot.define do
         Rails.root.join('spec/fixtures/pipeline_artifacts/code_coverage.json'), 'application/json')
     end
 
+    trait :unlocked do
+      association :pipeline, :unlocked, factory: :ci_pipeline
+    end
+
     trait :checksummed do
       verification_checksum { 'abc' }
     end

spec/factories/ci/pipelines.rb

@@ -81,6 +81,10 @@ FactoryBot.define do
         status { :failed }
       end
 
+      trait :unlocked do
+        locked { Ci::Pipeline.lockeds[:unlocked] }
+      end
+
       trait :protected do
         add_attribute(:protected) { true }
       end

spec/models/ci/pipeline_artifact_spec.rb

@@ -50,6 +50,30 @@ RSpec.describe Ci::PipelineArtifact, type: :model do
     end
   end
 
+  describe 'scopes' do
+    describe '.unlocked' do
+      subject(:pipeline_artifacts) { described_class.unlocked }
+
+      context 'when pipeline is locked' do
+        it 'returns an empty collection' do
+          expect(pipeline_artifacts).to be_empty
+        end
+      end
+
+      context 'when pipeline is unlocked' do
+        before do
+          create(:ci_pipeline_artifact, :with_coverage_report)
+        end
+
+        it 'returns unlocked artifacts' do
+          codequality_report = create(:ci_pipeline_artifact, :with_codequality_mr_diff_report, :unlocked)
+
+          expect(pipeline_artifacts).to eq([codequality_report])
+        end
+      end
+    end
+  end
+
   describe 'file is being stored' do
     subject { create(:ci_pipeline_artifact, :with_coverage_report) }
 

spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb

@@ -30,7 +30,7 @@ RSpec.describe Ci::PipelineArtifacts::DestroyAllExpiredService do
         stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::LOOP_LIMIT', 1)
         stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1)
 
-        create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago)
+        create_list(:ci_pipeline_artifact, 2, :unlocked, expire_at: 1.week.ago)
       end
 
       it 'destroys one artifact' do
@@ -46,7 +46,7 @@ RSpec.describe Ci::PipelineArtifacts::DestroyAllExpiredService do
       before do
         stub_const('Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1)
 
-        create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago)
+        create_list(:ci_pipeline_artifact, 2, :unlocked, expire_at: 1.week.ago)
       end
 
       it 'destroys all expired artifacts' do
@@ -60,7 +60,21 @@ RSpec.describe Ci::PipelineArtifacts::DestroyAllExpiredService do
 
     context 'when artifacts are not expired' do
       before do
-        create(:ci_pipeline_artifact, expire_at: 2.days.from_now)
+        create(:ci_pipeline_artifact, :unlocked, expire_at: 2.days.from_now)
+      end
+
+      it 'does not destroy pipeline artifacts' do
+        expect { subject }.not_to change { Ci::PipelineArtifact.count }
+      end
+
+      it 'reports the number of destroyed artifacts' do
+        is_expected.to eq(0)
+      end
+    end
+
+    context 'when pipeline is locked' do
+      before do
+        create(:ci_pipeline_artifact, expire_at: 2.weeks.ago)
       end
 
       it 'does not destroy pipeline artifacts' do

spec/workers/ci/pipeline_artifacts/expire_artifacts_worker_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Ci::PipelineArtifacts::ExpireArtifactsWorker do
 
   describe '#perform' do
     let_it_be(:pipeline_artifact) do
-      create(:ci_pipeline_artifact, :with_coverage_report, expire_at: 1.week.ago)
+      create(:ci_pipeline_artifact, :with_coverage_report, :unlocked, expire_at: 1.week.ago)
     end
 
     it 'executes a service' do