Run rubocop fixups over Openstack targeted refresh code

app/models/manageiq/providers/openstack/cloud_manager/refresher.rb

@@ -6,12 +6,12 @@ def collect_inventory_for_targets(ems, targets)
       targets_with_data = targets.collect do |target|
         target_name = target.try(:name) || target.try(:event_type)
 
-        _log.info "Filtering inventory for #{target.class} [#{target_name}] id: [#{target.id}]..."
+        _log.info("Filtering inventory for #{target.class} [#{target_name}] id: [#{target.id}]...")
         if ::Settings.ems.ems_openstack.try(:refresh).try(:inventory_object_refresh)
           inventory = ManageIQ::Providers::Openstack::Builder.build_inventory(ems, target)
         end
 
-        _log.info "Filtering inventory...Complete"
+        _log.info("Filtering inventory...Complete")
         [target, inventory]
       end
 
@@ -31,15 +31,15 @@ def save_inventory(ems, target, inventory_collections)
 
     def parse_targeted_inventory(ems, _target, inventory)
       log_header = format_ems_for_logging(ems)
-      _log.debug "#{log_header} Parsing inventory..."
+      _log.debug("#{log_header} Parsing inventory...")
       hashes, = Benchmark.realtime_block(:parse_inventory) do
         if ::Settings.ems.ems_openstack.try(:refresh).try(:inventory_object_refresh)
           inventory.inventory_collections
         else
           ManageIQ::Providers::Openstack::CloudManager::RefreshParser.ems_inv_to_hashes(ems, refresher_options)
         end
       end
-      _log.debug "#{log_header} Parsing inventory...Complete"
+      _log.debug("#{log_header} Parsing inventory...Complete")
 
       hashes
     end
@@ -49,7 +49,7 @@ def preprocess_targets
         if targets.any? { |t| t.kind_of?(ExtManagementSystem) }
           ems             = @ems_by_ems_id[ems_id]
           targets_for_log = targets.map { |t| "#{t.class} [#{t.name}] id [#{t.id}] " }
-          _log.info "Defaulting to full refresh for EMS: [#{ems.name}], id: [#{ems.id}], from targets: #{targets_for_log}" if targets.length > 1
+          _log.info("Defaulting to full refresh for EMS: [#{ems.name}], id: [#{ems.id}], from targets: #{targets_for_log}") if targets.length > 1
         end
 
         # We want all targets of class EmsEvent to be merged into one target,

app/models/manageiq/providers/openstack/inventory/collector/target_collection.rb

@@ -180,7 +180,7 @@ def infer_related_vm_ems_refs_db!
 
       all_stacks.collect(&:ems_ref).compact.each { |ems_ref| add_simple_target!(:orchestration_stacks, ems_ref) }
       vm.cloud_networks.collect(&:ems_ref).compact.each { |ems_ref| add_simple_target!(:cloud_networks, ems_ref) }
-      vm.floating_ips.collect(&:ems_ref).compact.each { |address| add_simple_target!(:floating_ips, ems_ref) }
+      vm.floating_ips.collect(&:ems_ref).compact.each { |_address| add_simple_target!(:floating_ips, ems_ref) }
       vm.network_ports.collect(&:ems_ref).compact.each do |ems_ref|
         add_simple_target!(:network_ports, ems_ref)
       end

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

@@ -59,11 +59,11 @@ def cloud_tenants
       tenant.description = t.description
       tenant.enabled = t.enabled
       tenant.ems_ref = t.id
-      if t.try(:parent_id).blank?
-        tenant.parent = nil
-      else
-        tenant.parent = persister.cloud_tenants.lazy_find(t.try(:parent_id))
-      end
+      tenant.parent = if t.try(:parent_id).blank?
+                        nil
+                      else
+                        persister.cloud_tenants.lazy_find(t.try(:parent_id))
+                      end
     end
   end
 
@@ -375,9 +375,9 @@ def compose_ems_ref(*keys)
   # Identify whether the given image is publicly available
   def public_image?(image)
     # Glance v1
-    return image.is_public if image.respond_to? :is_public
+    return image.is_public if image.respond_to?(:is_public)
     # Glance v2
-    image.visibility != 'private' if image.respond_to? :visibility
+    image.visibility != 'private' if image.respond_to?(:visibility)
   end
 
   # Identify whether the given image has a 32 or 64 bit architecture
@@ -395,7 +395,7 @@ def parse_image_parent_id(image)
       # What version of openstack is this glance v1 on some old openstack version?
       return image.copy_from["id"] if image.respond_to?(:copy_from) && image.copy_from
       # Glance V2
-      return image.instance_uuid if image.respond_to? :instance_uuid
+      return image.instance_uuid if image.respond_to?(:instance_uuid)
       # Glance V1
       image.properties.try(:[], 'instance_uuid')
     elsif image.server

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

@@ -38,7 +38,7 @@ def initialize_inventory_collections
       ),
       :strategy => strategy,
       :targeted => true,
-      :parent => manager.network_manager
+      :parent   => manager.network_manager
     )
 
     ######## Custom processing of Ancestry ##########

app/models/manageiq/providers/openstack/network_manager/refresher.rb

@@ -6,13 +6,13 @@ def collect_inventory_for_targets(ems, targets)
       targets_with_data = targets.collect do |target|
         target_name = target.try(:name) || target.try(:event_type)
 
-        _log.info "Filtering inventory for #{target.class} [#{target_name}] id: [#{target.id}]..."
+        _log.info("Filtering inventory for #{target.class} [#{target_name}] id: [#{target.id}]...")
 
         if ::Settings.ems.ems_openstack.try(:refresh).try(:inventory_object_refresh)
           inventory = ManageIQ::Providers::Openstack::Builder.build_inventory(ems, target)
         end
 
-        _log.info "Filtering inventory...Complete"
+        _log.info("Filtering inventory...Complete")
         [target, inventory]
       end
 
@@ -25,15 +25,15 @@ def parse_legacy_inventory(ems)
 
     def parse_targeted_inventory(ems, _target, inventory)
       log_header = format_ems_for_logging(ems)
-      _log.debug "#{log_header} Parsing inventory..."
+      _log.debug("#{log_header} Parsing inventory...")
       hashes, = Benchmark.realtime_block(:parse_inventory) do
         if ::Settings.ems.ems_openstack.try(:refresh).try(:inventory_object_refresh)
           inventory.inventory_collections
         else
           ManageIQ::Providers::Openstack::NetworkManager::RefreshParser.ems_inv_to_hashes(ems, refresher_options)
         end
       end
-      _log.debug "#{log_header} Parsing inventory...Complete"
+      _log.debug("#{log_header} Parsing inventory...Complete")
 
       hashes
     end

spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb

@@ -66,7 +66,7 @@ def assert_with_errors
       expect(AvailabilityZone.count).to            eq 1 # just NoZone
 
       # We have broken flavor list, but there is fallback for private flavors using get, which will collect used flavors
-      expect(Flavor.count).to              eq 2
+      expect(Flavor.count).to eq 2
 
       expect(ExtManagementSystem.count).to               eq 4 # Can this be not hardcoded?
       expect(security_groups_without_defaults.count).to  eq security_groups_count
@@ -90,7 +90,6 @@ def assert_with_errors
       expect(Relationship.count).to        be > 0
       # Just check that queue is not empty
       expect(MiqQueue.count).to            be > 0
-
     end
 
     def assert_with_skips
@@ -265,7 +264,6 @@ def assert_table_counts
       expect(MiqQueue.count).to            be > 0
       expect(CloudService.count).to        be > 0
       expect(CloudResourceQuota.count).to  be > 0
-
     end
 
     def assert_table_counts_orchestration
@@ -280,7 +278,7 @@ def assert_table_counts_orchestration
 
     def assert_table_counts_storage
       if storage_supported?
-        volumes_backup = CloudObjectStoreContainer.where({ key: "volumes_backup" })
+        volumes_backup = CloudObjectStoreContainer.where(:key => "volumes_backup")
         expect(CloudObjectStoreContainer.count).to eq storage_data.directories.count + volumes_backup.count
         expect(CloudObjectStoreObject.count).to    eq storage_data.files.count
       end
@@ -310,18 +308,18 @@ def assert_ems
     end
 
     def assert_flavors
-      gigabyte_transformation = -> (x) { x.gigabyte }
+      gigabyte_transformation = ->(x) { x.gigabyte }
       blacklisted_attributes = [:is_public] # TODO(lsmola) model blacklisted attrs
       # Disks are supported from havana and above apparently
       blacklisted_attributes += [:disk, :ephemeral, :swap] if environment_release_number < 4
 
       assert_objects_with_hashes(ManageIQ::Providers::Openstack::CloudManager::Flavor.all,
                                  compute_data.flavors,
                                  compute_data.flavor_translate_table,
-                                 {:ram       => -> (x) { x * 1_024 * 1_024 },
+                                 {:ram       => ->(x) { x * 1_024 * 1_024 },
                                   :disk      => gigabyte_transformation,
                                   :ephemeral => gigabyte_transformation,
-                                  :swap      => -> (x) { x.megabyte }},
+                                  :swap      => ->(x) { x.megabyte }},
                                  blacklisted_attributes)
 
       ManageIQ::Providers::Openstack::CloudManager::Flavor.all.each do |flavor|
@@ -358,7 +356,8 @@ def assert_private_flavor_tenant_mapping
     def assert_specific_az
       # This tests OpenStack functionality more than ManageIQ
       @nova_az = ManageIQ::Providers::Openstack::CloudManager::AvailabilityZone.where(
-        :type => ManageIQ::Providers::Openstack::CloudManager::AvailabilityZone.name, :ems_id => @ems.id).first
+        :type => ManageIQ::Providers::Openstack::CloudManager::AvailabilityZone.name, :ems_id => @ems.id
+      ).first
       # standard openstack AZs have their ems_ref set to their name ("nova" in the test case)...
       # the "null" openstack AZ has a unique ems_ref and name
       expect(@nova_az).to have_attributes(
@@ -444,7 +443,7 @@ def assert_security_group_rules_nova(security_group)
       assert_objects_with_hashes(security_group.firewall_rules,
                                  test_data,
                                  network_data.security_groups_rule_translate_table,
-                                 {:ip_range => -> (x) { x ? x[:cidr] : x }},
+                                 {:ip_range => ->(x) { x ? x[:cidr] : x }},
                                  [:group])
     end
 
@@ -481,15 +480,15 @@ def assert_networks
         assert_objects_with_hashes(network.cloud_subnets,
                                    network_data.subnets(network.name),
                                    network_data.subnet_translate_table,
-                                   {:ip_version => -> (x) { "ipv#{x}" }},
+                                   {:ip_version => ->(x) { "ipv#{x}" }},
                                    [:allocation_pools]) # TODO(lsmola) model blacklisted attrs
 
         if network.external_facing?
           vms = network.private_networks.map { |x| (network_vms(x) + network_stacks(x)) }.flatten.uniq
           expect(network.public_network_vms.count).to eq vms.count
 
           non_stack_vms          = network.public_network_vms.select { |x| x.orchestration_stack.blank? }
-          non_stack_expected_vms = network.private_networks.map { |x| (network_vms(x)) }.flatten.uniq
+          non_stack_expected_vms = network.private_networks.map { |x| network_vms(x) }.flatten.uniq
           expect(non_stack_vms.map(&:name)).to match_array(non_stack_expected_vms.map { |x| x[:name] })
         else
           vms = network_vms(network) + network_stacks(network)
@@ -551,9 +550,9 @@ def assert_subnets
       assert_objects_with_hashes(subnets,
                                  network_data.subnets,
                                  network_data.subnet_translate_table,
-                                 {:ip_version      => -> (x) { "ipv#{x}" },
-                                  :dns_nameservers => -> (x) { x.nil? ? [] : x },
-                                  :enable_dhcp     => -> (x) { x.nil? ? true : x }},
+                                 {:ip_version      => ->(x) { "ipv#{x}" },
+                                  :dns_nameservers => ->(x) { x.nil? ? [] : x },
+                                  :enable_dhcp     => ->(x) { x.nil? ? true : x }},
                                  [:allocation_pools]) # TODO(lsmola) model blacklisted attrs
 
       subnets.each do |subnet|
@@ -595,7 +594,6 @@ def assert_specific_volumes
       # assert_objects_with_hashes(volumes, volume_data.volumes)
     end
 
-
     def assert_specific_directories
       return unless storage_supported?
 
@@ -621,7 +619,7 @@ def assert_templates
       assert_objects_with_hashes(templates,
                                  image_data.images + image_data.servers_snapshots,
                                  image_data.images_translate_table,
-                                 :is_public => -> (x) { x.nil? ? false : x })
+                                 :is_public => ->(x) { x.nil? ? false : x })
     end
 
     def assert_specific_templates
@@ -787,19 +785,19 @@ def assert_specific_vm(vm_name, attributes)
       # TODO(lsmola) the flavor disk data should be stored in Flavor model, getting it from test data now
       flavor_expected = compute_data.flavors.detect { |x| x[:name] == vm.flavor.name }
 
-      disk = vm.hardware.disks.find_by_device_name("Root disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Root disk")
       expect(disk).to have_attributes(
         :device_name => "Root disk",
         :device_type => "disk",
         :size        => flavor_expected[:disk].gigabyte
       )
-      disk = vm.hardware.disks.find_by_device_name("Ephemeral disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Ephemeral disk")
       expect(disk).to have_attributes(
         :device_name => "Ephemeral disk",
         :device_type => "disk",
         :size        => flavor_expected[:ephemeral].gigabyte
       )
-      disk = vm.hardware.disks.find_by_device_name("Swap disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Swap disk")
       expect(disk).to have_attributes(
         :device_name => "Swap disk",
         :device_type => "disk",
@@ -823,7 +821,8 @@ def assert_specific_vm(vm_name, attributes)
     # TODO(lsmola) specific checks below, do we need them?
     def assert_specific_template_created_from_vm
       @snap = ManageIQ::Providers::Openstack::CloudManager::Template.where(
-        :name => "EmsRefreshSpec-PoweredOn-SnapShot").first
+        :name => "EmsRefreshSpec-PoweredOn-SnapShot"
+      ).first
       expect(@snap).not_to be_nil
       # FIXME: @snap.parent.should == @vm
     end
@@ -912,7 +911,7 @@ def assert_targeted_vm(vm_name, attributes)
         expect(vm.cloud_subnets.count).to   be > 0
         expect(vm.cloud_subnets.first).to   be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudSubnet)
 
-        expect(vm.fixed_ip_addresses.count).to    be > 0
+        expect(vm.fixed_ip_addresses.count).to be > 0
         expect(vm.private_networks.map(&:name)).to match_array vm_expected[:__network_names]
       end
 
@@ -933,19 +932,19 @@ def assert_targeted_vm(vm_name, attributes)
       # TODO(lsmola) the flavor disk data should be stored in Flavor model, getting it from test data now
       flavor_expected = compute_data.flavors.detect { |x| x[:name] == vm.flavor.name }
 
-      disk = vm.hardware.disks.find_by_device_name("Root disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Root disk")
       expect(disk).to have_attributes(
         :device_name => "Root disk",
         :device_type => "disk",
         :size        => flavor_expected[:disk].gigabyte
       )
-      disk = vm.hardware.disks.find_by_device_name("Ephemeral disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Ephemeral disk")
       expect(disk).to have_attributes(
         :device_name => "Ephemeral disk",
         :device_type => "disk",
         :size        => flavor_expected[:ephemeral].gigabyte
       )
-      disk = vm.hardware.disks.find_by_device_name("Swap disk")
+      disk = vm.hardware.disks.find_by(:device_name => "Swap disk")
       expect(disk).to have_attributes(
         :device_name => "Swap disk",
         :device_type => "disk",