Merge pull request #492 from agrare/storage_belongs_to_ems

Update storages to belong to a single EMS

app/models/manageiq/providers/vmware/infra_manager/inventory/parser/datastore.rb

@@ -28,24 +28,13 @@ def parse_datastore_capability(storage_hash, props)
     end
 
     def parse_datastore_host_mount(storage, datastore_ref, props)
-      location = parse_datastore_location(props)
-
-      # Since a targeted refresh of a single datastore can actually impact
-      # multiple Storage records in the database we have to send the
-      # host_storages for every single datastore with the same location
-      # everytime we parse a single datastore
-      cache["Datastore"].each do |mor, ds|
-        next if location != parse_datastore_location(ds)
-
-        ds[:host].to_a.each do |host_mount|
-          persister.host_storages.build(
-            :storage    => storage,
-            :host       => persister.hosts.lazy_find(host_mount.key._ref),
-            :ems_ref    => mor,
-            :read_only  => host_mount.mountInfo.accessMode == "readOnly",
-            :accessible => host_mount.mountInfo.accessible
-          )
-        end
+      props[:host].to_a.each do |host_mount|
+        persister.host_storages.build(
+          :storage    => storage,
+          :host       => persister.hosts.lazy_find(host_mount.key._ref),
+          :read_only  => host_mount.mountInfo.accessMode == "readOnly",
+          :accessible => host_mount.mountInfo.accessible,
+        )
       end
     end
 

app/models/manageiq/providers/vmware/infra_manager/inventory/parser/virtual_machine.rb

@@ -71,8 +71,7 @@ def parse_virtual_machine_storage(vm_hash, props)
         cache.find(ds)&.dig(:summary, :name) == datastore_name
       end
 
-      datastore_props = cache.find(datastore) if datastore
-      vm_hash[:storage] = persister.storages.lazy_find(parse_datastore_location(datastore_props)) if datastore_props
+      vm_hash[:storage] = persister.storages.lazy_find(datastore._ref)
     end
 
     def parse_virtual_machine_summary_runtime(vm_hash, props)
@@ -207,10 +206,7 @@ def parse_virtual_machine_disks(_vm, hardware, props)
           disk_hash[:filename] = backing.fileName
 
           if backing.datastore
-            datastore_props    = cache.find(backing.datastore)
-            datastore_location = parse_datastore_location(datastore_props) if datastore_props
-
-            disk_hash[:storage] = persister.storages.lazy_find(datastore_location) if datastore_location
+            disk_hash[:storage] = persister.storages.lazy_find(backing.datastore._ref)
           end
         when RbVmomi::VIM::VirtualDeviceRemoteDeviceBackingInfo
           disk_hash[:filename] = backing.deviceName

app/models/manageiq/providers/vmware/infra_manager/provision/cloning.rb

@@ -166,16 +166,7 @@ def clone_vm(vim_clone_options)
   end
 
   def datastore_ems_ref(clone_opts)
-    host_ids = if clone_opts[:host]
-                 clone_opts[:host].id
-               else
-                 clone_opts[:cluster].hosts.pluck(:id)
-               end
-
-    # Find a host in the cluster that has this storage mounted to get the right ems_ref for this
-    # datastore in the datacenter
-    datastore = HostStorage.find_by(:storage_id => clone_opts[:datastore].id, :host_id => host_ids)
-
+    datastore = Storage.find_by(:id => clone_opts[:datastore].id)
     datastore.try(:ems_ref)
   end
 

app/models/manageiq/providers/vmware/infra_manager/refresh_parser.rb

@@ -123,7 +123,6 @@ def self.dvpg_inv_to_hashes(inv, dvs_uids)
       def self.storage_inv_to_hashes(inv)
         result = []
         result_uids = {}
-        result_locs = {}
         return result, result_uids if inv.nil?
 
         inv.each do |mor, storage_inv|
@@ -135,10 +134,6 @@ def self.storage_inv_to_hashes(inv)
           capability = storage_inv["capability"]
 
           loc = uid = normalize_storage_uid(storage_inv)
-          unless result_locs[loc].nil?
-            result_uids[mor] = result_locs[loc]
-            next
-          end
 
           new_result = {
             :ems_ref            => mor,
@@ -162,7 +157,6 @@ def self.storage_inv_to_hashes(inv)
 
           result << new_result
           result_uids[mor] = new_result
-          result_locs[loc] = new_result
         end
         return result, result_uids
       end
@@ -820,8 +814,7 @@ def self.host_inv_to_host_storages_hashes(inv, storage_inv, storage_uids)
           result << {
             :storage    => storage_uids[s_mor],
             :read_only  => read_only,
-            :accessible => accessible,
-            :ems_ref    => s_mor
+            :accessible => accessible
           }
         end
 

app/models/manageiq/providers/vmware/infra_manager/vm/reconfigure.rb

@@ -398,7 +398,7 @@ def connect_cdrom_config_spec(vim_obj, vmcs, hardware, cdrom)
     device = vim_obj.getDeviceByLabel(cdrom[:device_name], hardware)
     raise "connect_cdrom_config_spec: no virtual device associated with: #{cdrom[:device_name]}" unless device
 
-    datastore_ref = HostStorage.find_by(:storage_id => cdrom[:storage_id], :host_id => host.id).try(:ems_ref)
+    datastore_ref = Storage.find_by(:id => cdrom[:storage_id]).try(:ems_ref)
     raise "connect_cdrom_config_spec: could not find datastore reference for storage ID [#{cdrom[:storage_id]}] and host ID [#{host.id}]" unless datastore_ref
 
     add_device_config_spec(vmcs, VirtualDeviceConfigSpecOperation::Edit) do |vdcs|

spec/models/manageiq/providers/vmware/infra_manager/inventory/collector_spec.rb

@@ -596,7 +596,7 @@ def assert_ems
       expect(ems.host_operating_systems.count).to eq(16)
       expect(ems.operating_systems.count).to eq(64)
       expect(ems.resource_pools.count).to eq(12)
-      expect(ems.storages.count).to eq(1)
+      expect(ems.storages.count).to eq(2)
       expect(ems.vms_and_templates.count).to eq(64)
       expect(ems.switches.count).to eq(18)
       expect(ems.lans.count).to eq(36)
@@ -636,9 +636,9 @@ def assert_specific_datastore
         :raw_disk_mappings_supported   => true,
       )
 
-      expect(storage.hosts.count).to eq(16)
-      expect(storage.disks.count).to eq(64)
-      expect(storage.vms.count).to   eq(64)
+      expect(storage.hosts.count).to eq(8)
+      expect(storage.disks.count).to eq(32)
+      expect(storage.vms.count).to   eq(32)
     end
 
     def assert_specific_folder

spec/models/manageiq/providers/vmware/infra_manager/provision_spec.rb

@@ -292,10 +292,10 @@
 
       context "#start_clone" do
         before(:each) do
-          ds_mor = "datastore-0"
-          storage = FactoryBot.create(:storage_nfs, :ems_ref => ds_mor, :ems_ref_type => "Datastore")
-
           Array.new(2) do |i|
+            ds_mor = "datastore-#{i}"
+            storage = FactoryBot.create(:storage_nfs, :ems_ref => ds_mor, :ems_ref_type => "Datastore")
+
             cluster_mor = "cluster-#{i}"
             cluster     = FactoryBot.create(:ems_cluster, :ems_ref => cluster_mor)
 
@@ -310,7 +310,6 @@
             FactoryBot.create(:host_vmware, host_props).tap do |host|
               host.storages = [storage]
               hs = host.host_storages.first
-              hs.ems_ref = "datastore-#{i}"
               hs.save
             end
           end
@@ -321,10 +320,11 @@
           dest_datastore_mor = "datastore-1"
           task_mor           = "task-1"
 
+          host = Host.find_by(:ems_ref => dest_host_mor)
           clone_opts = {
             :name      => @target_vm_name,
-            :host      => Host.find_by(:ems_ref => dest_host_mor),
-            :datastore => Storage.first
+            :host      => host,
+            :datastore => host.storages.first
           }
 
           expected_vim_clone_opts = {
@@ -351,10 +351,11 @@
           dest_datastore_mor = "datastore-1"
           task_mor           = "task-1"
 
+          cluster = EmsCluster.find_by(:ems_ref => dest_cluster_mor)
           clone_opts = {
             :name      => @target_vm_name,
-            :cluster   => EmsCluster.find_by(:ems_ref => dest_cluster_mor),
-            :datastore => Storage.first
+            :cluster   => cluster,
+            :datastore => cluster.storages.first
           }
 
           expected_vim_clone_opts = {

spec/models/manageiq/providers/vmware/infra_manager/refresher_spec.rb

@@ -254,7 +254,7 @@ def assert_table_counts
     expect(VmOrTemplate.count).to eq(101)
     expect(Vm.count).to eq(92)
     expect(MiqTemplate.count).to eq(9)
-    expect(Storage.count).to eq(33)
+    expect(Storage.count).to eq(50)
     expect(StorageProfile.count).to eq(6)
 
     expect(CustomAttribute.count).to eq(3)
@@ -285,7 +285,7 @@ def assert_ems
     expect(@ems.ems_folders.size).to eq(32)
     expect(@ems.ems_clusters.size).to eq(1)
     expect(@ems.resource_pools.size).to eq(17)
-    expect(@ems.storages.size).to eq(30)
+    expect(@ems.storages.size).to eq(50)
     expect(@ems.hosts.size).to eq(4)
     expect(@ems.vms_and_templates.size).to eq(101)
     expect(@ems.vms.size).to eq(92)

spec/models/manageiq/providers/vmware/infra_manager/vm/reconfigure_spec.rb

@@ -2,7 +2,7 @@
   let(:storage) { FactoryBot.create(:storage_vmware) }
   let(:host) do
     FactoryBot.create(:host_vmware_esx).tap do |host|
-      host.host_storages.create(:storage_id => storage.id, :host_id => host.id, :ems_ref => "datastore-1")
+      host.host_storages.create(:storage_id => storage.id, :host_id => host.id)
     end
   end
   let(:vm) do