Skip to content

Commit

Permalink
Load Lans effeciently in MiqProvisionVirtWorkflow
Browse files Browse the repository at this point in the history
Instead of preloading Lan records, splitting them up based on if they are
shareable and instantiating entire ActiveRecord to only use the name
column in a hash, use a specific query to gather and filter those values
and generate a hash from that.
  • Loading branch information
NickLaMuro committed May 3, 2018
1 parent c2e01c9 commit 3dcee40
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
10 changes: 5 additions & 5 deletions app/models/miq_provision_virt_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
26 changes: 17 additions & 9 deletions spec/models/miq_provision_virt_workflow_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 3dcee40

Please sign in to comment.