Issue 4092: Mark "FAILED" droplets/packages as expired (#4093)

* otherwise they will never be cleaned up
* create package delete jobs only if there is a hash (= package has a blob)

Author: jochenehret

lib/cloud_controller/bits_expiration.rb

@@ -11,7 +11,7 @@ def initialize(config=Config.config)
 
     def expire_droplets!(app)
       expirable_candidates = DropletModel.
-                             where(state: DropletModel::STAGED_STATE, app_guid: app.guid).
+                             where(state: [DropletModel::STAGED_STATE, DropletModel::FAILED_STATE], app_guid: app.guid).
                              exclude(guid: app.droplet_guid)
 
       return if expirable_candidates.count < droplets_storage_count
@@ -28,7 +28,7 @@ def expire_packages!(app)
       current_package_guid = app.droplet.try(:package_guid)
 
       expirable_candidates = PackageModel.
-                             where(state: PackageModel::READY_STATE, app_guid: app.guid).
+                             where(state: [PackageModel::READY_STATE, PackageModel::FAILED_STATE], app_guid: app.guid).
                              exclude(guid: current_package_guid)
 
       return if expirable_candidates.count < packages_storage_count
@@ -37,7 +37,7 @@ def expire_packages!(app)
 
       packages_to_expire.each do |package|
         package.update(state: PackageModel::EXPIRED_STATE)
-        enqueue_package_delete_job(package.guid)
+        enqueue_package_delete_job(package.guid) if package.package_hash
       end
     end
 

spec/unit/lib/cloud_controller/bits_expiration_spec.rb

@@ -30,10 +30,12 @@ module VCAP::CloudController
       expect(BitsExpiration.new(changed_config).packages_storage_count).to eq(10)
     end
 
-    context 'with an app with few droplets / packages' do
+    context 'with an app with few droplets / packages and one failed droplet / package' do
       it 'does not mark any as expired' do
         3.times { DropletModel.make(state: DropletModel::STAGED_STATE, app_guid: app.guid) }
         3.times { PackageModel.make(state: PackageModel::READY_STATE, app_guid: app.guid) }
+        DropletModel.make(state: DropletModel::FAILED_STATE, app_guid: app.guid)
+        PackageModel.make(state: PackageModel::FAILED_STATE, app_guid: app.guid)
         BitsExpiration.new.expire_droplets!(app)
         BitsExpiration.new.expire_packages!(app)
         expect(DropletModel.where(state: DropletModel::EXPIRED_STATE).count).to eq(0)
@@ -77,10 +79,19 @@ module VCAP::CloudController
         )
         app.update(droplet: @current)
 
-        10.times do |i|
+        2.times do |i|
           DropletModel.make(
             app_guid: app.guid,
             created_at: t + i,
+            droplet_hash: nil,
+            state: DropletModel::FAILED_STATE
+          )
+        end
+
+        10.times do |i|
+          DropletModel.make(
+            app_guid: app.guid,
+            created_at: t + i + 2,
             droplet_hash: 'current_droplet_hash'
           )
         end
@@ -133,11 +144,18 @@ module VCAP::CloudController
         )
         app.update(droplet: @current)
 
+        2.times do |i|
+          PackageModel.make(package_hash: nil,
+                            state: PackageModel::FAILED_STATE,
+                            app_guid: app.guid,
+                            created_at: t + i)
+        end
+
         10.times do |i|
           PackageModel.make(package_hash: 'real hash!',
                             state: PackageModel::READY_STATE,
                             app_guid: app.guid,
-                            created_at: t + i)
+                            created_at: t + i + 2)
         end
       end