Merge branch '288812-cleanup-policies-hard-limit-the-number-of-tags-to-delete' into 'master'

Limit the number of container tags to delete in a single pass

See merge request gitlab-org/gitlab!49961

app/models/application_setting.rb

@@ -303,6 +303,9 @@ class ApplicationSetting < ApplicationRecord
   validates :container_registry_delete_tags_service_timeout,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
+  validates :container_registry_cleanup_tags_service_max_list_size,
+            numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+
   validates :container_registry_expiration_policies_worker_capacity,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 

app/services/container_expiration_policies/cleanup_service.rb

@@ -4,6 +4,8 @@ module ContainerExpirationPolicies
   class CleanupService
     attr_reader :repository
 
+    SERVICE_RESULT_FIELDS = %i[original_size before_truncate_size after_truncate_size before_delete_size].freeze
+
     def initialize(repository)
       @repository = repository
     end
@@ -13,28 +15,37 @@ module ContainerExpirationPolicies
 
       repository.start_expiration_policy!
 
-      result = Projects::ContainerRepository::CleanupTagsService
+      service_result = Projects::ContainerRepository::CleanupTagsService
         .new(project, nil, policy_params.merge('container_expiration_policy' => true))
         .execute(repository)
 
-      if result[:status] == :success
+      if service_result[:status] == :success
         repository.update!(
           expiration_policy_cleanup_status: :cleanup_unscheduled,
           expiration_policy_started_at: nil,
           expiration_policy_completed_at: Time.zone.now
         )
-        success(:finished)
+        success(:finished, service_result)
       else
         repository.cleanup_unfinished!
 
-        success(:unfinished)
+        success(:unfinished, service_result)
       end
     end
 
     private
 
-    def success(cleanup_status)
-      ServiceResponse.success(message: "cleanup #{cleanup_status}", payload: { cleanup_status: cleanup_status, container_repository_id: repository.id })
+    def success(cleanup_status, service_result)
+      payload = {
+        cleanup_status: cleanup_status,
+        container_repository_id: repository.id
+      }
+
+      SERVICE_RESULT_FIELDS.each do |field|
+        payload["cleanup_tags_service_#{field}".to_sym] = service_result[field]
+      end
+
+      ServiceResponse.success(message: "cleanup #{cleanup_status}", payload: payload)
     end
 
     def policy_params

app/services/projects/container_repository/cleanup_tags_service.rb

@@ -8,12 +8,26 @@ module Projects
         return error('invalid regex') unless valid_regex?
 
         tags = container_repository.tags
+        original_size = tags.size
+
         tags = without_latest(tags)
         tags = filter_by_name(tags)
+
+        before_truncate_size = tags.size
+        tags = truncate(tags)
+        after_truncate_size = tags.size
+
         tags = filter_keep_n(tags)
         tags = filter_by_older_than(tags)
 
-        delete_tags(container_repository, tags)
+        delete_tags(container_repository, tags).tap do |result|
+          result[:original_size] = original_size
+          result[:before_truncate_size] = before_truncate_size
+          result[:after_truncate_size] = after_truncate_size
+          result[:before_delete_size] = tags.size
+
+          result[:status] = :error if before_truncate_size != after_truncate_size
+        end
       end
 
       private
@@ -23,12 +37,14 @@ module Projects
 
         tag_names = tags.map(&:name)
 
-        Projects::ContainerRepository::DeleteTagsService
-          .new(container_repository.project,
+        service = Projects::ContainerRepository::DeleteTagsService.new(
+          container_repository.project,
           current_user,
           tags: tag_names,
-               container_expiration_policy: params['container_expiration_policy'])
-          .execute(container_repository)
+          container_expiration_policy: params['container_expiration_policy']
+        )
+
+        service.execute(container_repository)
       end
 
       def without_latest(tags)
@@ -54,7 +70,7 @@ module Projects
         return tags unless params['keep_n']
 
         tags = order_by_date(tags)
-        tags.drop(params['keep_n'].to_i)
+        tags.drop(keep_n)
       end
 
       def filter_by_older_than(tags)
@@ -83,6 +99,31 @@ module Projects
         ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id)
         false
       end
+
+      def truncate(tags)
+        return tags unless throttling_enabled?
+        return tags if max_list_size == 0
+
+        # truncate the list to make sure that after the #filter_keep_n
+        # execution, the resulting list will be max_list_size
+        truncated_size = max_list_size + keep_n
+
+        return tags if tags.size <= truncated_size
+
+        tags.sample(truncated_size)
+      end
+
+      def throttling_enabled?
+        Feature.enabled?(:container_registry_expiration_policies_throttling)
+      end
+
+      def max_list_size
+        ::Gitlab::CurrentSettings.current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i
+      end
+
+      def keep_n
+        params['keep_n'].to_i
+      end
     end
   end
 end

app/workers/container_expiration_policies/cleanup_container_repository_worker.rb

@@ -12,6 +12,14 @@ module ContainerExpirationPolicies
     worker_resource_boundary :unknown
     idempotent!
 
+    LOG_ON_DONE_FIELDS = %i[
+      cleanup_status
+      cleanup_tags_service_original_size
+      cleanup_tags_service_before_truncate_size
+      cleanup_tags_service_after_truncate_size
+      cleanup_tags_service_before_delete_size
+    ].freeze
+
     def perform_work
       return unless throttling_enabled?
       return unless container_repository
@@ -26,7 +34,7 @@ module ContainerExpirationPolicies
 
       result = ContainerExpirationPolicies::CleanupService.new(container_repository)
                                                           .execute
-      log_extra_metadata_on_done(:cleanup_status, result.payload[:cleanup_status])
+      log_on_done(result)
     end
 
     def remaining_work_count
@@ -92,5 +100,15 @@ module ContainerExpirationPolicies
     def log_info(extra_structure)
       logger.info(structured_payload(extra_structure))
     end
+
+    def log_on_done(result)
+      LOG_ON_DONE_FIELDS.each do |field|
+        value = result.payload[field]
+
+        next if value.nil?
+
+        log_extra_metadata_on_done(field, value)
+      end
+    end
   end
 end

changelogs/unreleased/288812-cleanup-policies-hard-limit-the-number-of-tags-to-delete.yml

+---
+title: Limit the number of container tags to delete when deleting them in bulk
+merge_request: 49961
+author:
+type: changed

db/migrate/20201214111858_add_container_registry_cleanup_tags_service_max_list_size_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddContainerRegistryCleanupTagsServiceMaxListSizeToApplicationSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column(:application_settings, :container_registry_cleanup_tags_service_max_list_size, :integer, default: 200, null: false)
+  end
+end

db/migrate/20201214112752_add_app_settings_container_reg_cleanup_tags_service_max_list_size_constraint.rb

+# frozen_string_literal: true
+
+class AddAppSettingsContainerRegCleanupTagsServiceMaxListSizeConstraint < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  CONSTRAINT_NAME = 'app_settings_container_reg_cleanup_tags_max_list_size_positive'
+
+  disable_ddl_transaction!
+
+  def up
+    add_check_constraint :application_settings, 'container_registry_cleanup_tags_service_max_list_size >= 0', CONSTRAINT_NAME
+  end
+
+  def down
+    remove_check_constraint :application_settings, CONSTRAINT_NAME
+  end
+end

db/schema_migrations/20201214111858

+e3eb306d7956e396f038f07a20c674929fc4aae3e188b24e1087305b3fea8b4d
\ No newline at end of file

db/schema_migrations/20201214112752

+59906a3528499dd9c0b4c30088539299c08383ad3b36be8c862a7cc9ef1d50b2
\ No newline at end of file

db/structure.sql

@@ -9377,6 +9377,8 @@ CREATE TABLE application_settings (
     disable_feed_token boolean DEFAULT false NOT NULL,
     personal_access_token_prefix text,
     rate_limiting_response_text text,
+    container_registry_cleanup_tags_service_max_list_size integer DEFAULT 200 NOT NULL,
+    CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),

doc/api/container_registry.md

@@ -313,6 +313,13 @@ You can run this at most once an hour for a given container repository. This
 action doesn't delete blobs. To delete them and recycle disk space,
 [run the garbage collection](https://docs.gitlab.com/omnibus/maintenance/README.html#removing-unused-layers-not-referenced-by-manifests).
 
+WARNING:
+The number of tags deleted by this API is limited on GitLab.com
+because of the scale of the Container Registry there.
+If your Container Registry has a large number of tags to delete,
+only some of them will be deleted, and you might need to call this API multiple times.
+To schedule tags for automatic deletion, use a [cleanup policy](../user/packages/container_registry/index.md#cleanup-policy) instead.
+
 NOTE:
 In GitLab 12.4 and later, individual tags are deleted.
 For more details, see the [discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/15737).

spec/models/application_setting_spec.rb

@@ -72,6 +72,7 @@ RSpec.describe ApplicationSetting do
     it { is_expected.not_to allow_value(nil).for(:push_event_activities_limit) }
 
     it { is_expected.to validate_numericality_of(:container_registry_delete_tags_service_timeout).only_integer.is_greater_than_or_equal_to(0) }
+    it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) }
 
     it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) }

spec/services/container_expiration_policies/cleanup_service_spec.rb

@@ -34,10 +34,14 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
     end
 
     context 'without a successful cleanup tags service execution' do
-      it 'partially clean up the repository' do
+      let(:cleanup_tags_service_response) { { status: :error, message: 'timeout' } }
+
+      before do
         expect(Projects::ContainerRepository::CleanupTagsService)
-            .to receive(:new).and_return(double(execute: { status: :error, message: 'timeout' }))
+          .to receive(:new).and_return(double(execute: cleanup_tags_service_response))
+      end
 
+      it 'partially clean up the repository' do
         response = subject
 
         aggregate_failures "checking the response and container repositories" do
@@ -49,6 +53,39 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
           expect(repository.expiration_policy_completed_at).to eq(nil)
         end
       end
+
+      context 'with a truncated cleanup tags service response' do
+        let(:cleanup_tags_service_response) do
+          {
+            status: :error,
+            original_size: 1000,
+            before_truncate_size: 800,
+            after_truncate_size: 200,
+            before_delete_size: 100
+          }
+        end
+
+        it 'partially clean up the repository' do
+          response = subject
+
+          aggregate_failures "checking the response and container repositories" do
+            expect(response.success?).to eq(true)
+            expect(response.payload)
+              .to include(
+                cleanup_status: :unfinished,
+                container_repository_id: repository.id,
+                cleanup_tags_service_original_size: 1000,
+                cleanup_tags_service_before_truncate_size: 800,
+                cleanup_tags_service_after_truncate_size: 200,
+                cleanup_tags_service_before_delete_size: 100
+              )
+            expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
+            expect(repository.reload.cleanup_unfinished?).to be_truthy
+            expect(repository.expiration_policy_started_at).not_to eq(nil)
+            expect(repository.expiration_policy_completed_at).to eq(nil)
+          end
+        end
+      end
     end
 
     context 'with no repository' do

spec/services/projects/container_repository/cleanup_tags_service_spec.rb

@@ -3,11 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Projects::ContainerRepository::CleanupTagsService do
+  using RSpec::Parameterized::TableSyntax
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project, :private) }
   let_it_be(:repository) { create(:container_repository, :root, project: project) }
 
   let(:service) { described_class.new(project, user, params) }
+  let(:tags) { %w[latest A Ba Bb C D E] }
 
   before do
     project.add_maintainer(user)
@@ -16,7 +19,8 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
 
     stub_container_registry_tags(
       repository: repository.path,
-      tags: %w(latest A Ba Bb C D E))
+      tags: tags
+    )
 
     stub_tag_digest('latest', 'sha256:configA')
     stub_tag_digest('A', 'sha256:configA')
@@ -30,6 +34,8 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
     stub_digest_config('sha256:configB', 5.days.ago)
     stub_digest_config('sha256:configC', 1.month.ago)
     stub_digest_config('sha256:configD', nil)
+
+    stub_feature_flags(container_registry_expiration_policies_throttling: false)
   end
 
   describe '#execute' do
@@ -42,7 +48,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService)
           .not_to receive(:execute)
 
-        is_expected.to include(status: :success, deleted: [])
+        is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0))
       end
     end
 
@@ -51,7 +57,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         it 'does remove all tags except latest' do
           expect_delete(%w(A Ba Bb C D E))
 
-          is_expected.to include(status: :success, deleted: %w(A Ba Bb C D E))
+          is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E)))
         end
       end
 
@@ -78,12 +84,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
           subject
         end
 
-        it 'returns an error' do
-          response = subject
-
-          expect(response[:status]).to eq(:error)
-          expect(response[:message]).to eq('invalid regex')
-        end
+        it { is_expected.to eq(status: :error, message: 'invalid regex') }
 
         it 'calls error tracking service' do
           expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original
@@ -119,7 +120,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       it 'does remove C and D' do
         expect_delete(%w(C D))
 
-        is_expected.to include(status: :success, deleted: %w(C D))
+        is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2))
       end
 
       context 'with overriding allow regex' do
@@ -131,7 +132,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         it 'does not remove C' do
           expect_delete(%w(D))
 
-          is_expected.to include(status: :success, deleted: %w(D))
+          is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1))
         end
       end
 
@@ -144,7 +145,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         it 'does not remove C' do
           expect_delete(%w(D))
 
-          is_expected.to include(status: :success, deleted: %w(D))
+          is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1))
         end
       end
     end
@@ -158,7 +159,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       it 'does not remove B*' do
         expect_delete(%w(A C D E))
 
-        is_expected.to include(status: :success, deleted: %w(A C D E))
+        is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4))
       end
     end
 
@@ -173,7 +174,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
 
         expect(service).to receive(:order_by_date).and_call_original
 
-        is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+        is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3))
       end
     end
 
@@ -187,7 +188,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
 
         expect(service).not_to receive(:order_by_date)
 
-        is_expected.to include(status: :success, deleted: %w(A Ba Bb C))
+        is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4))
       end
     end
 
@@ -200,7 +201,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       it 'does remove B* and C as they are the oldest' do
         expect_delete(%w(Bb Ba C))
 
-        is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+        is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
       end
     end
 
@@ -213,7 +214,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       it 'does remove B* and C as they are older than 1 day' do
         expect_delete(%w(Ba Bb C))
 
-        is_expected.to include(status: :success, deleted: %w(Ba Bb C))
+        is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3))
       end
     end
 
@@ -227,7 +228,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       it 'does remove B* and C' do
         expect_delete(%w(Bb Ba C))
 
-        is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+        is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
       end
     end
 
@@ -245,7 +246,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         it 'succeeds without a user' do
           expect_delete(%w(Bb Ba C), container_expiration_policy: true)
 
-          is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+          is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
         end
       end
 
@@ -257,8 +258,71 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
         end
 
         it 'fails' do
-          is_expected.to include(status: :error, message: 'access denied')
+          is_expected.to eq(status: :error, message: 'access denied')
+        end
+      end
+    end
+
+    context 'truncating the tags list' do
+      let(:params) do
+        {
+          'name_regex_delete' => '.*',
+          'keep_n' => 1
+        }
+      end
+
+      shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:|
+        it 'returns the response' do
+          result = subject
+
+          service_response = expected_service_response(
+            status: status,
+            original_size: original_size,
+            before_truncate_size: before_truncate_size,
+            after_truncate_size: after_truncate_size,
+            before_delete_size: before_delete_size,
+            deleted: nil
+          )
+
+          expect(result).to eq(service_response.compact)
+        end
+      end
+
+      where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do
+        false | 10 | :success | :success | false
+        false | 10 | :error   | :error   | false
+        false | 3  | :success | :success | false
+        false | 3  | :error   | :error   | false
+        false | 0  | :success | :success | false
+        false | 0  | :error   | :error   | false
+        true  | 10 | :success | :success | false
+        true  | 10 | :error   | :error   | false
+        true  | 3  | :success | :error   | true
+        true  | 3  | :error   | :error   | true
+        true  | 0  | :success | :success | false
+        true  | 0  | :error   | :error   | false
+      end
+
+      with_them do
+        before do
+          stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled)
+          stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size)
+          allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service|
+            expect(service).to receive(:execute).and_return(status: delete_tags_service_status)
+          end
         end
+
+        original_size = 7
+        keep_n = 1
+
+        it_behaves_like(
+          'returning the response',
+          status: params[:expected_status],
+          original_size: original_size,
+          before_truncate_size: original_size - keep_n,
+          after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n,
+          before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter
+        )
       end
     end
   end
@@ -295,4 +359,16 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
       .to receive(:execute)
       .with(repository) { { status: :success, deleted: tags } }
   end
+
+  # all those -1 because the default tags on L13 have a "latest" that will be filtered out
+  def expected_service_response(status: :success, deleted: [], original_size: tags.size, before_truncate_size: tags.size - 1, after_truncate_size: tags.size - 1, before_delete_size: tags.size - 1)
+    {
+      status: status,
+      deleted: deleted,
+      original_size: original_size,
+      before_truncate_size: before_truncate_size,
+      after_truncate_size: after_truncate_size,
+      before_delete_size: before_delete_size
+    }.compact
+  end
 end

spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb

@@ -19,24 +19,35 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
 
     RSpec.shared_examples 'handling all repository conditions' do
       it 'sends the repository for cleaning' do
+        service_response = cleanup_service_response(repository: repository)
         expect(ContainerExpirationPolicies::CleanupService)
-          .to receive(:new).with(repository).and_return(double(execute: cleanup_service_response(repository: repository)))
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :finished)
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id)
+          .to receive(:new).with(repository).and_return(double(execute: service_response))
+        expect_log_extra_metadata(service_response: service_response)
 
         subject
       end
 
       context 'with unfinished cleanup' do
         it 'logs an unfinished cleanup' do
+          service_response = cleanup_service_response(status: :unfinished, repository: repository)
           expect(ContainerExpirationPolicies::CleanupService)
-            .to receive(:new).with(repository).and_return(double(execute: cleanup_service_response(status: :unfinished, repository: repository)))
-          expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :unfinished)
-          expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id)
+            .to receive(:new).with(repository).and_return(double(execute: service_response))
+          expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished)
+
+          subject
+        end
+
+        context 'with a truncated list of tags to delete' do
+          it 'logs an unfinished cleanup' do
+            service_response = cleanup_service_response(status: :unfinished, repository: repository, cleanup_tags_service_after_truncate_size: 10, cleanup_tags_service_before_delete_size: 5)
+            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)
 
             subject
           end
         end
+      end
 
       context 'with policy running shortly' do
         before do
@@ -87,10 +98,10 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
       let_it_be(:another_repository) { create(:container_repository, :cleanup_unfinished) }
 
       it 'process the cleanup scheduled repository first' do
+        service_response = cleanup_service_response(repository: repository)
         expect(ContainerExpirationPolicies::CleanupService)
-          .to receive(:new).with(repository).and_return(double(execute: cleanup_service_response(repository: repository)))
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :finished)
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id)
+          .to receive(:new).with(repository).and_return(double(execute: service_response))
+        expect_log_extra_metadata(service_response: service_response)
 
         subject
       end
@@ -105,10 +116,10 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
       end
 
       it 'process the repository with the oldest expiration_policy_started_at' do
+        service_response = cleanup_service_response(repository: repository)
         expect(ContainerExpirationPolicies::CleanupService)
-          .to receive(:new).with(repository).and_return(double(execute: cleanup_service_response(repository: repository)))
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :finished)
-        expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id)
+          .to receive(:new).with(repository).and_return(double(execute: service_response))
+        expect_log_extra_metadata(service_response: service_response)
 
         subject
       end
@@ -164,8 +175,27 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
       end
     end
 
-    def cleanup_service_response(status: :finished, repository:)
-      ServiceResponse.success(message: "cleanup #{status}", payload: { cleanup_status: status, container_repository_id: repository.id })
+    def cleanup_service_response(status: :finished, repository:, cleanup_tags_service_original_size: 100, cleanup_tags_service_before_truncate_size: 80, cleanup_tags_service_after_truncate_size: 80, cleanup_tags_service_before_delete_size: 50)
+      ServiceResponse.success(
+        message: "cleanup #{status}",
+        payload: {
+          cleanup_status: status,
+          container_repository_id: repository.id,
+          cleanup_tags_service_original_size: cleanup_tags_service_original_size,
+          cleanup_tags_service_before_truncate_size: cleanup_tags_service_before_truncate_size,
+          cleanup_tags_service_after_truncate_size: cleanup_tags_service_after_truncate_size,
+          cleanup_tags_service_before_delete_size: cleanup_tags_service_before_delete_size
+        }.compact
+      )
+    end
+
+    def expect_log_extra_metadata(service_response:, cleanup_status: :finished)
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, cleanup_status)
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id)
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_tags_service_original_size, service_response.payload[:cleanup_tags_service_original_size])
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_tags_service_before_truncate_size, service_response.payload[:cleanup_tags_service_before_truncate_size])
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_tags_service_after_truncate_size, service_response.payload[:cleanup_tags_service_after_truncate_size])
+      expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_tags_service_before_delete_size, service_response.payload[:cleanup_tags_service_before_delete_size])
     end
   end