Skip to content

Commit

Permalink
Merge pull request #13997 from Ladas/remove_duplicated_data_in_graph_…
Browse files Browse the repository at this point in the history 
…refresh

Remove duplicated data in graph refresh
  • Loading branch information
@agrare
agrare authored Feb 24, 2017
2 parents 9579706 + 7d795eb commit 81458a9
Show file tree
Hide file tree
Showing 2 changed files with 239 additions and 3 deletions.
13 changes: 10 additions & 3 deletions app/models/manager_refresh/save_collection/helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,16 @@ def create_or_update_inventory!(inventory_collection, record_index, association)
unique_index_keys = inventory_collection.manager_ref_to_cols

association.find_each do |record|
# TODO(lsmola) the old code was able to deal with duplicate records, should we do that? The old data still can
# have duplicate methods, so we should clean them up. It will slow up the indexing though.
record_index[inventory_collection.object_index_with_keys(unique_index_keys, record)] = record
index = inventory_collection.object_index_with_keys(unique_index_keys, record)
if record_index[index]
# We have a duplicate in the DB, destroy it. A find_each method does automatically .order(:id => :asc)
# so we always keep the oldest record in the case of duplicates.
_log.warn("A duplicate record was detected and destroyed, inventory_collection: '#{inventory_collection}', "\
"record: '#{record}', duplicate_index: '#{index}'")
record.destroy
else
record_index[index] = record
end
end

inventory_collection_size = inventory_collection.size
Expand Down
229 changes: 229 additions & 0 deletions spec/models/manager_refresh/save_inventory/acyclic_graph_of_inventory_collections_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,235 @@
expect(vm12.key_pairs.pluck(:id)).to match_array([@key_pair1.id, @key_pair12.id])
expect(vm2.key_pairs.pluck(:id)).to match_array([@key_pair2.id])
end

it "cleans up duplicates while" do
initialize_mocked_records

@vm1_dup1 = FactoryGirl.create(
:vm_cloud,
vm_data(1).merge(
:flavor => @flavor_1,
:genealogy_parent => @image1,
:key_pairs => [@key_pair1],
:location => 'host_10_10_10_1.com',
:ext_management_system => @ems,
)
)

@vm1_dup2 = FactoryGirl.create(
:vm_cloud,
vm_data(1).merge(
:flavor => @flavor_1,
:genealogy_parent => @image1,
:key_pairs => [@key_pair1],
:location => 'host_10_10_10_1.com',
:ext_management_system => @ems,
)
)

# Assert that the mocked data in the DB do have duplicate vm_ems_ref_1
assert_all_records_match_hashes(
[Vm.all, @ems.vms],
{
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_12",
:name => "vm_name_12",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_2",
:name => "vm_name_2",
:location => "host_10_10_10_2.com",
}, {
:ems_ref => "vm_ems_ref_4",
:name => "vm_name_4",
:location => "default_value_unknown",
}
)

# Now save the records using InventoryCollections
# Fill the InventoryCollections with data, that have a modified name
initialize_data_and_inventory_collections
add_data_to_inventory_collection(@data[:vms],
@vm_data_1.merge(:name => "vm_name_1_changed"),
@vm_data_12.merge(:name => "vm_name_12_changed"),
@vm_data_2.merge(:name => "vm_name_2_changed"),
@vm_data_4.merge(:name => "vm_name_4_changed"),
vm_data(5))
add_data_to_inventory_collection(@data[:miq_templates], @image_data_1, @image_data_2, @image_data_3)
add_data_to_inventory_collection(@data[:key_pairs], @key_pair_data_1, @key_pair_data_12, @key_pair_data_2,
@key_pair_data_3)
add_data_to_inventory_collection(@data[:hardwares], @hardware_data_1, @hardware_data_2, @hardware_data_12)
add_data_to_inventory_collection(@data[:disks],
@disk_data_1.merge(:device_type => "nvme_ssd_1"),
@disk_data_12.merge(:device_type => "nvme_ssd_12"),
@disk_data_13.merge(:device_type => "nvme_ssd_13"),
@disk_data_2.merge(:device_type => "nvme_ssd_2"))
add_data_to_inventory_collection(@data[:networks], @public_network_data_1, @public_network_data_12,
@public_network_data_13, @public_network_data_14, @public_network_data_2)
add_data_to_inventory_collection(@data[:flavors], @flavor_data_1, @flavor_data_2, @flavor_data_3)

# Invoke the InventoryCollections saving
ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values)

# Assert that saved data have the updated values and the duplicate vm_ems_ref_1 are gone
assert_all_records_match_hashes(
[Vm.all, @ems.vms],
{
:id => anything, # There is no guarantee which duplicates are deleted
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1_changed",
:location => "host_10_10_10_1.com",
}, {
:id => @vm12.id,
:ems_ref => "vm_ems_ref_12",
:name => "vm_name_12_changed",
:location => "host_10_10_10_1.com",
}, {
:id => @vm2.id,
:ems_ref => "vm_ems_ref_2",
:name => "vm_name_2_changed",
:location => "host_10_10_10_2.com",
}, {
:id => @vm4.id,
:ems_ref => "vm_ems_ref_4",
:name => "vm_name_4_changed",
:location => "default_value_unknown",
}, {
:id => anything,
:ems_ref => "vm_ems_ref_5",
:name => "vm_name_5",
:location => "vm_location_5",
}
)
end

it "cleans up duplicates, leaving the one with service relation" do
initialize_mocked_records

service = FactoryGirl.create(:service)
# Add service to this Vm1
service.add_resource!(@vm1)

@vm1_dup1 = FactoryGirl.create(
:vm_cloud,
vm_data(1).merge(
:flavor => @flavor_1,
:genealogy_parent => @image1,
:key_pairs => [@key_pair1],
:location => 'host_10_10_10_1_dup_1.com',
:ext_management_system => @ems,
)
)

@vm1_dup2 = FactoryGirl.create(
:vm_cloud,
vm_data(1).merge(
:flavor => @flavor_1,
:genealogy_parent => @image1,
:key_pairs => [@key_pair1],
:location => 'host_10_10_10_1_dup_2.com',
:ext_management_system => @ems,
)
)

# Assert that the mocked data in the DB have the duplicates
assert_all_records_match_hashes(
[Vm.all, @ems.vms],
{
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1_dup_1.com",
}, {
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1",
:location => "host_10_10_10_1_dup_2.com",
}, {
:ems_ref => "vm_ems_ref_12",
:name => "vm_name_12",
:location => "host_10_10_10_1.com",
}, {
:ems_ref => "vm_ems_ref_2",
:name => "vm_name_2",
:location => "host_10_10_10_2.com",
}, {
:ems_ref => "vm_ems_ref_4",
:name => "vm_name_4",
:location => "default_value_unknown",
}
)

# Now save the records using InventoryCollections
# Fill the InventoryCollections with data, that have a modified name
initialize_data_and_inventory_collections
add_data_to_inventory_collection(@data[:vms],
@vm_data_1.merge(:name => "vm_name_1_changed"),
@vm_data_12.merge(:name => "vm_name_12_changed"),
@vm_data_2.merge(:name => "vm_name_2_changed"),
@vm_data_4.merge(:name => "vm_name_4_changed"),
vm_data(5))
add_data_to_inventory_collection(@data[:miq_templates], @image_data_1, @image_data_2, @image_data_3)
add_data_to_inventory_collection(@data[:key_pairs], @key_pair_data_1, @key_pair_data_12, @key_pair_data_2,
@key_pair_data_3)
add_data_to_inventory_collection(@data[:hardwares], @hardware_data_1, @hardware_data_2, @hardware_data_12)
add_data_to_inventory_collection(@data[:disks],
@disk_data_1.merge(:device_type => "nvme_ssd_1"),
@disk_data_12.merge(:device_type => "nvme_ssd_12"),
@disk_data_13.merge(:device_type => "nvme_ssd_13"),
@disk_data_2.merge(:device_type => "nvme_ssd_2"))
add_data_to_inventory_collection(@data[:networks], @public_network_data_1, @public_network_data_12,
@public_network_data_13, @public_network_data_14, @public_network_data_2)
add_data_to_inventory_collection(@data[:flavors], @flavor_data_1, @flavor_data_2, @flavor_data_3)

# Invoke the InventoryCollections saving
ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values)

# Assert that saved data have the updated values and we kept the Vm with service association while deleting
# others
assert_all_records_match_hashes(
[Vm.all, @ems.vms],
{
:id => @vm1.id, # The vm with service relation remains in the DB
:ems_ref => "vm_ems_ref_1",
:name => "vm_name_1_changed",
:location => "host_10_10_10_1.com",
}, {
:id => @vm12.id,
:ems_ref => "vm_ems_ref_12",
:name => "vm_name_12_changed",
:location => "host_10_10_10_1.com",
}, {
:id => @vm2.id,
:ems_ref => "vm_ems_ref_2",
:name => "vm_name_2_changed",
:location => "host_10_10_10_2.com",
}, {
:id => @vm4.id,
:ems_ref => "vm_ems_ref_4",
:name => "vm_name_4_changed",
:location => "default_value_unknown",
}, {
:id => anything,
:ems_ref => "vm_ems_ref_5",
:name => "vm_name_5",
:location => "vm_location_5",
}
)
end
end

context "lazy_find vs find" do
Expand Down

0 comments on commit 81458a9

Please sign in to comment.