Merge pull request #190 from knikolla/feature/ignore_hours

 Support hours when providing outage intervals in billing

Author: knikolla

src/coldfront_plugin_cloud/management/commands/calculate_storage_gb_hours.py

@@ -14,11 +14,25 @@
 from coldfront.core.allocation.models import Allocation, AllocationStatusChoice
 import pytz
 
-from nerc_rates import load_from_url
 
 logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
 
+_RATES = None
+
+
+def get_rates():
+    # nerc-rates doesn't work with Python 3.9, which is what ColdFront is currently
+    # using in Production. Lazily load the rates only when either of the storage rates
+    # is not set via CLI arguments, so we can keep providing them via CLI until we upgrade
+    # Python version.
+    global _RATES
+
+    if _RATES is None:
+        from nerc_rates import load_from_url
+        _RATES = load_from_url()
+    return _RATES
+
 
 @dataclasses.dataclass
 class InvoiceRow:
@@ -68,7 +82,7 @@ def get_values(self):
 
 
 def datetime_type(v):
-    return pytz.utc.localize(datetime.strptime(v, '%Y-%m-%d'))
+    return pytz.utc.localize(datetime.fromisoformat(v))
 
 
 class Command(BaseCommand):
@@ -87,19 +101,19 @@ def add_arguments(self, parser):
         )
         parser.add_argument('--output', type=str, default='invoices.csv',
                              help='CSV file to write invoices to.')
-        parser.add_argument('--openstack-gb-rate', type=Decimal, required=True,
+        parser.add_argument('--openstack-gb-rate', type=Decimal, required=False,
                             help='Rate for OpenStack Volume and Object GB/hour.')
-        parser.add_argument('--openshift-gb-rate', type=Decimal, required=True,
+        parser.add_argument('--openshift-gb-rate', type=Decimal, required=False,
                             help='Rate for OpenShift GB/hour.')
         parser.add_argument('--s3-endpoint-url', type=str,
                             default='https://s3.us-east-005.backblazeb2.com')
         parser.add_argument('--s3-bucket-name', type=str,
                             default='nerc-invoicing')
         parser.add_argument('--upload-to-s3', default=False, action='store_true',
                           help='Upload generated CSV invoice to S3 storage.')
-        parser.add_argument('--excluded-date-ranges', type=str, 
+        parser.add_argument('--excluded-time-ranges', type=str,
                             default=None, nargs='+',
-                            help='List of date ranges excluded from billing')
+                            help='List of time ranges excluded from billing, in ISO format.')
 
     @staticmethod
     def default_start_argument():
@@ -176,9 +190,9 @@ def process_invoice_row(allocation, attrs, su_name, rate):
         logger.info(f'Processing invoices for {options["invoice_month"]}.')
         logger.info(f'Interval {options["start"] - options["end"]}.')
 
-        if options["excluded_date_ranges"]:
+        if options["excluded_time_ranges"]:
             excluded_intervals_list = utils.load_excluded_intervals(
-                options["excluded_date_ranges"]
+                options["excluded_time_ranges"]
             )
         else:
             excluded_intervals_list = None
@@ -200,15 +214,19 @@ def process_invoice_row(allocation, attrs, su_name, rate):
             resources__in=openshift_resources
         )
 
-        rates = load_from_url()
-        openstack_storage_rate = openshift_storage_rate = Decimal(
-            rates.get_value_at('Storage GB Rate', options["invoice_month"]))
-        
         if options['openstack_gb_rate']:
             openstack_storage_rate = options['openstack_gb_rate']
+        else:
+            openstack_storage_rate = Decimal(
+                get_rates().get_value_at('Storage GB Rate', options["invoice_month"])
+            )
 
         if options['openshift_gb_rate']:
             openshift_storage_rate = options['openshift_gb_rate']
+        else:
+            openshift_storage_rate = Decimal(
+                get_rates().get_value_at('Storage GB Rate', options["invoice_month"])
+            )
 
         logger.info(f'Using storage rate {openstack_storage_rate} (Openstack) and '
                     f'{openshift_storage_rate} (Openshift) for {options["invoice_month"]}')

src/coldfront_plugin_cloud/tests/unit/openshift/__init__.py

@@ -1,6 +1,7 @@
 import datetime
 import unittest
 import pytz
+import tempfile
 
 import freezegun
 
@@ -9,6 +10,7 @@
 from coldfront_plugin_cloud import utils
 
 from coldfront.core.allocation import models as allocation_models
+from django.core.management import call_command
 
 
 SECONDS_IN_DAY = 3600 * 24
@@ -42,6 +44,32 @@ def test_new_allocation_quota(self):
         )
         self.assertEqual(value, 96)
 
+        with tempfile.NamedTemporaryFile() as fp:
+            call_command(
+                'calculate_storage_gb_hours',
+                '--output', fp.name,
+                '--start', '2020-03-01',
+                '--end', '2020-03-31',
+                '--openstack-gb-rate','0.0000087890625',
+                '--openshift-gb-rate','0.0000087890625',
+                '--invoice-month','2020-03'
+            )
+
+        # Let's test a complete CLI call including excluded time, while we're at it. This is not for testing
+        # the validity but just the unerrored execution of the complete pipeline.
+        # Tests that verify the correct output are further down in the test file.
+        with tempfile.NamedTemporaryFile() as fp:
+            call_command(
+                'calculate_storage_gb_hours',
+                '--output', fp.name,
+                '--start', '2020-03-01',
+                '--end', '2020-03-31',
+                '--openstack-gb-rate','0.0000087890625',
+                '--openshift-gb-rate','0.0000087890625',
+                '--invoice-month','2020-03',
+                '--excluded-time-ranges', '2020-03-02 00:00:00,2020-03-03 05:00:00'
+            )
+
 
     def test_new_allocation_quota_expired(self):
         """Test that expiration doesn't affect invoicing."""
@@ -391,16 +419,17 @@ def get_excluded_interval_datetime_list(excluded_interval_list):
             ]
 
         # Single interval within active period
-        excluded_intervals = get_excluded_interval_datetime_list(
-            (((2020, 3, 15), (2020, 3, 16)),)
-        )
+        excluded_intervals = [
+            (datetime.datetime(2020, 3, 15, 9, 30, 0),
+             datetime.datetime(2020, 3, 16, 10, 30, 0)),
+        ]
 
         value = utils.get_included_duration(
             datetime.datetime(2020, 3, 15, 0, 0, 0),
             datetime.datetime(2020, 3, 17, 0, 0, 0),
             excluded_intervals
         )
-        self.assertEqual(value, SECONDS_IN_DAY * 1)
+        self.assertEqual(value, SECONDS_IN_DAY * 1 - 3600)
 
         # Interval starts before active period
         excluded_intervals = get_excluded_interval_datetime_list(
@@ -470,21 +499,21 @@ def test_load_excluded_intervals(self):
         ]
         output = utils.load_excluded_intervals(interval_list)
         self.assertEqual(output, [
-            [datetime.datetime(2023, 1, 1, 0, 0, 0),
-            datetime.datetime(2023, 1, 2, 0, 0, 0)]
+            [pytz.utc.localize(datetime.datetime(2023, 1, 1, 0, 0, 0)),
+            pytz.utc.localize(datetime.datetime(2023, 1, 2, 0, 0, 0))]
         ])
 
         # More than 1 interval
         interval_list = [
             "2023-01-01,2023-01-02",
-            "2023-01-04,2023-01-15",
+            "2023-01-04 09:00:00,2023-01-15 10:00:00",
         ]
         output = utils.load_excluded_intervals(interval_list)
         self.assertEqual(output, [
-            [datetime.datetime(2023, 1, 1, 0, 0, 0),
-            datetime.datetime(2023, 1, 2, 0, 0, 0)],
-            [datetime.datetime(2023, 1, 4, 0, 0, 0),
-            datetime.datetime(2023, 1, 15, 0, 0, 0)]
+            [pytz.utc.localize(datetime.datetime(2023, 1, 1, 0, 0, 0)),
+            pytz.utc.localize(datetime.datetime(2023, 1, 2, 0, 0, 0))],
+            [pytz.utc.localize(datetime.datetime(2023, 1, 4, 9, 0, 0)),
+            pytz.utc.localize(datetime.datetime(2023, 1, 15, 10, 0, 0))]
         ])
 
     def test_load_excluded_intervals_invalid(self):

src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py

@@ -185,12 +185,12 @@ def check_overlapping_intervals(excluded_intervals_list):
     excluded_intervals_list = list()
     for interval in excluded_interval_arglist:
         start, end = interval.strip().split(",")
-        start_dt, end_dt = [datetime.datetime.strptime(i, "%Y-%m-%d") for i in [start, end]]
+        start_dt, end_dt = [datetime.datetime.fromisoformat(i) for i in [start, end]]
         assert end_dt > start_dt, f"Interval end date ({end}) is before start date ({start})!"
         excluded_intervals_list.append(
             [
-                datetime.datetime.strptime(start, "%Y-%m-%d"),
-                datetime.datetime.strptime(end, "%Y-%m-%d")
+                pytz.utc.localize(datetime.datetime.fromisoformat(start)),
+                pytz.utc.localize(datetime.datetime.fromisoformat(end))
             ] 
         )