Merge branch '330325-improve-logs' into 'master'

Cleanup policies worker, improve logs

See merge request gitlab-org/gitlab!66182

app/workers/container_expiration_policies/cleanup_container_repository_worker.rb

@@ -63,16 +63,23 @@ module ContainerExpirationPolicies
     def container_repository
       strong_memoize(:container_repository) do
         ContainerRepository.transaction do
-          # We need a lock to prevent two workers from picking up the same row
-          container_repository = next_container_repository
+          repository = next_container_repository
 
-          container_repository&.tap(&:cleanup_ongoing!)
+          repository&.tap do |repo|
+            log_info(
+              project_id: repo.project_id,
+              container_repository_id: repo.id
+            )
+
+            repo.cleanup_ongoing!
+          end
         end
       end
     end
 
     def next_container_repository
       # rubocop: disable CodeReuse/ActiveRecord
+      # We need a lock to prevent two workers from picking up the same row
       next_one_requiring = ContainerRepository.requiring_cleanup
                                               .order(:expiration_policy_cleanup_status, :expiration_policy_started_at)
                                               .limit(1)

spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb

@@ -25,6 +25,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
         expect(ContainerExpirationPolicies::CleanupService)
           .to receive(:new).with(repository).and_return(double(execute: service_response))
         expect_log_extra_metadata(service_response: service_response)
+        expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
         subject
       end
@@ -35,6 +36,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
           expect(ContainerExpirationPolicies::CleanupService)
             .to receive(:new).with(repository).and_return(double(execute: service_response))
           expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished)
+          expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
           subject
         end
@@ -45,6 +47,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
             expect(ContainerExpirationPolicies::CleanupService)
               .to receive(:new).with(repository).and_return(double(execute: service_response))
             expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished, truncated: true)
+            expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
             subject
           end
@@ -65,6 +68,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
               expect(ContainerExpirationPolicies::CleanupService)
                 .to receive(:new).with(repository).and_return(double(execute: service_response))
               expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished, truncated: truncated)
+              expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
               subject
             end
@@ -78,6 +82,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
           expect(ContainerExpirationPolicies::CleanupService)
             .to receive(:new).with(repository).and_return(double(execute: service_response))
           expect_log_extra_metadata(service_response: service_response, cleanup_status: :error)
+          expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
           subject
         end
@@ -361,6 +366,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
         expect(ContainerExpirationPolicies::CleanupService)
           .to receive(:new).with(repository).and_return(double(execute: service_response))
         expect_log_extra_metadata(service_response: service_response)
+        expect_log_info(project_id: project.id, container_repository_id: repository.id)
 
         subject
       end
@@ -396,6 +402,11 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
         expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_error_message, service_response.message)
       end
     end
+
+    def expect_log_info(structure)
+      expect(worker.logger)
+        .to receive(:info).with(worker.structured_payload(structure))
+    end
   end
 
   describe '#remaining_work_count' do
@@ -446,6 +457,12 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
       end
 
       it { is_expected.to eq(0) }
+
+      it 'does not log a selected container' do
+        expect(worker).not_to receive(:log_info)
+
+        subject
+      end
     end
   end