Add provisioned? lifecycle status info to retireable? check

app/models/service.rb

@@ -156,6 +156,10 @@ def request_type
   end
 
   def retireable?
+    return false unless provisioned?
+
+    # top level services do not have types; this method is used only in creating tasks for child services which always have types
+    # please see https://github.com/ManageIQ/manageiq/pull/17317#discussion_r186528878
     parent.present? ? true : type.present?
   end
 

spec/models/mixins/ci_feature_mixin_spec.rb

@@ -20,9 +20,15 @@
     end
 
     it "service is retireable" do
-      FactoryBot.create(:service_resource, :service => service, :resource => FactoryBot.create(:service_ansible_tower, :type => ServiceAnsibleTower))
+      FactoryBot.create(:service_resource, :service => service, :resource => FactoryBot.create(:service_ansible_tower, :type => ServiceAnsibleTower, :lifecycle_state => 'provisioned'))
 
       expect(service.service_resources.first.resource.retireable?).to eq(true)
     end
+
+    it "unprov'd service is not retireable" do
+      FactoryBot.create(:service_resource, :service => service, :resource => FactoryBot.create(:service_ansible_tower, :type => ServiceAnsibleTower))
+
+      expect(service.service_resources.first.resource.retireable?).to eq(false)
+    end
   end
 end

spec/models/service_retire_task_spec.rb

@@ -1,7 +1,7 @@
 describe ServiceRetireTask do
   let(:user) { FactoryBot.create(:user_with_group) }
   let(:vm) { FactoryBot.create(:vm) }
-  let(:service) { FactoryBot.create(:service) }
+  let(:service) { FactoryBot.create(:service, :lifecycle_state => 'provisioned') }
   let(:miq_request) { FactoryBot.create(:service_retire_request, :requester => user, :source => service) }
   let(:service_retire_task) { FactoryBot.create(:service_retire_task, :source => service, :miq_request => miq_request, :options => {:src_ids => [service.id] }) }
   let(:reason) { "Why Not?" }
@@ -70,10 +70,18 @@
         miq_request.approve(approver, reason)
       end
 
-      it "creates service retire subtask" do
+      it "doesn't create service retire subtask for unprov'd service" do
         service.add_resource!(FactoryBot.create(:service_orchestration))
         service_retire_task.after_request_task_create
 
+        expect(service_retire_task.description).to eq("Service Retire for: #{service.name}")
+        expect(ServiceRetireTask.count).to eq(1)
+      end
+
+      it "creates service retire subtask" do
+        service.add_resource!(FactoryBot.create(:service_orchestration, :lifecycle_state => 'provisioned'))
+        service_retire_task.after_request_task_create
+
         expect(service_retire_task.description).to eq("Service Retire for: #{service.name}")
         expect(ServiceRetireTask.count).to eq(2)
       end
@@ -109,13 +117,21 @@
       end
 
       it "creates service retire subtask" do
-        service.add_resource!(FactoryBot.create(:service))
+        service.add_resource!(FactoryBot.create(:service, :lifecycle_state => 'provisioned'))
         service_retire_task.after_request_task_create
 
         expect(service_retire_task.description).to eq("Service Retire for: #{service.name}")
         expect(ServiceRetireTask.count).to eq(2)
       end
 
+      it "doesn't create service retire subtask for unprovisioned service" do
+        service.add_resource!(FactoryBot.create(:service))
+        service_retire_task.after_request_task_create
+
+        expect(service_retire_task.description).to eq("Service Retire for: #{service.name}")
+        expect(ServiceRetireTask.count).to eq(1)
+      end
+
       it "creates stack retire subtask" do
         service.add_resource!(FactoryBot.create(:orchestration_stack))
         service_retire_task.after_request_task_create
@@ -175,18 +191,20 @@
       end
     end
 
-    context "bundled service retires all children" do
-      let(:service_c1) { FactoryBot.create(:service) }
+    context "bundled service retires all valid children" do
+      let(:service_c1) { FactoryBot.create(:service, :lifecycle_state => 'provisioned') }
+      let(:service_c2) { FactoryBot.create(:service) }
 
       before do
         service.add_resource!(service_c1)
+        service.add_resource!(service_c2)
         service.add_resource!(FactoryBot.create(:service_template))
         @miq_request = FactoryBot.create(:service_retire_request, :requester => user)
         @miq_request.approve(approver, reason)
         @service_retire_task = FactoryBot.create(:service_retire_task, :source => service, :miq_request => @miq_request, :options => {:src_ids => [service.id] })
       end
 
-      it "creates subtask for services but not templates" do
+      it "creates subtask for provisioned services but not templates" do
         @service_retire_task.after_request_task_create
 
         expect(ServiceRetireTask.count).to eq(2)

spec/models/service_spec.rb

@@ -674,14 +674,20 @@
   end
 
   describe "#retireable?" do
-    let(:service_with_type) { FactoryBot.create(:service, :type => "thing") }
+    let(:service_with_type) { FactoryBot.create(:service, :type => "thing", :lifecycle_state => 'provisioned') }
+    let(:unprovd_service_with_type) { FactoryBot.create(:service, :type => "thing") }
     let(:service_without_type) { FactoryBot.create(:service, :type => nil) }
-    let(:service_with_parent) { FactoryBot.create(:service, :service => FactoryBot.create(:service)) }
+    let(:service_with_parent) { FactoryBot.create(:service, :service => FactoryBot.create(:service), :lifecycle_state => 'provisioned') }
+    let(:unprovisioned_service_with_parent) { FactoryBot.create(:service, :service => FactoryBot.create(:service)) }
     context "with no parent" do
       context "with type" do
         it "true" do
           expect(service_with_type.retireable?).to be(true)
         end
+
+        it "true" do
+          expect(unprovd_service_with_type.retireable?).to be(false)
+        end
       end
 
       context "without type" do
@@ -694,6 +700,7 @@
     context "with parent" do
       it "true" do
         expect(service_with_parent.retireable?).to be(true)
+        expect(unprovisioned_service_with_parent.retireable?).to be(false)
       end
     end
   end