Skip to content

Move hardcoded quota names from attributes.py to tests #324

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

Open
marcoagonzales007 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Move hardcoded quota names from attributes.py to tests #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 1 addition & 27 deletions src/coldfront_plugin_cloud/attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,35 +87,9 @@ class CloudAllocationAttribute:
CloudAllocationAttribute(name=ALLOCATION_ALERT, type="Int", is_changeable=True),
]

###########################################################
# OpenStack Quota Attributes
QUOTA_INSTANCES = "OpenStack Compute Instance Quota"
QUOTA_RAM = "OpenStack Compute RAM Quota (MiB)"
QUOTA_VCPU = "OpenStack Compute vCPU Quota"

QUOTA_VOLUMES = "OpenStack Number of Volumes Quota"
QUOTA_VOLUMES_GB = "OpenStack Volume Quota (GiB)"

QUOTA_FLOATING_IPS = "OpenStack Floating IP Quota"
QUOTA_NETWORKS = "Openstack Network Quota"

QUOTA_OBJECT_GB = "OpenStack Swift Quota (GiB)"

# QUOTE_GPU is still registered because count_gpu_usage command relies on it
QUOTA_GPU = "OpenStack GPU Quota"

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the comment! It helps clarify why we still have one attribute


###########################################################
# OpenShift Quota Attributes
QUOTA_LIMITS_CPU = "OpenShift Limit on CPU Quota"
QUOTA_LIMITS_MEMORY = "OpenShift Limit on RAM Quota (MiB)"
QUOTA_LIMITS_EPHEMERAL_STORAGE_GB = "OpenShift Limit on Ephemeral Storage Quota (GiB)"
QUOTA_REQUESTS_NESE_STORAGE = "OpenShift Request on NESE Storage Quota (GiB)"
QUOTA_REQUESTS_IBM_STORAGE = "OpenShift Request on IBM Storage Quota (GiB)"
QUOTA_REQUESTS_GPU = "OpenShift Request on GPU Quota"
QUOTA_REQUESTS_VM_GPU_A100_SXM4 = "OpenShift Request on GPU A100 SXM4"
QUOTA_REQUESTS_VM_GPU_V100 = "OpenShift Request on GPU V100"
QUOTA_REQUESTS_VM_GPU_H100 = "OpenShift Request on GPU H100"
QUOTA_PVC = "OpenShift Persistent Volume Claims Quota"


ALLOCATION_QUOTA_ATTRIBUTES = [
CloudAllocationAttribute(name=QUOTA_GPU),
Expand Down
34 changes: 18 additions & 16 deletions src/coldfront_plugin_cloud/management/commands/calculate_storage_gb_hours.py
View file
Open in desktop

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing the hardcoded quota strings from this file will be more complicated, as we are trying to do a more significant refactor. Refer to this comment to understand the bigger idea of what we are trying to achieve.

In summary, we want to encode in the quota display name information about the key of the quota on nerc-rates. This will require making a coordinated change with nerc-rates. We eventually want this command to be in such a state that there should be no code that is statically fetching certain storage rates (ibm-storage), but is dynamically fetching when only given the storage quotaspec in each allocation.

This refactor will be somewhat significant, so for now, I would suggest that you revert this file, and replace references to attributes with just hardcoded string literals or top-level constants. You can refactor this file in another PR.

Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ def process_invoice_row(allocation, attrs, su_name, rate):
f, delimiter=",", quotechar="|", quoting=csv.QUOTE_MINIMAL
)
csv_invoice_writer.writerow(InvoiceRow.get_headers())

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stray whitespace removal

for allocation in openstack_allocations:
allocation_str = (
f'{allocation.pk} of project "{allocation.project.title}"'
Expand All @@ -337,29 +336,32 @@ def process_invoice_row(allocation, attrs, su_name, rate):
quotaspec.invoice_name,
openstack_nese_storage_rate,
)
openshift_storage_rates = {
"OpenShift NESE Storage": openshift_nese_storage_rate,
"OpenShift IBM Scale Storage": openshift_ibm_storage_rate,
}

for allocation in openshift_allocations:
allocation_str = (
f'{allocation.pk} of project "{allocation.project.title}"'
)
logger.debug(f"Starting billing for allocation {allocation_str}.")

process_invoice_row(
allocation,
[
attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB,
attributes.QUOTA_REQUESTS_NESE_STORAGE,
],
"OpenShift NESE Storage",
openshift_nese_storage_rate,
)
attrs_by_invoice_name = {}
for quota_name, quotaspec in get_storage_quotaspecs(allocation).items():
attrs_by_invoice_name.setdefault(quotaspec.invoice_name, []).append(
quota_name
)

process_invoice_row(
allocation,
[attributes.QUOTA_REQUESTS_IBM_STORAGE],
"OpenShift IBM Scale Storage",
openshift_ibm_storage_rate,
)
for invoice_name, quota_names in attrs_by_invoice_name.items():
rate = openshift_storage_rates.get(invoice_name)
if rate is None:
logger.warning(
f"No rate configured for invoice name {invoice_name!r} "
f"on allocation {allocation_str}, skipping."
)
continue
process_invoice_row(allocation, quota_names, invoice_name, rate)

if options["upload_to_s3"]:
logger.info(f"Uploading to S3 endpoint {options['s3_endpoint_url']}.")
Expand Down
14 changes: 8 additions & 6 deletions src/coldfront_plugin_cloud/management/commands/convert_swift_quota_to_gib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from coldfront.core.allocation.models import Allocation, AllocationStatusChoice

logging.basicConfig(level=logging.INFO)
SWIFT_QUOTA_ATTRIBUTE = "OpenStack Swift Quota (GiB)"
logger = logging.getLogger(__name__)


Expand All @@ -26,19 +27,20 @@ def handle(self, *args, **options):
resources__in=openstack_resources,
)
for allocation in openstack_allocations:
if not (
swift_quota := allocation.get_attribute(attributes.QUOTA_OBJECT_GB)
):
if not (swift_quota := allocation.get_attribute(SWIFT_QUOTA_ATTRIBUTE)):
continue

allocation_str = f'{allocation.pk} of project "{allocation.project.title}"'
obj_key = openstack.QUOTA_KEY_MAPPING["object"]["keys"][
attributes.QUOTA_OBJECT_GB
]
allocator = openstack.OpenStackResourceAllocator(
allocation.resources.first(), allocation
)

object_labels = allocator._get_resource_quota_labels_by_service("object")
if not object_labels:
logger.error(f"{allocation_str} has no object quota defined, skipping.")
continue
obj_key = object_labels[0]

project_id = allocation.get_attribute(attributes.ALLOCATION_PROJECT_ID)
if not project_id:
logger.error(f"{allocation_str} is active but has no Project ID set.")
Expand Down
26 changes: 13 additions & 13 deletions src/coldfront_plugin_cloud/management/commands/register_default_quotas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,31 +37,31 @@ def handle(self, *args, **options):
# Define quotas for each resource type
openshift_quotas = [
{
"display_name": attributes.QUOTA_LIMITS_CPU,
"display_name": "OpenShift Limit on CPU Quota",
"quota_label": "limits.cpu",
"multiplier": 1,
},
{
"display_name": attributes.QUOTA_LIMITS_MEMORY,
"display_name": "OpenShift Limit on RAM Quota (MiB)",
"quota_label": "limits.memory",
"multiplier": 4096,
"unit_suffix": "Mi",
},
{
"display_name": attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB,
"display_name": "OpenShift Limit on Ephemeral Storage Quota (GiB)",
"quota_label": "limits.ephemeral-storage",
"multiplier": 5,
"unit_suffix": "Gi",
"resource_type": STORAGE_RESOURCE_TYPE_NAME,
"invoice_name": OPENSHIFT_STORAGE_INVOICE_NAME,
},
{
"display_name": attributes.QUOTA_PVC,
"display_name": "OpenShift Persistent Volume Claims Quota",
"quota_label": "persistentvolumeclaims",
"multiplier": 2,
},
{
"display_name": attributes.QUOTA_REQUESTS_NESE_STORAGE,
"display_name": "OpenShift Request on NESE Storage Quota (GiB)",
"quota_label": "ocs-external-storagecluster-ceph-rbd.storageclass.storage.k8s.io/requests.storage",
"multiplier": 20,
"static_quota": 0,
Expand All @@ -70,48 +70,48 @@ def handle(self, *args, **options):
"invoice_name": OPENSHIFT_STORAGE_INVOICE_NAME,
},
{
"display_name": attributes.QUOTA_REQUESTS_GPU,
"display_name": "OpenShift Request on GPU Quota",
"quota_label": "requests.nvidia.com/gpu",
"multiplier": 0,
},
]

openstack_quotas = [
{
"display_name": attributes.QUOTA_INSTANCES,
"display_name": "OpenStack Compute Instance Quota",
"quota_label": "compute.instances",
"multiplier": 1,
},
{
"display_name": attributes.QUOTA_VCPU,
"display_name": "OpenStack Compute vCPU Quota",
"quota_label": "compute.cores",
"multiplier": 1,
},
{
"display_name": attributes.QUOTA_RAM,
"display_name": "OpenStack Compute RAM Quota (MiB)",
"quota_label": "compute.ram",
"multiplier": 4096,
},
{
"display_name": attributes.QUOTA_VOLUMES,
"display_name": "OpenStack Number of Volumes Quota",
"quota_label": "volume.volumes",
"multiplier": 2,
},
{
"display_name": attributes.QUOTA_VOLUMES_GB,
"display_name": "OpenStack Volume Quota (GiB)",
"quota_label": "volume.gigabytes",
"multiplier": 20,
"resource_type": STORAGE_RESOURCE_TYPE_NAME,
"invoice_name": OPENSTACK_STORAGE_INVOICE_NAME,
},
{
"display_name": attributes.QUOTA_FLOATING_IPS,
"display_name": "OpenStack Floating IP Quota",
"quota_label": "network.floatingip",
"multiplier": 0,
"static_quota": 2,
},
{
"display_name": attributes.QUOTA_OBJECT_GB,
"display_name": "OpenStack Swift Quota (GiB)",
"quota_label": "object.x-account-meta-quota-bytes",
"multiplier": 1,
"resource_type": STORAGE_RESOURCE_TYPE_NAME,
Expand Down
17 changes: 7 additions & 10 deletions src/coldfront_plugin_cloud/openstack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ def _set_object_quota(self, project_id, payload):
# Note(knikolla): For consistency with other OpenStack
# quotas we're storing this as GB on the attribute and
# converting to bytes for Swift.
_, obj_q_mapping = self._extract_quota_label(
self.resource_quotaspecs.root[attributes.QUOTA_OBJECT_GB]
)
payload[obj_q_mapping] *= GB_IN_BYTES
if payload[obj_q_mapping] <= 0:
payload[obj_q_mapping] = 1
for obj_q_mapping in self._get_resource_quota_labels_by_service("object"):
if obj_q_mapping not in payload:
continue
Comment on lines +225 to +227

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As it stands, this function expects the object service to only have one quota (object.x-account-meta-quota-bytes), as such, it looks misleading to use a for loop. This function would likely cause unintended behavior if there was ever more than one object quota. I would suggest:

obj_q_mapping = self._get_resource_quota_labels_by_service("object")[0]

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, why was there a need for the if clause?

payload[obj_q_mapping] *= GB_IN_BYTES
if payload[obj_q_mapping] <= 0:
payload[obj_q_mapping] = 1
self.object(project_id).post_account(headers=payload)
except ksa_exceptions.catalog.EndpointNotFound:
logger.debug("No swift available, skipping its quota.")
Expand Down Expand Up @@ -291,10 +291,7 @@ def get_quota(self, project_id):

quotas = self._get_network_quota(quotas, project_id)

if object_quotaspec := self.resource_quotaspecs.root.get(
attributes.QUOTA_OBJECT_GB
):
_, key = self._extract_quota_label(object_quotaspec)
for key in self._get_resource_quota_labels_by_service("object"):

@QuanMPhm QuanMPhm Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does look aesthetically neater, but could lead to the same confusion mentioned above.

try:
swift = self.object(project_id).head_account()
except ksa_exceptions.catalog.EndpointNotFound:
Expand Down
27 changes: 27 additions & 0 deletions src/coldfront_plugin_cloud/tests/attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from coldfront_plugin_cloud.attributes import * # noqa: F403


# OpenStack Quota Attributes
QUOTA_INSTANCES = "OpenStack Compute Instance Quota"
QUOTA_RAM = "OpenStack Compute RAM Quota (MiB)"
QUOTA_VCPU = "OpenStack Compute vCPU Quota"

QUOTA_VOLUMES = "OpenStack Number of Volumes Quota"
QUOTA_VOLUMES_GB = "OpenStack Volume Quota (GiB)"

QUOTA_FLOATING_IPS = "OpenStack Floating IP Quota"
QUOTA_NETWORKS = "Openstack Network Quota"

QUOTA_OBJECT_GB = "OpenStack Swift Quota (GiB)"

# OpenShift Quota Attributes
QUOTA_LIMITS_CPU = "OpenShift Limit on CPU Quota"
QUOTA_LIMITS_MEMORY = "OpenShift Limit on RAM Quota (MiB)"
QUOTA_LIMITS_EPHEMERAL_STORAGE_GB = "OpenShift Limit on Ephemeral Storage Quota (GiB)"
QUOTA_REQUESTS_NESE_STORAGE = "OpenShift Request on NESE Storage Quota (GiB)"
QUOTA_REQUESTS_IBM_STORAGE = "OpenShift Request on IBM Storage Quota (GiB)"
QUOTA_REQUESTS_GPU = "OpenShift Request on GPU Quota"
QUOTA_REQUESTS_VM_GPU_A100_SXM4 = "OpenShift Request on GPU A100 SXM4"
QUOTA_REQUESTS_VM_GPU_V100 = "OpenShift Request on GPU V100"
QUOTA_REQUESTS_VM_GPU_H100 = "OpenShift Request on GPU H100"
QUOTA_PVC = "OpenShift Persistent Volume Claims Quota"
3 changes: 2 additions & 1 deletion src/coldfront_plugin_cloud/tests/functional/esi/test_allocations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import uuid
import time

from coldfront_plugin_cloud import attributes, openstack, esi, tasks
from coldfront_plugin_cloud import openstack, esi, tasks
from coldfront_plugin_cloud.tests import attributes
from coldfront_plugin_cloud.tests import base

from django.core.management import call_command
Expand Down
3 changes: 2 additions & 1 deletion src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import unittest
import uuid

from coldfront_plugin_cloud import attributes, openshift, tasks, utils
from coldfront_plugin_cloud import openshift, tasks, utils
from coldfront_plugin_cloud.tests import attributes
from coldfront_plugin_cloud.tests import base

from django.core.management import call_command
Expand Down
3 changes: 2 additions & 1 deletion src/coldfront_plugin_cloud/tests/functional/openshift_vm/test_allocation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import os
import unittest

from coldfront_plugin_cloud import attributes, openshift_vm, tasks
from coldfront_plugin_cloud import openshift_vm, tasks
from coldfront_plugin_cloud.tests import attributes
from coldfront_plugin_cloud.tests import base

from django.core.management import call_command
Expand Down
3 changes: 2 additions & 1 deletion src/coldfront_plugin_cloud/tests/functional/openstack/test_allocation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import uuid
import time

from coldfront_plugin_cloud import attributes, openstack, tasks, utils
from coldfront_plugin_cloud import openstack, tasks, utils
from coldfront_plugin_cloud.tests import attributes
from coldfront_plugin_cloud.tests import base

from django.core.management import call_command
Expand Down
2 changes: 1 addition & 1 deletion src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import freezegun

from coldfront_plugin_cloud import attributes
from coldfront_plugin_cloud.tests import attributes
from coldfront_plugin_cloud.tests import base
from coldfront_plugin_cloud import utils

Expand Down
2 changes: 1 addition & 1 deletion src/coldfront_plugin_cloud/tests/unit/test_register_default_quotas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.core.management import call_command

from coldfront_plugin_cloud.tests.base import TestBase
from coldfront_plugin_cloud import attributes
from coldfront_plugin_cloud.tests import attributes


class TestRegisterDefaultQuotas(TestBase):
Expand Down
Loading