Merge branch '296616-remove-archive-after-failed-attempts' into 'master'

Clean up failed archive when no more attempts left

See merge request gitlab-org/gitlab!71878

app/services/ci/archive_trace_service.rb

@@ -3,10 +3,15 @@
 module Ci
   class ArchiveTraceService
     def execute(job, worker_name:)
+      unless job.trace.archival_attempts_available?
+        Sidekiq.logger.warn(class: worker_name, message: 'The job is out of archival attempts.', job_id: job.id)
+
+        job.trace.attempt_archive_cleanup!
+        return
+      end
+
       unless job.trace.can_attempt_archival_now?
-        Sidekiq.logger.warn(class: worker_name,
-                            message: job.trace.archival_attempts_message,
-                            job_id: job.id)
+        Sidekiq.logger.warn(class: worker_name, message: 'The job can not be archived right now.', job_id: job.id)
         return
       end
 

lib/gitlab/ci/trace.rb

@@ -25,7 +25,7 @@ module Gitlab
 
       delegate :old_trace, to: :job
       delegate :can_attempt_archival_now?, :increment_archival_attempts!,
-        :archival_attempts_message, to: :trace_metadata
+        :archival_attempts_message, :archival_attempts_available?, to: :trace_metadata
 
       def initialize(job)
         @job = job
@@ -122,6 +122,10 @@ module Gitlab
         end
       end
 
+      def attempt_archive_cleanup!
+        destroy_any_orphan_trace_data!
+      end
+
       def update_interval
         if being_watched?
           UPDATE_FREQUENCY_WHEN_BEING_WATCHED
@@ -191,7 +195,10 @@ module Gitlab
       def unsafe_archive!
         raise ArchiveError, 'Job is not finished yet' unless job.complete?
 
-        unsafe_trace_conditionally_cleanup_before_retry!
+        already_archived?.tap do |archived|
+          destroy_any_orphan_trace_data!
+          raise AlreadyArchivedError, 'Could not archive again' if archived
+        end
 
         if job.trace_chunks.any?
           Gitlab::Ci::Trace::ChunkedIO.new(job) do |stream|
@@ -214,16 +221,15 @@ module Gitlab
       def already_archived?
         # TODO check checksum to ensure archive completed successfully
         # See https://gitlab.com/gitlab-org/gitlab/-/issues/259619
-        trace_artifact.archived_trace_exists?
+        trace_artifact&.archived_trace_exists?
       end
 
-      def unsafe_trace_conditionally_cleanup_before_retry!
+      def destroy_any_orphan_trace_data!
         return unless trace_artifact
 
         if already_archived?
           # An archive already exists, so make sure to remove the trace chunks
           erase_trace_chunks!
-          raise AlreadyArchivedError, 'Could not archive again'
         else
           # An archive already exists, but its associated file does not, so remove it
           trace_artifact.destroy!

spec/services/ci/archive_trace_service_spec.rb

@@ -88,6 +88,32 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do
 
         subject
       end
+
+      context 'job has archive and chunks' do
+        let(:job) { create(:ci_build, :success, :trace_artifact) }
+
+        before do
+          create(:ci_build_trace_chunk, build: job, chunk_index: 0)
+        end
+
+        context 'archive is not completed' do
+          before do
+            job.job_artifacts_trace.file.remove!
+          end
+
+          it 'cleanups any stale archive data' do
+            expect(job.job_artifacts_trace).to be_present
+
+            subject
+
+            expect(job.reload.job_artifacts_trace).to be_nil
+          end
+        end
+
+        it 'removes trace chunks' do
+          expect { subject }.to change { job.trace_chunks.count }.to(0)
+        end
+      end
     end
 
     context 'when the archival process is backed off' do