From 7d01d513f9d0ac58bbb708e2a8165112d3df5bdc Mon Sep 17 00:00:00 2001 From: d-m-u Date: Thu, 23 Aug 2018 09:29:02 -0400 Subject: [PATCH 1/2] Add CiFeatureMixin --- .../embedded_ansible/automation_manager/job.rb | 5 +++++ app/models/mixins/ci_feature_mixin.rb | 7 +++++++ app/models/orchestration_stack.rb | 1 + app/models/service.rb | 1 + app/models/service_retire_task.rb | 14 +++++--------- app/models/service_template.rb | 1 + app/models/vm.rb | 1 + 7 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 app/models/mixins/ci_feature_mixin.rb diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/job.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/job.rb index 8fb78ffe135..0d6bfaa53d3 100644 --- a/app/models/manageiq/providers/embedded_ansible/automation_manager/job.rb +++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/job.rb @@ -1,5 +1,6 @@ class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Job < ManageIQ::Providers::EmbeddedAutomationManager::OrchestrationStack include ManageIQ::Providers::AnsibleTower::Shared::AutomationManager::Job + include CiFeatureMixin require_nested :Status @@ -19,4 +20,8 @@ def raw_stdout_via_worker(userid, format = 'txt') super(userid, format, 'embedded_ansible') end + + def retireable? + false + end end diff --git a/app/models/mixins/ci_feature_mixin.rb b/app/models/mixins/ci_feature_mixin.rb new file mode 100644 index 00000000000..e0f5f0159f9 --- /dev/null +++ b/app/models/mixins/ci_feature_mixin.rb @@ -0,0 +1,7 @@ +module CiFeatureMixin + extend ActiveSupport::Concern + + def retireable? + resource.present? && resource.respond_to?(:retire_now) && resource.type.present? + end +end diff --git a/app/models/orchestration_stack.rb b/app/models/orchestration_stack.rb index 539a90ccb86..42dca64a541 100644 --- a/app/models/orchestration_stack.rb +++ b/app/models/orchestration_stack.rb @@ -11,6 +11,7 @@ class OrchestrationStack < ApplicationRecord include TenantIdentityMixin include CustomActionsMixin include SupportsFeatureMixin + include CiFeatureMixin acts_as_miq_taggable diff --git a/app/models/service.rb b/app/models/service.rb index 8c72c7d59b1..17f4ca8caa9 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -66,6 +66,7 @@ class Service < ApplicationRecord include ProcessTasksMixin include TenancyMixin include SupportsFeatureMixin + include CiFeatureMixin include Metric::CiMixin include_concern 'RetirementManagement' diff --git a/app/models/service_retire_task.rb b/app/models/service_retire_task.rb index d6fe3f5456c..d1fe985cbc8 100644 --- a/app/models/service_retire_task.rb +++ b/app/models/service_retire_task.rb @@ -32,7 +32,11 @@ def after_request_task_create def create_retire_subtasks(parent_service) parent_service.direct_service_children.each { |child| create_retire_subtasks(child) } parent_service.service_resources.collect do |svc_rsc| - next unless retireable?(svc_rsc, parent_service) + next unless svc_rsc.try(:retireable?) + # TODO: the next line deals with the filtering for provisioning + # (https://github.com/ManageIQ/manageiq/blob/3921e87915b5a69937b9d4a70bb24ab71b97c165/app/models/service_template/filter.rb#L5) + # which should be extended to retirement as part of later work + # svc_rsc.resource_type != "ServiceTemplate" || self.class.include_service_template?(self, srr.id, parent_service) nh = attributes.except("id", "created_on", "updated_on", "type", "state", "status", "message") nh['options'] = options.except(:child_tasks) # Initial Options[:dialog] to an empty hash so we do not pass down dialog values to child services tasks @@ -45,14 +49,6 @@ def create_retire_subtasks(parent_service) end.compact! end - def retireable?(svc_rsc, parent_service) - srr = svc_rsc.resource - srr.present? && - srr.respond_to?(:retire_now) && - srr.type.present? && - (svc_rsc.resource_type != "ServiceTemplate" || self.class.include_service_template?(self, srr.id, parent_service)) - end - def create_task(svc_rsc, parent_service, nh) new_task = (svc_rsc.resource.type.demodulize + "RetireTask").constantize.new(nh) new_task.options.merge!( diff --git a/app/models/service_template.rb b/app/models/service_template.rb index 57f77d9a1e4..dc877ab77f3 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -41,6 +41,7 @@ class ServiceTemplate < ApplicationRecord include NewWithTypeStiMixin include TenancyMixin include ArchivedMixin + include CiFeatureMixin include_concern 'Filter' belongs_to :tenant diff --git a/app/models/vm.rb b/app/models/vm.rb index 5332406a036..1dc2a56d939 100644 --- a/app/models/vm.rb +++ b/app/models/vm.rb @@ -7,6 +7,7 @@ class Vm < VmOrTemplate extend InterRegionApiMethodRelay include CustomActionsMixin + include CiFeatureMixin include_concern 'Operations' From 15909ca95a5c7d370a84a5737ca7e623f1c51e68 Mon Sep 17 00:00:00 2001 From: d-m-u Date: Thu, 23 Aug 2018 11:11:46 -0400 Subject: [PATCH 2/2] Add tests --- app/models/mixins/ci_feature_mixin.rb | 4 +- app/models/service.rb | 4 ++ app/models/service_retire_task.rb | 2 +- spec/models/mixins/ci_feature_mixin_spec.rb | 41 ++++++++++++++++ spec/models/service_retire_task_spec.rb | 52 --------------------- 5 files changed, 47 insertions(+), 56 deletions(-) create mode 100644 spec/models/mixins/ci_feature_mixin_spec.rb diff --git a/app/models/mixins/ci_feature_mixin.rb b/app/models/mixins/ci_feature_mixin.rb index e0f5f0159f9..baee0b1c8c9 100644 --- a/app/models/mixins/ci_feature_mixin.rb +++ b/app/models/mixins/ci_feature_mixin.rb @@ -1,7 +1,5 @@ module CiFeatureMixin - extend ActiveSupport::Concern - def retireable? - resource.present? && resource.respond_to?(:retire_now) && resource.type.present? + true end end diff --git a/app/models/service.rb b/app/models/service.rb index 17f4ca8caa9..867f081300a 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -215,6 +215,10 @@ def composite? children.present? end + def retireable? + type.present? + end + def atomic? children.empty? end diff --git a/app/models/service_retire_task.rb b/app/models/service_retire_task.rb index d1fe985cbc8..0178ad104fc 100644 --- a/app/models/service_retire_task.rb +++ b/app/models/service_retire_task.rb @@ -32,7 +32,7 @@ def after_request_task_create def create_retire_subtasks(parent_service) parent_service.direct_service_children.each { |child| create_retire_subtasks(child) } parent_service.service_resources.collect do |svc_rsc| - next unless svc_rsc.try(:retireable?) + next unless svc_rsc.resource.try(:retireable?) # TODO: the next line deals with the filtering for provisioning # (https://github.com/ManageIQ/manageiq/blob/3921e87915b5a69937b9d4a70bb24ab71b97c165/app/models/service_template/filter.rb#L5) # which should be extended to retirement as part of later work diff --git a/spec/models/mixins/ci_feature_mixin_spec.rb b/spec/models/mixins/ci_feature_mixin_spec.rb new file mode 100644 index 00000000000..30fe3d8a3fa --- /dev/null +++ b/spec/models/mixins/ci_feature_mixin_spec.rb @@ -0,0 +1,41 @@ +describe CiFeatureMixin do + let(:service) { FactoryGirl.create(:service) } + describe "#retireable?" do + it "vm is retireable" do + FactoryGirl.create(:service_resource, :service => service, :resource => FactoryGirl.create(:vm)) + + expect(service.service_resources.first.resource.retireable?).to eq(true) + end + + it "orchestration stack is retireable" do + FactoryGirl.create(:service_resource, :service => service, :resource => FactoryGirl.create(:orchestration_stack_amazon)) + + expect(service.service_resources.first.resource.retireable?).to eq(true) + end + + it "job not retireable" do + FactoryGirl.create(:service_resource, :service => service, :resource => FactoryGirl.create(:embedded_ansible_job)) + + expect(service.service_resources.first.resource.retireable?).to eq(false) + end + + context "service" do + context "with type" do + let(:service1) { FactoryGirl.create(:service_ansible_tower, :type => ServiceAnsibleTower) } + it "is retireable" do + FactoryGirl.create(:service_resource, :service => service, :resource => service1) + + expect(service.service_resources.first.resource.retireable?).to eq(true) + end + end + + context "without type" do + it "is not retireable" do + FactoryGirl.create(:service_resource, :service => service, :resource => FactoryGirl.create(:service)) + + expect(service.service_resources.first.resource.retireable?).to eq(false) + end + end + end + end +end diff --git a/spec/models/service_retire_task_spec.rb b/spec/models/service_retire_task_spec.rb index d9ec99fd483..be98a73a28e 100644 --- a/spec/models/service_retire_task_spec.rb +++ b/spec/models/service_retire_task_spec.rb @@ -97,58 +97,6 @@ end end - describe "#retireable?" do - include_context "service_bundle" - - context "without srr" do - it "is not retireable" do - service.service_resources << FactoryGirl.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => nil) - @service_retire_task = FactoryGirl.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] }) - - expect(@service_retire_task.retireable?(service.service_resources.first, service)).to be(false) - end - end - - context "srr nonresponsive to retireable?" do - it "is not retireable" do - service.service_resources << FactoryGirl.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => User.first.id) - @service_retire_task = FactoryGirl.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] }) - - expect(@service_retire_task.retireable?(service.service_resources.first, service)).to be(false) - end - end - - context "srr sans type" do - let(:vm_without_type) { FactoryGirl.create(:vm, :type => nil) } - it "is not retireable" do - service.service_resources << FactoryGirl.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => vm_without_type.id) - @service_retire_task = FactoryGirl.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] }) - - expect(@service_retire_task.retireable?(service.service_resources.first, service)).to be(false) - end - end - - context "svc_rsc resource_type is not ServiceTemplate" do - it "is not retireable" do - service.service_resources << FactoryGirl.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => service_c1.id) - @service_retire_task = FactoryGirl.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] }) - allow(@service_retire_task.class).to receive(:include_service_template?).with(@service_retire_task, service.service_resources.first.id, service).and_return(true) - - expect(@service_retire_task.retireable?(service.service_resources.first, service)).to be(false) - end - end - - context "class includes service template" do - it "is not retireable" do - service.service_resources << FactoryGirl.create(:service_resource, :resource_type => "ServiceTemplate", :service_id => service_c1.id, :resource_id => vm.id) - @service_retire_task = FactoryGirl.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] }) - allow(@service_retire_task.class).to receive(:include_service_template?).with(@service_retire_task, service.service_resources.first.id, service).and_return(false) - - expect(@service_retire_task.retireable?(service.service_resources.first, service)).to be(false) - end - end - end - describe "deliver_to_automate" do before do allow(MiqServer).to receive(:my_zone).and_return(Zone.seed.name)