Merge pull request #12608 from isimluk/extract-rates-cache

Extract rates cache to separate class

app/models/chargeback.rb

@@ -21,7 +21,7 @@ def self.build_results_for_report_chargeback(options)
  _log.info("Calculating chargeback costs...")
  @options = options = ReportOptions.new_from_h(options)
 
- cb = new
+ rates = RatesCache.new
 
  base_rollup = MetricRollup.includes(
  :resource => [:hardware, :tenant, :tags, :vim_performance_states, :custom_attributes],
@@ -61,7 +61,7 @@ def self.build_results_for_report_chargeback(options)
  # we need to select ChargebackRates for groups of MetricRollups records
  # and rates are selected by first MetricRollup record
  metric_rollup_record = metric_rollup_records.first
- rates_to_apply = cb.get_rates(metric_rollup_record)
+ rates_to_apply = rates.get(metric_rollup_record)
 
  # key contains resource_id and timestamp (query_start_time...query_end_time)
  # extra_fields there some extra field like resource name and
@@ -124,17 +124,6 @@ def self.get_tag_keys_and_fields(perf, ts_key)
  [key, extra_fields]
  end
 
- def get_rates(perf)
- @rates ||= {}
- @rates[perf.hash_features_affecting_rate] ||=
- begin
- prefix = Chargeback.report_cb_model(self.class.name).underscore
- ChargebackRate.get_assigned_for_target(perf.resource,
- :tag_list => perf.tag_list_reconstruct.map! { |t| prefix + t },
- :parents => get_rate_parents(perf))
- end
- end
-
  def calculate_costs(metric_rollup_records, rates, hours_in_interval)
  chargeback_fields_present = metric_rollup_records.count(&:chargeback_fields_present?)
  self.fixed_compute_metric = chargeback_fields_present if chargeback_fields_present
@@ -161,10 +150,6 @@ def self.report_tag_field
  "tag_name"
  end
 
- def self.get_rate_parents
- raise "Chargeback: get_rate_parents must be implemented in child class."
- end
-
  def self.set_chargeback_report_options(rpt, group_by, header_for_tag, tz)
  rpt.cols = %w(start_date display_range)
 

app/models/chargeback/rates_cache.rb

@@ -0,0 +1,24 @@
+class Chargeback
+ class RatesCache
+ def get(perf)
+ @rates ||= {}
+ @rates[perf.hash_features_affecting_rate] ||=
+ begin
+ prefix = tag_prefix(perf)
+ ChargebackRate.get_assigned_for_target(perf.resource,
+ :tag_list => perf.tag_list_reconstruct.map! { |t| prefix + t },
+ :parents => perf.parents_determining_rate)
+ end
+ end
+
+ private
+
+ def tag_prefix(perf)
+ case perf.resource_type
+ when Container.name then 'container_image'
+ when VmOrTemplate.name then 'vm'
+ when ContainerProject.name then 'container_project'
+ end
+ end
+ end
+end

app/models/chargeback_container_image.rb

@@ -88,9 +88,4 @@ def self.report_col_options
  "total_cost" => {:grouping => [:total]}
  }
  end
-
- def get_rate_parents(_perf)
- # get rates from image tags only
- []
- end
 end # class ChargebackContainerImage

app/models/chargeback_container_project.rb

@@ -80,9 +80,4 @@ def self.report_col_options
  "total_cost" => {:grouping => [:total]}
  }
  end
-
- def get_rate_parents(perf)
- # Get rate from assigned containers providers only
- [perf.parent_ems].compact
- end
 end # class Chargeback

app/models/chargeback_vm.rb

@@ -149,10 +149,4 @@ def self.report_col_options
  "total_cost" => {:grouping => [:total]}
  }
  end
-
- def get_rate_parents(perf)
- @enterprise ||= MiqEnterprise.my_enterprise
- parents = perf.resource_parents + [@enterprise]
- parents.compact
- end
 end # class Chargeback

app/models/metric/chargeback_helper.rb

@@ -24,4 +24,15 @@ def resource_parents
  resource.try(:tenant)
  ].compact
  end
+
+ def parents_determining_rate
+ case resource_type
+ when VmOrTemplate.name
+ (resource_parents + [MiqEnterprise.my_enterprise]).compact
+ when ContainerProject.name
+ [parent_ems].compact
+ when Container.name
+ []
+ end
+ end
 end

spec/models/chargeback_vm_spec.rb

@@ -50,79 +50,6 @@
  Timecop.return
  end
 
- describe "#get_rate_parents" do
- let(:vm) do
- FactoryGirl.create(:vm_vmware, :name => "test_vm", :evm_owner => @admin, :ems_ref => "ems_ref",
- :ems_cluster => @ems_cluster, :storage => @storage, :host => @host1,
- :ext_management_system => @ems
- )
- end
-
- let(:metric_rollup_without_parents) do
- FactoryGirl.create(:metric_rollup_vm_hr,
- :cpu_usagemhz_rate_average => @cpu_usagemhz_rate,
- :derived_vm_numvcpus => @cpu_count,
- :derived_memory_available => @memory_available,
- :derived_memory_used => @memory_used,
- :disk_usage_rate_average => @disk_usage_rate,
- :net_usage_rate_average => @net_usage_rate,
- :derived_vm_used_disk_storage => @vm_used_disk_storage.gigabytes,
- :derived_vm_allocated_disk_storage => @vm_allocated_disk_storage.gigabytes,
- :tag_names => "environment/prod",
- :resource => vm
- )
- end
-
- let(:metric_rollup_with_parents) do
- FactoryGirl.create(:metric_rollup_vm_hr,
- :cpu_usagemhz_rate_average => @cpu_usagemhz_rate,
- :derived_vm_numvcpus => @cpu_count,
- :derived_memory_available => @memory_available,
- :derived_memory_used => @memory_used,
- :disk_usage_rate_average => @disk_usage_rate,
- :net_usage_rate_average => @net_usage_rate,
- :derived_vm_used_disk_storage => @vm_used_disk_storage.gigabytes,
- :derived_vm_allocated_disk_storage => @vm_allocated_disk_storage.gigabytes,
- :tag_names => "environment/prod",
- :resource => vm,
- :parent_host_id => @host1.id,
- :parent_ems_cluster_id => @ems_cluster.id,
- :parent_ems_id => @ems.id,
- :parent_storage_id => @storage.id,
- )
- end
-
- it "uses resource's host, cluster, storage and ems" do
- parents = described_class.new.send(:get_rate_parents, metric_rollup_without_parents)
-
- expected_array = [
- vm.host,
- vm.ems_cluster,
- vm.storage,
- vm.ext_management_system,
- vm.tenant,
- MiqEnterprise.my_enterprise
- ].compact
-
- expect(parents).to match_array(expected_array)
- end
-
- it "uses host, cluster, storage ems from MetricRollup record" do
- parents = described_class.new.send(:get_rate_parents, metric_rollup_with_parents)
-
- expected_array = [
- metric_rollup_with_parents.parent_host,
- metric_rollup_with_parents.parent_ems_cluster,
- metric_rollup_with_parents.parent_storage,
- metric_rollup_with_parents.parent_ems,
- vm.tenant,
- MiqEnterprise.my_enterprise
- ].compact
-
- expect(parents).to match_array(expected_array)
- end
- end
-
  let(:report_static_fields) { %w(vm_name) }
 
  it "uses static fields" do
@@ -874,7 +801,7 @@ def used_average_for(metric, hours_in_interval)
 
  before do
  ChargebackRate.set_assignments(:compute, [rate_assignment_options])
- @rate = chargeback_vm.get_rates(metric_rollup).first
+ @rate = Chargeback::RatesCache.new.get(metric_rollup).first
  @assigned_rate = ChargebackRate.get_assignments("Compute").first
  end
 

spec/models/metric_rollup/chargeback_helper_spec.rb

@@ -0,0 +1,62 @@
+describe MetricRollup do
+ describe '#parents_determining_rate' do
+ before do
+ MiqRegion.seed
+ MiqEnterprise.seed
+ end
+
+ context 'VmOrTemplate' do
+ let(:ems) { FactoryGirl.build(:ems_vmware) }
+ let(:ems_cluster) { FactoryGirl.build(:ems_cluster, :ext_management_system => ems) }
+ let(:storage) { FactoryGirl.build(:storage_target_vmware) }
+ let(:host) { FactoryGirl.build(:host) }
+ let(:vm) do
+ FactoryGirl.create(:vm_vmware, :name => 'test_vm', :ems_ref => 'ems_ref',
+ :ems_cluster => ems_cluster, :storage => storage, :host => host,
+ :ext_management_system => ems
+ )
+ end
+
+ subject { metric_rollup.parents_determining_rate }
+
+ context 'metric_rollup record with parents not nil' do
+ let(:metric_rollup) do
+ FactoryGirl.build(:metric_rollup_vm_hr,
+ :resource => vm,
+ :parent_host => host,
+ :parent_ems_cluster => ems_cluster,
+ :parent_ems => ems,
+ :parent_storage => storage,
+ )
+ end
+
+ let(:parents_from_rollup) do
+ [
+ metric_rollup.parent_host,
+ metric_rollup.parent_ems_cluster,
+ metric_rollup.parent_storage,
+ metric_rollup.parent_ems,
+ MiqEnterprise.my_enterprise
+ ]
+ end
+
+ it { is_expected.to match_array(parents_from_rollup) }
+ end
+
+ context 'metric_rollup record with parents nil' do
+ let(:metric_rollup) { FactoryGirl.build(:metric_rollup_vm_hr, :resource => vm) }
+ let(:parents_from_vm) do
+ [
+ vm.host,
+ vm.ems_cluster,
+ vm.storage,
+ vm.ext_management_system,
+ MiqEnterprise.my_enterprise
+ ]
+ end
+
+ it { is_expected.to match_array(parents_from_vm) }
+ end
+ end
+ end
+end