Fix issue where cleanup-stale-apps command fails to delete apps with volumesets

CHANGELOG.md

@@ -14,6 +14,10 @@ Changes since the last non-beta release.
 
 _Please add entries here for your pull requests that are not yet released._
 
+### Fixed
+
+- Fixed issue where `cleanup-stale-apps` command fails to delete apps with volumesets. [PR 175](https://github.com/shakacode/heroku-to-control-plane/pull/175) by [Rafael Gomes](https://github.com/rafaelgomesxyz).
+
 ## [2.0.0] - 2024-05-14
 
 ### BREAKING CHANGES

docs/commands.md

@@ -66,7 +66,8 @@ cpl cleanup-images -a $APP_NAME
 
 ### `cleanup-stale-apps`
 
-- Deletes the whole app (GVC with all workloads and all images) for all stale apps
+- Deletes the whole app (GVC with all workloads, all volumesets and all images) for all stale apps
+- Also unbinds the app from the secrets policy, as long as both the identity and the policy exist (and are bound)
 - Stale apps are identified based on the creation date of the latest image
 - Specify the amount of days after an app should be considered stale through `stale_app_image_deployed_days` in the `.controlplane/controlplane.yml` file
 - If `match_if_app_name_starts_with` is `true` in the `.controlplane/controlplane.yml` file, it will delete all stale apps that start with the name

lib/command/cleanup_stale_apps.rb

@@ -7,9 +7,10 @@ class CleanupStaleApps < Base
       app_option(required: true),
       skip_confirm_option
     ].freeze
-    DESCRIPTION = "Deletes the whole app (GVC with all workloads and all images) for all stale apps"
+    DESCRIPTION = "Deletes the whole app (GVC with all workloads, all volumesets and all images) for all stale apps"
     LONG_DESCRIPTION = <<~DESC
-      - Deletes the whole app (GVC with all workloads and all images) for all stale apps
+      - Deletes the whole app (GVC with all workloads, all volumesets and all images) for all stale apps
+      - Also unbinds the app from the secrets policy, as long as both the identity and the policy exist (and are bound)
       - Stale apps are identified based on the creation date of the latest image
       - Specify the amount of days after an app should be considered stale through `stale_app_image_deployed_days` in the `.controlplane/controlplane.yml` file
       - If `match_if_app_name_starts_with` is `true` in the `.controlplane/controlplane.yml` file, it will delete all stale apps that start with the name
@@ -28,8 +29,8 @@ def call # rubocop:disable Metrics/MethodLength
 
       progress.puts
       stale_apps.each do |app|
-        delete_gvc(app)
-        delete_images(app)
+        delete_app(app[:name])
+        progress.puts
       end
     end
 
@@ -57,8 +58,7 @@ def stale_apps # rubocop:disable Metrics/MethodLength
 
             apps.push({
                         name: app_name,
-                        date: created_date,
-                        images: images.map { |current_image| current_image["name"] }
+                        date: created_date
                       })
           end
 
@@ -72,18 +72,8 @@ def confirm_delete
       Shell.confirm("\nAre you sure you want to delete these #{stale_apps.length} apps?")
     end
 
-    def delete_gvc(app)
-      step("Deleting app '#{app[:name]}'") do
-        cp.gvc_delete(app[:name])
-      end
-    end
-
-    def delete_images(app)
-      app[:images].each do |image|
-        step("Deleting image '#{image}'") do
-          cp.image_delete(image)
-        end
-      end
+    def delete_app(app)
+      Cpl::Cli.start(["delete", "-a", app, "--yes"])
     end
   end
 end

lib/command/delete.rb

@@ -36,7 +36,9 @@ def call
     private
 
     def delete_single_workload(workload)
-      return progress.puts("Workload '#{workload}' does not exist.") if cp.fetch_workload(workload).nil?
+      if cp.fetch_workload(workload).nil?
+        return progress.puts("Workload '#{workload}' does not exist in app '#{config.app}'.")
+      end
       return unless confirm_delete(workload)
 
       delete_workload(workload)
@@ -57,19 +59,19 @@ def delete_whole_app
 
     def check_volumesets
       @volumesets = cp.fetch_volumesets["items"]
-      return progress.puts("No volumesets to delete.") unless @volumesets.any?
+      return progress.puts("No volumesets to delete from app '#{config.app}'.") unless @volumesets.any?
 
-      message = "The following volumesets will be deleted along with the app:"
+      message = "The following volumesets will be deleted along with the app '#{config.app}':"
       volumesets_list = @volumesets.map { |volumeset| "- #{volumeset['name']}" }.join("\n")
       progress.puts("#{Shell.color(message, :red)}\n#{volumesets_list}\n\n")
     end
 
     def check_images
       @images = cp.query_images["items"]
                   .select { |image| image["name"].start_with?("#{config.app}:") }
-      return progress.puts("No images to delete.") unless @images.any?
+      return progress.puts("No images to delete from app '#{config.app}'.") unless @images.any?
 
-      message = "The following images will be deleted along with the app:"
+      message = "The following images will be deleted along with the app '#{config.app}':"
       images_list = @images.map { |image| "- #{image['name']}" }.join("\n")
       progress.puts("#{Shell.color(message, :red)}\n#{images_list}\n\n")
     end
@@ -91,14 +93,14 @@ def delete_gvc
     end
 
     def delete_workload(workload)
-      step("Deleting workload '#{workload}'") do
+      step("Deleting workload '#{workload}' from app '#{config.app}'") do
         cp.delete_workload(workload)
       end
     end
 
     def delete_volumesets
       @volumesets.each do |volumeset|
-        step("Deleting volumeset '#{volumeset['name']}'") do
+        step("Deleting volumeset '#{volumeset['name']}' from app '#{config.app}'") do
           # If the volumeset is attached to a workload, we need to delete the workload first
           workload = volumeset.dig("status", "usedByWorkload")&.split("/")&.last
           cp.delete_workload(workload) if workload
@@ -110,7 +112,7 @@ def delete_volumesets
 
     def delete_images
       @images.each do |image|
-        step("Deleting image '#{image['name']}'") do
+        step("Deleting image '#{image['name']}' from app '#{config.app}'") do
           cp.image_delete(image["name"])
         end
       end
@@ -127,7 +129,7 @@ def unbind_identity_from_policy
       end
       return unless is_bound
 
-      step("Unbinding identity from policy") do
+      step("Unbinding identity from policy for app '#{config.app}'") do
         cp.unbind_identity_from_policy(app_identity_link, app_secrets_policy)
       end
     end

spec/command/cleanup_stale_apps_spec.rb

@@ -32,8 +32,8 @@
     let!(:app2) { dummy_test_app("with-stale-app-image-deployed-days") }
 
     before do
-      run_cpl_command!("apply-template", "app", "-a", app1)
-      run_cpl_command!("apply-template", "app", "-a", app2)
+      run_cpl_command!("apply-template", "app", "postgres-with-volume", "-a", app1)
+      run_cpl_command!("apply-template", "app", "postgres-with-volume", "-a", app2)
       run_cpl_command!("build-image", "-a", app1)
       run_cpl_command!("build-image", "-a", app2)
     end
@@ -64,10 +64,12 @@
 
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
+      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume' from app '#{app1}'[.]+? done!/)
       expect(result[:stderr]).to match(/Deleting app '#{app1}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting image '#{app1}:1'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting image '#{app1}:1' from app '#{app1}'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume' from app '#{app2}'[.]+? done!/)
       expect(result[:stderr]).to match(/Deleting app '#{app2}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting image '#{app2}:1'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting image '#{app2}:1' from app '#{app2}'[.]+? done!/)
     end
 
     it "skips confirmation and deletes stale apps", :slow do
@@ -79,10 +81,12 @@
 
       expect(Shell).not_to have_received(:confirm)
       expect(result[:status]).to eq(0)
+      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume' from app '#{app1}'[.]+? done!/)
       expect(result[:stderr]).to match(/Deleting app '#{app1}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting image '#{app1}:1'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting image '#{app1}:1' from app '#{app1}'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume' from app '#{app2}'[.]+? done!/)
       expect(result[:stderr]).to match(/Deleting app '#{app2}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting image '#{app2}:1'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting image '#{app2}:1' from app '#{app2}'[.]+? done!/)
     end
   end
 

spec/command/delete_spec.rb

@@ -32,8 +32,8 @@
 
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to include("No volumesets to delete")
-      expect(result[:stderr]).to include("No images to delete")
+      expect(result[:stderr]).to include("No volumesets to delete from app '#{app}'")
+      expect(result[:stderr]).to include("No images to delete from app '#{app}'")
       expect(result[:stderr]).not_to include("Deleting app")
     end
 
@@ -44,8 +44,8 @@
 
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to include("No volumesets to delete")
-      expect(result[:stderr]).to include("No images to delete")
+      expect(result[:stderr]).to include("No volumesets to delete from app '#{app}'")
+      expect(result[:stderr]).to include("No images to delete from app '#{app}'")
       expect(result[:stderr]).to match(/Deleting app '#{app}'[.]+? done!/)
     end
 
@@ -56,8 +56,8 @@
 
       expect(Shell).not_to have_received(:confirm)
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to include("No volumesets to delete")
-      expect(result[:stderr]).to include("No images to delete")
+      expect(result[:stderr]).to include("No volumesets to delete from app '#{app}'")
+      expect(result[:stderr]).to include("No images to delete from app '#{app}'")
       expect(result[:stderr]).to match(/Deleting app '#{app}'[.]+? done!/)
     end
   end
@@ -81,10 +81,10 @@
 
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to match(/Deleting volumeset 'detached-volume'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting volumeset 'detached-volume' from app '#{app}'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting volumeset 'postgres-volume' from app '#{app}'[.]+? done!/)
       expect(result[:stderr]).to match(/Deleting app '#{app}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Deleting image '#{app}:1'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting image '#{app}:1' from app '#{app}'[.]+? done!/)
     end
   end
 
@@ -103,7 +103,7 @@
       result = run_cpl_command("delete", "-a", app, "--workload", "rails")
 
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to include("Workload 'rails' does not exist")
+      expect(result[:stderr]).to include("Workload 'rails' does not exist in app '#{app}'")
     end
   end
 
@@ -135,7 +135,7 @@
 
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to match(/Deleting workload 'rails'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting workload 'rails' from app '#{app}'[.]+? done!/)
     end
 
     it "skips confirmation and deletes workload" do
@@ -145,7 +145,7 @@
 
       expect(Shell).not_to have_received(:confirm)
       expect(result[:status]).to eq(0)
-      expect(result[:stderr]).to match(/Deleting workload 'rails'[.]+? done!/)
+      expect(result[:stderr]).to match(/Deleting workload 'rails' from app '#{app}'[.]+? done!/)
     end
   end
 
@@ -223,7 +223,7 @@
       expect(Shell).to have_received(:confirm).once
       expect(result[:status]).to eq(0)
       expect(result[:stderr]).to match(/Deleting app '#{app}'[.]+? done!/)
-      expect(result[:stderr]).to match(/Unbinding identity from policy[.]+? done!/)
+      expect(result[:stderr]).to match(/Unbinding identity from policy for app '#{app}'[.]+? done!/)
     end
   end
 end