From b6788f9cd21ec370797263035af0e9b06fecadb6 Mon Sep 17 00:00:00 2001 From: Oleg Barenboim Date: Fri, 29 May 2020 12:29:23 -0400 Subject: [PATCH] Merge pull request #20222 from agrare/fix_vm_supports_terminate Fix Vm#supports_terminate? checking EMS#vm_destroy (cherry picked from commit 74d620c2016d2264f4463841c07d9b4a21426f0c) --- app/models/vm_or_template/operations.rb | 6 --- spec/models/vm_or_template_spec.rb | 54 ++++++++++++++++--------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/app/models/vm_or_template/operations.rb b/app/models/vm_or_template/operations.rb index a9500cf414b..571f5d959b6 100644 --- a/app/models/vm_or_template/operations.rb +++ b/app/models/vm_or_template/operations.rb @@ -139,12 +139,6 @@ def validate_vm_control_shelve_offload_action end unsupported_reason_add(:control, msg) if msg end - - supports :terminate do - msg = unsupported_reason(:control) unless supports_control? - msg ||= _("Provider doesn't support vm_destroy") unless ext_management_system.respond_to?(:vm_destroy) - unsupported_reason_add(:terminate, msg) if msg - end end def validate_vm_control_powered_on diff --git a/spec/models/vm_or_template_spec.rb b/spec/models/vm_or_template_spec.rb index 39a606dec31..69404c77c7b 100644 --- a/spec/models/vm_or_template_spec.rb +++ b/spec/models/vm_or_template_spec.rb @@ -576,35 +576,49 @@ end end - context "#supports_terminate?" do - let(:ems_does_vm_destroy) { FactoryBot.create(:ems_vmware) } - let(:ems_doesnot_vm_destroy) { FactoryBot.create(:ems_storage) } + context "#supports_control?" do + let(:retired_vm) { FactoryBot.create(:vm, :retired => true, :ext_management_system => ems, :host => host) } + let(:template) { FactoryBot.create(:miq_template) } + let(:terminated_vm) { FactoryBot.create(:vm_amazon, :raw_power_state => "terminated") } + let(:vm_no_host) { FactoryBot.create(:vm, :ext_management_system => ems) } + let(:disconnected_vm) { FactoryBot.create(:vm, :host => host, :ext_management_system => ems, :connection_state => "disconnected") } + let(:archived_vm) { FactoryBot.create(:vm, :host => host) } + let(:vm) { FactoryBot.create(:vm, :ext_management_system => ems, :host => host) } + let(:ems) { FactoryBot.create(:ems_infra) } let(:host) { FactoryBot.create(:host) } - it "returns true for a VM not terminated" do - vm = FactoryBot.create(:vm, :host => host, :ext_management_system => ems_does_vm_destroy) - allow(vm).to receive_messages(:terminated? => false) - expect(vm.supports_terminate?).to eq(true) + it "returns false for a retired vm" do + expect(retired_vm.supports_control?).to be_falsey + expect(retired_vm.unsupported_reason(:control)).to eq("The VM is retired") + end + + it "returns false for a template" do + expect(template.supports_control?).to be_falsey + expect(template.unsupported_reason(:control)).to eq("The VM is a template") end it "returns false for a terminated VM" do - vm = FactoryBot.create(:vm, :host => host, :ext_management_system => ems_does_vm_destroy) - allow(vm).to receive_messages(:terminated? => true) - expect(vm.supports_terminate?).to eq(false) - expect(vm.unsupported_reason(:terminate)).to eq("The VM is terminated") + expect(terminated_vm.supports_control?).to eq(false) + expect(terminated_vm.unsupported_reason(:control)).to eq("The VM is terminated") + end + + it "returns false for a vm without a host" do + expect(vm_no_host.supports_control?).to be_falsey + expect(vm_no_host.unsupported_reason(:control)).to eq("The VM is not connected to a Host") + end + + it "returns false for a disconnected vm" do + expect(disconnected_vm.supports_control?).to be_falsey + expect(disconnected_vm.unsupported_reason(:control)).to eq("The VM does not have a valid connection state") end - it "returns false for a provider doesn't support vm_destroy" do - vm = FactoryBot.create(:vm, :host => host, :ext_management_system => ems_doesnot_vm_destroy) - allow(vm).to receive_messages(:terminated? => false) - expect(vm.supports_terminate?).to eq(false) - expect(vm.unsupported_reason(:terminate)).to eq("Provider doesn't support vm_destroy") + it "returns false for an archived vm" do + expect(archived_vm.supports_control?).to be_falsey + expect(archived_vm.unsupported_reason(:control)).to eq("The VM is not connected to an active Provider") end - it "returns false for a WMware VM" do - vm = FactoryBot.create(:vm, :host => host, :ext_management_system => ems_does_vm_destroy) - allow(vm).to receive_messages(:terminated? => false) - expect(vm.supports_terminate?).to eq(true) + it "returns true for a valid vm" do + expect(vm.supports_control?).to be_truthy end end