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

[EUWE] Chargebacks for SCVMM (rollup-less) [2/2] #13554

Merged
merged 25 commits into from
Jan 18, 2017

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Jan 18, 2017

What?

This is continuation of #13419. However, much smaller. Most of the commits here just amend specs. Specs needed to be amended in order to fix time handling issues we had in CB.

Related PRs

This is equivalent of backporting the following prs and resolving conflicts.
#12807
#13144
#13134
#13373
#13451

Related BZs

@miq-bot add_label chargebacks

of ChargebackRateDetail.

(cherry picked from commit ae27ca5)
ChargebackRateDetail object should not know anything about some
interval. It should just maintain the logic to calculate the cost.

The @hours_in_interval is a function of given consumption slice.

Moreover, there is a problem with @hours_in_interval -- the interval can
be for instance one week, and the rate can be monthly, then we divide
monthly cost by week. B@ng!

(cherry picked from commit 09cecec)
When calculation hourly_rate for ChargebackRateDetail with monthly rate,
we had assumed that lenght of report interval was good indicator of
hours in month. This is however truth only for monthly reports.

For example, assume monthly rate of $4 for fixed compute. On monthly
report we charge correctly $4 (the hourly_rate is ($4 / 1.month). On
weekly report, however we charge $4 per each week, as we assume that
hourly_rate is ($4 / 1.week).

(cherry picked from commit c3d3e85)
(cherry picked from commit 4f7e02c)

Conflicts:
	spec/models/chargeback_rate_detail_spec.rb
The before branch is run many many times again and again creating admin
user for no good.

(cherry picked from commit e9b9e30)

Conflicts:
	spec/models/chargeback_vm_spec.rb
(cherry picked from commit d8dafb4)

Conflicts:
	spec/models/chargeback_container_project_spec.rb
	spec/models/chargeback_vm_spec.rb
(cherry picked from commit 99ce628)
This code was definititelly meant to be written this way.

(cherry picked from commit 385e8d2)
(cherry picked from commit 63141c0)
(cherry picked from commit bbe59f0)

Conflicts:
	spec/models/chargeback_vm_spec.rb
(cherry picked from commit 5cc0a3c)

Conflicts:
	spec/models/chargeback_container_project_spec.rb
	spec/models/chargeback_vm_spec.rb
and fix related rubocop

(cherry picked from commit f870d3c)
For variable_rate, we charge only that much that has been consumed.

Perhaps I'll end the service tmrw, I should not be charged in advanced.

https://bugzilla.redhat.com/show_bug.cgi?id=1402072
(cherry picked from commit c94a8fb)
Otherwise, nobody will ever understand why we have them.

Now each file includes single hardcoded datetime and everything else is
relative to it. Think about it. That's actually meaning of the specs
prescription. The hardcoded values were hardcoded to be relative to
other hc values.

(cherry picked from commit 4609f3c)

Conflicts:
	spec/models/chargeback_container_image_spec.rb
	spec/models/chargeback_container_project_spec.rb
	spec/models/chargeback_vm_spec.rb
We are gonna change code to charge only for consumption since the
resource creation time. First, we need to make sure all our test subject
have proper created_on timestamp.

(cherry picked from commit e2d0561)

Conflicts:
	spec/models/chargeback_container_project_spec.rb
When resource did not exist in given interval (day, week, month) we will
not yield Consumption object into the chargeback engine.

As a result the report won't contain rows with zero consumption.

(cherry picked from commit 166896f)
Nice catch @lpichler!

(cherry picked from commit 3703d88)
@isimluk
Copy link
Member Author

isimluk commented Jan 18, 2017

@miq-bot assign @simaishi
/cc @lpichler @gtanzillo

@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2017

@isimluk Cannot apply the following label because they are not recognized: chargebacks

@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2017

Checked commits isimluk/manageiq@96b46ad~...1ef7a98 with ruby 2.2.6, rubocop 0.46.0, and haml-lint 0.19.0
9 files checked, 27 offenses detected

app/models/chargeback/consumption.rb

spec/models/chargeback_container_image_spec.rb

spec/models/chargeback_container_project_spec.rb

spec/models/chargeback_rate_detail_spec.rb

spec/models/chargeback_vm/ongoing_time_period_spec.rb

spec/models/chargeback_vm_spec.rb

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Thanks @isimluk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants