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.

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

Chargeback: Fix rate adjustment rounding bug #13331

Merged
merged 11 commits into from
Jan 23, 2017
57 changes: 18 additions & 39 deletions app/models/chargeback_rate_detail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ def fixed?
def find_rate(value)
fixed_rate = 0.0
variable_rate = 0.0
tier_found, = chargeback_tiers.select do |tier|
tier.starts_with_zero? && value.zero? ||
value > rate_adjustment(tier.start) && value <= rate_adjustment(tier.finish)
end
tier_found = chargeback_tiers.detect { |tier| tier.includes?(value / rate_adjustment) }
unless tier_found.nil?
fixed_rate = tier_found.fixed_rate
variable_rate = tier_found.variable_rate
Expand All @@ -86,7 +83,7 @@ def hourly_cost(value, consumption)
hourly_fixed_rate = hourly(fixed_rate, consumption)
hourly_variable_rate = hourly(variable_rate, consumption)

hourly_fixed_rate + rate_adjustment(hourly_variable_rate) * value
hourly_fixed_rate + rate_adjustment * value * hourly_variable_rate
end

def hourly(rate, consumption)
Expand All @@ -102,41 +99,23 @@ def hourly(rate, consumption)
hourly_rate
end

# Scale the rate in the unit difine by user to the default unit of the metric
# It showing the default units of the metrics:
# cpu_usagemhz_rate_average --> megahertz
# derived_memory_used --> megabytes
# derived_memory_available -->megabytes
# net_usage_rate_average --> kbps
# disk_usage_rate_average --> kbps
# derived_vm_allocated_disk_storage --> bytes
# derived_vm_used_disk_storage --> bytes

def rate_adjustment(hr)
case metric
when "cpu_usagemhz_rate_average" then
per_unit == 'megahertz' ? hr : hr = adjustment_measure(hr, 'megahertz')
when "derived_memory_used", "derived_memory_available" then
per_unit == 'megabytes' ? hr : hr = adjustment_measure(hr, 'megabytes')
when "net_usage_rate_average", "disk_usage_rate_average" then
per_unit == 'kbps' ? hr : hr = adjustment_measure(hr, 'kbps')
when "derived_vm_allocated_disk_storage", "derived_vm_used_disk_storage" then
per_unit == 'bytes' ? hr : hr = adjustment_measure(hr, 'bytes')
else hr
end
end
# Scale the rate in the unit defined by user -> to the default unit of the metric
METRIC_UNITS = {
'cpu_usagemhz_rate_average' => 'megahertz',
'derived_memory_used' => 'megabytes',
'derived_memory_available' => 'megabytes',
'net_usage_rate_average' => 'kbps',
'disk_usage_rate_average' => 'kbps',
'derived_vm_allocated_disk_storage' => 'bytes',
'derived_vm_used_disk_storage' => 'bytes'
}.freeze

# Adjusts the hourly rate to the per unit by default
def adjustment_measure(hr, pu_destiny)
measure = detail_measure
pos_pu_destiny = measure.units.index(pu_destiny)
pos_per_unit = measure.units.index(per_unit)
jumps = (pos_per_unit - pos_pu_destiny).abs
if pos_per_unit > pos_pu_destiny
hr.to_f / (measure.step**jumps)
else
hr * (measure.step**jumps)
end
def rate_adjustment
@rate_adjustment ||= if METRIC_UNITS[metric]
detail_measure.adjust(per_unit, METRIC_UNITS[metric])
else
1
end
end

def affects_report_fields(report_cols)
Expand Down
6 changes: 6 additions & 0 deletions app/models/chargeback_rate_detail_measure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ def measures
Hash[units_display.zip(units)]
end

def adjust(from_unit, to_unit)
return 1 if from_unit == to_unit
jumps = units.index(to_unit) - units.index(from_unit)
BigDecimal.new(step)**jumps
end

private def units_same_length
unless (units.count == units_display.count)
errors.add("Units Problem", "Units_display length diferent that the units length")
Expand Down
4 changes: 4 additions & 0 deletions app/models/chargeback_tier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ def self.to_float(s)
end
end

def includes?(value)
starts_with_zero? && value.zero? || value > start && value.to_f <= finish
end

def starts_with_zero?
start.zero?
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/chargeback_rate_detail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
].each_slice(2) do |per_unit, rate_adjustment|
cbd = FactoryGirl.build(:chargeback_rate_detail, :per_unit => per_unit, :metric => 'derived_memory_available',
:chargeback_rate_detail_measure_id => cbdm.id)
expect(cbd.rate_adjustment(value)).to eq(rate_adjustment)
expect(cbd.rate_adjustment * value).to eq(rate_adjustment)
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/models/chargeback_vm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
@service << @vm1
@service.save

@vm2 = FactoryGirl.create(:vm_vmware, :name => "test_vm 2", :evm_owner => admin)
@vm2 = FactoryGirl.create(:vm_vmware, :name => "test_vm 2", :evm_owner => admin, :created_on => month_beginning)

Range.new(month_beginning, month_end, true).step_value(12.hours).each do |time|
[@vm1, @vm2].each do |vm|
Expand Down Expand Up @@ -233,7 +233,8 @@
before do
@tenant = FactoryGirl.create(:tenant)
@tenant_child = FactoryGirl.create(:tenant, :ancestry => @tenant.id)
@vm_tenant = FactoryGirl.create(:vm_vmware, :tenant_id => @tenant_child.id, :name => "test_vm_tenant")
@vm_tenant = FactoryGirl.create(:vm_vmware, :tenant_id => @tenant_child.id,
:name => "test_vm_tenant", :created_on => month_beginning)

Range.new(start_time, finish_time, true).step_value(1.hour).each do |t|
@vm_tenant.metric_rollups <<
Expand Down