From ec5989eb315aa9337682624b42e6497cfdd329d4 Mon Sep 17 00:00:00 2001 From: lpichler Date: Wed, 23 May 2018 19:31:15 +0200 Subject: [PATCH 1/5] Use current tags for filtering resources in chargeback --- app/models/chargeback/consumption_with_rollups.rb | 8 +++++++- app/models/chargeback_vm.rb | 3 ++- spec/models/chargeback_vm_spec.rb | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/models/chargeback/consumption_with_rollups.rb b/app/models/chargeback/consumption_with_rollups.rb index 5a96dd35809..d1e187294b9 100644 --- a/app/models/chargeback/consumption_with_rollups.rb +++ b/app/models/chargeback/consumption_with_rollups.rb @@ -6,6 +6,8 @@ class ConsumptionWithRollups < Consumption attr_accessor :start_time, :end_time + TAG_PREFIX = '/managed/'.freeze + def initialize(metric_rollup_records, start_time, end_time) super(start_time, end_time) @rollups = metric_rollup_records @@ -22,7 +24,11 @@ def hash_features_affecting_rate def tag_names @tag_names ||= @rollups.inject([]) do |memo, rollup| - memo |= rollup.tag_names.split('|') if rollup.tag_names.present? + resource = rollup.resource + tag_names = [] + tag_names |= resource.tags.collect(&:name).map { |x| x.gsub(TAG_PREFIX, "") } if resource + tag_names |= rollup.tag_names.split('|') if rollup.tag_names.present? + memo |= tag_names if tag_names.present? memo end end diff --git a/app/models/chargeback_vm.rb b/app/models/chargeback_vm.rb index 149a6d0fa68..6fb57c58484 100644 --- a/app/models/chargeback_vm.rb +++ b/app/models/chargeback_vm.rb @@ -83,7 +83,8 @@ def self.build_results_for_report_ChargebackVm(options) def self.where_clause(records, options, region) scope = records.where(:resource_type => "VmOrTemplate") if options[:tag] && (@report_user.nil? || !@report_user.self_service?) - scope.for_tag_names(options[:tag].split("/")[2..-1]) + scope_with_current_tags = scope.where(:resource => Vm.find_tagged_with(:any => @options[:tag], :ns => '*')) + scope.for_tag_names(options[:tag].split("/")[2..-1]).or(scope_with_current_tags) else scope.where(:resource => vms(region)) end diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb index bb4a46b4412..30122a402e5 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -905,19 +905,24 @@ def result_row_for_vm(vm) FactoryGirl.create(:metric_rollup_vm_hr, :timestamp => report_run_time - 1.day - 17.hours, :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 => @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 => '*') ChargebackRate.set_assignments(:storage, [rate_assignment_options_1, rate_assignment_options_2]) + @vm = FactoryGirl.create(:vm_vmware, :name => "test_vm_1", :evm_owner => admin, :ems_ref => "ems_ref", :created_on => month_beginning) end it "return only one chargeback rate according to tag name of Vm" do + 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) - uniq_rates = chargeback_vm.get(consumption) + @vm.tag_with(["/managed/#{metric_rollup.tag_names}"], :ns => '*') + @vm.reload + uniq_rates = Chargeback::RatesCache.new.get(consumption) consumption.instance_variable_set(:@tag_names, nil) consumption.instance_variable_set(:@hash_features_affecting_rate, nil) expect([rate_assignment[:cb_rate]]).to match_array(uniq_rates) From 5d2e130dacf6bdb1ec46defcc12dad27e044f66e Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 24 May 2018 21:05:48 +0200 Subject: [PATCH 2/5] Consider current tags of resource in Metric::ChargebackHelper::tag_list_with_prefix --- app/models/metric/chargeback_helper.rb | 5 ++++- spec/models/chargeback_vm_spec.rb | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/models/metric/chargeback_helper.rb b/app/models/metric/chargeback_helper.rb index 1af763b7324..754bdacae14 100644 --- a/app/models/metric/chargeback_helper.rb +++ b/app/models/metric/chargeback_helper.rb @@ -26,7 +26,10 @@ def tag_list_with_prefix end end - "#{image_tag_name}#{tag_names}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) + current_tag_names = resource ? resource.tags.collect(&:name).map { |x| x.gsub("/managed/", "") } : [] + rollup_tag_names = tag_names ? tag_names.split("|") : [] + + "#{image_tag_name}#{(current_tag_names | rollup_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) end def resource_parents diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb index 30122a402e5..dde47f902f5 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -627,6 +627,21 @@ def result_row_for_vm(vm) subject { ChargebackVm.build_results_for_report_ChargebackVm(options).first.first } + context "when MetricRollup#tag_names are not considered" do + before do + # report filter is set to different tag + @vm1.metric_rollups.each { |mr| mr.update(:tag_names => 'registered/no|folder_path_yellow/datacenters') } + end + + it "cpu" do + expect(subject.cpu_allocated_metric).to eq(cpu_count) + used_metric = used_average_for(:cpu_usagemhz_rate_average, hours_in_month, @vm1) + expect(subject.cpu_used_metric).to be_within(0.01).of(used_metric) + expect(subject.cpu_used_cost).to be_within(0.01).of(used_metric * hourly_rate * hours_in_month) + expect(subject.cpu_allocated_cost).to be_within(0.01).of(cpu_count * count_hourly_rate * hours_in_month) + end + end + context "chargeback rate contains rate unrelated to chargeback vm" do let!(:chargeback_rate) do FactoryGirl.create(:chargeback_rate, :detail_params => detail_params.merge(:chargeback_rate_detail_cpu_cores_allocated => {:tiers => [count_hourly_variable_tier_rate]})) From bc49fe11afef75d904bc50d7ba45773068bcb089 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 24 May 2018 21:16:45 +0200 Subject: [PATCH 3/5] DRY: add method resource_current_tag_names in Metric::ChargebackHelper --- app/models/chargeback/consumption_with_rollups.rb | 6 +----- app/models/metric/chargeback_helper.rb | 7 +++++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/models/chargeback/consumption_with_rollups.rb b/app/models/chargeback/consumption_with_rollups.rb index d1e187294b9..e53f4f450aa 100644 --- a/app/models/chargeback/consumption_with_rollups.rb +++ b/app/models/chargeback/consumption_with_rollups.rb @@ -6,8 +6,6 @@ class ConsumptionWithRollups < Consumption attr_accessor :start_time, :end_time - TAG_PREFIX = '/managed/'.freeze - def initialize(metric_rollup_records, start_time, end_time) super(start_time, end_time) @rollups = metric_rollup_records @@ -24,9 +22,7 @@ def hash_features_affecting_rate def tag_names @tag_names ||= @rollups.inject([]) do |memo, rollup| - resource = rollup.resource - tag_names = [] - tag_names |= resource.tags.collect(&:name).map { |x| x.gsub(TAG_PREFIX, "") } if resource + tag_names = rollup.resource_current_tag_names tag_names |= rollup.tag_names.split('|') if rollup.tag_names.present? memo |= tag_names if tag_names.present? memo diff --git a/app/models/metric/chargeback_helper.rb b/app/models/metric/chargeback_helper.rb index 754bdacae14..13a82a03381 100644 --- a/app/models/metric/chargeback_helper.rb +++ b/app/models/metric/chargeback_helper.rb @@ -26,10 +26,9 @@ def tag_list_with_prefix end end - current_tag_names = resource ? resource.tags.collect(&:name).map { |x| x.gsub("/managed/", "") } : [] rollup_tag_names = tag_names ? tag_names.split("|") : [] - "#{image_tag_name}#{(current_tag_names | rollup_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) + "#{image_tag_name}#{(resource_current_tag_names | rollup_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) end def resource_parents @@ -50,4 +49,8 @@ def parents_determining_rate [parent_ems].compact end end + + def resource_current_tag_names + resource ? resource.tags.collect(&:name).map { |x| x.gsub("/managed/", "") } : [] + end end From 927151435283f00711fe9d2f46f39a5327887041 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 24 May 2018 21:19:58 +0200 Subject: [PATCH 4/5] DRY: add method resource_tag_names in Metric::ChargebackHelper --- app/models/chargeback/consumption_with_rollups.rb | 3 +-- app/models/metric/chargeback_helper.rb | 8 +++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/chargeback/consumption_with_rollups.rb b/app/models/chargeback/consumption_with_rollups.rb index e53f4f450aa..427cdb95554 100644 --- a/app/models/chargeback/consumption_with_rollups.rb +++ b/app/models/chargeback/consumption_with_rollups.rb @@ -22,8 +22,7 @@ def hash_features_affecting_rate def tag_names @tag_names ||= @rollups.inject([]) do |memo, rollup| - tag_names = rollup.resource_current_tag_names - tag_names |= rollup.tag_names.split('|') if rollup.tag_names.present? + tag_names = rollup.resource_current_tag_names | rollup.resource_tag_names memo |= tag_names if tag_names.present? memo end diff --git a/app/models/metric/chargeback_helper.rb b/app/models/metric/chargeback_helper.rb index 13a82a03381..5bcf80f6d87 100644 --- a/app/models/metric/chargeback_helper.rb +++ b/app/models/metric/chargeback_helper.rb @@ -26,9 +26,7 @@ def tag_list_with_prefix end end - rollup_tag_names = tag_names ? tag_names.split("|") : [] - - "#{image_tag_name}#{(resource_current_tag_names | rollup_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) + "#{image_tag_name}#{(resource_current_tag_names | resource_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) end def resource_parents @@ -53,4 +51,8 @@ def parents_determining_rate def resource_current_tag_names resource ? resource.tags.collect(&:name).map { |x| x.gsub("/managed/", "") } : [] end + + def resource_tag_names + tag_names ? tag_names.split("|") : [] + end end From 9130b570b045ba26ea2ff24bf47e5a33a533e4c6 Mon Sep 17 00:00:00 2001 From: lpichler Date: Thu, 24 May 2018 21:25:49 +0200 Subject: [PATCH 5/5] DRY: add method all_tag_names in Metric::ChargebackHelper --- app/models/chargeback/consumption_with_rollups.rb | 3 +-- app/models/metric/chargeback_helper.rb | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/models/chargeback/consumption_with_rollups.rb b/app/models/chargeback/consumption_with_rollups.rb index 427cdb95554..1aba22c7c84 100644 --- a/app/models/chargeback/consumption_with_rollups.rb +++ b/app/models/chargeback/consumption_with_rollups.rb @@ -22,8 +22,7 @@ def hash_features_affecting_rate def tag_names @tag_names ||= @rollups.inject([]) do |memo, rollup| - tag_names = rollup.resource_current_tag_names | rollup.resource_tag_names - memo |= tag_names if tag_names.present? + memo |= rollup.all_tag_names memo end end diff --git a/app/models/metric/chargeback_helper.rb b/app/models/metric/chargeback_helper.rb index 5bcf80f6d87..916bb736789 100644 --- a/app/models/metric/chargeback_helper.rb +++ b/app/models/metric/chargeback_helper.rb @@ -26,7 +26,7 @@ def tag_list_with_prefix end end - "#{image_tag_name}#{(resource_current_tag_names | resource_tag_names).join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) + "#{image_tag_name}#{all_tag_names.join("|")}".split("|").reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + (labels || []) end def resource_parents @@ -55,4 +55,8 @@ def resource_current_tag_names def resource_tag_names tag_names ? tag_names.split("|") : [] end + + def all_tag_names + resource_current_tag_names | resource_tag_names + end end