From ae7a083391db158df8e487586688133a48e0d335 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 3 Apr 2019 16:13:19 -0400 Subject: [PATCH] Return vm records with the same ems_ref first If there is a vm record with the same ems_ref then update that even if it has a different uid_ems. https://bugzilla.redhat.com/show_bug.cgi?id=1695008 --- app/models/ems_refresh/save_inventory.rb | 6 +- .../models/ems_refresh/save_inventory_spec.rb | 88 +++++-------------- 2 files changed, 28 insertions(+), 66 deletions(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index f7e85c92da8..1b4d393f722 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -53,6 +53,8 @@ def save_vms_inventory(ems, hashes, target = nil, disconnect = true) ] remove_keys = child_keys + extra_infra_keys + extra_cloud_keys + vms_by_ems_ref = ems.vms_and_templates.index_by(&:ems_ref) + # Query for all of the Vms once across all EMSes, to handle any moving VMs vms_uids = hashes.collect { |h| h[:uid_ems] }.compact vms = VmOrTemplate.where(:uid_ems => vms_uids).to_a @@ -89,7 +91,7 @@ def save_vms_inventory(ems, hashes, target = nil, disconnect = true) # Find the Vm in the database with the current uid_ems. In the event # of duplicates, try to determine which one is correct. - found = vms_by_uid_ems[h[:uid_ems]] || [] + found = Array(vms_by_ems_ref[h[:ems_ref]] || vms_by_uid_ems[h[:uid_ems]]) if found.length > 1 || (found.length == 1 && found.first.ems_id) found_dups = found @@ -113,7 +115,7 @@ def save_vms_inventory(ems, hashes, target = nil, disconnect = true) # build a type-specific vm or template found = ems.vms_and_templates.klass.new(h) else - vms_by_uid_ems[h[:uid_ems]].delete(found) + vms_by_uid_ems[h[:uid_ems]]&.delete(found) h.delete(:type) _log.info("#{log_header} Updating #{type} [#{found.name}] id: [#{found.id}] location: [#{found.location}] storage id: [#{found.storage_id}] uid_ems: [#{found.uid_ems}] ems_ref: [#{h[:ems_ref]}]") diff --git a/spec/models/ems_refresh/save_inventory_spec.rb b/spec/models/ems_refresh/save_inventory_spec.rb index 28e162930e3..9af7f1b773a 100644 --- a/spec/models/ems_refresh/save_inventory_spec.rb +++ b/spec/models/ems_refresh/save_inventory_spec.rb @@ -128,24 +128,14 @@ EmsRefresh.save_vms_inventory(@ems, data) vms = Vm.all - expect(vms.length).to eq(3) - - disconnected, connected = vms.partition { |v| v.ems_id.nil? } - expect(disconnected.length).to eq(1) - expect(connected.length).to eq(2) - - d = disconnected.first - c1, c2 = connected.sort_by(&:id) - - expect(d.id).to eq(@vm2.id) - expect(d.uid_ems).to eq(@vm2.uid_ems) + expect(vms.length).to eq(2) + v1, v2 = vms.sort_by(&:id) - expect(c1.id).to eq(@vm1.id) - expect(c1.uid_ems).to eq(@vm1.uid_ems) + expect(v1.id).to eq(@vm1.id) + expect(v1.uid_ems).to eq(@vm1.uid_ems) - expect(c2.id).not_to eq(@vm1.id) - expect(c2.id).not_to eq(@vm2.id) - expect(c2.uid_ems).to eq(@vm1.uid_ems) + expect(v2.id).to eq(@vm2.id) + expect(v2.uid_ems).to eq(@vm1.uid_ems) end end @@ -161,24 +151,14 @@ EmsRefresh.save_vms_inventory(@ems, data) vms = Vm.all - expect(vms.length).to eq(3) - - disconnected, connected = vms.partition { |v| v.ems_id.nil? } - expect(disconnected.length).to eq(1) - expect(connected.length).to eq(2) - - d = disconnected.first - c1, c2 = connected.sort_by(&:id) - - expect(d.id).to eq(@vm2.id) - expect(d.uid_ems).to eq(@vm2.uid_ems) + expect(vms.length).to eq(2) + v1, v2 = vms.sort_by(&:id) - expect(c1.id).to eq(@vm1.id) - expect(c1.uid_ems).to eq(@vm1.uid_ems) + expect(v1.id).to eq(@vm1.id) + expect(v1.uid_ems).to eq(@vm1.uid_ems) - expect(c2.id).not_to eq(@vm1.id) - expect(c2.id).not_to eq(@vm2.id) - expect(c2.uid_ems).not_to eq(@vm1.uid_ems) + expect(v2.id).to eq(@vm2.id) + expect(v2.uid_ems).not_to eq(@vm1.uid_ems) end it "should handle dups in the raw data" do @@ -209,24 +189,14 @@ EmsRefresh.save_vms_inventory(@ems, data) vms = Vm.all - expect(vms.length).to eq(3) - - disconnected, connected = vms.partition { |v| v.ems_id.nil? } - expect(disconnected.length).to eq(1) - expect(connected.length).to eq(2) - - d = disconnected.first - c1, c2 = connected.sort_by(&:id) - - expect(d.id).to eq(@vm2.id) - expect(d.uid_ems).to eq(@vm2.uid_ems) + expect(vms.length).to eq(2) + v1, v2 = vms.sort_by(&:id) - expect(c1.id).to eq(@vm1.id) - expect(c1.uid_ems).to eq(@vm1.uid_ems) + expect(v1.id).to eq(@vm1.id) + expect(v1.uid_ems).to eq(@vm1.uid_ems) - expect(c2.id).not_to eq(@vm1.id) - expect(c2.id).not_to eq(@vm2.id) - expect(c2.uid_ems).not_to eq(@vm1.uid_ems) + expect(v2.id).to eq(@vm2.id) + expect(v2.uid_ems).not_to eq(@vm2.uid_ems) end it "should handle dups in the raw data" do @@ -329,24 +299,14 @@ EmsRefresh.save_vms_inventory(@ems, data) vms = Vm.all - expect(vms.length).to eq(3) - - disconnected, connected = vms.partition { |v| v.ems_id.nil? } - expect(disconnected.length).to eq(1) - expect(connected.length).to eq(2) - - d = disconnected.first - c1, c2 = connected.sort_by(&:id) - - expect(d.id).to eq(@vm2.id) - expect(d.uid_ems).to eq(@vm2.uid_ems) + expect(vms.length).to eq(2) + v1, v2 = vms.sort_by(&:id) - expect(c1.id).to eq(@vm1.id) - expect(c1.uid_ems).to eq(@vm1.uid_ems) + expect(v1.id).to eq(@vm1.id) + expect(v1.uid_ems).to eq(@vm1.uid_ems) - expect(c2.id).not_to eq(@vm1.id) - expect(c2.id).not_to eq(@vm2.id) - expect(c2.uid_ems).to eq(@vm1.uid_ems) + expect(v2.id).to eq(@vm2.id) + expect(v2.uid_ems).not_to eq(@vm2.uid_ems) end end end