Merge pull request cloudfoundry#1909 from cloudfoundry/fix-docker-app-deletion-package-registry

fix docker app deletion when using a package registry

[finishes #175295965]

Author: tcdowney

app/actions/package_delete.rb

@@ -8,12 +8,8 @@ def delete(packages)
       packages = Array(packages)
 
       packages.each do |package|
-        package_delete = if VCAP::CloudController::Config.config.package_image_registry_configured?
-                           Jobs::Kubernetes::RegistryDelete.new(package.bits_image_reference)
-                         else
-                           Jobs::Runtime::BlobstoreDelete.new(package.guid, :package_blobstore)
-                         end
-        Jobs::Enqueuer.new(package_delete, queue: Jobs::Queues.generic).enqueue
+        package_src_delete_job = create_package_source_deletion_job(package)
+        Jobs::Enqueuer.new(package_src_delete_job, queue: Jobs::Queues.generic).enqueue if package_src_delete_job
         package.destroy
 
         Repositories::PackageEventRepository.record_app_package_delete(
@@ -23,5 +19,17 @@ def delete(packages)
 
       []
     end
+
+    private
+
+    def create_package_source_deletion_job(package)
+      return Jobs::Runtime::BlobstoreDelete.new(package.guid, :package_blobstore) unless package_registry_configured?
+
+      package.bits? ? Jobs::Kubernetes::RegistryDelete.new(package.bits_image_reference) : nil
+    end
+
+    def package_registry_configured?
+      VCAP::CloudController::Config.config.package_image_registry_configured?
+    end
   end
 end

app/jobs/runtime/delete_expired_package_blob.rb

@@ -17,11 +17,7 @@ def perform
           package = PackageModel.find(guid: package_guid)
           return unless package
 
-          if VCAP::CloudController::Config.config.package_image_registry_configured?
-            VCAP::CloudController::Jobs::Kubernetes::RegistryDelete.new(package.bits_image_reference).perform
-          else
-            BlobstoreDelete.new(package_guid, :package_blobstore).perform
-          end
+          create_package_source_deletion_job(package)&.perform
           package.update(package_hash: nil, sha256_checksum: nil)
         end
 
@@ -36,6 +32,18 @@ def max_attempts
         def logger
           @logger ||= Steno.logger('cc.background')
         end
+
+        private
+
+        def package_registry_configured?
+          VCAP::CloudController::Config.config.package_image_registry_configured?
+        end
+
+        def create_package_source_deletion_job(package)
+          return Jobs::Runtime::BlobstoreDelete.new(package.guid, :package_blobstore) unless package_registry_configured?
+
+          package.bits? ? Jobs::Kubernetes::RegistryDelete.new(package.bits_image_reference) : nil
+        end
       end
     end
   end

spec/unit/actions/package_delete_spec.rb

@@ -24,18 +24,32 @@ module VCAP::CloudController
             TestConfig.override(packages: { image_registry: { base_path: 'hub.example.com/user' } })
           end
 
-          it 'schedules a job to the delete the blobstore item' do
-            expect {
-              package_delete.delete(package)
-            }.to change {
-              Delayed::Job.count
-            }.by(1)
+          context 'when the package type is bits' do
+            it 'schedules a job to the delete the package from the container registry' do
+              expect {
+                package_delete.delete(package)
+              }.to change {
+                Delayed::Job.count
+              }.by(1)
 
-            job = Delayed::Job.last
-            expect(job.handler).to include('VCAP::CloudController::Jobs::Kubernetes::RegistryDelete')
-            expect(job.handler).to include(package.bits_image_reference(digest: false))
-            expect(job.queue).to eq(Jobs::Queues.generic)
-            expect(job.guid).not_to be_nil
+              job = Delayed::Job.last
+              expect(job.handler).to include('VCAP::CloudController::Jobs::Kubernetes::RegistryDelete')
+              expect(job.handler).to include(package.bits_image_reference(digest: false))
+              expect(job.queue).to eq(Jobs::Queues.generic)
+              expect(job.guid).not_to be_nil
+            end
+          end
+
+          context 'when the package type is docker' do
+            let!(:package) { PackageModel.make(type: PackageModel::DOCKER_TYPE) }
+
+            it 'does not schedule a deletion job since there was no source code uploaded' do
+              expect {
+                package_delete.delete(package)
+              }.to change {
+                Delayed::Job.count
+              }.by(0)
+            end
           end
         end
 

spec/unit/jobs/runtime/delete_expired_package_blob_spec.rb

@@ -4,7 +4,8 @@ module VCAP::CloudController
   module Jobs::Runtime
     RSpec.describe DeleteExpiredPackageBlob, job_context: :worker do
       subject(:job) { DeleteExpiredPackageBlob.new(package.guid) }
-      let(:package) { PackageModel.make(package_hash: 'some-hash', sha256_checksum: 'example-256-checksum') }
+      let(:type) { PackageModel::BITS_TYPE }
+      let(:package) { PackageModel.make(package_hash: 'some-hash', sha256_checksum: 'example-256-checksum', type: type) }
 
       it { is_expected.to be_a_valid_job }
 
@@ -17,12 +18,25 @@ module Jobs::Runtime
           allow(registry_delete).to receive(:perform)
         end
 
-        it 'delegates to registry delete job' do
-          job.perform
+        context 'when the package type is bits' do
+          it 'delegates to registry delete job' do
+            job.perform
+
+            expect(VCAP::CloudController::Jobs::Kubernetes::RegistryDelete).to have_received(:new).
+              with(package.bits_image_reference(digest: false))
+            expect(registry_delete).to have_received(:perform)
+          end
+        end
+
+        context 'when the package type is docker' do
+          let(:type) { PackageModel::DOCKER_TYPE }
+
+          it 'does not perform registry deletion' do
+            job.perform
 
-          expect(VCAP::CloudController::Jobs::Kubernetes::RegistryDelete).to have_received(:new).
-            with(package.bits_image_reference(digest: false))
-          expect(registry_delete).to have_received(:perform)
+            expect(VCAP::CloudController::Jobs::Kubernetes::RegistryDelete).not_to have_received(:new)
+            expect(registry_delete).not_to have_received(:perform)
+          end
         end
 
         it 'nils the package_hash and sha256_checksum values' do