From 9b04398168e647999ae1143bdd977c57c66d7a00 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 19 Jul 2017 15:43:49 +0200 Subject: [PATCH 1/4] Add custom_reconnect_block to handle complex reconnect logic Add custom_reconnect_block to handle complex reconnect logic --- .../manager_refresh/inventory_collection.rb | 85 +++++++++++++------ 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 561c9644971..d277d13f452 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -9,7 +9,8 @@ class InventoryCollection :custom_db_finder, :check_changed, :arel, :builder_params, :loaded_references, :db_data_index, :inventory_object_attributes, :name, :saver_strategy, :parent_inventory_collections, :manager_uuids, :skeletal_manager_uuids, :targeted_arel, :targeted, :manager_ref_allowed_nil, :use_ar_object, - :secondary_refs, :secondary_indexes, :created_records, :updated_records, :deleted_records + :secondary_refs, :secondary_indexes, :created_records, :updated_records, :deleted_records, + :custom_reconnect_block delegate :each, :size, :to => :to_a @@ -142,6 +143,34 @@ class InventoryCollection # stack.update_attribute(:parent, parent) # end # end + # @param custom_reconnect_block [Proc] A custom lambda for reconnect logic of previously disconnected records + # + # Example - Reconnect disconnected Vms + # ManagerRefresh::InventoryCollection.new({ + # :association => :orchestration_stack_ancestry, + # :custom_reconnect_block => vms_custom_reconnect_block, + # }) + # + # And the labmda is defined as: + # vms_custom_reconnect_block = lambda do |inventory_collection, inventory_objects_index, attributes_index| + # inventory_objects_index.each_slice(1000) do |batch| + # Vm.where(:ems_ref => batch.map(&:second).map(&:manager_uuid)).each do |record| + # index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record) + # + # # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it + # # would be sent for create. + # inventory_object = inventory_objects_index.delete(index) + # hash = attributes_index.delete(index) + # + # record.assign_attributes(hash.except(:id, :type)) + # if !inventory_collection.check_changed? || record.changed? + # record.save! + # inventory_collection.store_updated_records(record) + # end + # + # inventory_object.id = record.id + # end + # end # @param delete_method [Symbol] A delete method that will be used for deleting of the InventoryObject, if the # object is marked for deletion. A default is :destroy, the instance method must be defined on the # :model_class. @@ -345,32 +374,34 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil check_changed: nil, custom_manager_uuid: nil, custom_db_finder: nil, arel: nil, builder_params: {}, inventory_object_attributes: nil, unique_index_columns: nil, name: nil, saver_strategy: nil, parent_inventory_collections: nil, manager_uuids: [], all_manager_uuids: nil, targeted_arel: nil, - targeted: nil, manager_ref_allowed_nil: nil, secondary_refs: {}, use_ar_object: nil) - @model_class = model_class - @manager_ref = manager_ref || [:ems_ref] - @secondary_refs = secondary_refs - @custom_manager_uuid = custom_manager_uuid - @custom_db_finder = custom_db_finder - @association = association || [] - @parent = parent || nil - @arel = arel - @dependency_attributes = dependency_attributes || {} - @data = data || [] - @data_index = data_index || {} - @secondary_indexes = secondary_refs.map {|name, keys| [name, {}] }.to_h - @saved = saved || false - @strategy = process_strategy(strategy) - @delete_method = delete_method || :destroy - @custom_save_block = custom_save_block - @check_changed = check_changed.nil? ? true : check_changed - @internal_attributes = [:__feedback_edge_set_parent] - @complete = complete.nil? ? true : complete - @update_only = update_only.nil? ? false : update_only - @builder_params = builder_params - @unique_index_columns = unique_index_columns - @name = name || association || model_class.to_s.demodulize.tableize - @saver_strategy = process_saver_strategy(saver_strategy) - @use_ar_object = use_ar_object || false + targeted: nil, manager_ref_allowed_nil: nil, secondary_refs: {}, use_ar_object: nil, + custom_reconnect_block: nil) + @model_class = model_class + @manager_ref = manager_ref || [:ems_ref] + @secondary_refs = secondary_refs + @custom_manager_uuid = custom_manager_uuid + @custom_db_finder = custom_db_finder + @association = association || [] + @parent = parent || nil + @arel = arel + @dependency_attributes = dependency_attributes || {} + @data = data || [] + @data_index = data_index || {} + @secondary_indexes = secondary_refs.map {|name, keys| [name, {}]}.to_h + @saved = saved || false + @strategy = process_strategy(strategy) + @delete_method = delete_method || :destroy + @custom_save_block = custom_save_block + @custom_reconnect_block = custom_reconnect_block + @check_changed = check_changed.nil? ? true : check_changed + @internal_attributes = [:__feedback_edge_set_parent] + @complete = complete.nil? ? true : complete + @update_only = update_only.nil? ? false : update_only + @builder_params = builder_params + @unique_index_columns = unique_index_columns + @name = name || association || model_class.to_s.demodulize.tableize + @saver_strategy = process_saver_strategy(saver_strategy) + @use_ar_object = use_ar_object || false @manager_ref_allowed_nil = manager_ref_allowed_nil || [] From ce7072cf750a2123d2c148a63621f87842153ad0 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 19 Jul 2017 15:44:49 +0200 Subject: [PATCH 2/4] Invoke custom_reconnect_block if defined as part of saving Invoke custom_reconnect_block if defined as part of saving, allowing us to do reconnec instead of saving. --- app/models/manager_refresh/save_collection/saver/base.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/manager_refresh/save_collection/saver/base.rb b/app/models/manager_refresh/save_collection/saver/base.rb index c890538e399..11e23db451c 100644 --- a/app/models/manager_refresh/save_collection/saver/base.rb +++ b/app/models/manager_refresh/save_collection/saver/base.rb @@ -67,6 +67,10 @@ def save!(inventory_collection, association) end end + unless inventory_collection.custom_reconnect_block.nil? + inventory_collection.custom_reconnect_block.call(inventory_collection, inventory_objects_index, attributes_index) + end + # Records that were not found in the DB but sent for saving, we will be creating these in the DB. if inventory_collection.create_allowed? ActiveRecord::Base.transaction do From 6f261c445c34b7ca0898c88a5ea47a47903de9bf Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 19 Jul 2017 15:46:19 +0200 Subject: [PATCH 3/4] Test that reconnect logic works Test that reconnect logic works --- .../single_inventory_collection_spec.rb | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb b/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb index 632e7bb0d66..a12d4053859 100644 --- a/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb +++ b/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb @@ -698,6 +698,63 @@ end end end + + context 'testing reconnect logic' do + it 'reconnects existing VM' do + # Fill DB with test Vms + @vm1 = FactoryGirl.create(:vm_cloud, vm_data(1).merge(:ext_management_system => nil)) + @vm2 = FactoryGirl.create(:vm_cloud, vm_data(2).merge(:ext_management_system => @ems)) + + vms_custom_reconnect_block = lambda do |inventory_collection, inventory_objects_index, attributes_index| + inventory_objects_index.each_slice(1000) do |batch| + Vm.where(:ems_ref => batch.map(&:second).map(&:manager_uuid)).each do |record| + index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record) + + # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it + # would be sent for create. + inventory_object = inventory_objects_index.delete(index) + hash = attributes_index.delete(index) + + record.assign_attributes(hash.except(:id, :type)) + if !inventory_collection.check_changed? || record.changed? + record.save! + inventory_collection.store_updated_records(record) + end + + inventory_object.id = record.id + end + end + end + + @data = {} + @data[:vms] = ::ManagerRefresh::InventoryCollection.new( + :model_class => ManageIQ::Providers::CloudManager::Vm, + :parent => @ems, + :association => :vms, + :custom_reconnect_block => vms_custom_reconnect_block, + ) + + # Fill the InventoryCollections with data, that have a modified name + add_data_to_inventory_collection(@data[:vms], + vm_data(1).merge(:name => "vm_changed_name_1"), + vm_data(2).merge(:name => "vm_changed_name_2")) + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Check the InventoryCollection result matches what was created/deleted/updated + expect(@data[:vms].created_records).to match_array([]) + expect(@data[:vms].updated_records).to match_array([{:id => @vm1.id}, {:id => @vm2.id}]) + expect(@data[:vms].deleted_records).to match_array([]) + + # Assert that saved data have the updated values, checking id to make sure the original records are updated + assert_all_records_match_hashes( + [Vm.all, @ems.vms], + {:id => @vm1.id, :ems_ref => "vm_ems_ref_1", :name => "vm_changed_name_1", :location => "vm_location_1"}, + {:id => @vm2.id, :ems_ref => "vm_ems_ref_2", :name => "vm_changed_name_2", :location => "vm_location_2"} + ) + end + end end end end From f805115a2318971552b0083bd0332721244bc131 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 20 Jul 2017 10:55:05 +0200 Subject: [PATCH 4/4] Fix rubocop issues Fix rubocop issues --- app/models/manager_refresh/inventory_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index d277d13f452..e386183ab23 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -387,7 +387,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil @dependency_attributes = dependency_attributes || {} @data = data || [] @data_index = data_index || {} - @secondary_indexes = secondary_refs.map {|name, keys| [name, {}]}.to_h + @secondary_indexes = secondary_refs.map { |n, _k| [n, {}] }.to_h @saved = saved || false @strategy = process_strategy(strategy) @delete_method = delete_method || :destroy