diff --git a/ci/run_functional_tests_openshift.sh b/ci/run_functional_tests_openshift.sh index 060ef547..87c37540 100755 --- a/ci/run_functional_tests_openshift.sh +++ b/ci/run_functional_tests_openshift.sh @@ -17,6 +17,8 @@ fi export DJANGO_SETTINGS_MODULE="local_settings" export FUNCTIONAL_TESTS="True" export OS_AUTH_URL="https://onboarding-onboarding.cluster.local" +export OS_API_URL="https://onboarding-onboarding.cluster.local:6443" + coverage run --source="." -m django test coldfront_plugin_cloud.tests.functional.openshift coverage report diff --git a/src/coldfront_plugin_cloud/attributes.py b/src/coldfront_plugin_cloud/attributes.py index a410d150..89f680ff 100644 --- a/src/coldfront_plugin_cloud/attributes.py +++ b/src/coldfront_plugin_cloud/attributes.py @@ -19,6 +19,7 @@ class CloudAllocationAttribute: RESOURCE_AUTH_URL = 'Identity Endpoint URL' +RESOURCE_API_URL = 'OpenShift API Endpoint URL' RESOURCE_IDENTITY_NAME = 'OpenShift Identity Provider Name' RESOURCE_ROLE = 'Role for User in Project' @@ -33,6 +34,7 @@ class CloudAllocationAttribute: RESOURCE_ATTRIBUTES = [ CloudResourceAttribute(name=RESOURCE_AUTH_URL), + CloudResourceAttribute(name=RESOURCE_API_URL), CloudResourceAttribute(name=RESOURCE_IDENTITY_NAME), CloudResourceAttribute(name=RESOURCE_FEDERATION_PROTOCOL), CloudResourceAttribute(name=RESOURCE_IDP), diff --git a/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py b/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py index 6e96bcb9..91a2fc22 100644 --- a/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py +++ b/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py @@ -17,6 +17,10 @@ def add_arguments(self, parser): help='Name of OpenShift resource') parser.add_argument('--auth-url', type=str, required=True, help='URL of the openshift-acct-mgt endpoint') + parser.add_argument('--api-url', type=str, required=True, + help='API URL of the openshift cluster') + parser.add_argument('--idp', type=str, required=True, + help='Name of Openshift identity provider') parser.add_argument('--role', type=str, default='edit', help='Role for user when added to project (default: edit)') @@ -37,6 +41,18 @@ def handle(self, *args, **options): resource=openshift, value=options['auth_url'] ) + ResourceAttribute.objects.get_or_create( + resource_attribute_type=ResourceAttributeType.objects.get( + name=attributes.RESOURCE_API_URL), + resource=openshift, + value=options['api_url'] + ) + ResourceAttribute.objects.get_or_create( + resource_attribute_type=ResourceAttributeType.objects.get( + name=attributes.RESOURCE_IDENTITY_NAME), + resource=openshift, + value=options['idp'] + ) ResourceAttribute.objects.get_or_create( resource_attribute_type=ResourceAttributeType.objects.get( name=attributes.RESOURCE_ROLE), diff --git a/src/coldfront_plugin_cloud/management/commands/validate_allocations.py b/src/coldfront_plugin_cloud/management/commands/validate_allocations.py index ca9d5379..85206970 100644 --- a/src/coldfront_plugin_cloud/management/commands/validate_allocations.py +++ b/src/coldfront_plugin_cloud/management/commands/validate_allocations.py @@ -183,7 +183,7 @@ def handle(self, *args, **options): ) continue - quota = allocator.get_quota(project_id)["Quota"] + quota = allocator.get_quota(project_id) failed_validation = Command.sync_users(project_id, allocation, allocator, options["apply"]) diff --git a/src/coldfront_plugin_cloud/openshift.py b/src/coldfront_plugin_cloud/openshift.py index 7edb9c43..681f2947 100644 --- a/src/coldfront_plugin_cloud/openshift.py +++ b/src/coldfront_plugin_cloud/openshift.py @@ -36,12 +36,12 @@ def clean_openshift_metadata(obj): return obj QUOTA_KEY_MAPPING = { - attributes.QUOTA_LIMITS_CPU: lambda x: {":limits.cpu": f"{x * 1000}m"}, - attributes.QUOTA_LIMITS_MEMORY: lambda x: {":limits.memory": f"{x}Mi"}, - attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB: lambda x: {":limits.ephemeral-storage": f"{x}Gi"}, - attributes.QUOTA_REQUESTS_STORAGE: lambda x: {":requests.storage": f"{x}Gi"}, - attributes.QUOTA_REQUESTS_GPU: lambda x: {":requests.nvidia.com/gpu": f"{x}"}, - attributes.QUOTA_PVC: lambda x: {":persistentvolumeclaims": f"{x}"}, + attributes.QUOTA_LIMITS_CPU: lambda x: {"limits.cpu": f"{x * 1000}m"}, + attributes.QUOTA_LIMITS_MEMORY: lambda x: {"limits.memory": f"{x}Mi"}, + attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB: lambda x: {"limits.ephemeral-storage": f"{x}Gi"}, + attributes.QUOTA_REQUESTS_STORAGE: lambda x: {"requests.storage": f"{x}Gi"}, + attributes.QUOTA_REQUESTS_GPU: lambda x: {"requests.nvidia.com/gpu": f"{x}"}, + attributes.QUOTA_PVC: lambda x: {"persistentvolumeclaims": f"{x}"}, } @@ -77,7 +77,7 @@ def __init__(self, resource, allocation): def k8_client(self): # Load Endpoint URL and Auth token for new k8 client openshift_token = os.getenv(f"OPENSHIFT_{self.safe_resource_name}_TOKEN") - openshift_url = self.resource.get_attribute(attributes.RESOURCE_AUTH_URL) + openshift_url = self.resource.get_attribute(attributes.RESOURCE_API_URL) k8_config = kubernetes.client.Configuration() k8_config.api_key["authorization"] = openshift_token @@ -146,20 +146,41 @@ def create_project(self, suggested_project_name): project_name = project_id self._create_project(project_name, project_id) return self.Project(project_name, project_id) + + def delete_moc_quotas(self, project_id): + """deletes all resourcequotas from an openshift project""" + resourcequotas = self._openshift_get_resourcequotas(project_id) + for resourcequota in resourcequotas: + self._openshift_delete_resourcequota(project_id, resourcequota["metadata"]["name"]) + + logger.info(f"All quotas for {project_id} successfully deleted") def set_quota(self, project_id): - url = f"{self.auth_url}/projects/{project_id}/quota" - payload = dict() + """Sets the quota for a project, creating a minimal resourcequota + object in the project namespace with no extra scopes""" + + quota_spec = {} for key, func in QUOTA_KEY_MAPPING.items(): 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) + quota_spec.update(func(x)) + + quota_def = { + "metadata": {"name": f"{project_id}-project"}, + "spec": {"hard": quota_spec}, + } + + self.delete_moc_quotas(project_id) + self._openshift_create_resourcequota(project_id, quota_def) + + logger.info(f"Quota for {project_id} successfully created") def get_quota(self, project_id): - url = f"{self.auth_url}/projects/{project_id}/quota" - r = self.session.get(url) - return self.check_response(r) + cloud_quotas = self._openshift_get_resourcequotas(project_id) + combined_quota = {} + for cloud_quota in cloud_quotas: + combined_quota.update(cloud_quota["spec"]["hard"]) + + return combined_quota def create_project_defaults(self, project_id): pass @@ -181,7 +202,7 @@ def reactivate_project(self, project_id): def get_federated_user(self, username): if ( self._openshift_user_exists(username) - and self._openshift_get_identity(username) + and self._openshift_identity_exists(username) and self._openshift_useridentitymapping_exists(username, username) ): return {'username': username} @@ -264,25 +285,84 @@ def _openshift_get_identity(self, id_user): def _openshift_user_exists(self, user_name): try: self._openshift_get_user(user_name) - except kexc.NotFoundError: - return False + except kexc.NotFoundError as e: + # Ensures error raise because resource not found, + # not because of other reasons, like incorrect url + e_info = json.loads(e.body) + if ( + e_info["reason"] == "NotFound" + and e_info["details"]["name"] == user_name + ): + return False + raise e return True def _openshift_identity_exists(self, id_user): try: self._openshift_get_identity(id_user) - except kexc.NotFoundError: - return False + except kexc.NotFoundError as e: + e_info = json.loads(e.body) + if e_info.get("reason") == "NotFound": + return False + raise e return True def _openshift_useridentitymapping_exists(self, user_name, id_user): try: user = self._openshift_get_user(user_name) - except kexc.NotFoundError: - return False + except kexc.NotFoundError as e: + e_info = json.loads(e.body) + if e_info.get("reason") == "NotFound": + return False + raise e return any( identity == self.qualified_id_user(id_user) for identity in user.get("identities", []) ) + def _openshift_get_project(self, project_name): + api = self.get_resource_api(API_PROJECT, "Project") + return clean_openshift_metadata(api.get(name=project_name).to_dict()) + + def _openshift_get_resourcequotas(self, project_id): + """Returns a list of resourcequota objects in namespace with name `project_id`""" + # Raise a NotFound error if the project doesn't exist + self._openshift_get_project(project_id) + api = self.get_resource_api(API_CORE, "ResourceQuota") + res = clean_openshift_metadata(api.get(namespace=project_id).to_dict()) + + return res["items"] + + def _wait_for_quota_to_settle(self, project_id, resource_quota): + """Wait for quota on resourcequotas to settle. + + When creating a new resourcequota that sets a quota on resourcequota objects, we need to + wait for OpenShift to calculate the quota usage before we attempt to create any new + resourcequota objects. + """ + + if "resourcequotas" in resource_quota["spec"]["hard"]: + logger.info("waiting for resourcequota quota") + + api = self.get_resource_api(API_CORE, "ResourceQuota") + while True: + resp = clean_openshift_metadata( + api.get( + namespace=project_id, name=resource_quota["metadata"]["name"] + ).to_dict() + ) + if "resourcequotas" in resp["status"].get("used", {}): + break + time.sleep(0.1) + + def _openshift_create_resourcequota(self, project_id, quota_def): + api = self.get_resource_api(API_CORE, "ResourceQuota") + res = api.create(namespace=project_id, body=quota_def).to_dict() + self._wait_for_quota_to_settle(project_id, res) + + def _openshift_delete_resourcequota(self, project_id, resourcequota_name): + """In an openshift namespace {project_id) delete a specified resourcequota""" + api = self.get_resource_api(API_CORE, "ResourceQuota") + return api.delete(namespace=project_id, name=resourcequota_name).to_dict() + diff --git a/src/coldfront_plugin_cloud/tests/base.py b/src/coldfront_plugin_cloud/tests/base.py index 305410a1..14184c33 100644 --- a/src/coldfront_plugin_cloud/tests/base.py +++ b/src/coldfront_plugin_cloud/tests/base.py @@ -81,13 +81,15 @@ def new_openstack_resource(name=None, auth_url=None) -> Resource: return Resource.objects.get(name=resource_name) @staticmethod - def new_openshift_resource(name=None, auth_url=None) -> Resource: + def new_openshift_resource(name=None, auth_url=None, api_url=None, idp=None) -> Resource: resource_name = name or uuid.uuid4().hex call_command( 'add_openshift_resource', name=resource_name, auth_url=auth_url or 'https://onboarding-onboarding.cluster.local', + api_url=api_url or 'https://onboarding-onboarding.cluster.local:6443', + idp=idp or 'developer', ) return Resource.objects.get(name=resource_name) 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 483d224c..8e4c10bc 100644 --- a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py +++ b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py @@ -15,7 +15,8 @@ def setUp(self) -> None: super().setUp() self.resource = self.new_openshift_resource( name='Microshift', - auth_url=os.getenv('OS_AUTH_URL') + auth_url=os.getenv('OS_AUTH_URL'), + api_url=os.getenv('OS_API_URL'), ) def test_new_allocation(self): @@ -36,7 +37,8 @@ def test_new_allocation(self): allocator._get_project(project_id) # Check user and roles - allocator.get_federated_user(user.username) + user_info = allocator.get_federated_user(user.username) + self.assertEqual(user_info, {'username': user.username}) allocator._get_role(user.username, project_id) @@ -73,7 +75,8 @@ def test_add_remove_user(self): tasks.add_user_to_allocation(allocation_user2.pk) allocator._get_role(user.username, project_id) - allocator.get_federated_user(user2.username) + user_info = allocator.get_federated_user(user.username) + self.assertEqual(user_info, {'username': user.username}) allocator._get_role(user.username, project_id) allocator._get_role(user2.username, project_id) @@ -123,17 +126,16 @@ def test_new_allocation_quota(self): 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'] - quota = {k: v for k, v in quota.items() if v is not None} + quota = allocator.get_quota(project_id) # The return value will update to the most relevant unit, so # 2000m cores becomes 2 and 8192Mi becomes 8Gi self.assertEqual(quota, { - ":limits.cpu": "2", - ":limits.memory": "8Gi", - ":limits.ephemeral-storage": "10Gi", - ":requests.storage": "40Gi", - ":requests.nvidia.com/gpu": "0", - ":persistentvolumeclaims": "4", + "limits.cpu": "2", + "limits.memory": "8Gi", + "limits.ephemeral-storage": "10Gi", + "requests.storage": "40Gi", + "requests.nvidia.com/gpu": "0", + "persistentvolumeclaims": "4", }) # change a bunch of attributes @@ -154,16 +156,16 @@ def test_new_allocation_quota(self): # This call should update the openshift quota to match the current attributes call_command('validate_allocations', apply=True) - quota = allocator.get_quota(project_id)['Quota'] + quota = allocator.get_quota(project_id) quota = {k: v for k, v in quota.items() if v is not None} self.assertEqual(quota, { - ":limits.cpu": "6", - ":limits.memory": "8Gi", - ":limits.ephemeral-storage": "50Gi", - ":requests.storage": "100Gi", - ":requests.nvidia.com/gpu": "1", - ":persistentvolumeclaims": "10", + "limits.cpu": "6", + "limits.memory": "8Gi", + "limits.ephemeral-storage": "50Gi", + "requests.storage": "100Gi", + "requests.nvidia.com/gpu": "1", + "persistentvolumeclaims": "10", }) def test_reactivate_allocation(self): @@ -180,19 +182,17 @@ def test_reactivate_allocation(self): self.assertEqual(allocation.get_attribute(attributes.QUOTA_LIMITS_CPU), 2) - quota = allocator.get_quota(project_id)['Quota'] + quota = allocator.get_quota(project_id) - # https://github.com/CCI-MOC/openshift-acct-mgt - quota = {k: v for k, v in quota.items() if v is not None} # The return value will update to the most relevant unit, so # 2000m cores becomes 2 and 8192Mi becomes 8Gi self.assertEqual(quota, { - ":limits.cpu": "2", - ":limits.memory": "8Gi", - ":limits.ephemeral-storage": "10Gi", - ":requests.storage": "40Gi", - ":requests.nvidia.com/gpu": "0", - ":persistentvolumeclaims": "4", + "limits.cpu": "2", + "limits.memory": "8Gi", + "limits.ephemeral-storage": "10Gi", + "requests.storage": "40Gi", + "requests.nvidia.com/gpu": "0", + "persistentvolumeclaims": "4", }) # Simulate an attribute change request and subsequent approval which @@ -201,17 +201,16 @@ def test_reactivate_allocation(self): tasks.activate_allocation(allocation.pk) allocation.refresh_from_db() - quota = allocator.get_quota(project_id)['Quota'] - quota = {k: v for k, v in quota.items() if v is not None} + quota = allocator.get_quota(project_id) # The return value will update to the most relevant unit, so # 3000m cores becomes 3 and 8192Mi becomes 8Gi self.assertEqual(quota, { - ":limits.cpu": "3", - ":limits.memory": "8Gi", - ":limits.ephemeral-storage": "10Gi", - ":requests.storage": "40Gi", - ":requests.nvidia.com/gpu": "0", - ":persistentvolumeclaims": "4", + "limits.cpu": "3", + "limits.memory": "8Gi", + "limits.ephemeral-storage": "10Gi", + "requests.storage": "40Gi", + "requests.nvidia.com/gpu": "0", + "persistentvolumeclaims": "4", }) allocator._get_role(user.username, project_id) diff --git a/src/coldfront_plugin_cloud/tests/unit/openshift/test_quota.py b/src/coldfront_plugin_cloud/tests/unit/openshift/test_quota.py new file mode 100644 index 00000000..74d9f0a3 --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/openshift/test_quota.py @@ -0,0 +1,99 @@ +from unittest import mock + +from coldfront_plugin_cloud.tests import base +from coldfront_plugin_cloud.openshift import OpenShiftResourceAllocator + + +class TestOpenshiftQuota(base.TestBase): + def setUp(self) -> None: + mock_resource = mock.Mock() + mock_allocation = mock.Mock() + self.allocator = OpenShiftResourceAllocator(mock_resource, mock_allocation) + self.allocator.id_provider = "fake_idp" + self.allocator.k8_client = mock.Mock() + + @mock.patch("coldfront_plugin_cloud.openshift.OpenShiftResourceAllocator._openshift_get_project", mock.Mock()) + def test_get_resourcequotas(self): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = {"items": []} + self.allocator.k8_client.resources.get.return_value.get.return_value = fake_quota + res = self.allocator._openshift_get_resourcequotas("fake-project") + self.allocator.k8_client.resources.get.return_value.get.assert_called() + assert res == [] + + + def test_delete_quota(self): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = {} + self.allocator.k8_client.resources.get.return_value.delete.return_value = fake_quota + res = self.allocator._openshift_delete_resourcequota("test-project", "test-quota") + self.allocator.k8_client.resources.get.return_value.delete.assert_called() + assert res == {} + + + @mock.patch("coldfront_plugin_cloud.openshift.OpenShiftResourceAllocator._openshift_get_resourcequotas") + def test_delete_moc_quota(self, fake_get_resourcequotas): + fake_get_resourcequotas.return_value = [{"metadata": {"name": "fake-quota"}}] + self.allocator.delete_moc_quotas("test-project") + self.allocator.k8_client.resources.get.return_value.delete.assert_any_call( + namespace="test-project", name="fake-quota" + ) + + + @mock.patch("coldfront_plugin_cloud.openshift.OpenShiftResourceAllocator._wait_for_quota_to_settle") + def test_create_shift_quotas(self, fake_wait_quota): + quotadefs = { + "metadata": {"name": "fake-project-project"}, + "spec": {"hard": {"configmaps": "1", "cpu": "1", "resourcequotas": "1"}}, + } + + self.allocator.k8_client.resources.get.return_value.create.return_value = mock.Mock() + + self.allocator._openshift_create_resourcequota("fake-project", quotadefs) + + self.allocator.k8_client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "fake-project-project"}, + "spec": {"hard": {"configmaps": "1", "cpu": "1", "resourcequotas": "1"}}, + }, + ) + + fake_wait_quota.assert_called() + + + def test_wait_for_quota_to_settle(self): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = { + "metadata": {"name": "fake-quota"}, + "spec": {"hard": {"resourcequotas": "1"}}, + "status": {"used": {"resourcequotas": "1"}}, + } + self.allocator.k8_client.resources.get.return_value.get.return_value = fake_quota + + self.allocator._wait_for_quota_to_settle("fake-project", fake_quota.to_dict()) + + self.allocator.k8_client.resources.get.return_value.get.assert_called_with( + namespace="fake-project", + name="fake-quota", + ) + + @mock.patch("coldfront_plugin_cloud.openshift.OpenShiftResourceAllocator._openshift_get_resourcequotas") + def test_get_moc_quota(self, fake_get_quota): + expected_quota = { + "services": "2", + "configmaps": None, + "cpu": "1000", + } + fake_get_quota.return_value = [ + { + "spec": { + "hard": expected_quota + }, + } + ] + res = self.allocator.get_quota("fake-project") + self.assertEqual(res, expected_quota) + + + diff --git a/src/coldfront_plugin_cloud/tests/unit/openshift/test_user.py b/src/coldfront_plugin_cloud/tests/unit/openshift/test_user.py index 7b53dc5a..bec1c94c 100644 --- a/src/coldfront_plugin_cloud/tests/unit/openshift/test_user.py +++ b/src/coldfront_plugin_cloud/tests/unit/openshift/test_user.py @@ -1,4 +1,7 @@ from unittest import mock +import json + +import kubernetes.dynamic.exceptions as kexc from coldfront_plugin_cloud.tests import base from coldfront_plugin_cloud.openshift import OpenShiftResourceAllocator @@ -21,9 +24,14 @@ def test_get_federated_user(self): self.assertEqual(output, {"username": "fake_user"}) def test_get_federated_user_not_exist(self): - fake_user = mock.Mock(spec=["to_dict"]) - fake_user.to_dict.return_value = {"identities": ["fake_idp:fake_user"]} - self.allocator.k8_client.resources.get.return_value.get.return_value = fake_user + fake_error = kexc.NotFoundError(mock.Mock()) + fake_error.body = json.dumps({ + "reason": "NotFound", + "details": { + "name": "fake_user_2", + }, + }) + self.allocator.k8_client.resources.get.return_value.get.side_effect = fake_error output = self.allocator.get_federated_user("fake_user_2") self.assertEqual(output, None)