CCI-MOC / openstack-billing-from-db

Simple billing from the database
Apache License 2.0
1 stars 3 forks source link

Refactor SU logic based on flavor name #57

Closed knikolla closed 2 days ago

knikolla commented 5 months ago

30 removed the dependence on flavor names to figure out the SU type. VMs that had a flavor with a100 in their name where detected as OpenStack GPUA100. After #30 merged, the way we figure out with SU a VM is, is from the PCI Request field in the instance table.

However, we still used that information to reconstruct a flavor name

https://github.com/CCI-MOC/openstack-billing-from-db/blob/7dfbc657d2e93db660b755fa5f6d63475ae57d44/src/openstack_billing_db/model.py#L257-L258

And then further down the execution of the code still use flavor name to find out the flavor type.

https://github.com/CCI-MOC/openstack-billing-from-db/blob/7dfbc657d2e93db660b755fa5f6d63475ae57d44/src/openstack_billing_db/model.py#L21-L54

We should refactor that part of the code to not depend on flavor names anymore.

QuanMPhm commented 4 months ago

@knikolla My proposal will be to do the following code changes:

...the su_name string will be renamed as su_type, and will be defined as follows:

su_type = f"gpu_{pci_name}"

Given that we know pci_name will be a string within this list... https://github.com/CCI-MOC/openstack-billing-from-db/blob/fe0f1f9a61404947f12541d59f5747f6d52804c2/src/openstack_billing_db/billing.py#L189-L196

...this will allow us to change this code block... https://github.com/CCI-MOC/openstack-billing-from-db/blob/fe0f1f9a61404947f12541d59f5747f6d52804c2/src/openstack_billing_db/billing.py#L110-L123

...into this:

su_hour_attr = f"{i.service_unit_type}_su_hours"
if hasattr(invoice, su_hour_attr):
    invoice.__setattr__(su_hour_attr, invoice.__getattribute__(su_hour_attr) + su_hours)
else:
    raise Exception("Invalid flavor.")
QuanMPhm commented 4 months ago

@knikolla Do you have any opinion on my proposal?

knikolla commented 4 months ago

Sounds okay, start with that and I'll comment more on the code itself.

QuanMPhm commented 4 months ago

@knikolla Should I add a test case to make sure the flavor logic is handled correctly?

knikolla commented 4 months ago

@knikolla Should I add a test case to make sure the flavor logic is handled correctly?

Yes!