Merge pull request #17381 from NickLaMuro/faster_lan_lookup_in_miq_pr…

…ovision_virt_workflow

Load Lans effeciently in MiqProvisionVirtWorkflow

app/models/miq_provision_virt_workflow.rb

@@ -230,8 +230,6 @@ def available_vlans_and_hosts(options = {})
     hosts = get_selected_hosts(src)
     unless @vlan_options[:vlans] == false
       rails_logger('allowed_vlans', 0)
-      # TODO: Use Active Record to preload this data?
-      MiqPreloader.preload(hosts, :lans => :switches)
       load_allowed_vlans(hosts, vlans)
       rails_logger('allowed_vlans', 1)
     end
@@ -244,9 +242,11 @@ def load_allowed_vlans(hosts, vlans)
   end
 
   def load_hosts_vlans(hosts, vlans)
-    hosts.each do |h|
-      h.lans.each { |l| vlans[l.name] = l.name unless l.switch.shared? }
-    end
+    Lan.joins(:switch => :hosts)
+       .where(:hosts => {:id => hosts.map(&:id)})
+       .where(:switches => {:shared => [nil, false]})
+       .distinct.pluck(:name)
+       .each_with_object(vlans) { |v, h| h[v] = v }
   end
 
   def filter_by_tags(target, options)

spec/models/miq_provision_virt_workflow_spec.rb

@@ -67,9 +67,11 @@
 
   context 'network selection' do
     before do
-      @ems    = FactoryGirl.create(:ems_vmware)
-      @host1  = FactoryGirl.create(:host_vmware, :ems_id => @ems.id)
-      @src_vm = FactoryGirl.create(:vm_vmware, :host => @host1, :ems_id => @ems.id)
+      @ems      = FactoryGirl.create(:ems_vmware)
+      @host1    = FactoryGirl.create(:host_vmware, :ems_id => @ems.id)
+      @host2    = FactoryGirl.create(:host_vmware, :ems_id => @ems.id)
+      @src_vm   = FactoryGirl.create(:vm_vmware, :host => @host1, :ems_id => @ems.id)
+      @other_vm = FactoryGirl.create(:vm_vmware, :host => @host2, :ems_id => @ems.id)
       allow(Rbac).to receive(:search) do |hash|
         [Array.wrap(hash[:targets])]
       end
@@ -84,24 +86,30 @@
         s11 = FactoryGirl.create(:switch, :name => "A")
         s12 = FactoryGirl.create(:switch, :name => "B")
         s13 = FactoryGirl.create(:switch, :name => "C")
-        @src_vm.host.switches = [s11, s12, s13]
+        s14 = FactoryGirl.create(:switch, :name => "D")
+        @src_vm.host.switches   = [s11, s12, s13]
+        @other_vm.host.switches = [s14]
         @lan11 = FactoryGirl.create(:lan, :name => "lan_A", :switch_id => s11.id)
         @lan12 = FactoryGirl.create(:lan, :name => "lan_B", :switch_id => s12.id)
         @lan13 = FactoryGirl.create(:lan, :name => "lan_C", :switch_id => s13.id)
+        @lan14 = FactoryGirl.create(:lan, :name => "lan_D", :switch_id => s14.id)
       end
 
       it '#allowed_vlans' do
-        allow(workflow).to receive(:allowed_hosts).with(no_args).and_return([workflow.host_to_hash_struct(@host1)])
+        allowed_hosts = [
+          workflow.host_to_hash_struct(@host1),
+          workflow.host_to_hash_struct(@host2)
+        ]
+        allow(workflow).to receive(:allowed_hosts).with(no_args).and_return(allowed_hosts)
         vlans = workflow.allowed_vlans(:vlans => true, :dvs => false)
-        lan_keys = [@lan11.name, @lan13.name, @lan12.name]
+        lan_keys = [@lan11.name, @lan13.name, @lan12.name, @lan14.name]
         expect(vlans.keys).to match_array(lan_keys)
         expect(vlans.values).to match_array(lan_keys)
       end
 
       it '#load_hosts_vlans' do
-        hosts = [@host1]
-        MiqPreloader.preload(hosts, :lans => :switches)
-        expect { workflow.load_hosts_vlans(hosts, {}) }.not_to exceed_query_limit(0)
+        hosts = [@host1, @host2]
+        expect { workflow.load_hosts_vlans(hosts, {}) }.not_to exceed_query_limit(1)
       end
     end
   end