Add tenant for matching assigned chargeback rate

[lpichler]

https://bugzilla.redhat.com/show_bug.cgi?id=1337657

we are not considering current tenant when we want to determine assigned rate,
so

I am making suggestion for you @aljesusg, what do you think ?

cc @gtanzillo

[lpichler]

@miq-bot add-label wip

[lpichler]

@miq-bot add-label blocker

[aljesusg]

@lpichler Why we should add the current tenant of user?

[lpichler]

@aljesusg

1. we have assigned certain rate for certain tenant as you did.
2. then when we are generating chargeback report we need to determine which rate should be used -> and it is according to current tenant.

and selection is here:
https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/assignment_mixin.rb#L155

alist are assigments and we need to select the one and I think that it should be according current_tenant

Maybe we should think we what we will do if for the current tenant there is no such assigment

Does it make sense to you ?

[aljesusg]

Yeah you're right :) . Thanks @lpichler

[lpichler]

I realized another thing. @aljesusg or maybe it should not be a current user.
because who is the current user when reporting workers are running ? and probably does not make sense because existing code deriving it from metric_rollup table (variable perf) here
https://github.com/ManageIQ/manageiq/blob/master/app/models/chargeback.rb#L79

so I think that we should determine tenant for somehow from metric rollup records (perf)
MetricRollup doesn't direclty belongs to tenant but MetricRollup does have
resource (perf.resource) what is for example provider, host ... and this recause has tenant

So my other proposal is to use perf.resource.tenant

parents = [perf.parent_host, perf.parent_ems_cluster, perf.parent_storage, perf.parent_ems, @enterprise, perf.resource.tenant].compact

@gtanzillo What do you think about using perf.resource.tenant for determing rates ?

It seems ok to me because there is basically the same thing
https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/assignment_mixin.rb#L110
but we are using options[:parents] instead of ASSIGNMENT_PARENT_ASSOCIATIONS so we should add it to parents

but drawback can be perfomance, but logically it sounds good to me.

Thanks!

[gtanzillo]

@lpichler @aljesusg Yes, I think it should take the rates from the tenant that owns the resource.

[lpichler]

@miq-bot remove-label wip

[lpichler]

I added resource's tenant to parents with spec.

@aljesusg @gtanzillo please review

[miq-bot]

Checked commits lpichler/manageiq@3ca68f6~...75b08e3 with ruby 2.2.4, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. ⭐

[gtanzillo]

👍 Looks good!