From 61ceeb5f05a10be7afce3834b9df9e5d3bd92856 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 7 Jun 2018 13:31:33 +0200 Subject: [PATCH 1/5] Update ChargebackVm tests to use persitent DB objects --- spec/models/chargeback_vm_spec.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb index e3f81f5ffec..20f428601db 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -853,7 +853,7 @@ def result_row_for_vm(vm) let(:report_options) { Chargeback::ReportOptions.new } let(:timestamp_key) { 'Fri, 13 May 2016 10:40:00 UTC +00:00' } let(:beginning_of_day) { timestamp_key.in_time_zone.beginning_of_day } - let(:metric_rollup) { FactoryGirl.build(:metric_rollup_vm_hr, :timestamp => timestamp_key, :resource => @vm1) } + let(:metric_rollup) { FactoryGirl.create(:metric_rollup_vm_hr, :timestamp => timestamp_key, :resource => @vm1) } let(:consumption) { Chargeback::ConsumptionWithRollups.new([metric_rollup], nil, nil) } subject { described_class.report_row_key(consumption) } before do @@ -879,7 +879,7 @@ def result_row_for_vm(vm) context 'with parent ems' do let(:metric_rollup) do - FactoryGirl.build(:metric_rollup_vm_hr, :tag_names => 'environment/prod', + FactoryGirl.create(:metric_rollup_vm_hr, :tag_names => 'environment/prod', :parent_host_id => @host1.id, :parent_ems_cluster_id => @ems_cluster.id, :parent_ems_id => ems.id, :parent_storage_id => @storage.id, :resource => @vm1, :resource_name => @vm1.name) @@ -892,7 +892,7 @@ def result_row_for_vm(vm) context 'when parent ems is missing' do let(:metric_rollup) do - FactoryGirl.build(:metric_rollup_vm_hr, :tag_names => 'environment/prod', + FactoryGirl.create(:metric_rollup_vm_hr, :tag_names => 'environment/prod', :parent_host_id => @host1.id, :parent_ems_cluster_id => @ems_cluster.id, :parent_storage_id => @storage.id, :resource => @vm1, :resource_name => @vm1.name) @@ -922,7 +922,6 @@ def result_row_for_vm(vm) :parent_ems_id => ems.id, :parent_storage_id => @storage.id, :resource => @vm) end - let(:consumption) { Chargeback::ConsumptionWithRollups.new([metric_rollup], nil, nil) } before do @storage.tag_with([classification_1.tag.name, classification_2.tag.name], :ns => '*') @@ -934,9 +933,11 @@ def result_row_for_vm(vm) skip('this feature needs to be added to new chargeback') if Settings.new_chargeback [rate_assignment_options_1, rate_assignment_options_2].each do |rate_assignment| - metric_rollup.tag_names = rate_assignment[:tag].first.tag.send(:name_path) + metric_rollup.update_attributes!(:tag_names => rate_assignment[:tag].first.tag.send(:name_path)) @vm.tag_with(["/managed/#{metric_rollup.tag_names}"], :ns => '*') @vm.reload + + consumption = Chargeback::ConsumptionWithRollups.new([metric_rollup], nil, nil) uniq_rates = Chargeback::RatesCache.new.get(consumption) consumption.instance_variable_set(:@tag_names, nil) consumption.instance_variable_set(:@hash_features_affecting_rate, nil) @@ -1039,6 +1040,8 @@ def find_result_by_vm_name_and_region(chargeback_result, vm_name, region) subject! { ChargebackVm.build_results_for_report_ChargebackVm(options_tenant).first } it "report from all regions and only for tenant_1" do + skip('this feature needs to be added to new chargeback rating') if Settings.new_chargeback + # report only VMs from tenant 1 vm_ids = subject.map(&:vm_id) vm_ids_from_tenant = [tenant_1, tenant_1_region_1].map { |t| t.subtree.map(&:vms).map(&:ids) }.flatten @@ -1080,10 +1083,10 @@ def find_result_by_vm_name_and_region(chargeback_result, vm_name, region) let(:metering_used_hours) { 24 } let(:hardware) do - FactoryGirl.build(:hardware, + FactoryGirl.create(:hardware, :cpu_total_cores => cores, :memory_mb => mem_mb, - :disks => [FactoryGirl.build(:disk, :size => disk_b)]) + :disks => [FactoryGirl.create(:disk, :size => disk_b)]) end let(:fixed_cost) { hourly_rate * 24 } From 82ba7f4cefa3f9231d712723d0e88b6196533c49 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 7 Jun 2018 17:51:23 +0200 Subject: [PATCH 2/5] Use one select call in ConsumptionHistory --- app/models/chargeback/consumption_history.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/chargeback/consumption_history.rb b/app/models/chargeback/consumption_history.rb index e3bb04a078e..7d6a836026e 100644 --- a/app/models/chargeback/consumption_history.rb +++ b/app/models/chargeback/consumption_history.rb @@ -40,9 +40,9 @@ def self.base_rollup_scope :parent_ems_cluster => :tags, :parent_storage => :tags, :parent_ems => :tags) - .select(*Metric::BASE_COLS).order('resource_id, timestamp') + .select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') - base_rollup.select(*ChargeableField.chargeable_cols_on_metric_rollup).with_resource + base_rollup.with_resource end private_class_method :base_rollup_scope From 1d4790f0acae1455294c0807b75caef669fcb7b4 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 7 Jun 2018 18:38:05 +0200 Subject: [PATCH 3/5] Don't load everything in one query in ConsumptionHistory --- app/models/chargeback/consumption_history.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/models/chargeback/consumption_history.rb b/app/models/chargeback/consumption_history.rb index 7d6a836026e..8f436850260 100644 --- a/app/models/chargeback/consumption_history.rb +++ b/app/models/chargeback/consumption_history.rb @@ -33,15 +33,7 @@ def self.for_report(cb_class, options, region) end def self.base_rollup_scope - base_rollup = MetricRollup.includes( - :resource => [:hardware, :tenant, :tags, :vim_performance_states, :custom_attributes, - {:container_image => :custom_attributes}], - :parent_host => :tags, - :parent_ems_cluster => :tags, - :parent_storage => :tags, - :parent_ems => :tags) - .select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') - + base_rollup = MetricRollup.select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') base_rollup.with_resource end From 5318fbe2ff2af76956582611eddf70de8143726f Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 7 Jun 2018 18:45:17 +0200 Subject: [PATCH 4/5] Move resource_with to uniq_timestamp_record_map method in ConsumptionHistory --- app/models/chargeback/consumption_history.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/chargeback/consumption_history.rb b/app/models/chargeback/consumption_history.rb index 8f436850260..f7d1fba786e 100644 --- a/app/models/chargeback/consumption_history.rb +++ b/app/models/chargeback/consumption_history.rb @@ -33,14 +33,13 @@ def self.for_report(cb_class, options, region) end def self.base_rollup_scope - base_rollup = MetricRollup.select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') - base_rollup.with_resource + MetricRollup.select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') end private_class_method :base_rollup_scope def self.uniq_timestamp_record_map(report_scope, group_by_tenant = false) - main_select = MetricRollup.select(:id, :resource_id).arel.ast.to_sql + main_select = MetricRollup.select(:id, :resource_id).with_resource.arel.ast.to_sql .gsub("SELECT", "DISTINCT ON (resource_type, resource_id, timestamp)") .gsub(/ FROM.*$/, '') From e5b6698c277b1e720ac146c97589d0639c6ca41a Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 7 Jun 2018 20:54:03 +0200 Subject: [PATCH 5/5] Remove order from query in ConsumptionHistory ordering is in uniq_timestamp_record_map --- app/models/chargeback/consumption_history.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/chargeback/consumption_history.rb b/app/models/chargeback/consumption_history.rb index f7d1fba786e..db820b9e875 100644 --- a/app/models/chargeback/consumption_history.rb +++ b/app/models/chargeback/consumption_history.rb @@ -33,7 +33,7 @@ def self.for_report(cb_class, options, region) end def self.base_rollup_scope - MetricRollup.select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)).order('resource_id, timestamp') + MetricRollup.select(*(Metric::BASE_COLS + ChargeableField.chargeable_cols_on_metric_rollup)) end private_class_method :base_rollup_scope