Merge branch '296616-continue-trace-archive-if-not-complete' into 'master'

Retry CI trace archive if left in incomplete state

See merge request gitlab-org/gitlab!68906

lib/gitlab/ci/trace.rb

@@ -191,11 +191,7 @@ module Gitlab
       def unsafe_archive!
         raise ArchiveError, 'Job is not finished yet' unless job.complete?
 
-        if trace_artifact
-          unsafe_trace_cleanup!
-
-          raise AlreadyArchivedError, 'Could not archive again'
-        end
+        unsafe_trace_conditionally_cleanup_before_retry!
 
         if job.trace_chunks.any?
           Gitlab::Ci::Trace::ChunkedIO.new(job) do |stream|
@@ -215,12 +211,19 @@ module Gitlab
         end
       end
 
-      def unsafe_trace_cleanup!
+      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?
+      end
+
+      def unsafe_trace_conditionally_cleanup_before_retry!
         return unless trace_artifact
 
-        if trace_artifact.archived_trace_exists?
+        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

@@ -28,7 +28,7 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do
 
       context 'when live trace chunks still exist' do
         before do
-          create(:ci_build_trace_chunk, build: job)
+          create(:ci_build_trace_chunk, build: job, chunk_index: 0)
         end
 
         it 'removes the trace chunks' do
@@ -40,8 +40,14 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do
             job.job_artifacts_trace.file.remove!
           end
 
-          it 'removes the trace artifact' do
-            expect { subject }.to change { job.reload.job_artifacts_trace }.to(nil)
+          it 'removes the trace artifact and builds a new one' do
+            existing_trace = job.job_artifacts_trace
+            expect(existing_trace).to receive(:destroy!).and_call_original
+
+            subject
+
+            expect(job.reload.job_artifacts_trace).to be_present
+            expect(job.reload.job_artifacts_trace.file.file).to be_present
           end
         end
       end