Merge branch '330315-step-in-cron-worker' into 'master'

Regularly disable policies linked with no container repositories

See merge request gitlab-org/gitlab!62461

app/models/container_expiration_policy.rb

@@ -38,6 +38,16 @@ class ContainerExpirationPolicy < ApplicationRecord
     )
   end
 
+  def self.without_container_repositories
+    where.not(
+      'EXISTS(?)',
+      ContainerRepository.select(1)
+                         .where(
+                           'container_repositories.project_id = container_expiration_policies.project_id'
+                         )
+    )
+  end
+
   def self.keep_n_options
     {
       1 => _('%{tags} tag per image name') % { tags: 1 },

app/workers/container_expiration_policy_worker.rb

@@ -15,11 +15,19 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo
 
   def perform
     process_stale_ongoing_cleanups
+    disable_policies_without_container_repositories
     throttling_enabled? ? perform_throttled : perform_unthrottled
   end
 
   private
 
+  def disable_policies_without_container_repositories
+    ContainerExpirationPolicy.active.each_batch(of: BATCH_SIZE) do |policies|
+      policies.without_container_repositories
+              .update_all(enabled: false)
+    end
+  end
+
   def process_stale_ongoing_cleanups
     threshold = delete_tags_service_timeout.seconds + 30.minutes
     ContainerRepository.with_stale_ongoing_cleanup(threshold.ago)

doc/user/packages/container_registry/index.md

@@ -488,6 +488,10 @@ Cleanup policies can be run on all projects, with these exceptions:
   Feature.disable(:container_expiration_policies_historic_entry, Project.find(<project id>))
   ```
 
+WARNING:
+For performance reasons, enabled cleanup policies are automatically disabled for projects on
+GitLab.com that don't have a container image.
+
 ### How the cleanup policy works
 
 The cleanup policy collects all tags in the Container Registry and excludes tags

spec/models/container_expiration_policy_spec.rb

@@ -139,15 +139,23 @@ RSpec.describe ContainerExpirationPolicy, type: :model do
     end
   end
 
-  describe '.with_container_repositories' do
-    subject { described_class.with_container_repositories }
-
+  context 'policies with container repositories' do
     let_it_be(:policy1) { create(:container_expiration_policy) }
     let_it_be(:container_repository1) { create(:container_repository, project: policy1.project) }
     let_it_be(:policy2) { create(:container_expiration_policy) }
     let_it_be(:container_repository2) { create(:container_repository, project: policy2.project) }
     let_it_be(:policy3) { create(:container_expiration_policy) }
 
-    it { is_expected.to contain_exactly(policy1, policy2) }
+    describe '.with_container_repositories' do
+      subject { described_class.with_container_repositories }
+
+      it { is_expected.to contain_exactly(policy1, policy2) }
+    end
+
+    describe '.without_container_repositories' do
+      subject { described_class.without_container_repositories }
+
+      it { is_expected.to contain_exactly(policy3) }
+    end
   end
 end

spec/workers/container_expiration_policy_worker_spec.rb

@@ -123,5 +123,19 @@ RSpec.describe ContainerExpirationPolicyWorker do
         expect(stuck_cleanup.reload).to be_cleanup_unfinished
       end
     end
+
+    context 'policies without container repositories' do
+      let_it_be(:container_expiration_policy1) { create(:container_expiration_policy, enabled: true) }
+      let_it_be(:container_repository1) { create(:container_repository, project_id: container_expiration_policy1.project_id) }
+      let_it_be(:container_expiration_policy2) { create(:container_expiration_policy, enabled: true) }
+      let_it_be(:container_repository2) { create(:container_repository, project_id: container_expiration_policy2.project_id) }
+      let_it_be(:container_expiration_policy3) { create(:container_expiration_policy, enabled: true) }
+
+      it 'disables them' do
+        expect { subject }
+          .to change { ::ContainerExpirationPolicy.active.count }.from(3).to(2)
+        expect(container_expiration_policy3.reload.enabled).to be false
+      end
+    end
   end
 end