Fix blobstore errors for droplets from CF Docker apps (cloudfoundry#2588

)

* Don't trigger droplet deletion job for Docker apps but still make sure that Docker droplets are marked as "EXPIRED" in the db
* Refactor expired_blob_cleanup test to highlight the difference between buildpack and Docker droplet handling
* Fixes cloudfoundry#2582

Co-authored-by: Aftab Alam <81828613+iaftab-alam@users.noreply.github.com>

lib/cloud_controller/bits_expiration.rb

@@ -20,7 +20,7 @@ def expire_droplets!(app)
 
       droplets_to_expire.each do |droplet|
         droplet.update(state: DropletModel::EXPIRED_STATE)
-        enqueue_droplet_delete_job(droplet.guid)
+        enqueue_droplet_delete_job(droplet.guid) if droplet.droplet_hash
       end
     end
 

spec/unit/jobs/runtime/expired_blob_cleanup_spec.rb

@@ -12,40 +12,32 @@ module Jobs::Runtime
       end
 
       describe 'droplets' do
-        context 'expired' do
-          let!(:expired_droplet) { DropletModel.make(state: DropletModel::EXPIRED_STATE) }
-          let!(:non_expired_droplet) { DropletModel.make }
+        let!(:buildpack_droplet) { DropletModel.make(droplet_hash: 'not-nil', docker_receipt_image: nil) }
+        let!(:docker_droplet) { DropletModel.make(droplet_hash: nil, docker_receipt_image: 'repo/test-app') }
+        let!(:staged_droplet) { DropletModel.make(state: DropletModel::STAGED_STATE) }
 
-          it 'enqueues a deletion job when droplet_hash is not nil' do
-            expired_droplet.update(droplet_hash: 'not-nil')
+        context 'expired' do
+          before do
+            buildpack_droplet.update(state: DropletModel::EXPIRED_STATE)
+            docker_droplet.update(state: DropletModel::EXPIRED_STATE)
+          end
 
+          it 'enqueues a deletion job only for buildpack droplets' do
             expect { job.perform }.to change { Delayed::Job.count }.by(1)
             expect(Delayed::Job.last.handler).to include('DeleteExpiredDropletBlob')
           end
-
-          it 'does nothing when droplet_hash is nil' do
-            expired_droplet.update(droplet_hash: nil)
-
-            expect { job.perform }.not_to change { Delayed::Job.count }.from(0)
-          end
         end
 
         context 'failed' do
-          let!(:expired_droplet) { DropletModel.make(state: DropletModel::FAILED_STATE) }
-          let!(:non_expired_droplet) { DropletModel.make }
-
-          it 'enqueues a deletion job when droplet_hash is not nil' do
-            expired_droplet.update(droplet_hash: 'not-nil')
+          before do
+            buildpack_droplet.update(state: DropletModel::FAILED_STATE)
+            docker_droplet.update(state: DropletModel::FAILED_STATE)
+          end
 
+          it 'enqueues a deletion job only for buildpack droplets' do
             expect { job.perform }.to change { Delayed::Job.count }.by(1)
             expect(Delayed::Job.last.handler).to include('DeleteExpiredDropletBlob')
           end
-
-          it 'does nothing when droplet_hash is nil' do
-            expired_droplet.update(droplet_hash: nil)
-
-            expect { job.perform }.not_to change { Delayed::Job.count }.from(0)
-          end
         end
       end
 

spec/unit/lib/cloud_controller/bits_expiration_spec.rb

@@ -41,19 +41,47 @@ module VCAP::CloudController
       end
     end
 
+    context 'with docker cf apps' do
+      before do
+        t        = Time.now
+        @current = DropletModel.make(
+          app_guid:     app.guid,
+          created_at:   t,
+          droplet_hash: nil,
+          docker_receipt_image: 'repo/test-app',
+        )
+        app.update(droplet: @current)
+
+        10.times do |i|
+          DropletModel.make(
+            app_guid:     app.guid,
+            created_at:   t + i,
+            droplet_hash: nil,
+            docker_receipt_image: 'repo/test-app',
+          )
+        end
+      end
+
+      it 'does not enqueue a job to delete the blob' do
+        expect { BitsExpiration.new.expire_droplets!(app) }.not_to change { Delayed::Job.count }
+      end
+    end
+
     context 'with droplets' do
       before do
         t        = Time.now
         @current = DropletModel.make(
           app_guid:     app.guid,
-          created_at:   t
+          created_at:   t,
+          droplet_hash: 'current_droplet_hash',
         )
         app.update(droplet: @current)
 
         10.times do |i|
           DropletModel.make(
             app_guid:     app.guid,
             created_at:   t + i,
+            droplet_hash: 'current_droplet_hash',
           )
         end
       end