Merge pull request #560 from liu-samuel/clean-up-failed-provision

Clean up network interface and public ip on failure

app/models/manageiq/providers/azure/cloud_manager/provision/cloning.rb

@@ -176,6 +176,16 @@ def build_nic_options(ip_id = nil)
     network_options
   end
 
+  def start_clone_task
+    super
+  rescue Azure::Armrest::BadRequestException => err
+    phase_context[:exception_class]     = err.class.name
+    phase_context[:exception_message]   = err.message
+    phase_context[:exception_backtrace] = err.backtrace
+    phase_context[:error_phase]         = phase
+    signal :clone_failure_cleanup
+  end
+
   def start_clone(clone_options)
     source.with_provider_connection do |azure|
       vms = ::Azure::Armrest::VirtualMachineService.new(azure)

app/models/manageiq/providers/azure/cloud_manager/provision/state_machine.rb

@@ -6,4 +6,24 @@ def customize_destination
 
     signal :post_create_destination
   end
+
+  def clone_failure_cleanup
+    return if phase_context[:clone_options].nil?
+
+    delete_instance(phase_context[:clone_options]) if phase_context[:clone_options]
+    signal :provision_error
+  rescue Azure::Armrest::BadRequestException
+    requeue_phase(3.minutes)
+  end
+
+  def delete_instance(instance_attrs)
+    source.with_provider_connection do |azure|
+      nis = ::Azure::Armrest::Network::NetworkInterfaceService.new(azure)
+      ips = ::Azure::Armrest::Network::IpAddressService.new(azure)
+      ni = nis.get(instance_attrs[:name], resource_group.name)
+      ip = ips.get("#{instance_attrs[:name]}-publicIp", resource_group.name)
+      nis.delete(instance_attrs[:name], resource_group.name) if ni
+      ips.delete("#{instance_attrs[:name]}-publicIp", resource_group.name) if ip
+    end
+  end
 end

spec/factories/image.rb

@@ -1,3 +1,7 @@
 FactoryBot.define do
-  factory :azure_image, :class => 'ManageIQ::Providers::Azure::CloudManager::Template'
+  factory :azure_image, :class => 'ManageIQ::Providers::Azure::CloudManager::Template' do
+    sequence(:name) { |n| "Azure Image #{n}" }
+    location { "azure" }
+    vendor { "azure" }
+  end
 end

spec/models/manageiq/providers/azure/cloud_manager/provision/cloning_spec.rb

@@ -1,10 +1,18 @@
 require 'azure-armrest'
 
 describe ManageIQ::Providers::Azure::CloudManager::Provision::Cloning do
-  let(:provision) { ManageIQ::Providers::Azure::CloudManager::Provision.new }
+  let(:template) { FactoryBot.create(:azure_image, :ext_management_system => ems) }
+  let(:provision) { FactoryBot.create(:miq_provision_azure, :options => {:src_vm_id => template.id}, :miq_request => miq_request) }
+  let(:configuration) { double("azure armrest configuration") }
+  let(:vms) { double("virtual machine service") }
   let(:ems) { FactoryBot.create(:ems_azure_with_authentication) }
   let(:ip_address_service) { double('ip address service') }
+  let(:public_ip) { double('public ip') }
+  let(:network_interface) { double('network interface') }
+  let(:nic_service) { double("nic service") }
   let(:nic_id) { '/subscriptions/xyz/resourceGroups/foo/providers/Microsoft.Network/networkInterfaces/foo_nic' }
+  let(:miq_request) { FactoryBot.create(:miq_provision_request) }
+
 
   context "associated_nic" do
     before do
@@ -113,4 +121,48 @@
       expect(provision.create_nic(false)).to eql(@nic_object.id)
     end
   end
+
+  context "start_clone_task" do
+    before do
+      resource_group = FactoryBot.create(:azure_resource_group)
+      dest_name = "test"
+      clone_options = {clone_options: {name: "test-name", location: "germanycentralwest"}}
+
+      allow(Azure::Armrest::Network::IpAddressService).to receive(:new).and_return(ip_address_service)
+      allow(Azure::Armrest::Network::NetworkInterfaceService).to receive(:new).and_return(nic_service)
+      allow(Azure::Armrest::Configuration).to receive(:new).and_return(configuration)
+      allow(Azure::Armrest::VirtualMachineService).to receive(:new).and_return(vms)
+
+      allow(provision).to receive(:resource_group).and_return(resource_group)
+      allow(provision).to receive(:dest_name).and_return(dest_name)
+      allow(provision).to receive(:phase_context).and_return(clone_options)
+      allow(provision).to receive(:requeue_phase)
+
+      allow(vms).to receive(:create).and_raise(Azure::Armrest::BadRequestException.new('errors', 'details', 'info'))
+      allow(ip_address_service).to receive(:get).and_return(public_ip)
+      allow(nic_service).to receive(:get).and_return(network_interface)
+
+      allow(nic_service).to receive(:delete)
+      allow(ip_address_service).to receive(:delete)
+    end
+
+    it 'deletes the public IP and network interface when a BadRequestException is raised' do
+      provision.start_clone_task
+      expect(ip_address_service).to have_received(:delete)
+      expect(nic_service).to have_received(:delete)      
+    end
+
+    it 'phase_context is requeued properly after BadRequestException' do
+      allow(nic_service).to receive(:delete).and_raise(Azure::Armrest::BadRequestException.new('errors', 'details', 'info'))
+      provision.start_clone_task
+      expect(provision).to have_received(:requeue_phase).with(3.minutes)
+    end
+
+    it 'phase_context errors are properly set after BadRequestException' do
+      allow(nic_service).to receive(:delete).and_raise(Azure::Armrest::BadRequestException.new('errors', 'details', 'info'))
+      provision.start_clone_task
+      expect(provision.phase_context[:exception_class]).to eq('Azure::Armrest::BadRequestException')
+      expect(provision.phase_context[:exception_message]).to eq('details')
+    end
+  end
 end