Parse PhysicalDisks into Canisters

app/models/manageiq/providers/lenovo/inventory/parser/component_parser/physical_disk.rb

@@ -11,28 +11,32 @@ class Inventory::Parser::ComponentParser::PhysicalDisk < Inventory::Parser::Comp
       :disk_size       => 'size'
     }.freeze
 
-    def lazy_build(components, storage_uuid)
-      total_space = 0
+    def build(driver, index, storage = nil, canister = nil)
+      parsed_disk = parse_disk(driver, storage, index)
 
-      components.each do |component|
-        component['drives'].each do |drive|
-          properties = parse_drive(drive)
-          properties[:physical_storage] = @persister.physical_storages.lazy_find(storage_uuid)
+      add_parent(parsed_disk, :belongs_to => :physical_storage, :object => storage) if storage
+      add_parent(parsed_disk, :belongs_to => :canister, :object => canister) if canister
+      physical_disk = @persister.physical_disks.build(parsed_disk)
 
-          total_space += properties[:disk_size].to_i
+      physical_disk
+    end
 
-          @persister.physical_disks.build(properties)
+    def total_space(components)
+      total_space = 0
+      components.each do |component|
+        component['drives'].each do |driver|
+          total_space += driver['size'].to_i
         end
       end
-
       total_space
     end
 
-    def parse_drive(drive)
+    def parse_disk(driver, storage, index)
       result = {}
-      PHYSICAL_DISK.each_pair do |key, drive_key|
-        result[key] = drive[drive_key]
+      PHYSICAL_DISK.each_pair do |key, driver_key|
+        result[key] = driver[driver_key]
       end
+      result[:ems_ref] = storage[:ems_ref] + '_' + index
       result
     end
   end

app/models/manageiq/providers/lenovo/inventory/parser/component_parser/physical_storage.rb

@@ -45,7 +45,7 @@ class Inventory::Parser::ComponentParser::PhysicalStorage < Inventory::Parser::C
     def build(storage_xclarity, rack = nil, chassis = nil)
       properties = parse_physical_storage(storage_xclarity)
 
-      total_space = build_physical_disks(storage_xclarity)
+      total_space = get_total_space(storage_xclarity)
       properties[:total_space] = total_space.zero? ? nil : total_space.gigabytes
 
       add_parent(properties, :belongs_to => :physical_rack, :object => rack) if rack
@@ -74,30 +74,49 @@ def parse_physical_storage(storage_xclarity)
     private
 
     def build_associations(storage, storage_xclarity)
+      build_physical_disks(storage, storage_xclarity)
       build_canisters(storage, storage_xclarity)
       build_asset_detail(storage, storage_xclarity)
     end
 
+    def build_physical_disks(storage, storage_xclarity)
+      build_drivers_inside_multi_enclosures(storage, storage_xclarity) if storage_xclarity.enclosures.present?
+    end
+
+    def build_drivers_inside_multi_enclosures(storage, storage_xclarity)
+      enclosures = storage_xclarity.enclosures
+      enclosures.each do |enclosure|
+        build_drivers_inside_single_enclosure(enclosure, storage)
+      end
+    end
+
+    def build_drivers_inside_single_enclosure(enclosure, storage)
+      driver_index = 0
+      enclosure['drives'].each do |driver|
+        components(:physical_disks).build(driver, driver_index.to_s, storage)
+        driver_index += 1
+      end
+    end
+
     def build_canisters(storage, storage_xclarity)
       components(:canisters).build(storage_xclarity,
                                    :belongs_to => :physical_storage,
                                    :object     => storage)
     end
 
-    def build_physical_disks(storage_xclarity)
+    def build_asset_detail(storage, storage_xclarity)
+      super(storage, storage_xclarity, PHYSICAL_STORAGE[:asset_detail])
+    end
+
+    def get_total_space(storage_xclarity)
       total_space = 0
       components = storage_xclarity.canisters.presence || storage_xclarity.enclosures.presence
       if components.present?
-        total_space = components(:physical_disks).lazy_build(components,
-                                                             storage_xclarity.uuid)
+        total_space = components(:physical_disks).total_space(components)
       end
       total_space
     end
 
-    def build_asset_detail(storage, storage_xclarity)
-      super(storage, storage_xclarity, PHYSICAL_STORAGE[:asset_detail])
-    end
-
     def type(_storage)
       'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalStorage'
     end

app/models/manageiq/providers/lenovo/inventory/persister/definitions/physical_infra_collections.rb

@@ -224,8 +224,8 @@ def add_canisters
   def add_physical_disks
     add_collection(physical_infra, :physical_disks) do |builder|
       builder.add_properties(
-        :manager_ref             => %i(physical_storage location),
-        :manager_ref_allowed_nil => %i(location)
+        :manager_ref             => %i(physical_storage ems_ref),
+        :manager_ref_allowed_nil => %i(ems_ref)
       )
     end
   end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_disk_parser.rb

@@ -0,0 +1,36 @@
+module ManageIQ::Providers::Lenovo
+  class PhysicalInfraManager::Parser::PhysicalDiskParser < PhysicalInfraManager::Parser::ComponentParser
+    class << self
+      # Mapping between fields inside a [XClarityClient::Storage] to a [Hash] with symbols of PhysicalDisk fields
+      PHYSICAL_DISK = {
+        :model           => 'model',
+        :vendor          => 'vendorName',
+        :status          => 'status',
+        :location        => 'location',
+        :serial_number   => 'serialNumber',
+        :health_state    => 'health',
+        :controller_type => 'type',
+        :disk_size       => :disk_size
+      }.freeze
+
+      #
+      # Parse disk into a hash
+      #
+      # @param [Hash] disk_hash - hash containing physical disk raw data
+      # @param [Hash] canister - parsed canister data
+      #
+      # @return [Hash] containing the physical disk information
+      #
+      def parse_physical_disk(disk, canister: nil)
+        result = parse(disk, PHYSICAL_DISK)
+        result[:canister] = canister if canister
+
+        result
+      end
+
+      def disk_size(disk)
+        disk['size']
+      end
+    end
+  end
+end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_storage_parser.rb

@@ -59,33 +59,57 @@ def health_state(storage)
       end
 
       def physical_disks(storage)
-        return parse_drivers_inside_components(storage.canisters) if storage.canisters.present?
-        parse_drivers_inside_components(storage.enclosures) if storage.enclosures.present?
+        parse_drivers_inside_multi_enclosures(storage) if storage.enclosures.present?
       end
 
-      def parse_drivers_inside_components(components)
+      def parse_drivers_inside_multi_enclosures(storage)
         drivers = []
+        storage.enclosures.each do |enclosure|
+          drivers << parse_drivers_inside_single_component(enclosure, :storage => storage)
+        end
 
-        components.each do |component|
-          component['drives'].each do |driver|
-            drivers << parse_driver(driver)
-          end
+        drivers.flatten
+      end
+
+      def parse_drivers_inside_single_component(parent, storage: nil)
+        drivers = []
+        driver_index = 0
+
+        parent['drives']&.each do |driver|
+          drivers << if storage.present?
+                       parse_storage_driver(storage, driver, driver_index.to_s)
+                     else
+                       parse_canister_driver(parent, driver, driver_index.to_s)
+                     end
+          driver_index += 1
         end
 
         drivers
       end
 
-      def parse_driver(driver)
-        {
-          :model           => driver['model'],
-          :vendor          => driver['vendorName'],
-          :status          => driver['status'],
-          :location        => driver['location'],
-          :serial_number   => driver['serialNumber'],
-          :health_state    => driver['health'],
-          :controller_type => driver['type'],
-          :disk_size       => driver['size']
-        }
+      def parse_storage_driver(storage, driver, driver_index)
+        result = parent::PhysicalDiskParser.parse_physical_disk(driver)
+        result[:ems_ref] = storage.uuid + '_' + driver_index
+
+        result
+      end
+
+      def parse_canister_driver(canister, driver, driver_index)
+        result = parent::PhysicalDiskParser.parse_physical_disk(driver, :canister => canister)
+        result[:ems_ref] = canister['uuid'] + '_' + driver_index
+
+        result
+      end
+
+      #
+      # @param storage [Hash] Hash with parsed storage data (including physical_disks in it)
+      #
+      def get_total_space(storage)
+        total_space = 0
+        disks = storage[:physical_disks]
+        disks&.each { |disk| total_space += disk[:disk_size].to_i }
+
+        total_space.zero? ? nil : total_space.gigabytes # returns the size in bytes
       end
 
       #
@@ -127,6 +151,7 @@ def parse_canisters_inside_components(components)
 
       def parse_canister(canister)
         {
+          :ems_ref                      => canister['uuid'],
           :serial_number                => canister['serialNumber'],
           :name                         => canister['cmmDisplayName'],
           :position                     => canister['position'],
@@ -141,6 +166,7 @@ def parse_canister(canister)
           :power_state                  => canister['powerState'],
           :host_ports                   => canister['hostPorts'],
           :hardware_version             => canister['hardwareVersion'],
+          :physical_disks               => parse_drivers_inside_single_component(canister),
           :computer_system              => {
             :hardware => {
               :guest_devices => canister_guest_devices(canister)

spec/models/manageiq/providers/lenovo/physical_infra_manager/physical_infra_spec_common.rb

@@ -0,0 +1,35 @@
+module PhysicalInfraSpecCommon
+  extend ActiveSupport::Concern
+
+  MODELS = %i(
+    ext_management_system physical_rack physical_chassis physical_server
+    physical_switch physical_storage physical_disk customization_script
+  ).freeze
+
+  def serialize_inventory
+    skip_atributes = %w(created_at updated_at last_refresh_date updated_on)
+    inventory = {}
+    PhysicalInfraSpecCommon::MODELS.each do |rel|
+      inventory[rel] = rel.to_s.classify.constantize.all.collect do |e|
+        e.attributes.except(*skip_atributes)
+      end
+    end
+
+    inventory
+  end
+
+  def assert_models_not_changed(inventory_before, inventory_after)
+    aggregate_failures do
+      PhysicalInfraSpecCommon::MODELS.each do |model|
+        expect(inventory_after[model].count).to eq(inventory_before[model].count), "#{model} count"\
+               " doesn't fit \nexpected: #{inventory_before[model].count}\ngot#{inventory_after[model].count}"
+
+        inventory_after[model].each do |item_after|
+          item_before = inventory_before[model].detect { |i| i["id"] == item_after["id"] }
+          expect(item_after).to eq(item_before), \
+                                "class: #{model.to_s.classify}\nexpected: #{item_before}\ngot: #{item_after}"
+        end
+      end
+    end
+  end
+end

spec/models/manageiq/providers/lenovo/physical_infra_manager/refresh_parser_spec.rb

@@ -169,6 +169,7 @@
       expect(physical_disk_three).to_not be_nil
       expect(physical_disk_four).to_not be_nil
 
+      expect(physical_disk_one[:ems_ref]).to eq("208000C0FF2683AF_0")
       expect(physical_disk_one[:model]).to eq("ST9300653SS")
       expect(physical_disk_one[:vendor]).to eq("IBM-ESXS")
       expect(physical_disk_one[:status]).to eq("Up")
@@ -179,6 +180,22 @@
       expect(physical_disk_one[:disk_size]).to eq("300.0GB")
     end
 
+    it 'will parse physical disks inside a canister' do
+      physical_storage = @result[:physical_storages].first
+      canister_one = physical_storage[:canisters].first
+      canister_disk_one = canister_one[:physical_disks].first
+
+      expect(canister_disk_one[:ems_ref]).to eq("457784225BA511E18290B4F27BD253A5_0")
+      expect(canister_disk_one[:model]).to eq("Canister_Driver_Model")
+      expect(canister_disk_one[:vendor]).to eq("Canister_Driver_Vendor")
+      expect(canister_disk_one[:status]).to eq("Up")
+      expect(canister_disk_one[:location]).to eq("0.50")
+      expect(canister_disk_one[:serial_number]).to eq("20183QX50000B3492018")
+      expect(canister_disk_one[:health_state]).to eq("OK")
+      expect(canister_disk_one[:controller_type]).to eq("SAS")
+      expect(canister_disk_one[:disk_size]).to eq("300.0GB")
+    end
+
     it 'will have a reference to the physical rack where the storage is inside' do
       physical_rack = @result[:physical_racks].first
       physical_storage = @result[:physical_storages].second

spec/models/manageiq/providers/lenovo/physical_infra_manager/refresher_spec.rb

@@ -1,4 +1,7 @@
+require_relative 'physical_infra_spec_common'
 describe ManageIQ::Providers::Lenovo::PhysicalInfraManager::Refresher do
+  include PhysicalInfraSpecCommon
+
   before(:all) do
     vcr_path = File.dirname(described_class.name.underscore)
     options = {:allow_playback_repeats => true}
@@ -87,6 +90,18 @@
     end
   end
 
+  it 'will check inventory consistency' do
+    EmsRefresh.refresh(ems2)
+    ems2.reload
+    inventory_before = serialize_inventory
+
+    EmsRefresh.refresh(ems2)
+    ems2.reload
+    inventory_after = serialize_inventory
+
+    assert_models_not_changed(inventory_before, inventory_after)
+  end
+
   def assert_specific_rack
     rack = PhysicalRack.find_by(:ems_ref => "096F8C92-08D4-4A24-ABD8-FE56D482F8C4")
 
@@ -107,6 +122,7 @@ def assert_specific_server
 
   def assert_specific_storage
     storage = PhysicalStorage.find_by(:ems_ref => "208000C0FF2683AF")
+    storage_physical_disk_one = PhysicalDisk.find_by(:ems_ref => "208000C0FF2683AF_0")
 
     expect(storage.name).to eq("S3200-1")
     expect(storage.uid_ems).to eq("208000C0FF2683AF")
@@ -119,12 +135,25 @@ def assert_specific_storage
     expect(storage.drive_bays).to eq(12)
     expect(storage.enclosures).to eq(1)
     expect(storage.canister_slots).to eq(2)
+    expect(storage.physical_disks.count).to eq(4)
+    expect(storage.canisters.count).to eq(2)
+
+    expect(storage_physical_disk_one.model).to eq("ST9300653SS")
+    expect(storage_physical_disk_one.vendor).to eq("IBM-ESXS")
+    expect(storage_physical_disk_one.status).to eq("Up")
+    expect(storage_physical_disk_one.location).to eq("0.22")
+    expect(storage_physical_disk_one.serial_number).to eq("6XN43QX50000B349D4LY")
+    expect(storage_physical_disk_one.health_state).to eq("OK")
+    expect(storage_physical_disk_one.controller_type).to eq("SAS")
+    expect(storage_physical_disk_one.disk_size).to eq("300.0GB")
+    expect(storage_physical_disk_one.physical_storage_id).to eq(storage.id)
   end
 
   def assert_table_counts
     expect(PhysicalRack.count).to eq(3)
     expect(PhysicalServer.count).to eq(2)
     expect(PhysicalStorage.count).to eq(2)
+    expect(PhysicalDisk.count).to eq(4)
     expect(GuestDevice.count).to eq(8)
     expect(PhysicalNetworkPort.count).to eq(50)
   end