Merge branch '333694-no-op-composer-workers' into 'master'

Stop Composer cache file related workers

See merge request gitlab-org/gitlab!71230

app/models/packages/composer/cache_file.rb

@@ -16,8 +16,6 @@ module Packages
 
       scope :with_namespace, ->(namespace) { where(namespace: namespace) }
       scope :with_sha, ->(sha) { where(file_sha256: sha) }
-      scope :expired, -> { where("delete_at <= ?", Time.current) }
-      scope :without_namespace, -> { where(namespace_id: nil) }
     end
   end
 end

app/services/packages/composer/create_package_service.rb

@@ -17,10 +17,6 @@ module Packages
           })
         end
 
-        unless Feature.enabled?(:remove_composer_v1_cache_code, project)
-          ::Packages::Composer::CacheUpdateWorker.perform_async(created_package.project_id, created_package.name, nil)
-        end
-
         created_package
       end
 

app/workers/packages/composer/cache_cleanup_worker.rb

@@ -14,19 +14,7 @@ module Packages
       idempotent!
 
       def perform
-        ::Packages::Composer::CacheFile.without_namespace.find_in_batches do |cache_files|
-          cache_files.each(&:destroy)
-        rescue ActiveRecord::RecordNotFound
-          # ignore. likely due to object already being deleted.
-        end
-
-        ::Packages::Composer::CacheFile.expired.find_in_batches do |cache_files|
-          cache_files.each(&:destroy)
-        rescue ActiveRecord::RecordNotFound
-          # ignore. likely due to object already being deleted.
-        end
-      rescue StandardError => e
-        Gitlab::ErrorTracking.log_exception(e)
+        # no-op: to be removed after 14.5 https://gitlab.com/gitlab-org/gitlab/-/issues/333694
       end
     end
   end

app/workers/packages/composer/cache_update_worker.rb

@@ -7,20 +7,14 @@ module Packages
 
       data_consistency :always
 
-      sidekiq_options retry: 3
+      sidekiq_options retry: false
 
       feature_category :package_registry
 
       idempotent!
 
-      def perform(project_id, package_name, last_page_sha)
-        project = Project.find_by_id(project_id)
-
-        return unless project
-
-        Gitlab::Composer::Cache.new(project: project, name: package_name, last_page_sha: last_page_sha).execute
-      rescue StandardError => e
-        Gitlab::ErrorTracking.log_exception(e, project_id: project_id)
+      def perform(*args)
+        # no-op: to be removed after 14.5 https://gitlab.com/gitlab-org/gitlab/-/issues/333694
       end
     end
   end

config/feature_flags/development/remove_composer_v1_cache_code.yml

----
-name: remove_composer_v1_cache_code
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67843
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338264
-milestone: '14.2'
-type: development
-group: group::package
-default_enabled: false

config/initializers/1_settings.rb

@@ -466,9 +466,6 @@ Settings.cron_jobs['personal_access_tokens_expired_notification_worker']['job_cl
 Settings.cron_jobs['repository_archive_cache_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['repository_archive_cache_worker']['cron'] ||= '0 * * * *'
 Settings.cron_jobs['repository_archive_cache_worker']['job_class'] = 'RepositoryArchiveCacheWorker'
-Settings.cron_jobs['packages_composer_cache_cleanup_worker'] ||= Settingslogic.new({})
-Settings.cron_jobs['packages_composer_cache_cleanup_worker']['cron'] ||= '30 * * * *'
-Settings.cron_jobs['packages_composer_cache_cleanup_worker']['job_class'] = 'Packages::Composer::CacheCleanupWorker'
 Settings.cron_jobs['import_export_project_cleanup_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['import_export_project_cleanup_worker']['cron'] ||= '0 * * * *'
 Settings.cron_jobs['import_export_project_cleanup_worker']['job_class'] = 'ImportExportProjectCleanupWorker'

spec/services/packages/composer/create_package_service_spec.rb

@@ -24,25 +24,6 @@ RSpec.describe Packages::Composer::CreatePackageService do
 
     let(:created_package) { Packages::Package.composer.last }
 
-    shared_examples 'using the cache update worker' do
-      context 'with remove_composer_v1_cache_code enabled' do
-        it 'does not enqueue a cache update job' do
-          expect(::Packages::Composer::CacheUpdateWorker).not_to receive(:perform_async)
-
-          subject
-        end
-      end
-
-      context 'with remove_composer_v1_cache_code disabled' do
-        it 'enqueues a cache update job' do
-          stub_feature_flags(remove_composer_v1_cache_code: true)
-          expect(::Packages::Composer::CacheUpdateWorker).not_to receive(:perform_async)
-
-          subject
-        end
-      end
-    end
-
     context 'without an existing package' do
       context 'with a branch' do
         let(:branch) { project.repository.find_branch('master') }
@@ -64,7 +45,6 @@ RSpec.describe Packages::Composer::CreatePackageService do
 
         it_behaves_like 'assigns build to package'
         it_behaves_like 'assigns status to package'
-        it_behaves_like 'using the cache update worker'
       end
 
       context 'with a tag' do
@@ -89,7 +69,6 @@ RSpec.describe Packages::Composer::CreatePackageService do
 
         it_behaves_like 'assigns build to package'
         it_behaves_like 'assigns status to package'
-        it_behaves_like 'using the cache update worker'
       end
     end
 
@@ -106,8 +85,6 @@ RSpec.describe Packages::Composer::CreatePackageService do
             .to change { Packages::Package.composer.count }.by(0)
             .and change { Packages::Composer::Metadatum.count }.by(0)
         end
-
-        it_behaves_like 'using the cache update worker'
       end
 
       context 'belonging to another project' do
@@ -129,8 +106,6 @@ RSpec.describe Packages::Composer::CreatePackageService do
             .to change { Packages::Package.composer.count }.by(1)
             .and change { Packages::Composer::Metadatum.count }.by(1)
         end
-
-        it_behaves_like 'using the cache update worker'
       end
     end
   end

spec/workers/every_sidekiq_worker_spec.rb

@@ -359,7 +359,7 @@ RSpec.describe 'Every Sidekiq worker' do
         'ObjectPool::ScheduleJoinWorker' => 3,
         'ObjectStorage::BackgroundMoveWorker' => 5,
         'ObjectStorage::MigrateUploadsWorker' => 3,
-        'Packages::Composer::CacheUpdateWorker' => 3,
+        'Packages::Composer::CacheUpdateWorker' => false,
         'Packages::Go::SyncPackagesWorker' => 3,
         'Packages::Maven::Metadata::SyncWorker' => 3,
         'Packages::Nuget::ExtractionWorker' => 3,

spec/workers/packages/composer/cache_cleanup_worker_spec.rb

@@ -18,12 +18,8 @@ RSpec.describe Packages::Composer::CacheCleanupWorker, type: :worker do
       cache_file4.update_columns(namespace_id: nil)
     end
 
-    it 'deletes expired packages' do
-      expect { subject }.to change { Packages::Composer::CacheFile.count }.by(-2)
-      expect { cache_file1.reload }.not_to raise_error ActiveRecord::RecordNotFound
-      expect { cache_file2.reload }.not_to raise_error ActiveRecord::RecordNotFound
-      expect { cache_file3.reload }.to raise_error ActiveRecord::RecordNotFound
-      expect { cache_file4.reload }.to raise_error ActiveRecord::RecordNotFound
+    it 'does nothing' do
+      expect { subject }.not_to change { Packages::Composer::CacheFile.count }
     end
   end
 end

spec/workers/packages/composer/cache_update_worker_spec.rb

@@ -21,8 +21,8 @@ RSpec.describe Packages::Composer::CacheUpdateWorker, type: :worker do
 
     include_examples 'an idempotent worker' do
       context 'creating a package' do
-        it 'updates the cache' do
-          expect { subject }.to change { Packages::Composer::CacheFile.count }.by(1)
+        it 'does nothing' do
+          expect { subject }.to change { Packages::Composer::CacheFile.count }.by(0)
         end
       end
 
@@ -36,12 +36,12 @@ RSpec.describe Packages::Composer::CacheUpdateWorker, type: :worker do
           package.destroy!
         end
 
-        it 'marks the file for deletion' do
+        it 'does nothing' do
           expect { subject }.not_to change { Packages::Composer::CacheFile.count }
 
           cache_file = Packages::Composer::CacheFile.last
 
-          expect(cache_file.reload.delete_at).not_to be_nil
+          expect(cache_file.reload.delete_at).to be_nil
         end
       end
     end