From 3bc63e51e49a7c9f010c7f4443b87b7308aa97a1 Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Wed, 31 Jul 2024 16:15:33 -0400 Subject: [PATCH 1/2] Added test cases to ensure correct billing with allocation change requests --- .../unit/test_calculate_quota_unit_hours.py | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py index 972921ec..e9f8b3d6 100644 --- a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py +++ b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py @@ -1,4 +1,5 @@ import datetime +import unittest import pytz import freezegun @@ -177,6 +178,157 @@ def test_new_allocation_quota_never_approved(self): ) self.assertEqual(value, 0) + @unittest.expectedFailure + def test_change_request_decrease(self): + """Test for when a change request decreases the quota""" + self.resource = self.new_openshift_resource( + name="", + auth_url="", + ) + user = self.new_user() + project = self.new_project(pi=user) + allocation = self.new_allocation(project, self.resource, 2) + + with freezegun.freeze_time("2020-03-15 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 2) + + with freezegun.freeze_time("2020-03-17 00:00:00"): + cr = allocation_models.AllocationChangeRequest.objects.create( + allocation=allocation, + status = allocation_models.AllocationChangeStatusChoice.objects.filter( + name="Approved").first() + ) + attr = allocation_models.AllocationAttribute.objects.filter( + allocation_attribute_type__name=attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + allocation=allocation + ).first() + allocation_models.AllocationAttributeChangeRequest.objects.create( + allocation_change_request=cr, + allocation_attribute=attr, + new_value=0, + ) + + with freezegun.freeze_time("2020-03-19 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 0) + + allocation.refresh_from_db() + + value = utils.calculate_quota_unit_hours( + allocation, + attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + pytz.utc.localize(datetime.datetime(2020, 3, 1, 0, 0, 1)), + pytz.utc.localize(datetime.datetime(2020, 3, 31, 23, 59, 59)) + ) + self.assertEqual(value, 96) + + def test_change_request_increase(self): + """Test for when a change request increases the quota""" + self.resource = self.new_openshift_resource( + name="", + auth_url="", + ) + user = self.new_user() + project = self.new_project(pi=user) + allocation = self.new_allocation(project, self.resource, 2) + + with freezegun.freeze_time("2020-03-15 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 2) + + with freezegun.freeze_time("2020-03-17 00:00:00"): + cr = allocation_models.AllocationChangeRequest.objects.create( + allocation=allocation, + status = allocation_models.AllocationChangeStatusChoice.objects.filter( + name="Approved").first() + ) + attr = allocation_models.AllocationAttribute.objects.filter( + allocation_attribute_type__name=attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + allocation=allocation + ).first() + allocation_models.AllocationAttributeChangeRequest.objects.create( + allocation_change_request=cr, + allocation_attribute=attr, + new_value=4, + ) + + with freezegun.freeze_time("2020-03-19 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 4) + + allocation.refresh_from_db() + + value = utils.calculate_quota_unit_hours( + allocation, + attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + pytz.utc.localize(datetime.datetime(2020, 3, 1, 0, 0, 1)), + pytz.utc.localize(datetime.datetime(2020, 3, 20, 23, 59, 59)) + ) + self.assertEqual(value, 384) + + @unittest.expectedFailure + def test_change_request_decrease_multiple(self): + """Test for when multiple different change request decreases the quota""" + self.resource = self.new_openshift_resource( + name="", + auth_url="", + ) + user = self.new_user() + project = self.new_project(pi=user) + allocation = self.new_allocation(project, self.resource, 2) + + with freezegun.freeze_time("2020-03-15 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 2) + + # In this case, approved CR is the first CR submitted + with freezegun.freeze_time("2020-03-16 00:00:00"): + cr = allocation_models.AllocationChangeRequest.objects.create( + allocation=allocation, + status = allocation_models.AllocationChangeStatusChoice.objects.filter( + name="Approved").first() + ) + attr = allocation_models.AllocationAttribute.objects.filter( + allocation_attribute_type__name=attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + allocation=allocation + ).first() + allocation_models.AllocationAttributeChangeRequest.objects.create( + allocation_change_request=cr, + allocation_attribute=attr, + new_value=0, + ) + + with freezegun.freeze_time("2020-03-17 00:00:00"): + cr = allocation_models.AllocationChangeRequest.objects.create( + allocation=allocation, + status = allocation_models.AllocationChangeStatusChoice.objects.filter( + name="Approved").first() + ) + attr = allocation_models.AllocationAttribute.objects.filter( + allocation_attribute_type__name=attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + allocation=allocation + ).first() + allocation_models.AllocationAttributeChangeRequest.objects.create( + allocation_change_request=cr, + allocation_attribute=attr, + new_value=1, + ) + + with freezegun.freeze_time("2020-03-19 00:00:00"): + utils.set_attribute_on_allocation( + allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 0) + + allocation.refresh_from_db() + + value = utils.calculate_quota_unit_hours( + allocation, + attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, + pytz.utc.localize(datetime.datetime(2020, 3, 1, 0, 0, 1)), + pytz.utc.localize(datetime.datetime(2020, 3, 31, 23, 59, 59)) + ) + self.assertEqual(value, 48) + def test_new_allocation_quota_change_request(self): self.resource = self.new_openshift_resource( name="", From e5236751951cb19064f34a0ea876165089cc6719 Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Thu, 1 Aug 2024 08:55:42 -0400 Subject: [PATCH 2/2] Fixed bug for billing with change requests Due to an boolean statement, the SU hours for storage will not be correctly calculated when change requests (CRs) to quotas are involved. Our intention with CRs is that when an allocation's storage quota decreases, this decreased value becomes effective (for billing purposes) when the CR for it is created, not when the CR is approved (or the time at which the quota actually decreased). The aforementioned boolean statement would instead lead to undesired behavior. --- .../tests/unit/test_calculate_quota_unit_hours.py | 2 -- src/coldfront_plugin_cloud/utils.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py index e9f8b3d6..f779e829 100644 --- a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py +++ b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py @@ -178,7 +178,6 @@ def test_new_allocation_quota_never_approved(self): ) self.assertEqual(value, 0) - @unittest.expectedFailure def test_change_request_decrease(self): """Test for when a change request decreases the quota""" self.resource = self.new_openshift_resource( @@ -267,7 +266,6 @@ def test_change_request_increase(self): ) self.assertEqual(value, 384) - @unittest.expectedFailure def test_change_request_decrease_multiple(self): """Test for when multiple different change request decreases the quota""" self.resource = self.new_openshift_resource( diff --git a/src/coldfront_plugin_cloud/utils.py b/src/coldfront_plugin_cloud/utils.py index 4dc74207..cabda75a 100644 --- a/src/coldfront_plugin_cloud/utils.py +++ b/src/coldfront_plugin_cloud/utils.py @@ -124,7 +124,7 @@ def calculate_quota_unit_hours(allocation: Allocation, # find one that happened just before the next event. cr_created_at = cr.history.first().created if cr.history.first().created <= event_time: - if unbounded_last_event_time and unbounded_last_event_time < cr_created_at: + if unbounded_last_event_time and unbounded_last_event_time > cr_created_at: # But after the unbounded last event time. continue