From e4c2aaa4b25133b8b985df85d388ee18d79151c1 Mon Sep 17 00:00:00 2001 From: Justin Riley Date: Mon, 27 Nov 2023 12:53:39 -0500 Subject: [PATCH 1/2] openshift: fix bug in set_quota The OpenShift set_quota function was skipping quota values if they evaluated to false. This updates the function to include quota values as long as they're not None which allows for 0 values to be set. --- src/coldfront_plugin_cloud/openshift.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coldfront_plugin_cloud/openshift.py b/src/coldfront_plugin_cloud/openshift.py index b814fd9..6f45fa7 100644 --- a/src/coldfront_plugin_cloud/openshift.py +++ b/src/coldfront_plugin_cloud/openshift.py @@ -85,7 +85,7 @@ def set_quota(self, project_id): url = f"{self.auth_url}/projects/{project_id}/quota" payload = dict() for key, func in QUOTA_KEY_MAPPING.items(): - if x := self.allocation.get_attribute(key): + if (x := self.allocation.get_attribute(key)) is not None: payload.update(func(x)) r = self.session.put(url, data=json.dumps({'Quota': payload})) self.check_response(r) From f4000db703c9ccedadf4404695048c07f9e7ee2e Mon Sep 17 00:00:00 2001 From: Justin Riley Date: Mon, 27 Nov 2023 12:55:18 -0500 Subject: [PATCH 2/2] fix default GPU quota and update tests The default GPU quota multiplier should have been 0 given that we don't want a user to have access to a GPU until they explicitly request one via coldfront. Also updated the tests to include requesting an OpenShift GPU. --- src/coldfront_plugin_cloud/tasks.py | 2 +- .../tests/functional/openshift/test_allocation.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coldfront_plugin_cloud/tasks.py b/src/coldfront_plugin_cloud/tasks.py index d9a3626..8556bfc 100644 --- a/src/coldfront_plugin_cloud/tasks.py +++ b/src/coldfront_plugin_cloud/tasks.py @@ -33,7 +33,7 @@ attributes.QUOTA_LIMITS_MEMORY: 4096, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB: 5, attributes.QUOTA_REQUESTS_STORAGE: 20, - attributes.QUOTA_REQUESTS_GPU: 1, + attributes.QUOTA_REQUESTS_GPU: 0, attributes.QUOTA_PVC: 2 } } diff --git a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py index 0092d4c..483d224 100644 --- a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py +++ b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py @@ -120,6 +120,7 @@ def test_new_allocation_quota(self): self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_MEMORY), 2 * 4096) self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB), 2 * 5) self.assertEqual(allocation.get_attribute(attributes.QUOTA_REQUESTS_STORAGE), 2 * 20) + self.assertEqual(allocation.get_attribute(attributes.QUOTA_REQUESTS_GPU), 2 * 0) self.assertEqual(allocation.get_attribute(attributes.QUOTA_PVC), 2 * 2) quota = allocator.get_quota(project_id)['Quota'] @@ -131,6 +132,7 @@ def test_new_allocation_quota(self): ":limits.memory": "8Gi", ":limits.ephemeral-storage": "10Gi", ":requests.storage": "40Gi", + ":requests.nvidia.com/gpu": "0", ":persistentvolumeclaims": "4", }) @@ -139,12 +141,14 @@ def test_new_allocation_quota(self): utils.set_attribute_on_allocation(allocation, attributes.QUOTA_LIMITS_MEMORY, 8192) utils.set_attribute_on_allocation(allocation, attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB, 50) utils.set_attribute_on_allocation(allocation, attributes.QUOTA_REQUESTS_STORAGE, 100) + utils.set_attribute_on_allocation(allocation, attributes.QUOTA_REQUESTS_GPU, 1) utils.set_attribute_on_allocation(allocation, attributes.QUOTA_PVC, 10) self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_CPU), 6) self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_MEMORY), 8192) self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB), 50) self.assertEqual(allocation.get_attribute(attributes.QUOTA_REQUESTS_STORAGE), 100) + self.assertEqual(allocation.get_attribute(attributes.QUOTA_REQUESTS_GPU), 1) self.assertEqual(allocation.get_attribute(attributes.QUOTA_PVC), 10) # This call should update the openshift quota to match the current attributes @@ -158,6 +162,7 @@ def test_new_allocation_quota(self): ":limits.memory": "8Gi", ":limits.ephemeral-storage": "50Gi", ":requests.storage": "100Gi", + ":requests.nvidia.com/gpu": "1", ":persistentvolumeclaims": "10", }) @@ -186,6 +191,7 @@ def test_reactivate_allocation(self): ":limits.memory": "8Gi", ":limits.ephemeral-storage": "10Gi", ":requests.storage": "40Gi", + ":requests.nvidia.com/gpu": "0", ":persistentvolumeclaims": "4", }) @@ -204,6 +210,7 @@ def test_reactivate_allocation(self): ":limits.memory": "8Gi", ":limits.ephemeral-storage": "10Gi", ":requests.storage": "40Gi", + ":requests.nvidia.com/gpu": "0", ":persistentvolumeclaims": "4", })