From d54cba7f9c58f37011737a42ead7a8a03c8929f7 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 22 Aug 2018 16:00:22 +0200
Subject: [PATCH 1/9] Graph refresh with Builder and Inventory

Switching Refresher as ManagerRefresh subclass, added collector/parser/persister classes.
---
 .../manageiq/providers/google/builder.rb      |  29 ++
 .../google/cloud_manager/refresher.rb         |   2 +-
 .../manageiq/providers/google/inventory.rb    |   5 +
 .../providers/google/inventory/collector.rb   |  12 +
 .../inventory/collector/cloud_manager.rb      |  81 ++++
 .../inventory/collector/network_manager.rb    | 172 +++++++++
 .../providers/google/inventory/parser.rb      |   6 +
 .../google/inventory/parser/cloud_manager.rb  | 298 +++++++++++++++
 .../inventory/parser/network_manager.rb       | 358 ++++++++++++++++++
 .../providers/google/inventory/persister.rb   |  10 +
 .../inventory/persister/cloud_manager.rb      |   7 +
 .../definitions/cloud_collections.rb          |  74 ++++
 .../definitions/network_collections.rb        |  47 +++
 .../inventory/persister/network_manager.rb    |  20 +
 .../google/network_manager/refresher.rb       |   2 +-
 config/settings.yml                           |   9 +
 .../google/cloud_manager/refresher_spec.rb    |  53 ++-
 17 files changed, 1174 insertions(+), 11 deletions(-)
 create mode 100644 app/models/manageiq/providers/google/builder.rb
 create mode 100644 app/models/manageiq/providers/google/inventory.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/collector.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/collector/network_manager.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/parser.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/parser/network_manager.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/persister.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/persister/cloud_manager.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
 create mode 100644 app/models/manageiq/providers/google/inventory/persister/network_manager.rb

diff --git a/app/models/manageiq/providers/google/builder.rb b/app/models/manageiq/providers/google/builder.rb
new file mode 100644
index 00000000..0cecabe1
--- /dev/null
+++ b/app/models/manageiq/providers/google/builder.rb
@@ -0,0 +1,29 @@
+class ManageIQ::Providers::Google::Builder
+  class << self
+    def build_inventory(ems, target)
+      manager_type = ManageIQ::Providers::Inflector.manager_type(target)
+
+      manager_type = 'Cloud' unless %w(Cloud Network).include?(manager_type)
+
+      collector_class = "ManageIQ::Providers::Google::Inventory::Collector::#{manager_type}Manager".safe_constantize
+      persister_class = "ManageIQ::Providers::Google::Inventory::Persister::#{manager_type}Manager".safe_constantize
+      parser_class    = "ManageIQ::Providers::Google::Inventory::Parser::#{manager_type}Manager".safe_constantize
+
+      inventory(ems, target, collector_class, persister_class, [parser_class])
+    end
+
+    private
+
+    def inventory(manager, raw_target, collector_class, persister_class, parsers_classes)
+      # binding.pry
+      collector = collector_class.new(manager, raw_target)
+      persister = persister_class.new(manager, raw_target)
+
+      ::ManageIQ::Providers::Google::Inventory.new(
+        persister,
+        collector,
+        parsers_classes.map(&:new)
+      )
+    end
+  end
+end
diff --git a/app/models/manageiq/providers/google/cloud_manager/refresher.rb b/app/models/manageiq/providers/google/cloud_manager/refresher.rb
index 5db84d2d..23bd37e0 100644
--- a/app/models/manageiq/providers/google/cloud_manager/refresher.rb
+++ b/app/models/manageiq/providers/google/cloud_manager/refresher.rb
@@ -1,5 +1,5 @@
 module ManageIQ::Providers::Google
-  class CloudManager::Refresher < ManageIQ::Providers::BaseManager::Refresher
+  class CloudManager::Refresher < ManageIQ::Providers::BaseManager::ManagerRefresher
     def parse_legacy_inventory(ems)
       ManageIQ::Providers::Google::CloudManager::RefreshParser.ems_inv_to_hashes(ems, refresher_options)
     end
diff --git a/app/models/manageiq/providers/google/inventory.rb b/app/models/manageiq/providers/google/inventory.rb
new file mode 100644
index 00000000..b5dd9bf5
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory.rb
@@ -0,0 +1,5 @@
+class ManageIQ::Providers::Google::Inventory < ManagerRefresh::Inventory
+  require_nested :Collector
+  require_nested :Parser
+  require_nested :Persister
+end
diff --git a/app/models/manageiq/providers/google/inventory/collector.rb b/app/models/manageiq/providers/google/inventory/collector.rb
new file mode 100644
index 00000000..3b2e131f
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/collector.rb
@@ -0,0 +1,12 @@
+class ManageIQ::Providers::Google::Inventory::Collector < ManagerRefresh::Inventory::Collector
+  require_nested :CloudManager
+  require_nested :NetworkManager
+
+  def initialize(_manager, _target)
+    super
+  end
+
+  def connection
+    @connection ||= manager.connect
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
new file mode 100644
index 00000000..3b4ddaf9
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
@@ -0,0 +1,81 @@
+class ManageIQ::Providers::Google::Inventory::Collector::CloudManager < ManageIQ::Providers::Google::Inventory::Collector
+  attr_reader :project_key_pairs
+
+  def availability_zones
+    connection.zones.all
+  end
+
+  def flavors
+    flavors_by_zone = connection.list_aggregated_machine_types.items
+    flavors_by_zone.values.flat_map(&:machine_types).compact.uniq(&:id)
+  end
+
+  def flavor(flavor_uid, availability_zone_uid)
+    connection.get_machine_type(flavor_uid, availability_zone_uid)
+  end
+
+  def cloud_volumes
+    connection.disks.all
+  end
+
+  # !also parse vms! there
+  def cloud_volume_snapshots
+    connection.snapshots.all
+  end
+
+  def images
+    connection.images.all
+  end
+
+  def instances
+    connection.servers.all
+  end
+
+  def key_pairs(instances)
+    ssh_keys = []
+
+    # Find all key pairs added directly to GCE instances
+    instances.each do |instance|
+      ssh_keys |= parse_compute_metadata_ssh_keys(instance.metadata)
+    end
+
+    # Add ssh keys that are common to all instances in the project
+    project_common_metadata = connection.projects.get(manager.project).common_instance_metadata
+    @project_key_pairs      = parse_compute_metadata_ssh_keys(project_common_metadata)
+
+    ssh_keys |= @project_key_pairs
+    ssh_keys
+  end
+
+  def parse_compute_metadata(metadata, key)
+    metadata_item = metadata[:items].to_a.detect { |x| x[:key] == key }
+    metadata_item.to_h[:value]
+  end
+
+  def parse_compute_metadata_ssh_keys(metadata)
+    require 'sshkey'
+
+    ssh_keys = []
+
+    # Find the sshKeys property in the instance metadata
+    metadata_ssh_keys = parse_compute_metadata(metadata, "sshKeys")
+
+    metadata_ssh_keys.to_s.split("\n").each do |ssh_key|
+      # Google returns the key in the form username:public_key
+      name, public_key = ssh_key.split(":", 2)
+      begin
+        fingerprint = SSHKey.sha1_fingerprint(public_key)
+
+        ssh_keys << {
+          :name        => name,
+          :public_key  => public_key,
+          :fingerprint => fingerprint
+        }
+      rescue => err
+        _log.warn("Failed to parse public key #{name}: #{err}")
+      end
+    end
+
+    ssh_keys
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/collector/network_manager.rb b/app/models/manageiq/providers/google/inventory/collector/network_manager.rb
new file mode 100644
index 00000000..a53bbb9b
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/collector/network_manager.rb
@@ -0,0 +1,172 @@
+class ManageIQ::Providers::Google::Inventory::Collector::NetworkManager < ManageIQ::Providers::Google::Inventory::Collector
+  def cloud_networks
+    connection.networks.all
+  end
+
+  def cloud_subnets
+    if @subnetworks.nil?
+      @subnetworks = connection.subnetworks.all
+      # For a backwards compatibility, old GCE networks were created without subnet. It's not possible now, but
+      # GCE haven't migrated to new format. We will create a fake subnet for each network without subnets.
+      @subnetworks += connection.networks.select { |x| x.ipv4_range.present? }.map do |x|
+        Fog::Compute::Google::Subnetwork.new(
+          :name               => x.name,
+          :gateway_address    => x.gateway_ipv4,
+          :ip_cidr_range      => x.ipv4_range,
+          :id                 => x.id,
+          :network            => x.self_link,
+          :self_link          => x.self_link,
+          :description        => "Subnetwork placeholder for GCE legacy networks without subnetworks",
+          :creation_timestamp => x.creation_timestamp,
+          :kind               => x.kind
+        )
+      end
+    end
+
+    @subnetworks
+  end
+
+  def network_ports
+    if @network_ports.nil?
+      @network_ports = []
+      connection.servers.all.collect do |instance|
+        @network_ports += instance.network_interfaces.each do |i|
+          i[:device_id] = instance.id
+        end
+      end
+    end
+
+    @network_ports
+  end
+
+  def floating_ips(status = :assigned)
+    if status == :assigned
+      # Fetch assigned floating IPs
+      network_ports.flat_map do |network_port|
+        network_port[:access_configs].to_a.collect do |access_config|
+          {:fixed_ip => network_port[:network_ip], :external_ip => access_config[:nat_ip]}
+        end
+      end
+    else
+      # Fetch non assigned static floating IPs
+      connection.addresses.reject { |x| x.status == "IN USE" }
+    end
+  end
+
+  # for IC firewall_rules
+  def firewalls
+    connection.firewalls.all
+  end
+
+  # for IC load_balancers
+  def forwarding_rules
+    connection.forwarding_rules.all
+  end
+
+  # for IC load_balancer_pools
+  def target_pools
+    # Right now we only support network-based load-balancers, instead of the
+    # more complicated HTTP/HTTPS load balancers.
+    # TODO(jsselman): Add support for http/https proxies
+    connection.target_pools.all
+  end
+
+  def get_health_check_from_link(link)
+    parts = parse_health_check_link(link)
+    unless parts
+      _log.warn("Unable to parse health check link: #{link}")
+      return nil
+    end
+
+    return nil unless connection.project == parts[:project]
+    get_health_check_cached(parts[:health_check])
+  end
+
+  # Lookup a VM in fog via its link to get the VM id (which is equivalent to
+  # the ems_ref).
+  #
+  # @param link [String] the full url to the vm
+  # @return [String, nil] the vm id, or nil if it could not be found
+  def get_vm_id_from_link(link)
+    parts = parse_vm_link(link)
+    unless parts
+      _log.warn("Unable to parse vm link: #{link}")
+      return nil
+    end
+
+    # Ensure our connection is using the same project; if it's not we can't
+    # do much
+    return nil unless connection.project == parts[:project]
+
+    get_vm_id_cached(parts[:zone], parts[:instance])
+  end
+
+  private
+
+  # Parses a VM's self_link attribute to extract the project name, zone, and
+  # instance name. Used when other services refer to a VM by its link.
+  #
+  # @param vm_link [String] the full url to the vm (e.g.
+  #   "https://www.googleapis.com/compute/v1/projects/myproject/zones/us-central1-a/instances/foobar")
+  # @return [Hash{Symbol => String}, nil] a hash containing extracted components
+  #   for `:project`, `:zone`, and `:instance`, or nil if the link did not
+  #   match.
+  def parse_vm_link(vm_link)
+    link_regexp = %r{\Ahttps://www\.googleapis\.com/compute/v1/projects/([^/]+)/zones/([^/]+)/instances/([^/]+)\Z}
+    m = link_regexp.match(vm_link)
+    return nil if m.nil?
+
+    {
+      :project  => m[1],
+      :zone     => m[2],
+      :instance => m[3]
+    }
+  end
+
+  def parse_health_check_link(health_check_link)
+    link_regexp = %r{\Ahttps://www\.googleapis\.com/compute/v1/projects/([^/]+)/global/httpHealthChecks/([^/]+)\Z}
+
+    m = link_regexp.match(health_check_link)
+    return nil if m.nil?
+
+    {
+      :project      => m[1],
+      :health_check => m[2]
+    }
+  end
+
+  # Look up a VM in fog via a given zone and instance for the current
+  # project to get the VM id. Note this method caches matched values during
+  # this instance's entire lifetime.
+  #
+  # @param zone [String] the zone of the vm
+  # @param instance [String] the name of the vm
+  # @return [String, nil] the vm id, or nil if it could not be found
+  def get_vm_id_cached(zone, instance)
+    @vm_cache ||= {}
+
+    return @vm_cache.fetch_path(zone, instance) if @vm_cache.has_key_path?(zone, instance)
+
+    begin
+      @vm_cache.store_path(zone, instance, connection.get_server(instance, zone).id)
+    rescue Fog::Errors::Error, ::Google::Apis::ClientError => err
+      m = "Error during data collection for [#{manager&.name}] id: [#{manager&.id}] when querying link for vm_id: #{err}"
+      _log.warn(m)
+      _log.warn(err.backtrace.join("\n"))
+      nil
+    end
+  end
+
+  def get_health_check_cached(health_check)
+    @health_check_cache ||= {}
+
+    return @health_check_cache.fetch_path(health_check) if @health_check_cache.has_key_path?(health_check)
+
+    @health_check_cache.store_path(health_check, connection.http_health_checks.get(health_check))
+  rescue Fog::Errors::Error => err
+    m = "Error during data collection for [#{manager&.name}] id: [#{manager&.id}] when querying link for health check: #{err}"
+    _log.warn(m)
+    _log.warn(err.backtrace.join("\n"))
+    nil
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/parser.rb b/app/models/manageiq/providers/google/inventory/parser.rb
new file mode 100644
index 00000000..58f4bf7f
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/parser.rb
@@ -0,0 +1,6 @@
+class ManageIQ::Providers::Google::Inventory::Parser < ManagerRefresh::Inventory::Parser
+  require_nested :CloudManager
+  require_nested :NetworkManager
+
+  VENDOR_GOOGLE = "google".freeze
+end
diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
new file mode 100644
index 00000000..728128b9
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -0,0 +1,298 @@
+class ManageIQ::Providers::Google::Inventory::Parser::CloudManager < ManageIQ::Providers::Google::Inventory::Parser
+  include ManageIQ::Providers::Google::RefreshHelperMethods
+
+  def initialize
+    super
+    # Mapping from disk url to source image id.
+    @disk_to_source_image_id = {}
+    # Mapping from disk url to disk uid
+    @disk_to_id = {}
+
+    @project_key_pairs = Set.new
+  end
+
+  def parse
+    log_header = "MIQ(#{self.class.name}.#{__method__}) Collecting data for EMS name: [#{collector.manager.name}] id: [#{collector.manager.id}]"
+    _log.info("#{log_header}...")
+
+    # binding.pry
+    availability_zones
+    flavors
+    cloud_volumes
+    cloud_volume_snapshots
+
+    images do |image, inventory_object|
+      image_os(inventory_object, image)
+    end
+    instances
+
+    _log.info("#{log_header}...Complete")
+  end
+
+  private
+
+  def availability_zones
+    collector.availability_zones.each do |az|
+      persister.availability_zones.find_or_build(az.name).assign_attributes(
+        :name => az.name
+      )
+    end
+  end
+
+  def flavors
+    collector.flavors.each do |flavor|
+      instance_flavor(flavor)
+    end
+  end
+
+  def get_flavor(flavor_uid, availability_zone_uid)
+    flavor = collector.flavor(flavor_uid, availability_zone_uid)
+    instance_flavor(flavor)
+  end
+
+  def instance_flavor(flavor)
+    persister.flavors.find_or_build(flavor.name).assign_attributes(
+      :name        => flavor.name,
+      :description => flavor.description,
+      :enabled     => !flavor.deprecated,
+      :cpus        => flavor.guest_cpus,
+      :memory      => flavor.memory_mb * 1.megabyte
+    )
+  end
+
+  # TODO: where is cloud_tenant assoc?
+  def cloud_volumes
+    collector.cloud_volumes.each do |cloud_volume|
+      zone_id = parse_uid_from_url(cloud_volume.zone)
+
+      persister.cloud_volumes.find_or_build(cloud_volume.id.to_s).assign_attributes(
+        :availability_zone => persister.availability_zones.lazy_find(zone_id),
+        :base_snapshot     => persister.cloud_volume_snapshots.lazy_find(cloud_volume.source_snapshot),
+        :creation_time     => cloud_volume.creation_timestamp,
+        :description       => cloud_volume.description,
+        :name              => cloud_volume.name,
+        :size              => cloud_volume.size_gb.to_i.gigabyte,
+        :status            => cloud_volume.status,
+        :volume_type       => parse_uid_from_url(cloud_volume.type)
+      )
+
+      # Take note of the source_image_id so we can expose it in parse_instance
+      @disk_to_source_image_id[cloud_volume.self_link] = cloud_volume.source_image_id
+      @disk_to_id[cloud_volume.self_link] = cloud_volume.id.to_s
+    end
+  end
+
+  def cloud_volume_snapshots
+    collector.cloud_volume_snapshots.each do |snapshot|
+      persister.cloud_volume_snapshots.find_or_build(snapshot.id.to_s).assign_attributes(
+        :creation_time => snapshot.creation_timestamp,
+        :description   => snapshot.description,
+        :name          => snapshot.name,
+        :size          => snapshot.disk_size_gb.to_i.gigabytes,
+        :status        => snapshot.status,
+        :cloud_volume  => persister.cloud_volumes.lazy_find(snapshot.source_disk)
+      )
+
+      image = snapshot
+      uid = image.id.to_s
+
+      # TODO: duplicite code
+      persister_miq_template = persister.miq_templates.find_or_build(uid).assign_attributes(
+        :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
+        :location           => image.self_link,
+        :name               => image.name || uid,
+        :publicly_available => true,
+        :raw_power_state    => "never",
+        :template           => true,
+        :uid_ems            => uid,
+        :vendor             => VENDOR_GOOGLE
+      )
+      image_os(persister_miq_template, image)
+    end
+  end
+
+  def images
+    collector.images.each do |image|
+      uid = image.id.to_s
+
+      persister_image = persister.miq_templates.find_or_build(uid).assign_attributes(
+        :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
+        :location           => image.self_link,
+        :name               => image.name || uid,
+        :publicly_available => true,
+        :raw_power_state    => "never",
+        :template           => true,
+        :uid_ems            => uid,
+        :vendor             => VENDOR_GOOGLE
+      )
+      yield image, persister_image
+    end
+  end
+
+  def instances
+    instances = collector.instances
+
+    key_pairs(instances) # TODO: merge with loop below
+
+    instances.each do |instance|
+      uid = instance.id.to_s
+
+      flavor_uid = parse_uid_from_url(instance.machine_type)
+      zone_uid = parse_uid_from_url(instance.zone)
+
+      flavor = persister.flavors.find(flavor_uid)
+
+      # If the flavor isn't found in our index, check if it is a custom flavor
+      # that we have to get directly
+      flavor = get_flavor(flavor_uid, zone_uid) if flavor.nil?
+
+      availability_zone = persister.availability_zones.lazy_find(zone_uid)
+      parent_image_uid = parse_instance_parent_image(instance)
+
+      persister_instance = persister.vms.find_or_build(uid).assign_attributes(
+        :availability_zone => availability_zone,
+        :description       => instance.description,
+        :flavor            => flavor,
+        :location          => "unknown", # TODO: ??? || "unknown"
+        :name              => instance.name || uid,
+        :genealogy_parent  => persister.miq_templates.lazy_find(parent_image_uid),
+        :raw_power_state   => instance.status,
+        :uid_ems           => uid,
+        :vendor            => VENDOR_GOOGLE,
+      )
+
+      instance_os(persister_instance, parent_image_uid)
+      instance_hardware(persister_instance, instance, flavor)
+      instance_key_pairs(persister_instance, instance)
+      instance_advanced_settings(persister_instance, instance)
+      #
+    end
+  end
+
+  def key_pairs(instances)
+    collector.key_pairs(instances).each do |ssh_key|
+      persister.key_pairs.build(
+        :name        => ssh_key[:name],
+        :fingerprint => ssh_key[:fingerprint]
+      )
+    end
+  end
+
+  # TODO: collector.parse - refactoring needed
+  def instance_key_pairs(persister_instance, instance)
+    # Add project common ssh-keys with keys specific to this instance
+    instance_ssh_keys = collector.parse_compute_metadata_ssh_keys(instance.metadata) | collector.project_key_pairs
+    instance_ssh_keys.each do |ssh_key|
+      # select existing key pairs
+      # existing_key_pairs = persister.key_pairs.data.select do |kp|
+      #   kp.name == ssh_key[:name] && kp.fingerprint == ssh_key[:fingerprint]
+      # end
+
+      key_pair = persister.key_pairs.find(:name        => ssh_key[:name],
+                                          :fingerprint => ssh_key[:fingerprint])
+      if key_pair.nil?
+        key_pair = persister.key_pairs.build(
+          :name        => ssh_key[:name],       # manager_ref
+          :fingerprint => ssh_key[:fingerprint] # manager_ref
+        )
+      end
+
+      key_pair.vms ||= []
+      key_pair.vms << persister_instance
+    end
+  end
+
+  def instance_hardware(persister_instance, instance, series)
+    persister_hardware = persister.hardwares.build(
+      :vm_or_template  => persister_instance, # manager_ref
+      :cpu_total_cores => series[:cpus],
+      :memory_mb       => series[:memory] / 1.megabyte
+    )
+    hardware_disks(persister_hardware, instance)
+  end
+
+  def instance_advanced_settings(persister_instance, instance)
+    persister.advanced_settings.build(
+      :resource     => persister_instance, # manager_ref
+      :name         => "preemptible?",
+      :display_name => N_("Is VM Preemptible"),
+      :description  => N_("Whether or not the VM is 'preemptible'. See"\
+                               " https://cloud.google.com/compute/docs/instances/preemptible for more details."),
+      :value        => instance.scheduling[:preemptible].to_s,
+      :read_only    => true
+    )
+  end
+
+  # @param persister_template [InventoryObject]
+  def image_os(persister_template, image)
+    persister.operating_systems.build(
+      :vm_or_template => persister_template, # manager_ref
+      :product_name   => get_os_product_name(image)
+    )
+  end
+
+  # Operating system for Vm (i.e. instance)
+  # It's name is taken from parent image's OS
+  # Note: Not added when parent image not found
+  #
+  # @param persister_vm [InventoryObject] -> IC <ManageIQ::Providers::Google::CloudManager::Vm>
+  # @param parent_image_uid [String] UID of vm's template
+  def instance_os(persister_vm, parent_image_uid)
+    persister.operating_systems.build(
+      :vm_or_template => persister_vm,
+      :product_name   => persister.operating_systems.lazy_find(
+        persister.miq_templates.lazy_find(parent_image_uid),
+        :key => :product_name
+      )
+    )
+  end
+
+  def hardware_disks(persister_hardware, instance)
+    instance.disks.each do |attached_disk|
+      # TODO(mslemr): can't be better solution?
+      cloud_volume_ems_ref = @disk_to_id[attached_disk[:source]]
+      persister_cloud_volume = persister.cloud_volumes.find(cloud_volume_ems_ref)
+      next if persister_cloud_volume.nil?
+
+      persister.disks.build(
+        :backing         => persister_cloud_volume,
+        :backing_type    => 'CloudVolume',
+        :controller_type => VENDOR_GOOGLE,
+        :device_name     => attached_disk[:device_name], # manager_ref
+        :device_type     => "disk",
+        :hardware        => persister_hardware,          # manager_ref
+        :location        => attached_disk[:index],
+        :size            => persister_cloud_volume.size
+      )
+    end
+  end
+
+  # ---
+
+  def get_os_product_name(storage)
+    if storage.kind == 'compute#image'
+      OperatingSystem.normalize_os_name(storage.name)
+    else
+      'unknown'
+    end
+  end
+
+  # Get image's (miq_template's) ems_ref from
+  # instance disks (connected to cloud_volume's `self_link`)
+  def parse_instance_parent_image(instance)
+    parent_image_uid = nil
+
+    instance.disks.each do |disk|
+      parent_image_uid = @disk_to_source_image_id[disk[:source]]
+      next if parent_image_uid.nil?
+      break
+    end
+
+    parent_image_uid
+  end
+
+  # @param ssh_key [Hash]
+  def key_pairs_ems_ref(ssh_key)
+    "#{ssh_key[:name]}:#{ssh_key[:fingerprint]}"
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
new file mode 100644
index 00000000..65465d75
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -0,0 +1,358 @@
+class ManageIQ::Providers::Google::Inventory::Parser::NetworkManager < ManageIQ::Providers::Google::Inventory::Parser
+  include ManageIQ::Providers::Google::RefreshHelperMethods
+
+  GCP_HEALTH_STATUS_MAP = {
+    "HEALTHY"   => "InService",
+    "UNHEALTHY" => "OutOfService"
+  }.freeze
+
+  def initialize
+    super
+
+    # Simple mapping from target pool's self_link url to the created
+    # target pool entity.
+    @target_pool_index = {}
+
+    # Another simple mapping from target pool's self_link url to the set of
+    # lbs that point at it
+    @target_pool_link_to_load_balancers = {}
+  end
+
+  def parse
+    log_header = "MIQ(#{self.class.name}.#{__method__}) Collecting data for EMS name: [#{collector.manager.name}] id: [#{collector.manager.id}]"
+    _log.info("#{log_header}...")
+
+    # binding.pry
+    cloud_networks
+    security_groups
+    network_ports
+    floating_ips
+    load_balancers
+    # load_balancer_listeners
+    # load_balancer_health_checks
+
+    _log.info("#{log_header}...Complete")
+  end
+
+  private
+
+  def cloud_networks
+    collector.cloud_networks.each do |network|
+      persister_cloud_network = persister.cloud_networks.build(
+        :cidr    => network.ipv4_range,
+        :ems_ref => network.id.to_s, # manager_ref
+        :enabled => true,
+        :name    => network.name,
+        :status  => "active"
+      )
+
+      cloud_subnets(persister_cloud_network, network)
+    end
+  end
+
+  def cloud_subnets(persister_cloud_network, network)
+    @subnets_by_network_link ||= collector.cloud_subnets.each_with_object({}) { |x, subnets| (subnets[x.network] ||= []) << x }
+    @subnets_by_network_link[network.self_link].each do |cloud_subnet|
+      uid = cloud_subnet.id.to_s
+      persister.cloud_subnets.build(
+        :cidr          => cloud_subnet.ip_cidr_range,
+        :cloud_network => persister_cloud_network,
+        :ems_ref       => uid, # manager_ref
+        :gateway       => cloud_subnet.gateway_address,
+        :name          => cloud_subnet.name || uid,
+        :status        => "active",
+      )
+    end
+  end
+
+  def network_ports
+    collector.network_ports.each do |network_port|
+      uid = network_port[:network_ip]
+
+      persister_network_port = persister.network_ports.build(
+        :device      => persister.vms.lazy_find(network_port[:device_id]),
+        :device_ref  => network_port[:device_id].to_s,
+        :ems_ref     => uid,
+        :mac_address => nil,
+        :name        => network_port[:name],
+        :status      => nil,
+      )
+
+      persister_network_port.security_groups ||= []
+      persister_network_port.security_groups << persister.security_groups.lazy_find(parse_uid_from_url(network_port[:network]))
+
+      cloud_subnet_network_port(persister_network_port, network_port)
+    end
+  end
+
+  def cloud_subnet_network_port(persister_network_port, network_port)
+    subnets_by_link ||= collector.cloud_subnets.each_with_object({}) { |x, subnets| subnets[x.self_link] = x }
+
+    # For legacy GCE networks without subnets, we also try a network link
+    cloud_subnet = subnets_by_link[network_port[:subnetwork]] || subnets_by_link[network_port[:network]]
+
+    persister.cloud_subnet_network_ports.build(
+      :cloud_subnet => persister.cloud_subnets.lazy_find(cloud_subnet.id.to_s),
+      :network_port => persister_network_port,
+      :address      => network_port[:network_ip]
+    )
+  end
+
+  def floating_ips
+    collector.floating_ips(:non_assigned).each do |ip|
+      persister.floating_ips.build(
+        :address          => ip.address,
+        :ems_ref          => ip.address,
+        :fixed_ip_address => nil,
+        :network_port     => nil,
+        :vm               => nil
+      )
+    end
+    collector.floating_ips(:assigned).each do |ip|
+      network_port = persister.network_ports.find(ip[:fixed_ip])
+      if network_port.present?
+        vm = persister.vms.lazy_find(network_port.device_ref)
+      end
+      persister.floating_ips.build(
+        :address          => ip[:external_ip],
+        :ems_ref          => ip[:external_ip],
+        :fixed_ip_address => ip[:fixed_ip],
+        :network_port     => network_port,
+        :vm               => vm
+      )
+    end
+  end
+
+  def security_groups
+    firewalls = collector.firewalls
+
+    persister.cloud_networks.data.each do |cloud_network|
+      uid = cloud_network.name
+      persister_security_group = persister.security_groups.build(
+        :cloud_network => cloud_network,
+        :ems_ref       => uid,
+        :name          => uid
+      )
+
+      network_firewalls = firewalls.select do |firewall|
+        parse_uid_from_url(firewall.network) == cloud_network.name
+      end
+
+      # Build firewall rules
+      firewall_rules(persister_security_group, network_firewalls)
+    end
+  end
+
+  def firewall_rules(persister_security_group, firewalls)
+    firewalls.each do |firewall|
+      name = firewall.name
+      source_ip_range = firewall.source_ranges.nil? ? "0.0.0.0/0" : firewall.source_ranges.first
+
+      firewall.allowed.each do |fw_allowed|
+        protocol      = fw_allowed[:ip_protocol].upcase
+        allowed_ports = fw_allowed[:ports].to_a.first
+
+        if allowed_ports.nil?
+          # The ICMP protocol doesn't have ports so set to -1
+          from_port = to_port = -1
+        else
+          from_port, to_port = allowed_ports.split("-", 2)
+        end
+
+        persister.firewall_rules.build(
+          :direction             => "inbound",
+          :end_port              => to_port,
+          :host_protocol         => protocol,
+          :name                  => name,
+          :port                  => from_port,
+          :resource              => persister_security_group,
+          :source_ip_range       => source_ip_range,
+          :source_security_group => nil
+        )
+      end
+    end
+  end
+
+  def load_balancers
+    forwarding_rules = collector.forwarding_rules
+
+    forwarding_rules.each do |forwarding_rule|
+      persister_load_balancer = persister.load_balancers.build(
+        :ems_ref => forwarding_rule.id.to_s,
+        :name    => forwarding_rule.name
+      )
+
+      if forwarding_rule.target
+        # Make sure we link the target link back to this instance for future
+        # back-references
+        @target_pool_link_to_load_balancers[forwarding_rule.target] ||= Set.new
+        @target_pool_link_to_load_balancers[forwarding_rule.target].add(persister_load_balancer)
+      end
+    end
+
+    load_balancer_pools
+
+    forwarding_rules.each do |forwarding_rule|
+      load_balancer_listener(persister.load_balancers.lazy_find(forwarding_rule.id.to_s), forwarding_rule)
+    end
+  end
+
+  def load_balancer_listener(persister_load_balancer, forwarding_rule)
+    # Only TCP/UDP/SCTP forwarding rules have ports
+    has_ports = %w(TCP UDP SCTP).include?(forwarding_rule.ip_protocol)
+    port_range = (parse_port_range(forwarding_rule.port_range) if has_ports)
+
+    persister_load_balancer_listener = persister.load_balancer_listeners.build(
+      :name                     => forwarding_rule.name,
+      :ems_ref                  => forwarding_rule.id.to_s,
+      :load_balancer_protocol   => forwarding_rule.ip_protocol,
+      :instance_protocol        => forwarding_rule.ip_protocol,
+      :load_balancer_port_range => port_range,
+      :instance_port_range      => port_range,
+      :load_balancer            => persister_load_balancer
+    )
+
+    load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+  end
+
+  def load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+    persister_lb_listener_pool = persister.load_balancer_listener_pools.build(
+      :load_balancer_listener => persister_load_balancer_listener,
+      :load_balancer_pool     => @target_pool_index[forwarding_rule.target]
+    )
+
+    persister_load_balancer_listener.load_balancer_listener_pools ||= []
+    persister_load_balancer_listener.load_balancer_listener_pools << persister_lb_listener_pool
+  end
+
+  def load_balancer_pools
+    collector.target_pools.each do |target_pool|
+      persister_load_balancer_pool = persister.load_balancer_pools.build(
+        :ems_ref => target_pool.id.to_s,
+        :name    => target_pool.name
+      )
+
+      @target_pool_index[target_pool.self_link] = persister_load_balancer_pool
+
+      load_balancer_pool_members(persister_load_balancer_pool, target_pool)
+      load_balancer_health_check(target_pool)
+    end
+  end
+
+  def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
+    target_pool.instances.to_a.each do |member_link|
+      persister_lb_pool_member = persister.load_balancer_pool_members.find(Digest::MD5.base64digest(member_link))
+
+      if persister_lb_pool_member.nil?
+        vm_id = collector.get_vm_id_from_link(member_link)
+
+        persister_lb_pool_member = persister.load_balancer_pool_members.build(
+          :ems_ref => Digest::MD5.base64digest(member_link),
+          :vm      => (persister.vms.lazy_find(vm_id) if vm_id)
+        )
+      end
+      persister.load_balancer_pool_member_pools.find_or_build_by(
+        :load_balancer_pool        => persister_load_balancer_pool,
+        :load_balancer_pool_member => persister_lb_pool_member
+      )
+    end
+  end
+
+  def load_balancer_health_check(target_pool)
+    # Target pools aren't required to have health checks
+    return if target_pool.health_checks.blank?
+
+    # For some reason a target pool has a list of health checks, but the API
+    # won't accept more than one. Ignore the rest
+    if target_pool.health_checks.size > 1
+      _log.warn("Expected one health check on target pool but found many! Ignoring all but the first.")
+    end
+
+    health_check = collector.get_health_check_from_link(target_pool.health_checks.first)
+    @target_pool_link_to_load_balancers[target_pool.self_link].each do |persister_load_balancer|
+      # load_balancer and listener have same ems_ref
+      load_balancer_listener = persister.load_balancer_listeners.lazy_find(persister_load_balancer.ems_ref)
+
+      # TODO(mslemr) this return is in old refresh
+      # return nil if load_balancer_listener.nil? #TODO: return inside collect?
+
+      uid = "#{persister_load_balancer.ems_ref}_#{target_pool.id}_#{health_check.id}"
+      persister_load_balancer_health_check = \
+        persister.load_balancer_health_checks.build(
+          :ems_ref                => uid,
+          :healthy_threshold      => health_check.healthy_threshold,
+          :interval               => health_check.check_interval_sec,
+          :load_balancer          => persister_load_balancer,
+          :load_balancer_listener => load_balancer_listener,
+          :name                   => health_check.name,
+          :protocol               => "HTTP",
+          :port                   => health_check.port,
+          :timeout                => health_check.timeout_sec,
+          :unhealthy_threshold    => health_check.unhealthy_threshold,
+          :url_path               => health_check.request_path,
+        )
+
+      load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+    end
+  end
+
+  def load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+    return if target_pool.instances.blank?
+    # First attempt to get the health of the instance
+    # Due to a bug in fog, there's no way to get the health of an individual
+    # member. Instead we have to get the health of the entire target_pool,
+    # which if it fails means we skip.
+    # Issue here: https://github.com/fog/fog-google/issues/162
+    target_pool.get_health.collect do |instance_link, instance_health|
+      # attempt to look up the load balancer member
+      member = persister.load_balancer_pool_members.find(Digest::MD5.base64digest(instance_link))
+      return nil unless member
+
+      # Lookup our health state in the health status map; default to
+      # "OutOfService" if we can't find a mapping.
+      status = "OutOfService"
+      unless instance_health.nil?
+        gcp_status = instance_health[0][:health_state]
+
+        if GCP_HEALTH_STATUS_MAP.include?(gcp_status)
+          status = GCP_HEALTH_STATUS_MAP[gcp_status]
+        else
+          _log.warn("Unable to find an explicit health status mapping for state: #{gcp_status} - defaulting to 'OutOfService'")
+        end
+      end
+
+      persister.load_balancer_health_check_members.build(
+        :load_balancer_health_check => persister_load_balancer_health_check,
+        :load_balancer_pool_member  => member,
+        :status                     => status,
+        :status_reason              => ""
+      )
+    end
+  rescue Fog::Errors::Error => err
+    _log.warn("Caught unexpected error when probing health for target pool #{target_pool.name}: #{err}")
+    _log.warn(err.backtrace.join("\n"))
+    return []
+  end
+  #
+  # --- helpers ---
+  #
+
+  # Parses a port range returned by GCP from a string to a Range. Note that
+  # GCP treats the empty port range "" to mean all ports; hence this method
+  # returns 0..65535 when the input is the empty string.
+  #
+  # @param port_range [String] the port range (e.g. "" or "80-123" or "11")
+  # @return [Range] a range representing the port range
+  def parse_port_range(port_range)
+    # Three forms:
+    # "80"
+    # "5000-5010"
+    # "" (all ports)
+    m = /\A(\d+)(?:-(\d+))?\Z/.match(port_range)
+    return 0..65_535 unless m
+
+    start = Integer(m[1])
+    finish = m[2] ? Integer(m[2]) : start
+    start..finish
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/persister.rb b/app/models/manageiq/providers/google/inventory/persister.rb
new file mode 100644
index 00000000..54fc04a7
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/persister.rb
@@ -0,0 +1,10 @@
+class ManageIQ::Providers::Google::Inventory::Persister < ManagerRefresh::Inventory::Persister
+  require_nested :CloudManager
+  require_nested :NetworkManager
+
+  # @param manager [ManageIQ::Providers::BaseManager] A manager object
+  # @param target [Object] A refresh Target object
+  def initialize(manager, target = nil)
+    super
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/persister/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/persister/cloud_manager.rb
new file mode 100644
index 00000000..163996f1
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/persister/cloud_manager.rb
@@ -0,0 +1,7 @@
+class ManageIQ::Providers::Google::Inventory::Persister::CloudManager < ManageIQ::Providers::Google::Inventory::Persister
+  include ManageIQ::Providers::Google::Inventory::Persister::Definitions::CloudCollections
+
+  def initialize_inventory_collections
+    initialize_cloud_inventory_collections
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb b/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
new file mode 100644
index 00000000..4a59b6f1
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
@@ -0,0 +1,74 @@
+module ManageIQ::Providers::Google::Inventory::Persister::Definitions::CloudCollections
+  extend ActiveSupport::Concern
+
+  def initialize_cloud_inventory_collections
+    %i(availability_zones
+       disks
+       flavors
+       hardwares
+       operating_systems
+       vms).each do |name|
+
+      add_collection(cloud, name)
+    end
+
+    add_miq_templates
+
+    add_key_pairs
+
+    add_cloud_volumes
+    add_cloud_volume_snapshots
+
+    add_advanced_settings
+
+    # Custom processing of Ancestry
+    %i(vm_and_miq_template_ancestry).each do |name|
+      add_collection(cloud, name)
+    end
+  end
+
+  # ------ IC provider specific definitions -------------------------
+
+  def add_miq_templates
+    add_collection(cloud, :miq_templates) do |builder|
+      builder.add_properties(
+        :model_class => ManageIQ::Providers::Google::CloudManager::Template
+      )
+    end
+  end
+
+  def add_cloud_volumes
+    add_collection(cloud, :cloud_volumes) do |builder|
+      builder.add_default_values(
+        :ems_id => manager.id
+      )
+    end
+  end
+
+  def add_cloud_volume_snapshots
+    add_collection(cloud, :cloud_volume_snapshots) do |builder|
+      builder.add_default_values(
+        :ems_id => manager.id
+      )
+    end
+  end
+
+  def add_key_pairs
+    add_collection(cloud, :key_pairs) do |builder|
+      builder.add_properties(
+        :manager_ref => %i(name fingerprint),
+        :model_class => ManageIQ::Providers::Google::CloudManager::AuthKeyPair
+      )
+    end
+  end
+
+  # advanced_settings for VMs
+  def add_advanced_settings
+    add_collection(cloud, :advanced_settings) do |builder|
+      builder.add_properties(
+        :manager_ref                  => %i(resource),
+        :parent_inventory_collections => %i(vms)
+      )
+    end
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb b/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
new file mode 100644
index 00000000..66e986a2
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
@@ -0,0 +1,47 @@
+module ManageIQ::Providers::Google::Inventory::Persister::Definitions::NetworkCollections
+  extend ActiveSupport::Concern
+
+  def initialize_network_inventory_collections
+    %i(cloud_networks
+       cloud_subnets
+       floating_ips
+       load_balancers
+       load_balancer_health_checks
+       load_balancer_health_check_members
+       load_balancer_listeners
+       load_balancer_pools
+       load_balancer_pool_members
+       load_balancer_pool_member_pools
+       network_ports
+       security_groups).each do |name|
+
+      add_collection(network, name)
+    end
+
+    add_cloud_subnet_network_ports
+
+    add_firewall_rules
+
+    add_load_balancer_listener_pools
+  end
+
+  # ------ IC provider specific definitions -------------------------
+
+  def add_cloud_subnet_network_ports
+    add_collection(network, :cloud_subnet_network_ports) do |builder|
+      builder.add_properties(:manager_ref_allowed_nil => %i(cloud_subnet))
+    end
+  end
+
+  def add_firewall_rules
+    add_collection(network, :firewall_rules) do |builder|
+      builder.add_properties(:manager_ref_allowed_nil => %i(source_security_group))
+    end
+  end
+
+  def add_load_balancer_listener_pools
+    add_collection(network, :load_balancer_listener_pools) do |builder|
+      builder.add_properties(:manager_ref_allowed_nil => %i(load_balancer_pool))
+    end
+  end
+end
diff --git a/app/models/manageiq/providers/google/inventory/persister/network_manager.rb b/app/models/manageiq/providers/google/inventory/persister/network_manager.rb
new file mode 100644
index 00000000..e9c1978b
--- /dev/null
+++ b/app/models/manageiq/providers/google/inventory/persister/network_manager.rb
@@ -0,0 +1,20 @@
+class ManageIQ::Providers::Google::Inventory::Persister::NetworkManager < ManageIQ::Providers::Google::Inventory::Persister
+  include ManageIQ::Providers::Google::Inventory::Persister::Definitions::NetworkCollections
+
+  def initialize_inventory_collections
+    initialize_network_inventory_collections
+
+    initialize_cloud_inventory_collections
+  end
+
+  def initialize_cloud_inventory_collections
+    %i(vms).each do |name|
+      add_collection(cloud, name) do |builder|
+        builder.add_properties(
+          :parent   => manager.parent_manager,
+          :strategy => :local_db_cache_all
+        )
+      end
+    end
+  end
+end
diff --git a/app/models/manageiq/providers/google/network_manager/refresher.rb b/app/models/manageiq/providers/google/network_manager/refresher.rb
index e21c91fb..6b4f0f61 100644
--- a/app/models/manageiq/providers/google/network_manager/refresher.rb
+++ b/app/models/manageiq/providers/google/network_manager/refresher.rb
@@ -1,5 +1,5 @@
 module ManageIQ::Providers
-  class Google::NetworkManager::Refresher < ManageIQ::Providers::BaseManager::Refresher
+  class Google::NetworkManager::Refresher < ManageIQ::Providers::BaseManager::ManagerRefresher
     def parse_legacy_inventory(ems)
       ManageIQ::Providers::Google::NetworkManager::RefreshParser.ems_inv_to_hashes(ems, refresher_options)
     end
diff --git a/config/settings.yml b/config/settings.yml
index 3bb0f6f8..37edb072 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -4,6 +4,15 @@
     :blacklisted_event_names: []
     :event_handling:
       :event_groups:
+:ems_refresh:
+  :gce:
+    :inventory_object_refresh: true
+    :inventory_collections:
+      :saver_strategy: default
+  :gce_network:
+    :inventory_object_refresh: false
+    :inventory_collections:
+      :saver_strategy: default
 :http_proxy:
   :google:
     :host:
diff --git a/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb b/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
index 28b1c409..20081689 100644
--- a/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
+++ b/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
@@ -21,14 +21,32 @@
   let(:snapshot_location)     { "https://www.googleapis.com/compute/v1/projects/GOOGLE_PROJECT/global/snapshots/test-snapshot-1" }
 
   before(:each) do
+    stub_settings_merge(
+      :ems_refresh => {
+        :gce         => {
+          :inventory_object_refresh => true,
+          :inventory_collections    => {
+            :saver_strategy => "default"
+          }
+        },
+        :gce_network => {
+          :inventory_object_refresh => true,
+          :inventory_collections    => {
+            :saver_strategy => "default"
+          }
+        }
+      }
+    )
+
     @ems = FactoryGirl.create(:ems_google_with_vcr_authentication)
   end
 
+  # firewall_rule?
   MODELS = %i(
-    availability_zone cloud_network cloud_subnet disk ext_management_system flavor floating_ip
+    availability_zone cloud_network cloud_subnet disk ext_management_system firewall_rule flavor floating_ip
     guest_device hardware load_balancer load_balancer_health_check load_balancer_health_check_member
     load_balancer_listener load_balancer_listener_pool load_balancer_pool load_balancer_pool_member
-    load_balancer_pool_member_pool miq_template network network_port network_router operating_system
+    load_balancer_pool_member_pool miq_template network network_port network_port_security_group network_router operating_system
     orchestration_stack orchestration_stack_output orchestration_stack_parameter orchestration_stack_resource
     orchestration_template relationship resource_group security_group vm vm_or_template
   ).freeze
@@ -44,7 +62,7 @@
 
       assert_table_counts
       assert_ems
-      assert_specific_zone
+      assert_specific_availability_zone
       assert_specific_key_pair
       assert_specific_cloud_network
       assert_specific_cloud_subnet
@@ -59,6 +77,12 @@
       assert_specific_snapshot_template
       assert_specific_cloud_volume
       assert_specific_cloud_volume_snapshot
+
+      # TODO: missing asserts
+      # asserts_specific_network_port
+      # assert_specific_operating_system
+      # assert_specific_hardware
+      # assert_specific_disk
     end
   end
 
@@ -67,12 +91,13 @@ def expected_table_counts
       :availability_zone                 => 46,
       :cloud_network                     => 3,
       :cloud_subnet                      => 18,
-      :disk                              => 15,
+      :disk                              => 15, # same as :vm and :hardware
       :ext_management_system             => 2,
+      :firewall_rule                     => 19, # TODO: (old refresh doesn't meet it (first time == 20 ,second time == 19)), new refresh different
       :flavor                            => 29,
       :floating_ip                       => 13,
       :guest_device                      => 0,
-      :hardware                          => 15,
+      :hardware                          => 15, # same as :vm
       :key_pair                          => 1,
       :load_balancer                     => 4,
       :load_balancer_health_check_member => 1,
@@ -82,11 +107,13 @@ def expected_table_counts
       :load_balancer_listener_pool       => 4,
       :load_balancer_pool_member         => 3,
       :load_balancer_pool_member_pool    => 4,
-      :miq_template                      => 1511,
+      :miq_template                      => 1511, # 1510 (images) + 1 (cloud_volume_snapshots)
       :network                           => 0,
       :network_port                      => 15,
+      :network_port_security_group       => 15,
       :network_router                    => 0,
-      :operating_system                  => 1523,
+      :operating_system                  => 1526, # 1510 (images) + 1 (cloud_volume_snapshots) + 15 (instances)
+      # :operating_system                  => 1523, # in old refresh (OS skipped for instances with instance.image.nil?)
       :orchestration_stack               => 0,
       :orchestration_stack_output        => 0,
       :orchestration_stack_parameter     => 0,
@@ -96,7 +123,7 @@ def expected_table_counts
       :resource_group                    => 0,
       :security_group                    => 3,
       :vm                                => 15,
-      :vm_or_template                    => 1526,
+      :vm_or_template                    => 1526, # :miq_template + :vm
     }
   end
 
@@ -118,7 +145,7 @@ def assert_ems
     expect(@ems.miq_templates.size).to      eql(expected_table_counts[:miq_template])
   end
 
-  def assert_specific_zone
+  def assert_specific_availability_zone
     expect(zone_east).to have_attributes(
       :name   => "us-east1-c",
       :ems_id => @ems.id
@@ -369,6 +396,7 @@ def assert_specific_vm_powered_off
     assert_specific_vm_powered_off_hardware(vm_powered_off)
     assert_specific_vm_powered_off_hardware_disks(vm_powered_off)
     assert_specific_vm_powered_off_networks(vm_powered_off)
+    assert_specific_vm_powered_off_security_groups(vm_powered_off)
     assert_specific_vm_powered_off_floating_ips(vm_powered_off)
   end
 
@@ -418,6 +446,13 @@ def assert_specific_vm_powered_off_networks(v)
     expect(v.cloud_subnets.first).to  eq(v.cloud_subnet)
   end
 
+  def assert_specific_vm_powered_off_security_groups(v)
+    expect(v.security_groups.size).to eq(1)
+    expect(v.network_ports.first.security_groups.size).to eq(1)
+
+    expect(v.security_groups.first).to have_attributes(:name => 'default')
+  end
+
   def assert_specific_vm_powered_off_floating_ips(v)
     expect(v.floating_ips.size).to                      eql(0)
     expect(v.floating_ip).to                            eql(nil)

From d682dd1ef4bf3627835fd75b3fba4c30fa180150 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 22 Aug 2018 16:17:04 +0200
Subject: [PATCH 2/9] InventoryCollection's find_or_build -> build

---
 .../google/inventory/parser/cloud_manager.rb  | 29 ++++++++++++-------
 .../inventory/parser/network_manager.rb       |  2 +-
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index 728128b9..c110ecfe 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -33,8 +33,9 @@ def parse
 
   def availability_zones
     collector.availability_zones.each do |az|
-      persister.availability_zones.find_or_build(az.name).assign_attributes(
-        :name => az.name
+      persister.availability_zones.build(
+        :ems_ref => az.name,
+        :name    => az.name
       )
     end
   end
@@ -51,12 +52,13 @@ def get_flavor(flavor_uid, availability_zone_uid)
   end
 
   def instance_flavor(flavor)
-    persister.flavors.find_or_build(flavor.name).assign_attributes(
-      :name        => flavor.name,
+    persister.flavors.build(
+      :cpus        => flavor.guest_cpus,
       :description => flavor.description,
+      :ems_ref     => flavor.name,
       :enabled     => !flavor.deprecated,
-      :cpus        => flavor.guest_cpus,
-      :memory      => flavor.memory_mb * 1.megabyte
+      :memory      => flavor.memory_mb * 1.megabyte,
+      :name        => flavor.name
     )
   end
 
@@ -65,11 +67,12 @@ def cloud_volumes
     collector.cloud_volumes.each do |cloud_volume|
       zone_id = parse_uid_from_url(cloud_volume.zone)
 
-      persister.cloud_volumes.find_or_build(cloud_volume.id.to_s).assign_attributes(
+      persister.cloud_volumes.build(
         :availability_zone => persister.availability_zones.lazy_find(zone_id),
         :base_snapshot     => persister.cloud_volume_snapshots.lazy_find(cloud_volume.source_snapshot),
         :creation_time     => cloud_volume.creation_timestamp,
         :description       => cloud_volume.description,
+        :ems_ref           => cloud_volume.id.to_s,
         :name              => cloud_volume.name,
         :size              => cloud_volume.size_gb.to_i.gigabyte,
         :status            => cloud_volume.status,
@@ -84,9 +87,10 @@ def cloud_volumes
 
   def cloud_volume_snapshots
     collector.cloud_volume_snapshots.each do |snapshot|
-      persister.cloud_volume_snapshots.find_or_build(snapshot.id.to_s).assign_attributes(
+      persister.cloud_volume_snapshots.build(
         :creation_time => snapshot.creation_timestamp,
         :description   => snapshot.description,
+        :ems_ref       => snapshot.id.to_s,
         :name          => snapshot.name,
         :size          => snapshot.disk_size_gb.to_i.gigabytes,
         :status        => snapshot.status,
@@ -97,8 +101,9 @@ def cloud_volume_snapshots
       uid = image.id.to_s
 
       # TODO: duplicite code
-      persister_miq_template = persister.miq_templates.find_or_build(uid).assign_attributes(
+      persister_miq_template = persister.miq_templates.build(
         :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
+        :ems_ref            => uid,
         :location           => image.self_link,
         :name               => image.name || uid,
         :publicly_available => true,
@@ -115,8 +120,9 @@ def images
     collector.images.each do |image|
       uid = image.id.to_s
 
-      persister_image = persister.miq_templates.find_or_build(uid).assign_attributes(
+      persister_image = persister.miq_templates.build(
         :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
+        :ems_ref            => uid,
         :location           => image.self_link,
         :name               => image.name || uid,
         :publicly_available => true,
@@ -149,9 +155,10 @@ def instances
       availability_zone = persister.availability_zones.lazy_find(zone_uid)
       parent_image_uid = parse_instance_parent_image(instance)
 
-      persister_instance = persister.vms.find_or_build(uid).assign_attributes(
+      persister_instance = persister.vms.build(
         :availability_zone => availability_zone,
         :description       => instance.description,
+        :ems_ref           => uid,
         :flavor            => flavor,
         :location          => "unknown", # TODO: ??? || "unknown"
         :name              => instance.name || uid,
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index 65465d75..fe9083e3 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -251,7 +251,7 @@ def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
           :vm      => (persister.vms.lazy_find(vm_id) if vm_id)
         )
       end
-      persister.load_balancer_pool_member_pools.find_or_build_by(
+      persister.load_balancer_pool_member_pools.build(
         :load_balancer_pool        => persister_load_balancer_pool,
         :load_balancer_pool_member => persister_lb_pool_member
       )

From 79b810f2e6f33aee23a0b8b00ef7644c9509723a Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 22 Aug 2018 17:51:07 +0200
Subject: [PATCH 3/9] FirewallRule's manager_ref includes name

Name is needed to unique identification
---
 .../manageiq/providers/google/builder.rb      |  1 -
 .../google/inventory/parser/cloud_manager.rb  |  2 --
 .../inventory/parser/network_manager.rb       | 36 +++++++++----------
 .../definitions/network_collections.rb        |  5 ++-
 .../google/cloud_manager/refresher_spec.rb    |  4 +--
 5 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/app/models/manageiq/providers/google/builder.rb b/app/models/manageiq/providers/google/builder.rb
index 0cecabe1..2818ce09 100644
--- a/app/models/manageiq/providers/google/builder.rb
+++ b/app/models/manageiq/providers/google/builder.rb
@@ -15,7 +15,6 @@ def build_inventory(ems, target)
     private
 
     def inventory(manager, raw_target, collector_class, persister_class, parsers_classes)
-      # binding.pry
       collector = collector_class.new(manager, raw_target)
       persister = persister_class.new(manager, raw_target)
 
diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index c110ecfe..bf435c7a 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -15,7 +15,6 @@ def parse
     log_header = "MIQ(#{self.class.name}.#{__method__}) Collecting data for EMS name: [#{collector.manager.name}] id: [#{collector.manager.id}]"
     _log.info("#{log_header}...")
 
-    # binding.pry
     availability_zones
     flavors
     cloud_volumes
@@ -62,7 +61,6 @@ def instance_flavor(flavor)
     )
   end
 
-  # TODO: where is cloud_tenant assoc?
   def cloud_volumes
     collector.cloud_volumes.each do |cloud_volume|
       zone_id = parse_uid_from_url(cloud_volume.zone)
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index fe9083e3..7f95b75c 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -22,14 +22,11 @@ def parse
     log_header = "MIQ(#{self.class.name}.#{__method__}) Collecting data for EMS name: [#{collector.manager.name}] id: [#{collector.manager.id}]"
     _log.info("#{log_header}...")
 
-    # binding.pry
     cloud_networks
-    security_groups
+    # security_groups
     network_ports
     floating_ips
     load_balancers
-    # load_balancer_listeners
-    # load_balancer_health_checks
 
     _log.info("#{log_header}...Complete")
   end
@@ -37,6 +34,8 @@ def parse
   private
 
   def cloud_networks
+    firewalls = collector.firewalls
+
     collector.cloud_networks.each do |network|
       persister_cloud_network = persister.cloud_networks.build(
         :cidr    => network.ipv4_range,
@@ -47,6 +46,7 @@ def cloud_networks
       )
 
       cloud_subnets(persister_cloud_network, network)
+      security_group(persister_cloud_network, firewalls)
     end
   end
 
@@ -123,24 +123,20 @@ def floating_ips
     end
   end
 
-  def security_groups
-    firewalls = collector.firewalls
-
-    persister.cloud_networks.data.each do |cloud_network|
-      uid = cloud_network.name
-      persister_security_group = persister.security_groups.build(
-        :cloud_network => cloud_network,
-        :ems_ref       => uid,
-        :name          => uid
-      )
-
-      network_firewalls = firewalls.select do |firewall|
-        parse_uid_from_url(firewall.network) == cloud_network.name
-      end
+  def security_group(persister_cloud_network, firewalls)
+    uid = persister_cloud_network.name
+    persister_security_group = persister.security_groups.build(
+      :cloud_network => persister_cloud_network,
+      :ems_ref       => uid,
+      :name          => uid
+    )
 
-      # Build firewall rules
-      firewall_rules(persister_security_group, network_firewalls)
+    network_firewalls = firewalls.select do |firewall|
+      parse_uid_from_url(firewall.network) == persister_cloud_network.name
     end
+
+    # Build firewall rules
+    firewall_rules(persister_security_group, network_firewalls)
   end
 
   def firewall_rules(persister_security_group, firewalls)
diff --git a/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb b/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
index 66e986a2..3f96f862 100644
--- a/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
+++ b/app/models/manageiq/providers/google/inventory/persister/definitions/network_collections.rb
@@ -35,7 +35,10 @@ def add_cloud_subnet_network_ports
 
   def add_firewall_rules
     add_collection(network, :firewall_rules) do |builder|
-      builder.add_properties(:manager_ref_allowed_nil => %i(source_security_group))
+      builder.add_properties(
+        :manager_ref             => %i(name resource source_security_group direction host_protocol port end_port source_ip_range),
+        :manager_ref_allowed_nil => %i(source_security_group)
+      )
     end
   end
 
diff --git a/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb b/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
index 20081689..76471ef7 100644
--- a/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
+++ b/spec/models/manageiq/providers/google/cloud_manager/refresher_spec.rb
@@ -41,7 +41,6 @@
     @ems = FactoryGirl.create(:ems_google_with_vcr_authentication)
   end
 
-  # firewall_rule?
   MODELS = %i(
     availability_zone cloud_network cloud_subnet disk ext_management_system firewall_rule flavor floating_ip
     guest_device hardware load_balancer load_balancer_health_check load_balancer_health_check_member
@@ -59,7 +58,6 @@
         EmsRefresh.refresh(@ems.network_manager)
       end
       @ems.reload
-
       assert_table_counts
       assert_ems
       assert_specific_availability_zone
@@ -93,7 +91,7 @@ def expected_table_counts
       :cloud_subnet                      => 18,
       :disk                              => 15, # same as :vm and :hardware
       :ext_management_system             => 2,
-      :firewall_rule                     => 19, # TODO: (old refresh doesn't meet it (first time == 20 ,second time == 19)), new refresh different
+      :firewall_rule                     => 20, # TODO: (old refresh doesn't meet it (first time == 20 ,second time == 19))
       :flavor                            => 29,
       :floating_ip                       => 13,
       :guest_device                      => 0,

From ad4bfca3f551c449eae36c8640286a95d559e4b3 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 24 Aug 2018 09:55:11 +0200
Subject: [PATCH 4/9] InventoryCollection advanced_settings =>
 vms_and_templates_advanced_settings

---
 .../providers/google/inventory/parser/cloud_manager.rb         | 2 +-
 .../providers/google/inventory/parser/network_manager.rb       | 1 -
 .../inventory/persister/definitions/cloud_collections.rb       | 3 ++-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index bf435c7a..be7049b6 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -217,7 +217,7 @@ def instance_hardware(persister_instance, instance, series)
   end
 
   def instance_advanced_settings(persister_instance, instance)
-    persister.advanced_settings.build(
+    persister.vms_and_templates_advanced_settings.build(
       :resource     => persister_instance, # manager_ref
       :name         => "preemptible?",
       :display_name => N_("Is VM Preemptible"),
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index 7f95b75c..216b364f 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -23,7 +23,6 @@ def parse
     _log.info("#{log_header}...")
 
     cloud_networks
-    # security_groups
     network_ports
     floating_ips
     load_balancers
diff --git a/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb b/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
index 4a59b6f1..1c389bde 100644
--- a/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
+++ b/app/models/manageiq/providers/google/inventory/persister/definitions/cloud_collections.rb
@@ -64,9 +64,10 @@ def add_key_pairs
 
   # advanced_settings for VMs
   def add_advanced_settings
-    add_collection(cloud, :advanced_settings) do |builder|
+    add_collection(cloud, :vms_and_templates_advanced_settings) do |builder|
       builder.add_properties(
         :manager_ref                  => %i(resource),
+        :model_class                  => ::AdvancedSetting,
         :parent_inventory_collections => %i(vms)
       )
     end

From c5f401778a69abf8f05b3e0852877b54e1e8e373 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 24 Aug 2018 14:46:55 +0200
Subject: [PATCH 5/9] Cloud parser,collector refactoring

Also enabling network refresh with inventory in settings
---
 .../inventory/collector/cloud_manager.rb      |  50 +----
 .../google/inventory/parser/cloud_manager.rb  | 192 ++++++++++--------
 config/settings.yml                           |   2 +-
 3 files changed, 118 insertions(+), 126 deletions(-)

diff --git a/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
index 3b4ddaf9..8d71b5c1 100644
--- a/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/collector/cloud_manager.rb
@@ -31,51 +31,11 @@ def instances
     connection.servers.all
   end
 
-  def key_pairs(instances)
-    ssh_keys = []
-
-    # Find all key pairs added directly to GCE instances
-    instances.each do |instance|
-      ssh_keys |= parse_compute_metadata_ssh_keys(instance.metadata)
+  # Used for ssh keys common to all instances in the project
+  def project_instance_metadata
+    if @common_instance_metadata.nil?
+      @common_instance_metadata = connection.projects.get(manager.project).common_instance_metadata
     end
-
-    # Add ssh keys that are common to all instances in the project
-    project_common_metadata = connection.projects.get(manager.project).common_instance_metadata
-    @project_key_pairs      = parse_compute_metadata_ssh_keys(project_common_metadata)
-
-    ssh_keys |= @project_key_pairs
-    ssh_keys
-  end
-
-  def parse_compute_metadata(metadata, key)
-    metadata_item = metadata[:items].to_a.detect { |x| x[:key] == key }
-    metadata_item.to_h[:value]
-  end
-
-  def parse_compute_metadata_ssh_keys(metadata)
-    require 'sshkey'
-
-    ssh_keys = []
-
-    # Find the sshKeys property in the instance metadata
-    metadata_ssh_keys = parse_compute_metadata(metadata, "sshKeys")
-
-    metadata_ssh_keys.to_s.split("\n").each do |ssh_key|
-      # Google returns the key in the form username:public_key
-      name, public_key = ssh_key.split(":", 2)
-      begin
-        fingerprint = SSHKey.sha1_fingerprint(public_key)
-
-        ssh_keys << {
-          :name        => name,
-          :public_key  => public_key,
-          :fingerprint => fingerprint
-        }
-      rescue => err
-        _log.warn("Failed to parse public key #{name}: #{err}")
-      end
-    end
-
-    ssh_keys
+    @common_instance_metadata
   end
 end
diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index be7049b6..621f1031 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -3,12 +3,9 @@ class ManageIQ::Providers::Google::Inventory::Parser::CloudManager < ManageIQ::P
 
   def initialize
     super
-    # Mapping from disk url to source image id.
-    @disk_to_source_image_id = {}
-    # Mapping from disk url to disk uid
-    @disk_to_id = {}
 
-    @project_key_pairs = Set.new
+    @cloud_volume_url_to_source_image_id = {}
+    @cloud_volume_url_to_id = {}
   end
 
   def parse
@@ -16,14 +13,32 @@ def parse
     _log.info("#{log_header}...")
 
     availability_zones
+
     flavors
+
+    key_pairs
+
     cloud_volumes
-    cloud_volume_snapshots
 
-    images do |image, inventory_object|
-      image_os(inventory_object, image)
+    cloud_volume_snapshots do |persister_miq_template, snapshot|
+      image_os(persister_miq_template, snapshot)
+    end
+
+    images do |persister_miq_template, image|
+      image_os(persister_miq_template, image)
+    end
+
+    instances do |persister_instance, parent_image_uid, flavor, instance|
+      instance_os(persister_instance, parent_image_uid)
+
+      instance_hardware(persister_instance, flavor) do |persister_hardware|
+        hardware_disks(persister_hardware, instance)
+      end
+
+      instance_key_pairs(persister_instance, instance)
+
+      instance_advanced_settings(persister_instance, instance)
     end
-    instances
 
     _log.info("#{log_header}...Complete")
   end
@@ -41,16 +56,16 @@ def availability_zones
 
   def flavors
     collector.flavors.each do |flavor|
-      instance_flavor(flavor)
+      flavor(flavor)
     end
   end
 
-  def get_flavor(flavor_uid, availability_zone_uid)
+  def flavor_by_uid_and_zone_uid(flavor_uid, availability_zone_uid)
     flavor = collector.flavor(flavor_uid, availability_zone_uid)
-    instance_flavor(flavor)
+    flavor(flavor)
   end
 
-  def instance_flavor(flavor)
+  def flavor(flavor)
     persister.flavors.build(
       :cpus        => flavor.guest_cpus,
       :description => flavor.description,
@@ -78,8 +93,8 @@ def cloud_volumes
       )
 
       # Take note of the source_image_id so we can expose it in parse_instance
-      @disk_to_source_image_id[cloud_volume.self_link] = cloud_volume.source_image_id
-      @disk_to_id[cloud_volume.self_link] = cloud_volume.id.to_s
+      @cloud_volume_url_to_source_image_id[cloud_volume.self_link] = cloud_volume.source_image_id
+      @cloud_volume_url_to_id[cloud_volume.self_link] = cloud_volume.id.to_s
     end
   end
 
@@ -95,70 +110,58 @@ def cloud_volume_snapshots
         :cloud_volume  => persister.cloud_volumes.lazy_find(snapshot.source_disk)
       )
 
-      image = snapshot
-      uid = image.id.to_s
-
-      # TODO: duplicite code
-      persister_miq_template = persister.miq_templates.build(
-        :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
-        :ems_ref            => uid,
-        :location           => image.self_link,
-        :name               => image.name || uid,
-        :publicly_available => true,
-        :raw_power_state    => "never",
-        :template           => true,
-        :uid_ems            => uid,
-        :vendor             => VENDOR_GOOGLE
-      )
-      image_os(persister_miq_template, image)
+      persister_miq_template = image(snapshot)
+
+      yield persister_miq_template, snapshot
     end
   end
 
   def images
     collector.images.each do |image|
-      uid = image.id.to_s
-
-      persister_image = persister.miq_templates.build(
-        :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
-        :ems_ref            => uid,
-        :location           => image.self_link,
-        :name               => image.name || uid,
-        :publicly_available => true,
-        :raw_power_state    => "never",
-        :template           => true,
-        :uid_ems            => uid,
-        :vendor             => VENDOR_GOOGLE
-      )
-      yield image, persister_image
+      persister_miq_template = image(image)
+
+      yield(persister_miq_template, image)
     end
   end
 
-  def instances
-    instances = collector.instances
-
-    key_pairs(instances) # TODO: merge with loop below
+  def image(image)
+    uid = image.id.to_s
+
+    persister.miq_templates.build(
+      :deprecated         => image.kind == "compute#image" ? !image.deprecated.nil? : false,
+      :ems_ref            => uid,
+      :location           => image.self_link,
+      :name               => image.name || uid,
+      :publicly_available => true,
+      :raw_power_state    => "never",
+      :template           => true,
+      :uid_ems            => uid,
+      :vendor             => VENDOR_GOOGLE
+    )
+  end
 
-    instances.each do |instance|
+  def instances
+    collector.instances.each do |instance|
       uid = instance.id.to_s
 
       flavor_uid = parse_uid_from_url(instance.machine_type)
       zone_uid = parse_uid_from_url(instance.zone)
 
+      # TODO(mslemr) if possible, change to lazy_find (now result needed immediately)
       flavor = persister.flavors.find(flavor_uid)
 
       # If the flavor isn't found in our index, check if it is a custom flavor
       # that we have to get directly
-      flavor = get_flavor(flavor_uid, zone_uid) if flavor.nil?
+      flavor = flavor_by_uid_and_zone_uid(flavor_uid, zone_uid) if flavor.nil?
 
-      availability_zone = persister.availability_zones.lazy_find(zone_uid)
       parent_image_uid = parse_instance_parent_image(instance)
 
       persister_instance = persister.vms.build(
-        :availability_zone => availability_zone,
+        :availability_zone => persister.availability_zones.lazy_find(zone_uid),
         :description       => instance.description,
         :ems_ref           => uid,
         :flavor            => flavor,
-        :location          => "unknown", # TODO: ??? || "unknown"
+        :location          => "unknown", # TODO(mslemr) instance.self_link,
         :name              => instance.name || uid,
         :genealogy_parent  => persister.miq_templates.lazy_find(parent_image_uid),
         :raw_power_state   => instance.status,
@@ -166,16 +169,13 @@ def instances
         :vendor            => VENDOR_GOOGLE,
       )
 
-      instance_os(persister_instance, parent_image_uid)
-      instance_hardware(persister_instance, instance, flavor)
-      instance_key_pairs(persister_instance, instance)
-      instance_advanced_settings(persister_instance, instance)
-      #
+      yield persister_instance, parent_image_uid, flavor, instance
     end
   end
 
-  def key_pairs(instances)
-    collector.key_pairs(instances).each do |ssh_key|
+  # Adding global key-pairs (needed when instances count == 0)
+  def key_pairs
+    project_key_pairs.each do |ssh_key|
       persister.key_pairs.build(
         :name        => ssh_key[:name],
         :fingerprint => ssh_key[:fingerprint]
@@ -183,37 +183,28 @@ def key_pairs(instances)
     end
   end
 
-  # TODO: collector.parse - refactoring needed
   def instance_key_pairs(persister_instance, instance)
     # Add project common ssh-keys with keys specific to this instance
-    instance_ssh_keys = collector.parse_compute_metadata_ssh_keys(instance.metadata) | collector.project_key_pairs
+    instance_ssh_keys = project_key_pairs | parse_compute_metadata_ssh_keys(instance.metadata)
+
     instance_ssh_keys.each do |ssh_key|
-      # select existing key pairs
-      # existing_key_pairs = persister.key_pairs.data.select do |kp|
-      #   kp.name == ssh_key[:name] && kp.fingerprint == ssh_key[:fingerprint]
-      # end
-
-      key_pair = persister.key_pairs.find(:name        => ssh_key[:name],
-                                          :fingerprint => ssh_key[:fingerprint])
-      if key_pair.nil?
-        key_pair = persister.key_pairs.build(
-          :name        => ssh_key[:name],       # manager_ref
-          :fingerprint => ssh_key[:fingerprint] # manager_ref
-        )
-      end
+      key_pair = persister.key_pairs.build(
+        :name        => ssh_key[:name],       # manager_ref
+        :fingerprint => ssh_key[:fingerprint] # manager_ref
+      )
 
       key_pair.vms ||= []
       key_pair.vms << persister_instance
     end
   end
 
-  def instance_hardware(persister_instance, instance, series)
+  def instance_hardware(persister_instance, series)
     persister_hardware = persister.hardwares.build(
       :vm_or_template  => persister_instance, # manager_ref
       :cpu_total_cores => series[:cpus],
       :memory_mb       => series[:memory] / 1.megabyte
     )
-    hardware_disks(persister_hardware, instance)
+    yield persister_hardware
   end
 
   def instance_advanced_settings(persister_instance, instance)
@@ -255,7 +246,7 @@ def instance_os(persister_vm, parent_image_uid)
   def hardware_disks(persister_hardware, instance)
     instance.disks.each do |attached_disk|
       # TODO(mslemr): can't be better solution?
-      cloud_volume_ems_ref = @disk_to_id[attached_disk[:source]]
+      cloud_volume_ems_ref = @cloud_volume_url_to_id[attached_disk[:source]]
       persister_cloud_volume = persister.cloud_volumes.find(cloud_volume_ems_ref)
       next if persister_cloud_volume.nil?
 
@@ -288,7 +279,7 @@ def parse_instance_parent_image(instance)
     parent_image_uid = nil
 
     instance.disks.each do |disk|
-      parent_image_uid = @disk_to_source_image_id[disk[:source]]
+      parent_image_uid = @cloud_volume_url_to_source_image_id[disk[:source]]
       next if parent_image_uid.nil?
       break
     end
@@ -300,4 +291,45 @@ def parse_instance_parent_image(instance)
   def key_pairs_ems_ref(ssh_key)
     "#{ssh_key[:name]}:#{ssh_key[:fingerprint]}"
   end
+
+  # Ssh keys that are common to all instances in the project
+  def project_key_pairs
+    if @project_key_pairs.nil?
+      project_common_metadata = collector.project_instance_metadata
+      @project_key_pairs      = parse_compute_metadata_ssh_keys(project_common_metadata)
+    end
+    @project_key_pairs
+  end
+
+  def parse_compute_metadata(metadata, key)
+    metadata_item = metadata[:items].to_a.detect { |x| x[:key] == key }
+    metadata_item.to_h[:value]
+  end
+
+  def parse_compute_metadata_ssh_keys(metadata)
+    require 'sshkey'
+
+    ssh_keys = []
+
+    # Find the sshKeys property in the instance metadata
+    metadata_ssh_keys = parse_compute_metadata(metadata, "sshKeys")
+
+    metadata_ssh_keys.to_s.split("\n").each do |ssh_key|
+      # Google returns the key in the form username:public_key
+      name, public_key = ssh_key.split(":", 2)
+      begin
+        fingerprint = SSHKey.sha1_fingerprint(public_key)
+
+        ssh_keys << {
+          :name        => name,
+          :public_key  => public_key,
+          :fingerprint => fingerprint
+        }
+      rescue => err
+        _log.warn("Failed to parse public key #{name}: #{err}")
+      end
+    end
+
+    ssh_keys
+  end
 end
diff --git a/config/settings.yml b/config/settings.yml
index 37edb072..50c16a10 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -10,7 +10,7 @@
     :inventory_collections:
       :saver_strategy: default
   :gce_network:
-    :inventory_object_refresh: false
+    :inventory_object_refresh: true
     :inventory_collections:
       :saver_strategy: default
 :http_proxy:

From a28934b32b669b2d8a7769e02fd6ee464ed7d066 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 24 Aug 2018 15:54:44 +0200
Subject: [PATCH 6/9] Reducing direct .find in network parser

Replacing with lazy_find
---
 .../providers/google/inventory/parser/network_manager.rb   | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index 216b364f..81b0786a 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -108,10 +108,9 @@ def floating_ips
       )
     end
     collector.floating_ips(:assigned).each do |ip|
-      network_port = persister.network_ports.find(ip[:fixed_ip])
-      if network_port.present?
-        vm = persister.vms.lazy_find(network_port.device_ref)
-      end
+      network_port = persister.network_ports.lazy_find(ip[:fixed_ip])
+      vm = persister.network_ports.lazy_find(ip[:fixed_ip], :key => :device)
+
       persister.floating_ips.build(
         :address          => ip[:external_ip],
         :ems_ref          => ip[:external_ip],

From 0dbef4281d5b4a1598b3dcda4ce198ce469b27bc Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Mon, 27 Aug 2018 14:42:21 +0200
Subject: [PATCH 7/9] Inventory Builder inherits from core

And collector,parser,persister follows core renaming
---
 .../manageiq/providers/google/builder.rb      | 27 ++++---------------
 .../providers/google/inventory/collector.rb   |  2 +-
 .../providers/google/inventory/parser.rb      |  2 +-
 .../providers/google/inventory/persister.rb   |  2 +-
 4 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/app/models/manageiq/providers/google/builder.rb b/app/models/manageiq/providers/google/builder.rb
index 2818ce09..dd539aaa 100644
--- a/app/models/manageiq/providers/google/builder.rb
+++ b/app/models/manageiq/providers/google/builder.rb
@@ -1,28 +1,11 @@
-class ManageIQ::Providers::Google::Builder
+class ManageIQ::Providers::Google::Builder < ManageIQ::Providers::Inventory::Builder
   class << self
-    def build_inventory(ems, target)
-      manager_type = ManageIQ::Providers::Inflector.manager_type(target)
-
-      manager_type = 'Cloud' unless %w(Cloud Network).include?(manager_type)
-
-      collector_class = "ManageIQ::Providers::Google::Inventory::Collector::#{manager_type}Manager".safe_constantize
-      persister_class = "ManageIQ::Providers::Google::Inventory::Persister::#{manager_type}Manager".safe_constantize
-      parser_class    = "ManageIQ::Providers::Google::Inventory::Parser::#{manager_type}Manager".safe_constantize
-
-      inventory(ems, target, collector_class, persister_class, [parser_class])
+    def allowed_manager_types
+      %w(Cloud Network)
     end
 
-    private
-
-    def inventory(manager, raw_target, collector_class, persister_class, parsers_classes)
-      collector = collector_class.new(manager, raw_target)
-      persister = persister_class.new(manager, raw_target)
-
-      ::ManageIQ::Providers::Google::Inventory.new(
-        persister,
-        collector,
-        parsers_classes.map(&:new)
-      )
+    def default_manager_type
+      'Cloud'
     end
   end
 end
diff --git a/app/models/manageiq/providers/google/inventory/collector.rb b/app/models/manageiq/providers/google/inventory/collector.rb
index 3b2e131f..4b85ac3e 100644
--- a/app/models/manageiq/providers/google/inventory/collector.rb
+++ b/app/models/manageiq/providers/google/inventory/collector.rb
@@ -1,4 +1,4 @@
-class ManageIQ::Providers::Google::Inventory::Collector < ManagerRefresh::Inventory::Collector
+class ManageIQ::Providers::Google::Inventory::Collector < ManageIQ::Providers::Inventory::Collector
   require_nested :CloudManager
   require_nested :NetworkManager
 
diff --git a/app/models/manageiq/providers/google/inventory/parser.rb b/app/models/manageiq/providers/google/inventory/parser.rb
index 58f4bf7f..e1b4d0cc 100644
--- a/app/models/manageiq/providers/google/inventory/parser.rb
+++ b/app/models/manageiq/providers/google/inventory/parser.rb
@@ -1,4 +1,4 @@
-class ManageIQ::Providers::Google::Inventory::Parser < ManagerRefresh::Inventory::Parser
+class ManageIQ::Providers::Google::Inventory::Parser < ManageIQ::Providers::Inventory::Parser
   require_nested :CloudManager
   require_nested :NetworkManager
 
diff --git a/app/models/manageiq/providers/google/inventory/persister.rb b/app/models/manageiq/providers/google/inventory/persister.rb
index 54fc04a7..9b270e64 100644
--- a/app/models/manageiq/providers/google/inventory/persister.rb
+++ b/app/models/manageiq/providers/google/inventory/persister.rb
@@ -1,4 +1,4 @@
-class ManageIQ::Providers::Google::Inventory::Persister < ManagerRefresh::Inventory::Persister
+class ManageIQ::Providers::Google::Inventory::Persister < ManageIQ::Providers::Inventory::Persister
   require_nested :CloudManager
   require_nested :NetworkManager
 

From ced861dc80115fa7e85082575a8cdc616a42676d Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Tue, 28 Aug 2018 11:34:32 +0200
Subject: [PATCH 8/9] Refresh parsers refactoring and comments

---
 .../inventory/collector/network_manager.rb    |  2 +-
 .../google/inventory/parser/cloud_manager.rb  | 17 ++--
 .../inventory/parser/network_manager.rb       | 77 +++++++++++++------
 3 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/app/models/manageiq/providers/google/inventory/collector/network_manager.rb b/app/models/manageiq/providers/google/inventory/collector/network_manager.rb
index a53bbb9b..97d7b0dd 100644
--- a/app/models/manageiq/providers/google/inventory/collector/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/collector/network_manager.rb
@@ -55,7 +55,7 @@ def floating_ips(status = :assigned)
 
   # for IC firewall_rules
   def firewalls
-    connection.firewalls.all
+    @firewalls ||= connection.firewalls.all
   end
 
   # for IC load_balancers
diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index 621f1031..be68851a 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -13,11 +13,8 @@ def parse
     _log.info("#{log_header}...")
 
     availability_zones
-
     flavors
-
     key_pairs
-
     cloud_volumes
 
     cloud_volume_snapshots do |persister_miq_template, snapshot|
@@ -36,7 +33,6 @@ def parse
       end
 
       instance_key_pairs(persister_instance, instance)
-
       instance_advanced_settings(persister_instance, instance)
     end
 
@@ -183,6 +179,8 @@ def key_pairs
     end
   end
 
+  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param instance [Fog::Compute::Google::Server]
   def instance_key_pairs(persister_instance, instance)
     # Add project common ssh-keys with keys specific to this instance
     instance_ssh_keys = project_key_pairs | parse_compute_metadata_ssh_keys(instance.metadata)
@@ -198,6 +196,8 @@ def instance_key_pairs(persister_instance, instance)
     end
   end
 
+  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param series [InventoryObject<ManageIQ::Providers::Google::CloudManager::Flavor>]
   def instance_hardware(persister_instance, series)
     persister_hardware = persister.hardwares.build(
       :vm_or_template  => persister_instance, # manager_ref
@@ -207,6 +207,8 @@ def instance_hardware(persister_instance, series)
     yield persister_hardware
   end
 
+  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param instance [Fog::Compute::Google::Server]
   def instance_advanced_settings(persister_instance, instance)
     persister.vms_and_templates_advanced_settings.build(
       :resource     => persister_instance, # manager_ref
@@ -219,7 +221,8 @@ def instance_advanced_settings(persister_instance, instance)
     )
   end
 
-  # @param persister_template [InventoryObject]
+  # @param persister_template [InventoryObject<ManageIQ::Providers::Google::CloudManager::Template]
+  # @param image [Fog::Compute::Google::Snapshot, Fog::Compute::Google::Image]
   def image_os(persister_template, image)
     persister.operating_systems.build(
       :vm_or_template => persister_template, # manager_ref
@@ -231,7 +234,7 @@ def image_os(persister_template, image)
   # It's name is taken from parent image's OS
   # Note: Not added when parent image not found
   #
-  # @param persister_vm [InventoryObject] -> IC <ManageIQ::Providers::Google::CloudManager::Vm>
+  # @param persister_vm [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
   # @param parent_image_uid [String] UID of vm's template
   def instance_os(persister_vm, parent_image_uid)
     persister.operating_systems.build(
@@ -243,6 +246,8 @@ def instance_os(persister_vm, parent_image_uid)
     )
   end
 
+  # @param persister_hardware [InventoryObject<Hardware>]
+  # @param instance [Fog::Compute::Google::Server]
   def hardware_disks(persister_hardware, instance)
     instance.disks.each do |attached_disk|
       # TODO(mslemr): can't be better solution?
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index 81b0786a..84531f71 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -22,10 +22,34 @@ def parse
     log_header = "MIQ(#{self.class.name}.#{__method__}) Collecting data for EMS name: [#{collector.manager.name}] id: [#{collector.manager.id}]"
     _log.info("#{log_header}...")
 
-    cloud_networks
-    network_ports
+    cloud_networks do |persister_cloud_network, network|
+      cloud_subnets(persister_cloud_network, network)
+
+      security_group(persister_cloud_network) do |persister_security_group, network_firewalls|
+        firewall_rules(persister_security_group, network_firewalls)
+      end
+    end
+
+    network_ports do |persister_network_port, network_port|
+      cloud_subnet_network_port(persister_network_port, network_port)
+    end
+
     floating_ips
-    load_balancers
+
+    load_balancers do |forwarding_rules|
+      load_balancer_pools do |persister_load_balancer_pool, target_pool|
+        load_balancer_pool_members(persister_load_balancer_pool, target_pool)
+        load_balancer_health_check(target_pool) do |persister_load_balancer_health_check|
+          load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+        end
+      end
+
+      forwarding_rules.each do |forwarding_rule|
+        load_balancer_listener(forwarding_rule) do |persister_load_balancer_listener|
+          load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+        end
+      end
+    end
 
     _log.info("#{log_header}...Complete")
   end
@@ -33,8 +57,6 @@ def parse
   private
 
   def cloud_networks
-    firewalls = collector.firewalls
-
     collector.cloud_networks.each do |network|
       persister_cloud_network = persister.cloud_networks.build(
         :cidr    => network.ipv4_range,
@@ -43,12 +65,12 @@ def cloud_networks
         :name    => network.name,
         :status  => "active"
       )
-
-      cloud_subnets(persister_cloud_network, network)
-      security_group(persister_cloud_network, firewalls)
+      yield persister_cloud_network, network
     end
   end
 
+  # @param persister_cloud_network [InventoryObject<ManageIQ::Providers::Google::NetworkManager::CloudNetwork>]
+  # @param network [Fog::Compute::Google::Network]
   def cloud_subnets(persister_cloud_network, network)
     @subnets_by_network_link ||= collector.cloud_subnets.each_with_object({}) { |x, subnets| (subnets[x.network] ||= []) << x }
     @subnets_by_network_link[network.self_link].each do |cloud_subnet|
@@ -80,10 +102,12 @@ def network_ports
       persister_network_port.security_groups ||= []
       persister_network_port.security_groups << persister.security_groups.lazy_find(parse_uid_from_url(network_port[:network]))
 
-      cloud_subnet_network_port(persister_network_port, network_port)
+      yield persister_network_port, network_port
     end
   end
 
+  # @param persister_network_port [InventoryObject<ManageIQ::Providers::Google::MetworkManager::NetworkPort>]
+  # @param network_port [Hash]
   def cloud_subnet_network_port(persister_network_port, network_port)
     subnets_by_link ||= collector.cloud_subnets.each_with_object({}) { |x, subnets| subnets[x.self_link] = x }
 
@@ -121,7 +145,8 @@ def floating_ips
     end
   end
 
-  def security_group(persister_cloud_network, firewalls)
+  # @param persister_cloud_network [InventoryObject<ManageIQ::Providers::Google::NetworkManager::CloudNetwork>]
+  def security_group(persister_cloud_network)
     uid = persister_cloud_network.name
     persister_security_group = persister.security_groups.build(
       :cloud_network => persister_cloud_network,
@@ -129,14 +154,15 @@ def security_group(persister_cloud_network, firewalls)
       :name          => uid
     )
 
-    network_firewalls = firewalls.select do |firewall|
+    network_firewalls = collector.firewalls.select do |firewall|
       parse_uid_from_url(firewall.network) == persister_cloud_network.name
     end
 
-    # Build firewall rules
-    firewall_rules(persister_security_group, network_firewalls)
+    yield persister_security_group, network_firewalls
   end
 
+  # @param persister_security_group [InventoryObject<ManageIQ::Providers::Google::NetworkManager::SecurityGroup>]
+  # @param firewalls [Array<Fog::Compute::Google::Firewall>]
   def firewall_rules(persister_security_group, firewalls)
     firewalls.each do |firewall|
       name = firewall.name
@@ -184,14 +210,11 @@ def load_balancers
       end
     end
 
-    load_balancer_pools
-
-    forwarding_rules.each do |forwarding_rule|
-      load_balancer_listener(persister.load_balancers.lazy_find(forwarding_rule.id.to_s), forwarding_rule)
-    end
+    yield forwarding_rules
   end
 
-  def load_balancer_listener(persister_load_balancer, forwarding_rule)
+  # @param forwarding_rule [Fog::Compute::Google::ForwardingRule]
+  def load_balancer_listener(forwarding_rule)
     # Only TCP/UDP/SCTP forwarding rules have ports
     has_ports = %w(TCP UDP SCTP).include?(forwarding_rule.ip_protocol)
     port_range = (parse_port_range(forwarding_rule.port_range) if has_ports)
@@ -203,12 +226,14 @@ def load_balancer_listener(persister_load_balancer, forwarding_rule)
       :instance_protocol        => forwarding_rule.ip_protocol,
       :load_balancer_port_range => port_range,
       :instance_port_range      => port_range,
-      :load_balancer            => persister_load_balancer
+      :load_balancer            => persister.load_balancers.lazy_find(forwarding_rule.id.to_s)
     )
 
-    load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+    yield persister_load_balancer_listener
   end
 
+  # @param persister_load_balancer_listener [InventoryObject<ManageIQ::Providers::google::NetworkManager::LoadBalancerListener]
+  # @param forwarding_rule [Fog::Compute::Google::ForwardingRule]
   def load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
     persister_lb_listener_pool = persister.load_balancer_listener_pools.build(
       :load_balancer_listener => persister_load_balancer_listener,
@@ -228,11 +253,12 @@ def load_balancer_pools
 
       @target_pool_index[target_pool.self_link] = persister_load_balancer_pool
 
-      load_balancer_pool_members(persister_load_balancer_pool, target_pool)
-      load_balancer_health_check(target_pool)
+      yield persister_load_balancer_pool, target_pool
     end
   end
 
+  # @param persister_load_balancer_pool [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerPool>]
+  # @param target_pool [Fog::Compute::Google::TargetPool]
   def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
     target_pool.instances.to_a.each do |member_link|
       persister_lb_pool_member = persister.load_balancer_pool_members.find(Digest::MD5.base64digest(member_link))
@@ -252,6 +278,7 @@ def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
     end
   end
 
+  # @param target_pool [Fog::Compute::Google::TargetPool]
   def load_balancer_health_check(target_pool)
     # Target pools aren't required to have health checks
     return if target_pool.health_checks.blank?
@@ -286,10 +313,12 @@ def load_balancer_health_check(target_pool)
           :url_path               => health_check.request_path,
         )
 
-      load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+      yield persister_load_balancer_health_check
     end
   end
 
+  # @param persister_load_balancer_health_check [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerHealthCheck>]
+  # @param target_pool [Fog::Compute::Google::TargetPool]
   def load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
     return if target_pool.instances.blank?
     # First attempt to get the health of the instance

From ed51bfb4c6fc306c93cceee4fc80eb0a7b642af0 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 31 Aug 2018 10:05:38 +0200
Subject: [PATCH 9/9] Removing inventory builder, fixing inventory superclass

---
 .../manageiq/providers/google/builder.rb      | 11 -----
 .../manageiq/providers/google/inventory.rb    |  8 ++-
 .../google/inventory/parser/cloud_manager.rb  | 37 +++++++-------
 .../inventory/parser/network_manager.rb       | 49 ++++++++++---------
 4 files changed, 50 insertions(+), 55 deletions(-)
 delete mode 100644 app/models/manageiq/providers/google/builder.rb

diff --git a/app/models/manageiq/providers/google/builder.rb b/app/models/manageiq/providers/google/builder.rb
deleted file mode 100644
index dd539aaa..00000000
--- a/app/models/manageiq/providers/google/builder.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-class ManageIQ::Providers::Google::Builder < ManageIQ::Providers::Inventory::Builder
-  class << self
-    def allowed_manager_types
-      %w(Cloud Network)
-    end
-
-    def default_manager_type
-      'Cloud'
-    end
-  end
-end
diff --git a/app/models/manageiq/providers/google/inventory.rb b/app/models/manageiq/providers/google/inventory.rb
index b5dd9bf5..187edf21 100644
--- a/app/models/manageiq/providers/google/inventory.rb
+++ b/app/models/manageiq/providers/google/inventory.rb
@@ -1,5 +1,11 @@
-class ManageIQ::Providers::Google::Inventory < ManagerRefresh::Inventory
+class ManageIQ::Providers::Google::Inventory < ManageIQ::Providers::Inventory
   require_nested :Collector
   require_nested :Parser
   require_nested :Persister
+
+  # Default manager for building collector/parser/persister classes
+  # when failed to get class name from refresh target automatically
+  def self.default_manager_name
+    "CloudManager"
+  end
 end
diff --git a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
index be68851a..c3fd7d5e 100644
--- a/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb
@@ -25,15 +25,15 @@ def parse
       image_os(persister_miq_template, image)
     end
 
-    instances do |persister_instance, parent_image_uid, flavor, instance|
-      instance_os(persister_instance, parent_image_uid)
+    instances do |persister_vm, parent_image_uid, flavor, instance|
+      instance_os(persister_vm, parent_image_uid)
 
-      instance_hardware(persister_instance, flavor) do |persister_hardware|
+      instance_hardware(persister_vm, flavor) do |persister_hardware|
         hardware_disks(persister_hardware, instance)
       end
 
-      instance_key_pairs(persister_instance, instance)
-      instance_advanced_settings(persister_instance, instance)
+      instance_key_pairs(persister_vm, instance)
+      instance_advanced_settings(persister_vm, instance)
     end
 
     _log.info("#{log_header}...Complete")
@@ -143,7 +143,7 @@ def instances
       flavor_uid = parse_uid_from_url(instance.machine_type)
       zone_uid = parse_uid_from_url(instance.zone)
 
-      # TODO(mslemr) if possible, change to lazy_find (now result needed immediately)
+      # TODO(mslemr) lazy_find (now result needed immediately)
       flavor = persister.flavors.find(flavor_uid)
 
       # If the flavor isn't found in our index, check if it is a custom flavor
@@ -152,12 +152,12 @@ def instances
 
       parent_image_uid = parse_instance_parent_image(instance)
 
-      persister_instance = persister.vms.build(
+      persister_vm = persister.vms.build(
         :availability_zone => persister.availability_zones.lazy_find(zone_uid),
         :description       => instance.description,
         :ems_ref           => uid,
         :flavor            => flavor,
-        :location          => "unknown", # TODO(mslemr) instance.self_link,
+        :location          => "unknown", # TODO(mslemr) instance.self_link?,
         :name              => instance.name || uid,
         :genealogy_parent  => persister.miq_templates.lazy_find(parent_image_uid),
         :raw_power_state   => instance.status,
@@ -165,7 +165,7 @@ def instances
         :vendor            => VENDOR_GOOGLE,
       )
 
-      yield persister_instance, parent_image_uid, flavor, instance
+      yield persister_vm, parent_image_uid, flavor, instance
     end
   end
 
@@ -179,9 +179,9 @@ def key_pairs
     end
   end
 
-  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param persister_vm [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
   # @param instance [Fog::Compute::Google::Server]
-  def instance_key_pairs(persister_instance, instance)
+  def instance_key_pairs(persister_vm, instance)
     # Add project common ssh-keys with keys specific to this instance
     instance_ssh_keys = project_key_pairs | parse_compute_metadata_ssh_keys(instance.metadata)
 
@@ -192,26 +192,26 @@ def instance_key_pairs(persister_instance, instance)
       )
 
       key_pair.vms ||= []
-      key_pair.vms << persister_instance
+      key_pair.vms << persister_vm
     end
   end
 
-  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param persister_vm [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
   # @param series [InventoryObject<ManageIQ::Providers::Google::CloudManager::Flavor>]
-  def instance_hardware(persister_instance, series)
+  def instance_hardware(persister_vm, series)
     persister_hardware = persister.hardwares.build(
-      :vm_or_template  => persister_instance, # manager_ref
+      :vm_or_template  => persister_vm, # manager_ref
       :cpu_total_cores => series[:cpus],
       :memory_mb       => series[:memory] / 1.megabyte
     )
     yield persister_hardware
   end
 
-  # @param persister_instance [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
+  # @param persister_vm [InventoryObject<ManageIQ::Providers::Google::CloudManager::Vm>]
   # @param instance [Fog::Compute::Google::Server]
-  def instance_advanced_settings(persister_instance, instance)
+  def instance_advanced_settings(persister_vm, instance)
     persister.vms_and_templates_advanced_settings.build(
-      :resource     => persister_instance, # manager_ref
+      :resource     => persister_vm, # manager_ref
       :name         => "preemptible?",
       :display_name => N_("Is VM Preemptible"),
       :description  => N_("Whether or not the VM is 'preemptible'. See"\
@@ -250,7 +250,6 @@ def instance_os(persister_vm, parent_image_uid)
   # @param instance [Fog::Compute::Google::Server]
   def hardware_disks(persister_hardware, instance)
     instance.disks.each do |attached_disk|
-      # TODO(mslemr): can't be better solution?
       cloud_volume_ems_ref = @cloud_volume_url_to_id[attached_disk[:source]]
       persister_cloud_volume = persister.cloud_volumes.find(cloud_volume_ems_ref)
       next if persister_cloud_volume.nil?
diff --git a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
index 84531f71..0dec6a93 100644
--- a/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
+++ b/app/models/manageiq/providers/google/inventory/parser/network_manager.rb
@@ -37,16 +37,17 @@ def parse
     floating_ips
 
     load_balancers do |forwarding_rules|
-      load_balancer_pools do |persister_load_balancer_pool, target_pool|
-        load_balancer_pool_members(persister_load_balancer_pool, target_pool)
-        load_balancer_health_check(target_pool) do |persister_load_balancer_health_check|
-          load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+      load_balancer_pools do |persister_lb_pool, target_pool|
+        load_balancer_pool_members(persister_lb_pool, target_pool)
+
+        load_balancer_health_check(target_pool) do |persister_lb_health_check|
+          load_balancer_health_check_members(persister_lb_health_check, target_pool)
         end
       end
 
       forwarding_rules.each do |forwarding_rule|
-        load_balancer_listener(forwarding_rule) do |persister_load_balancer_listener|
-          load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+        load_balancer_listener(forwarding_rule) do |persister_lb_listener|
+          load_balancer_listener_pool(persister_lb_listener, forwarding_rule)
         end
       end
     end
@@ -219,7 +220,7 @@ def load_balancer_listener(forwarding_rule)
     has_ports = %w(TCP UDP SCTP).include?(forwarding_rule.ip_protocol)
     port_range = (parse_port_range(forwarding_rule.port_range) if has_ports)
 
-    persister_load_balancer_listener = persister.load_balancer_listeners.build(
+    persister_lb_listener = persister.load_balancer_listeners.build(
       :name                     => forwarding_rule.name,
       :ems_ref                  => forwarding_rule.id.to_s,
       :load_balancer_protocol   => forwarding_rule.ip_protocol,
@@ -229,37 +230,37 @@ def load_balancer_listener(forwarding_rule)
       :load_balancer            => persister.load_balancers.lazy_find(forwarding_rule.id.to_s)
     )
 
-    yield persister_load_balancer_listener
+    yield persister_lb_listener
   end
 
-  # @param persister_load_balancer_listener [InventoryObject<ManageIQ::Providers::google::NetworkManager::LoadBalancerListener]
+  # @param persister_lb_listener [InventoryObject<ManageIQ::Providers::google::NetworkManager::LoadBalancerListener]
   # @param forwarding_rule [Fog::Compute::Google::ForwardingRule]
-  def load_balancer_listener_pool(persister_load_balancer_listener, forwarding_rule)
+  def load_balancer_listener_pool(persister_lb_listener, forwarding_rule)
     persister_lb_listener_pool = persister.load_balancer_listener_pools.build(
-      :load_balancer_listener => persister_load_balancer_listener,
+      :load_balancer_listener => persister_lb_listener,
       :load_balancer_pool     => @target_pool_index[forwarding_rule.target]
     )
 
-    persister_load_balancer_listener.load_balancer_listener_pools ||= []
-    persister_load_balancer_listener.load_balancer_listener_pools << persister_lb_listener_pool
+    persister_lb_listener.load_balancer_listener_pools ||= []
+    persister_lb_listener.load_balancer_listener_pools << persister_lb_listener_pool
   end
 
   def load_balancer_pools
     collector.target_pools.each do |target_pool|
-      persister_load_balancer_pool = persister.load_balancer_pools.build(
+      persister_lb_pool = persister.load_balancer_pools.build(
         :ems_ref => target_pool.id.to_s,
         :name    => target_pool.name
       )
 
-      @target_pool_index[target_pool.self_link] = persister_load_balancer_pool
+      @target_pool_index[target_pool.self_link] = persister_lb_pool
 
-      yield persister_load_balancer_pool, target_pool
+      yield persister_lb_pool, target_pool
     end
   end
 
-  # @param persister_load_balancer_pool [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerPool>]
+  # @param persister_lb_pool [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerPool>]
   # @param target_pool [Fog::Compute::Google::TargetPool]
-  def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
+  def load_balancer_pool_members(persister_lb_pool, target_pool)
     target_pool.instances.to_a.each do |member_link|
       persister_lb_pool_member = persister.load_balancer_pool_members.find(Digest::MD5.base64digest(member_link))
 
@@ -272,7 +273,7 @@ def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
         )
       end
       persister.load_balancer_pool_member_pools.build(
-        :load_balancer_pool        => persister_load_balancer_pool,
+        :load_balancer_pool        => persister_lb_pool,
         :load_balancer_pool_member => persister_lb_pool_member
       )
     end
@@ -298,7 +299,7 @@ def load_balancer_health_check(target_pool)
       # return nil if load_balancer_listener.nil? #TODO: return inside collect?
 
       uid = "#{persister_load_balancer.ems_ref}_#{target_pool.id}_#{health_check.id}"
-      persister_load_balancer_health_check = \
+      persister_lb_health_check = \
         persister.load_balancer_health_checks.build(
           :ems_ref                => uid,
           :healthy_threshold      => health_check.healthy_threshold,
@@ -313,13 +314,13 @@ def load_balancer_health_check(target_pool)
           :url_path               => health_check.request_path,
         )
 
-      yield persister_load_balancer_health_check
+      yield persister_lb_health_check
     end
   end
 
-  # @param persister_load_balancer_health_check [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerHealthCheck>]
+  # @param persister_lb_health_check [InventoryObject<ManageIQ::Providers::Google::NetworkManager::LoadBalancerHealthCheck>]
   # @param target_pool [Fog::Compute::Google::TargetPool]
-  def load_balancer_health_check_members(persister_load_balancer_health_check, target_pool)
+  def load_balancer_health_check_members(persister_lb_health_check, target_pool)
     return if target_pool.instances.blank?
     # First attempt to get the health of the instance
     # Due to a bug in fog, there's no way to get the health of an individual
@@ -345,7 +346,7 @@ def load_balancer_health_check_members(persister_load_balancer_health_check, tar
       end
 
       persister.load_balancer_health_check_members.build(
-        :load_balancer_health_check => persister_load_balancer_health_check,
+        :load_balancer_health_check => persister_lb_health_check,
         :load_balancer_pool_member  => member,
         :status                     => status,
         :status_reason              => ""