Refresh operating system and fix normalized guest os

app/models/manageiq/providers/amazon/cloud_manager/refresh_parser.rb

@@ -171,10 +171,10 @@ def self.key_pair_type
   def parse_image(image, is_public)
     uid      = image.image_id
     location = image.image_location
-    guest_os = (image.platform == "windows") ? "windows" : "linux"
-    if guest_os == "linux"
+    guest_os = (image.platform == "windows") ? "windows_generic" : "linux_generic"
+    if guest_os == "linux_generic"
       guest_os = OperatingSystem.normalize_os_name(location)
-      guest_os = "linux" if guest_os == "unknown"
+      guest_os = "linux_generic" if guest_os == "unknown"
     end
 
     name = get_from_tags(image, :name)
@@ -198,7 +198,9 @@ def parse_image(image, is_public)
       # the is_public flag here avoids having to make an additional API call
       # per image, since we already know whether it's a public image
       :publicly_available => is_public,
-
+      :operating_system   => {
+        :product_name => guest_os # FIXME: duplicated information used by some default reports
+      },
       :hardware           => {
         :guest_os            => guest_os,
         :bitness             => architecture_to_bitness(image.architecture),
@@ -273,7 +275,9 @@ def parse_instance(instance)
     parent_image = @data_index.fetch_path(:vms, instance.image_id)
     if parent_image
       new_result[:parent_vm] = parent_image
-      new_result.store_path(:hardware, :guest_os, parent_image.fetch_path(:hardware, :guest_os))
+      guest_os = parent_image.fetch_path(:hardware, :guest_os)
+      new_result.store_path(:hardware, :guest_os, guest_os)
+      new_result.store_path(:operating_system, :product_name, guest_os) # FIXME: duplicated information used by some default reports
     end
 
     if flavor[:ephemeral_disk_count] > 0

app/models/manageiq/providers/amazon/inventory/parser/cloud_manager.rb

@@ -58,15 +58,16 @@ def images(images)
       )
 
       image_hardware(persister_image, image)
+      image_operating_system(persister_image, image)
       vm_and_template_labels(persister_image, image["tags"] || [])
     end
   end
 
   def image_hardware(persister_image, image)
-    guest_os = image['platform'] == "windows" ? "windows" : "linux"
-    if guest_os == "linux"
+    guest_os = image['platform'] == "windows" ? "windows_generic" : "linux_generic"
+    if guest_os == "linux_generic"
       guest_os = OperatingSystem.normalize_os_name(image['image_location'])
-      guest_os = "linux" if guest_os == "unknown"
+      guest_os = "linux_generic" if guest_os == "unknown"
     end
 
     persister.hardwares.find_or_build(persister_image).assign_attributes(
@@ -77,6 +78,13 @@ def image_hardware(persister_image, image)
     )
   end
 
+  def image_operating_system(persister_image, image)
+    persister.operating_systems.find_or_build(persister_image).assign_attributes(
+      # FIXME: duplicated information used by some default reports
+      :product_name => persister.hardwares.lazy_find(image['image_id'], :key => :guest_os)
+    )
+  end
+
   def vm_and_template_labels(resource, tags)
     tags.each do |tag|
       persister.vm_and_template_labels.find_or_build_by(:resource => resource, :name => tag["key"]).assign_attributes(
@@ -192,6 +200,7 @@ def instances
       )
 
       instance_hardware(persister_instance, instance, flavor)
+      instance_operating_system(persister_instance, instance)
       vm_and_template_labels(persister_instance, instance["tags"] || [])
     end
   end
@@ -213,6 +222,13 @@ def instance_hardware(persister_instance, instance, flavor)
     hardware_disks(persister_hardware, instance, flavor)
   end
 
+  def instance_operating_system(persister_instance, instance)
+    persister.operating_systems.find_or_build(persister_instance).assign_attributes(
+      # FIXME: duplicated information used by some default reports
+      :product_name => persister.hardwares.lazy_find(instance['image_id'], :key => :guest_os)
+    )
+  end
+
   def hardware_networks(persister_hardware, instance)
     hardware_network(persister_hardware,
                      instance['private_ip_address'].presence,

app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb

@@ -2,7 +2,7 @@ class ManageIQ::Providers::Amazon::Inventory::Persister::CloudManager < ManageIQ
   def initialize_inventory_collections
     add_inventory_collections(
       cloud,
-      %i(vms miq_templates hardwares networks disks availability_zones vm_and_template_labels
+      %i(vms miq_templates hardwares operating_systems networks disks availability_zones vm_and_template_labels
          flavors key_pairs orchestration_stacks orchestration_stacks_resources
          orchestration_stacks_outputs orchestration_stacks_parameters orchestration_templates)
     )

app/models/manageiq/providers/amazon/inventory/persister/target_collection.rb

@@ -16,8 +16,8 @@ def initialize_inventory_collections
     # Child models with references in the Parent InventoryCollections for Cloud
     add_inventory_collections(
       cloud,
-      %i(hardwares networks disks vm_and_template_labels orchestration_stacks_resources orchestration_stacks_outputs
-         orchestration_stacks_parameters)
+      %i(hardwares operating_systems networks disks vm_and_template_labels orchestration_stacks_resources
+         orchestration_stacks_outputs orchestration_stacks_parameters)
     )
 
     add_inventory_collection(cloud.orchestration_templates)

app/models/manageiq/providers/amazon/inventory_collection_default/cloud_manager.rb

@@ -72,6 +72,16 @@ def hardwares(extra_attributes = {})
       super(attributes.merge!(extra_attributes))
     end
 
+    def operating_systems(extra_attributes = {})
+      attributes = {
+        :inventory_object_attributes => [
+          :product_name,
+        ]
+      }
+
+      super(attributes.merge!(extra_attributes))
+    end
+
     def networks(extra_attributes = {})
       attributes = {
         :inventory_object_attributes => [

spec/models/manageiq/providers/amazon/aws_refresher_spec_common.rb

@@ -273,12 +273,16 @@ def assert_specific_template
     )
 
     expect(@template.ext_management_system).to eq(@ems)
-    expect(@template.operating_system).to       be_nil # TODO: This should probably not be nil
+    expect(@template.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(@template.custom_attributes.size).to eq(0)
     expect(@template.snapshots.size).to eq(0)
 
     expect(@template.hardware).to have_attributes(
-      :guest_os            => "linux",
+      :guest_os            => "linux_generic",
       :guest_os_full_name  => nil,
       :bios                => nil,
       :annotation          => nil,
@@ -336,7 +340,11 @@ def assert_specific_template_2
     )
 
     expect(@template2.ext_management_system).to eq(@ems)
-    expect(@template2.operating_system).to       be_nil # TODO: This should probably not be nil
+    expect(@template2.operating_system).to(
+      have_attributes(
+        :product_name => "linux_redhat",
+      )
+    )
     expect(@template2.custom_attributes.size).to eq(0)
     expect(@template2.snapshots.size).to eq(0)
 
@@ -520,13 +528,17 @@ def assert_specific_vm_powered_on
     expect(v.security_groups)
       .to match_array [sg_2, @sg]
 
-    expect(v.operating_system).to       be_nil # TODO: This should probably not be nil
+    expect(v.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(v.custom_attributes.size).to eq(2)
     expect(v.snapshots.size).to eq(0)
     # expect(v.tags.size).to eq(0)
 
     expect(v.hardware).to have_attributes(
-      :guest_os            => "linux",
+      :guest_os            => "linux_generic",
       :guest_os_full_name  => nil,
       :bios                => nil,
       :annotation          => nil,
@@ -613,7 +625,11 @@ def assert_specific_vm_powered_off
            .where(:name => "EmsRefreshSpec-SecurityGroup2").first
     expect(v.security_groups)
       .to match_array [sg_2, @sg]
-    expect(v.operating_system).to be_nil # TODO: This should probably not be nil
+    expect(v.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(v.custom_attributes.size).to eq(2)
     expect(v.custom_attributes.find_by(:name => "Name").value).to eq("EmsRefreshSpec-PoweredOff")
     expect(v.custom_attributes.find_by(:name => "owner").value).to eq("UNKNOWN")
@@ -622,7 +638,7 @@ def assert_specific_vm_powered_off
     expect(v.hardware).to have_attributes(
       :config_version       => nil,
       :virtual_hw_version   => nil,
-      :guest_os             => "linux",
+      :guest_os             => "linux_generic",
       :cpu_sockets          => 1,
       :bios                 => nil,
       :bios_location        => nil,
@@ -720,7 +736,11 @@ def assert_specific_vm_on_cloud_network
     ]
     expect(v.load_balancer_health_check_states_with_reason).to match_array healt_check_states_with_reason
 
-    expect(v.operating_system).to be_nil # TODO: This should probably not be nil
+    expect(v.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(v.custom_attributes.size).to eq(2)
     expect(v.custom_attributes.find_by(:name => "Name").value).to eq("EmsRefreshSpec-PoweredOn-VPC")
     expect(v.custom_attributes.find_by(:name => "owner").value).to eq("UNKNOWN")
@@ -730,7 +750,7 @@ def assert_specific_vm_on_cloud_network
       have_attributes(
         :config_version       => nil,
         :virtual_hw_version   => nil,
-        :guest_os             => "linux",
+        :guest_os             => "linux_generic",
         :cpu_sockets          => 1,
         :bios                 => nil,
         :bios_location        => nil,
@@ -864,7 +884,17 @@ def assert_specific_vm_on_cloud_network_public_ip
     expect(v.network_ports.first.ipaddresses).to match_array([@ip2.fixed_ip_address, @ip2.address])
     expect(v.ipaddresses).to match_array([@ip2.fixed_ip_address, @ip2.address])
 
-    expect(v.operating_system).to be_nil # TODO: This should probably not be nil
+    if options.inventory_object_refresh
+    expect(v.operating_system).to(
+      have_attributes(
+        :product_name => "linux_redhat",
+      )
+    )
+    else
+      # Old refresh can't fetch the public image and there fore also operating_system
+      expect(v.operating_system).to be_nil
+    end
+
     expect(v.custom_attributes.size).to eq(2)
     expect(v.custom_attributes.find_by(:name => "Name").value).to eq("EmsRefreshSpec-PoweredOn-VPC1")
     expect(v.custom_attributes.find_by(:name => "owner").value).to eq("UNKNOWN")
@@ -874,7 +904,7 @@ def assert_specific_vm_on_cloud_network_public_ip
       have_attributes(
         :config_version       => nil,
         :virtual_hw_version   => nil,
-        :guest_os             => @template2.try(:hardware).try(:guest_os),
+        :guest_os             => options.inventory_object_refresh ? "linux_redhat" : nil,
         :cpu_sockets          => 1,
         :bios                 => nil,
         :bios_location        => nil,

spec/models/manageiq/providers/amazon/aws_refresher_spec_counts.rb

@@ -137,6 +137,7 @@ def table_counts_from_api
       :network                       => networks_count,
       :network_port                  => network_ports_count,
       :network_router                => network_routers_count,
+      :operating_system              => instances_and_images_count,
       :orchestration_stack           => orchestration_stacks_count,
       :orchestration_stack_output    => orchestration_stack_outputs_count,
       :orchestration_stack_parameter => orchestration_stack_parameters_count,

spec/models/manageiq/providers/amazon/cloud_manager/stubbed_refresher_spec.rb

@@ -147,7 +147,7 @@ def expected_table_counts(disconnect = nil)
       :guest_device                      => 0,
       :hardware                          => vm_count_plus_disconnect_inv + image_count_plus_disconnect_inv,
       :network                           => vm_count_plus_disconnect_inv * 2,
-      :operating_system                  => 0,
+      :operating_system                  => vm_count_plus_disconnect_inv + image_count_plus_disconnect_inv,
       :snapshot                          => 0,
       :system_service                    => 0,
       :relationship                      => vm_count_plus_disconnect_inv + image_count_plus_disconnect_inv,

spec/models/manageiq/providers/amazon/cloud_manager/vcr_specs/refresher_other_region_spec.rb

@@ -45,6 +45,8 @@
     def table_counts_from_api
       counts = super
       counts[:network_router] = 0 # We do not collect NetworkRouters in old refresh
+      # Old refresh can't fetch some images, those will have missing operating_system
+      counts[:operating_system] = counts[:operating_system] - Vm.all.select { |x| x.genealogy_parent.nil? }.count
       counts
     end
   end
@@ -146,12 +148,16 @@ def assert_specific_template
     )
 
     expect(@template.ext_management_system).to eq(@ems)
-    expect(@template.operating_system).to       be_nil # TODO: This should probably not be nil
+    expect(@template.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(@template.custom_attributes.size).to eq(0)
     expect(@template.snapshots.size).to eq(0)
 
     expect(@template.hardware).to have_attributes(
-      :guest_os           => "linux",
+      :guest_os           => "linux_generic",
       :guest_os_full_name => nil,
       :bios               => nil,
       :annotation         => nil,
@@ -202,12 +208,16 @@ def assert_specific_vm_powered_on
     expect(v.cloud_subnet).to           be_nil
     expect(v.security_groups).to eq([@sg])
     expect(v.key_pairs).to eq([@kp])
-    expect(v.operating_system).to       be_nil # TODO: This should probably not be nil
+    expect(v.operating_system).to(
+      have_attributes(
+        :product_name => "linux_generic",
+      )
+    )
     expect(v.custom_attributes.size).to eq(1)
     expect(v.snapshots.size).to eq(0)
 
     expect(v.hardware).to have_attributes(
-      :guest_os           => "linux",
+      :guest_os           => "linux_generic",
       :guest_os_full_name => nil,
       :bios               => nil,
       :annotation         => nil,

spec/models/manageiq/providers/amazon/cloud_manager/vcr_specs/refresher_spec.rb

@@ -41,6 +41,8 @@
   def table_counts_from_api
     counts = super
     counts[:network_router] = 0 # We do not collect NetworkRouters in old refresh
+    # Old refresh can't fetch some images, those will have missing operating_system
+    counts[:operating_system] = counts[:operating_system] - Vm.all.select { |x| x.genealogy_parent.nil? }.count
     counts
   end
 end

spec/models/manageiq/providers/amazon/cloud_manager/vcr_specs/targeted_refresh_spec.rb

@@ -53,6 +53,7 @@
             :flavor                        => 3,
             :floating_ip                   => 2,
             :hardware                      => 2,
+            :operating_system              => 2,
             :miq_template                  => 1,
             :network                       => 2,
             :network_port                  => 2,
@@ -107,6 +108,7 @@
             :flavor                        => 3,
             :floating_ip                   => 3,
             :hardware                      => 2,
+            :operating_system              => 2,
             :miq_template                  => 1,
             :network                       => 2,
             :network_port                  => 3,
@@ -152,6 +154,7 @@
             :flavor                        => 3,
             :floating_ip                   => 1,
             :hardware                      => 2,
+            :operating_system              => 2,
             :miq_template                  => 1,
             :network                       => 2,
             :network_port                  => 1,
@@ -306,6 +309,7 @@
             :flavor                        => 3,
             :floating_ip                   => 1,
             :hardware                      => 2,
+            :operating_system              => 2,
             :miq_template                  => 1,
             :network                       => 2,
             :network_port                  => 1,