Merge branch '348176-dp-api-cleanup-worker-call' into 'master'

Call dependency proxy cleanup workers in purge

See merge request gitlab-org/gitlab!77043

app/workers/all_queues.yml

@@ -291,6 +291,15 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: cronjob:dependency_proxy_cleanup_dependency_proxy
+  :worker_name: DependencyProxy::CleanupDependencyProxyWorker
+  :feature_category: :dependency_proxy
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: cronjob:dependency_proxy_image_ttl_group_policy
   :worker_name: DependencyProxy::ImageTtlGroupPolicyWorker
   :feature_category: :dependency_proxy

app/workers/concerns/dependency_proxy/expireable.rb

+# frozen_string_literal: true
+
+module DependencyProxy
+  module Expireable
+    extend ActiveSupport::Concern
+
+    UPDATE_BATCH_SIZE = 100
+
+    private
+
+    def expire_artifacts(collection)
+      collection.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
+        batch.update_all(status: :expired)
+      end
+    end
+  end
+end

app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb

+# frozen_string_literal: true
+
+module DependencyProxy
+  class CleanupDependencyProxyWorker
+    include ApplicationWorker
+    include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+
+    data_consistency :always
+    idempotent!
+
+    feature_category :dependency_proxy
+
+    def perform
+      enqueue_blob_cleanup_job if DependencyProxy::Blob.expired.any?
+      enqueue_manifest_cleanup_job if DependencyProxy::Manifest.expired.any?
+    end
+
+    private
+
+    def enqueue_blob_cleanup_job
+      DependencyProxy::CleanupBlobWorker.perform_with_capacity
+    end
+
+    def enqueue_manifest_cleanup_job
+      DependencyProxy::CleanupManifestWorker.perform_with_capacity
+    end
+  end
+end

app/workers/dependency_proxy/image_ttl_group_policy_worker.rb

@@ -4,20 +4,19 @@ module DependencyProxy
   class ImageTtlGroupPolicyWorker # rubocop:disable Scalability/IdempotentWorker
     include ApplicationWorker
     include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+    include DependencyProxy::Expireable
 
     data_consistency :always
 
     feature_category :dependency_proxy
 
-    UPDATE_BATCH_SIZE = 100
-
     def perform
       DependencyProxy::ImageTtlGroupPolicy.enabled.each do |policy|
         qualified_blobs = policy.group.dependency_proxy_blobs.active.read_before(policy.ttl)
         qualified_manifests = policy.group.dependency_proxy_manifests.active.read_before(policy.ttl)
 
-        enqueue_blob_cleanup_job if expire_artifacts(qualified_blobs, DependencyProxy::Blob)
-        enqueue_manifest_cleanup_job if expire_artifacts(qualified_manifests, DependencyProxy::Manifest)
+        expire_artifacts(qualified_blobs)
+        expire_artifacts(qualified_manifests)
       end
 
       log_counts
@@ -25,25 +24,6 @@ module DependencyProxy
 
     private
 
-    def expire_artifacts(artifacts, model)
-      rows_updated = false
-
-      artifacts.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
-        rows = batch.update_all(status: :expired)
-        rows_updated ||= rows > 0
-      end
-
-      rows_updated
-    end
-
-    def enqueue_blob_cleanup_job
-      DependencyProxy::CleanupBlobWorker.perform_with_capacity
-    end
-
-    def enqueue_manifest_cleanup_job
-      DependencyProxy::CleanupManifestWorker.perform_with_capacity
-    end
-
     def log_counts
       use_replica_if_available do
         expired_blob_count = DependencyProxy::Blob.expired.count

app/workers/purge_dependency_proxy_cache_worker.rb

@@ -2,6 +2,7 @@
 
 class PurgeDependencyProxyCacheWorker
   include ApplicationWorker
+  include DependencyProxy::Expireable
 
   data_consistency :always
 
@@ -12,21 +13,14 @@ class PurgeDependencyProxyCacheWorker
   queue_namespace :dependency_proxy
   feature_category :dependency_proxy
 
-  UPDATE_BATCH_SIZE = 100
-
   def perform(current_user_id, group_id)
     @current_user = User.find_by_id(current_user_id)
     @group = Group.find_by_id(group_id)
 
     return unless valid?
 
-    @group.dependency_proxy_blobs.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
-      batch.update_all(status: :expired)
-    end
-
-    @group.dependency_proxy_manifests.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
-      batch.update_all(status: :expired)
-    end
+    expire_artifacts(@group.dependency_proxy_blobs)
+    expire_artifacts(@group.dependency_proxy_manifests)
   end
 
   private

config/initializers/1_settings.rb

@@ -534,6 +534,9 @@ Settings.cron_jobs['container_expiration_policy_worker']['job_class'] = 'Contain
 Settings.cron_jobs['image_ttl_group_policy_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['image_ttl_group_policy_worker']['cron'] ||= '40 0 * * *'
 Settings.cron_jobs['image_ttl_group_policy_worker']['job_class'] = 'DependencyProxy::ImageTtlGroupPolicyWorker'
+Settings.cron_jobs['cleanup_dependency_proxy_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['cleanup_dependency_proxy_worker']['cron'] ||= '20 3,15 * * *'
+Settings.cron_jobs['cleanup_dependency_proxy_worker']['job_class'] = 'DependencyProxy::CleanupDependencyProxyWorker'
 Settings.cron_jobs['x509_issuer_crl_check_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['x509_issuer_crl_check_worker']['cron'] ||= '30 1 * * *'
 Settings.cron_jobs['x509_issuer_crl_check_worker']['job_class'] = 'X509IssuerCrlCheckWorker'

doc/api/dependency_proxy.md

@@ -11,7 +11,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/11631) in GitLab 12.10.
 > - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/273655) from GitLab Premium to GitLab Free in 13.6.
 
-Deletes the cached manifests and blobs for a group. This endpoint requires the [Owner role](../user/permissions.md)
+Schedules for deletion the cached manifests and blobs for a group. This endpoint requires the
+[Owner role](../user/permissions.md)
 for the group.
 
 ```plaintext

spec/workers/dependency_proxy/cleanup_dependency_proxy_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DependencyProxy::CleanupDependencyProxyWorker do
+  describe '#perform' do
+    subject { described_class.new.perform }
+
+    context 'when there are records to be deleted' do
+      it_behaves_like 'an idempotent worker' do
+        it 'queues the cleanup jobs', :aggregate_failures do
+          create(:dependency_proxy_blob, :expired)
+          create(:dependency_proxy_manifest, :expired)
+
+          expect(DependencyProxy::CleanupBlobWorker).to receive(:perform_with_capacity).twice
+          expect(DependencyProxy::CleanupManifestWorker).to receive(:perform_with_capacity).twice
+
+          subject
+        end
+      end
+    end
+
+    context 'when there are not records to be deleted' do
+      it_behaves_like 'an idempotent worker' do
+        it 'does not queue the cleanup jobs', :aggregate_failures do
+          expect(DependencyProxy::CleanupBlobWorker).not_to receive(:perform_with_capacity)
+          expect(DependencyProxy::CleanupManifestWorker).not_to receive(:perform_with_capacity)
+
+          subject
+        end
+      end
+    end
+  end
+end

spec/workers/dependency_proxy/image_ttl_group_policy_worker_spec.rb

@@ -17,13 +17,6 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicyWorker do
       let_it_be_with_reload(:new_blob) { create(:dependency_proxy_blob, group: group) }
       let_it_be_with_reload(:new_manifest) { create(:dependency_proxy_manifest, group: group) }
 
-      it 'calls the limited capacity workers', :aggregate_failures do
-        expect(DependencyProxy::CleanupBlobWorker).to receive(:perform_with_capacity)
-        expect(DependencyProxy::CleanupManifestWorker).to receive(:perform_with_capacity)
-
-        subject
-      end
-
       it 'updates the old images to expired' do
         expect { subject }
           .to change { old_blob.reload.status }.from('default').to('expired')
@@ -33,15 +26,6 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicyWorker do
       end
     end
 
-    context 'when there are no images to expire' do
-      it 'does not do anything', :aggregate_failures do
-        expect(DependencyProxy::CleanupBlobWorker).not_to receive(:perform_with_capacity)
-        expect(DependencyProxy::CleanupManifestWorker).not_to receive(:perform_with_capacity)
-
-        subject
-      end
-    end
-
     context 'counts logging' do
       let_it_be(:expired_blob) { create(:dependency_proxy_blob, :expired, group: group) }
       let_it_be(:expired_blob2) { create(:dependency_proxy_blob, :expired, group: group) }