Merge branch '216117_retry_vulnerability_export_jobs' into 'master'

Retry failed vulnerability export jobs up to 3 times

See merge request gitlab-org/gitlab!33986

ee/app/models/vulnerabilities/export.rb

@@ -34,6 +34,10 @@ module Vulnerabilities
         transition [:created, :running] => :failed
       end
 
+      event :reset_state do
+        transition running: :created
+      end
+
       state :created
       state :running
       state :finished

ee/app/services/vulnerability_exports/export_service.rb

@@ -28,8 +28,7 @@ module VulnerabilityExports
 
     attr_accessor :vulnerability_export
 
-    delegate :exportable, to: :vulnerability_export, private: true
-    delegate :format, to: :vulnerability_export, private: true
+    delegate :exportable, :format, to: :vulnerability_export, private: true
 
     def lease_key
       "#{LEASE_NAMESPACE}:#{vulnerability_export.id}"
@@ -39,9 +38,9 @@ module VulnerabilityExports
       vulnerability_export.start!
       generate_export_file
       vulnerability_export.finish!
-    rescue => error
-      vulnerability_export.failed!
-      raise(error)
+    rescue StandardError
+      vulnerability_export.reset_state!
+      raise
     ensure
       schedule_export_deletion
     end

ee/app/workers/vulnerability_exports/export_worker.rb

@@ -5,20 +5,22 @@ module VulnerabilityExports
     include ApplicationWorker
     include ::Gitlab::ExclusiveLeaseHelpers
 
-    LEASE_TIMEOUT = 1.hour
+    sidekiq_options dead: false
 
     feature_category :vulnerability_management
     worker_resource_boundary :cpu
 
     idempotent!
 
+    sidekiq_retries_exhausted do |job|
+      Vulnerabilities::Export.find_by_id(job['args'].last).failed!
+    end
+
     def perform(_exportable_id = nil, vulnerability_export_id) # rubocop:disable Style/OptionalArguments
       vulnerability_export = Vulnerabilities::Export.find_by_id(vulnerability_export_id)
       return unless vulnerability_export
 
       ExportService.export(vulnerability_export)
-    rescue => error
-      logger.error class: self.class.name, message: error.message
     end
   end
 end

ee/changelogs/unreleased/216117_retry_vulnerability_export_jobs.yml

+---
+title: Retry failed vulnerability export background jobs
+merge_request: 33986
+author:
+type: added

ee/spec/services/vulnerability_exports/export_service_spec.rb

@@ -66,9 +66,9 @@ RSpec.describe VulnerabilityExports::ExportService do
           allow(service_object).to receive(:generate_export_file).and_raise(error)
         end
 
-        it 'marks the export object as `failed` and propagates the error to the caller' do
+        it 'sets the state of export back to `created`' do
           expect { export }.to raise_error(error)
-          expect(vulnerability_export.failed?).to be_truthy
+          expect(vulnerability_export.reload.created?).to be_truthy
         end
 
         it 'schedules the export deletion background job' do

ee/spec/workers/vulnerability_exports/export_worker_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe VulnerabilityExports::ExportWorker, type: :worker do
-  describe '#perform' do
-    let(:worker) { described_class.new }
+  let(:worker) { described_class.new }
 
+  describe '#perform' do
     subject(:export_vulnerabilities) { worker.perform(vulnerability_export_id) }
 
     before do
@@ -36,23 +36,35 @@ RSpec.describe VulnerabilityExports::ExportWorker, type: :worker do
 
         expect(VulnerabilityExports::ExportService).to have_received(:export).with(vulnerability_export)
       end
+    end
+  end
 
-      context 'when export fails' do
-        let(:error_message) { 'Foo' }
+  describe 'sidekiq_retries_exhausted callback' do
+    let(:sidekiq_retry_handler) { Sidekiq::JobRetry.new }
+    let(:vulnerability_export) { create(:vulnerability_export, :created, :csv) }
+    let(:default_job_payload) { { 'class' => described_class.name, 'args' => [vulnerability_export.id] } }
 
-        before do
-          allow(VulnerabilityExports::ExportService).to receive(:export).and_raise(error_message)
-        end
+    subject(:run_job) do
+      sidekiq_retry_handler.local(worker, job_payload, 'default') do
+        raise 'Foo'
+      end
+    end
+
+    context 'when the max retry count is not reached' do
+      let(:job_payload) { default_job_payload.merge('retry_count' => 1) }
 
-        it 'does not raise exception' do
-          expect { export_vulnerabilities }.not_to raise_error
-        end
+      it 'does not mark the vulnerability export object as failed' do
+        expect { run_job }.to raise_error(Sidekiq::JobRetry::Skip)
+                          .and not_change { vulnerability_export.reload.failed? }.from(false)
+      end
+    end
 
-        it 'logs error' do
-          export_vulnerabilities
+    context 'when the max retry count is reached' do
+      let(:job_payload) { default_job_payload.merge('retry_count' => 2) }
 
-          expect(Sidekiq.logger).to have_received(:error).with(class: described_class.name, message: error_message)
-        end
+      it 'marks the vulnerability export object as failed' do
+        expect { run_job }.to raise_error(Sidekiq::JobRetry::Skip)
+                          .and change { vulnerability_export.reload.failed? }.from(false).to(true)
       end
     end
   end