Merge pull request #16897 from gmcculloug/provision_vm_name

Refactor VM naming method in provisioning task to support call from automate
(cherry picked from commit ee6cb2d)

https://bugzilla.redhat.com/show_bug.cgi?id=1539752

app/models/miq_provision.rb

@@ -57,11 +57,16 @@ def placement_auto
   end
 
   def after_request_task_create
-    vm_name                      = get_next_vm_name
-    options[:vm_target_name]     = vm_name
-    options[:vm_target_hostname] = get_hostname(vm_name)
-    self.description             = self.class.get_description(self, vm_name)
-    save
+    update_vm_name(get_next_vm_name, :update_request => false)
+  end
+
+  def update_vm_name(new_name, update_request: true)
+    new_name = self.class.get_vm_full_name(new_name, self, true)
+    options[:vm_target_name]     = new_name
+    options[:vm_target_hostname] = get_hostname(new_name)
+
+    update_attributes(:description => self.class.get_description(self, new_name), :options => options)
+    miq_request.update_description_from_tasks if update_request
   end
 
   def after_ae_delivery(ae_result)

app/models/miq_provision/automate.rb

@@ -1,6 +1,20 @@
 module MiqProvision::Automate
   extend ActiveSupport::Concern
 
+  module ClassMethods
+    def vm_name_from_automate(prov_obj)
+      prov_obj.save
+      attrs = {'request' => 'UI_PROVISION_INFO', 'message' => 'get_vmname'}
+      MiqAeEngine.set_automation_attributes_from_objects([prov_obj.get_user], attrs)
+      MiqAeEngine.resolve_automation_object("REQUEST",
+                                            prov_obj.get_user,
+                                            attrs,
+                                            :vmdb_object => prov_obj).root("vmname").tap do
+        prov_obj.reload
+      end
+    end
+  end
+
   def get_placement_via_automate
     attrs = automate_attributes('get_placement')
     ws = MiqAeEngine.resolve_automation_object("REQUEST", get_user, attrs, :vmdb_object => self)

app/models/miq_provision/naming.rb

@@ -6,13 +6,7 @@ module MiqProvision::Naming
 
   module ClassMethods
     def get_next_vm_name(prov_obj, determine_index = true)
-      prov_obj.save
-      attrs = {'request' => 'UI_PROVISION_INFO', 'message' => 'get_vmname'}
-      MiqAeEngine.set_automation_attributes_from_objects([prov_obj.get_user], attrs)
-      ws = MiqAeEngine.resolve_automation_object("REQUEST", prov_obj.get_user, attrs, :vmdb_object => prov_obj)
-
-      unresolved_vm_name = ws.root("vmname")
-      prov_obj.reload
+      unresolved_vm_name = vm_name_from_automate(prov_obj)
 
       # Check if we need to force a unique target name
       if prov_obj.get_option(:miq_force_unique_name) == true && unresolved_vm_name !~ NAME_SEQUENCE_REGEX

app/models/miq_provision_request.rb

@@ -66,8 +66,12 @@ def set_description(force = false)
   end
 
   def post_create_request_tasks
+    update_description_from_tasks
+  end
+
+  def update_description_from_tasks
     return unless requested_task_idx.length == 1
-    update_attributes(:description => miq_request_tasks.first.description)
+    update_attributes(:description => miq_request_tasks.reload.first.description)
   end
 
   def my_role

spec/models/miq_provision_spec.rb

@@ -13,6 +13,8 @@
       }
     end
 
+    let(:miq_region) { MiqRegion.create }
+
     context "with VMware infrastructure" do
       before(:each) do
         @ems         = FactoryGirl.create(:ems_vmware_with_authentication)
@@ -54,7 +56,7 @@
         end
 
         it "should create a valid target_name and hostname" do
-          MiqRegion.seed
+          expect(MiqRegion).to receive(:my_region).and_return(miq_region).twice
           ae_workspace = double("ae_workspace")
           expect(ae_workspace).to receive(:root).and_return(@target_vm_name)
           expect(MiqAeEngine).to receive(:resolve_automation_object).and_return(ae_workspace).exactly(3).times
@@ -78,11 +80,40 @@
           expect(@vm_prov.get_option(:vm_target_hostname)).to eq(name_002.gsub(/ +|_+/, "-"))
         end
 
+        context "#update_vm_name" do
+          it "does not modify a fully resolved vm_name" do
+            @vm_prov.update_vm_name(@target_vm_name)
+            expect(@vm_prov.get_option(:vm_target_name)).to eq(@target_vm_name)
+          end
+
+          it "Enumerates vm_name that contains the naming sequence characters" do
+            expect(MiqRegion).to receive(:my_region).and_return(miq_region)
+
+            @vm_prov.update_vm_name("#{@target_vm_name}$n{3}")
+            expect(@vm_prov.get_option(:vm_target_name)).to eq("#{@target_vm_name}001")
+          end
+
+          it "Updates the request description with only name parameter passed" do
+            expect(@pr).to receive(:update_description_from_tasks).and_call_original
+
+            @vm_prov.update_vm_name(@target_vm_name)
+
+            expect(@vm_prov.description).to eq("Provision from [template1] to [clone test]")
+            expect(@pr.description).to eq(@vm_prov.description)
+          end
+
+          it "Does not update the request description when `update_request` parameter is false" do
+            expect(@pr).not_to receive(:update_description_from_tasks)
+
+            @vm_prov.update_vm_name(@target_vm_name, :update_request => false)
+          end
+        end
+
         context "when auto naming sequence exceeds the range" do
           before do
-            region = MiqRegion.seed
-            region.naming_sequences.create(:name => "#{@target_vm_name}$n{3}", :source => "provisioning", :value => 998)
-            region.naming_sequences.create(:name => "#{@target_vm_name}$n{4}", :source => "provisioning", :value => 10)
+            expect(MiqRegion).to receive(:my_region).exactly(3).times.and_return(miq_region)
+            miq_region.naming_sequences.create(:name => "#{@target_vm_name}$n{3}", :source => "provisioning", :value => 998)
+            miq_region.naming_sequences.create(:name => "#{@target_vm_name}$n{4}", :source => "provisioning", :value => 10)
           end
 
           it "should advance to next range but based on the existing sequence number for the new range" do