Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't charge hours before Vm/Container/Project was created #13373

Merged
merged 2 commits into from
Jan 10, 2017
Merged

Don't charge hours before Vm/Container/Project was created #13373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2d0561
Specs: resources should be created before their consumption
isimluk Jan 6, 2017
1317470
Do not charge for hours before resource creation
isimluk Jan 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions app/models/chargeback/consumption.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ def initialize(start_time, end_time)
@start_time, @end_time = start_time, end_time
end

def past_hours_in_interval
# We cannot charge for future hours (i.e. weekly report on Monday, should charge just monday)
@past_hours_in_interval ||= begin
past = (Time.current - @start_time).round / 1.hour
[past, hours_in_interval].min
end
def consumed_hours_in_interval
# Why we need this?
# 1) We cannot charge for hours until the resources existed (vm provisioned in the middle of month)
# 2) We cannot charge for future hours (i.e. weekly report on Monday, should charge just monday)
@consumed_hours_in_interval ||= begin
consuption_start = [@start_time, resource.try(:created_on)].compact.max
consumption_end = [Time.current, @end_time].min
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use Time.current.utc to ensure the time is not in the local time zone?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this for a while. It seems the TZ does not matter in this case.

The consumption_end is just used for substraction.

(consumption_end - consuption_start).round / 1.hour
end
end

def hours_in_month
Expand Down
2 changes: 1 addition & 1 deletion app/models/chargeback/consumption_with_rollups.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def max(metric)

def avg(metric)
metric_sum = values(metric).sum
metric_sum / past_hours_in_interval
metric_sum / consumed_hours_in_interval
end

def none?(metric)
Expand Down
2 changes: 1 addition & 1 deletion app/models/chargeback_rate_detail.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def cost_keys

def metric_and_cost_by(consumption)
metric_value = metric_value_by(consumption)
[metric_value, hourly_cost(metric_value, consumption) * consumption.past_hours_in_interval]
[metric_value, hourly_cost(metric_value, consumption) * consumption.consumed_hours_in_interval]
end

def first_tier?(tier,tiers)
Expand Down
5 changes: 3 additions & 2 deletions spec/models/chargeback_container_project_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
let(:base_options) { {:interval_size => 2, :end_interval_offset => 0, :ext_options => {:tz => 'UTC'} } }
let(:hourly_rate) { 0.01 }
let(:starting_date) { Time.parse('2012-09-01 23:59:59Z').utc }
let(:ts) { starting_date.in_time_zone(Metric::Helper.get_time_zone(options[:ext_options])) }
let(:ts) { starting_date.in_time_zone(Metric::Helper.get_time_zone(base_options[:ext_options])) }
let(:report_run_time) { month_end }
let(:month_beginning) { ts.beginning_of_month.utc }
let(:month_end) { ts.end_of_month.utc }
Expand Down Expand Up @@ -32,7 +32,8 @@
ChargebackRate.seed

EvmSpecHelper.create_guid_miq_server_zone
@project = FactoryGirl.create(:container_project, :name => "my project", :ext_management_system => ems)
@project = FactoryGirl.create(:container_project, :name => "my project", :ext_management_system => ems,
:created_on => month_beginning)

temp = {:cb_rate => chargeback_rate, :object => ems}
ChargebackRate.set_assignments(:compute, [temp])
Expand Down
3 changes: 2 additions & 1 deletion spec/models/chargeback_vm/ongoing_time_period_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
let(:tag) { Tag.find_by_name('/managed/environment/prod') }
let(:vm) do
ems = FactoryGirl.create(:ems_vmware)
vm = FactoryGirl.create(:vm_vmware, :name => 'test_vm', :evm_owner => admin, :ems_ref => 'ems_ref')
vm = FactoryGirl.create(:vm_vmware, :name => 'test_vm', :evm_owner => admin, :ems_ref => 'ems_ref',
:created_on => start_of_all_intervals)
vm.tag_with(tag.name, :ns => '*')
host = FactoryGirl.create(:host, :hardware => FactoryGirl.create(:hardware,
:memory_mb => 8124, :cpu_total_cores => 1,
Expand Down
5 changes: 3 additions & 2 deletions spec/models/chargeback_vm_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
c = FactoryGirl.create(:classification, :name => "prod", :description => "Production", :parent_id => cat.id)
@tag = Tag.find_by_name("/managed/environment/prod")

@vm1 = FactoryGirl.create(:vm_vmware, :name => "test_vm", :evm_owner => admin, :ems_ref => "ems_ref")
@vm1 = FactoryGirl.create(:vm_vmware, :name => "test_vm", :evm_owner => admin, :ems_ref => "ems_ref",
:created_on => month_beginning)
@vm1.tag_with(@tag.name, :ns => '*')

@host1 = FactoryGirl.create(:host, :hardware => FactoryGirl.create(:hardware, :memory_mb => 8124, :cpu_total_cores => 1, :cpu_speed => 9576), :vms => [@vm1])
Expand Down Expand Up @@ -527,7 +528,7 @@

context 'for SCVMM (hyper-v)' do
let!(:vm1) do
vm = FactoryGirl.create(:vm_microsoft)
vm = FactoryGirl.create(:vm_microsoft, :created_on => report_run_time - 1.day)
vm.tag_with(@tag.name, :ns => '*')
vm
end
Expand Down