From 5f25d71e4a17b9118ab73feb72914e8505ef0922 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Mon, 6 Jan 2020 12:17:55 -0500 Subject: [PATCH] change infra targets to be single ems centric remove cop around exclude_storage guard vs next statement --- .../infra_manager/metrics_capture.rb | 67 ++++++++----------- .../infra_manager/metrics_capture_spec.rb | 4 +- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/app/models/manageiq/providers/infra_manager/metrics_capture.rb b/app/models/manageiq/providers/infra_manager/metrics_capture.rb index 6fccd3f77b5..07326030a49 100644 --- a/app/models/manageiq/providers/infra_manager/metrics_capture.rb +++ b/app/models/manageiq/providers/infra_manager/metrics_capture.rb @@ -1,23 +1,16 @@ class ManageIQ::Providers::InfraManager::MetricsCapture < ManageIQ::Providers::BaseManager::MetricsCapture def capture_ems_targets(options = {}) - capture_infra_targets([ems], options) - end - - private - - # If a Cluster, standalone Host, or Storage is not enabled, skip it. - # If a Cluster is enabled, capture all of its Hosts. - # If a Host is enabled, capture all of its Vms. - def capture_infra_targets(emses, options) - load_infra_targets_data(emses, options) - all_hosts = capture_host_targets(emses) + load_infra_targets_data(ems, options) + all_hosts = capture_host_targets(ems) targets = enabled_hosts = only_enabled(all_hosts) targets += capture_storage_targets(all_hosts) unless options[:exclude_storages] - targets += capture_vm_targets(emses, enabled_hosts) + targets += capture_vm_targets(ems, enabled_hosts) targets end + private + # Filter to enabled hosts. If it has a cluster consult that, otherwise consult the host itself. # # NOTE: if capture_storage takes only enabled, then move @@ -33,9 +26,9 @@ def only_enabled(hosts) # tags are needed for determining if it is enabled. # ems is needed for determining queue name # cluster is used for hierarchies - def load_infra_targets_data(emses, options) - MiqPreloader.preload(emses, preload_hash_infra_targets_data(options)) - postload_infra_targets_data(emses, options) + def load_infra_targets_data(ems, options) + MiqPreloader.preload(ems, preload_hash_infra_targets_data(options)) + postload_infra_targets_data(ems, options) end def preload_hash_infra_targets_data(options) @@ -55,34 +48,32 @@ def preload_hash_infra_targets_data(options) # and since we also rely upon tags and clusters, this causes unnecessary data to be downloaded # # so we have introduced this to work around preload not working (and inverse_of) - def postload_infra_targets_data(emses, options) + def postload_infra_targets_data(ems, options) # populate ems (with tags / clusters) - emses.each do |ems| - ems.hosts.each do |host| - host.ems_cluster.association(:ext_management_system).target = ems if host.ems_cluster_id - unless options[:exclude_storages] - host.storages.each do |storage| - storage.ext_management_system = ems - end - end - end - host_ids = ems.hosts.index_by(&:id) - clusters = ems.hosts.flat_map(&:ems_cluster).uniq.compact.index_by(&:id) - ems.vms.each do |vm| - vm.association(:ext_management_system).target = ems if vm.ems_id - vm.association(:ems_cluster).target = clusters[vm.ems_cluster_id] if vm.ems_cluster_id - vm.association(:host).target = host_ids[vm.host_id] if vm.host_id + ems.hosts.each do |host| + host.ems_cluster.association(:ext_management_system).target = ems if host.ems_cluster_id + next if options[:exclude_storages] + + host.storages.each do |storage| + storage.ext_management_system = ems end end + host_ids = ems.hosts.index_by(&:id) + clusters = ems.hosts.flat_map(&:ems_cluster).uniq.compact.index_by(&:id) + ems.vms.each do |vm| + vm.association(:ext_management_system).target = ems if vm.ems_id + vm.association(:ems_cluster).target = clusters[vm.ems_cluster_id] if vm.ems_cluster_id + vm.association(:host).target = host_ids[vm.host_id] if vm.host_id + end end - def capture_host_targets(emses) + def capture_host_targets(ems) # NOTE: if capture_storage_targets takes only enabled hosts # merge only_enabled into this method - emses.flat_map(&:hosts) + ems.hosts end - # @param [Host] all hosts that have an ems + # @param [Array] all hosts that have an ems # NOTE: disabled hosts are passed in. # @return [Array] supported storages # hosts preloaded storages and tags @@ -90,11 +81,11 @@ def capture_storage_targets(hosts) hosts.flat_map(&:storages).uniq.select { |s| Storage.supports?(s.store_type) & s.perf_capture_enabled? } end - # @param [Array] emses Typically 1 ems for this zone - # @param [Host] hosts that are enabled or cluster enabled + # @param [ExtManagementSystem] ems + # @param [Array] hosts that are enabled or cluster enabled # we want to work with only enabled_hosts, so hosts needs to be further filtered - def capture_vm_targets(emses, hosts) + def capture_vm_targets(ems, hosts) enabled_host_ids = hosts.select(&:perf_capture_enabled?).index_by(&:id) - emses.flat_map { |e| e.vms.select { |v| enabled_host_ids.key?(v.host_id) && v.state == 'on' && v.supports_capture? } } + ems.vms.select { |v| enabled_host_ids.key?(v.host_id) && v.state == 'on' && v.supports_capture? } end end diff --git a/spec/models/manageiq/providers/infra_manager/metrics_capture_spec.rb b/spec/models/manageiq/providers/infra_manager/metrics_capture_spec.rb index a82908ce37d..b158461a04d 100644 --- a/spec/models/manageiq/providers/infra_manager/metrics_capture_spec.rb +++ b/spec/models/manageiq/providers/infra_manager/metrics_capture_spec.rb @@ -25,13 +25,13 @@ describe ".capture_ems_targets" do it "finds enabled targets" do - targets = described_class.new(nil, ems).send(:capture_ems_targets) + targets = described_class.new(nil, ems).capture_ems_targets assert_infra_targets_enabled targets expect(targets.map { |t| t.class.name }).to match_array(%w[ManageIQ::Providers::Vmware::InfraManager::Vm ManageIQ::Providers::Vmware::InfraManager::Host ManageIQ::Providers::Vmware::InfraManager::Host ManageIQ::Providers::Vmware::InfraManager::Vm ManageIQ::Providers::Vmware::InfraManager::Host Storage]) end it "finds enabled targets excluding storages" do - targets = described_class.new(nil, ems).send(:capture_ems_targets, :exclude_storages => true) + targets = described_class.new(nil, ems).capture_ems_targets(:exclude_storages => true) assert_infra_targets_enabled targets expect(targets.map { |t| t.class.name }).to match_array(%w[ManageIQ::Providers::Vmware::InfraManager::Vm ManageIQ::Providers::Vmware::InfraManager::Host ManageIQ::Providers::Vmware::InfraManager::Host ManageIQ::Providers::Vmware::InfraManager::Vm ManageIQ::Providers::Vmware::InfraManager::Host]) end