fix: adding users and showing quotas (#22)

* fix: pathching a resource pool

* fix: return full quota in all cases rather than id

.github/workflows/test_publish.yml

@@ -7,6 +7,9 @@ on:
     tags:
       - 'v*'
   pull_request:
+    branches:
+      - 'main'
+      - 'hotfix/**'
 
 jobs:
   test:

src/k8s/clients.py

@@ -42,7 +42,7 @@ def delete_namespaced_resource_quota(self, name: Any, namespace: Any, **kwargs:
 
     def patch_namespaced_resource_quota(self, name: Any, namespace: Any, body: Any, **kwargs: Any) -> Any:
         """Update a resource quota."""
-        return self.client.delete_namespaced_resource_quota(name, namespace, body, **kwargs)
+        return self.client.patch_namespaced_resource_quota(name, namespace, body, **kwargs)
 
 
 class K8sSchedulingClient(K8sSchedudlingClientInterface):  # pragma:nocover

src/k8s/quota.py

@@ -121,3 +121,18 @@ def update_quota(self, quota: models.Quota):
         """Update a specific resource quota."""
         quota_manifest = self._quota_to_manifest(quota)
         self.core_client.patch_namespaced_resource_quota(name=quota.id, namespace=self.namespace, body=quota_manifest)
+
+    def hydrate_resource_pool_quota(self, resource_pool: models.ResourcePool) -> models.ResourcePool:
+        """Replace the resource pool quota ID with a quota model."""
+
+        if resource_pool.quota is None or isinstance(resource_pool.quota, models.Quota):
+            return resource_pool
+        if isinstance(resource_pool.quota, str):
+            quota = self._get_quota(resource_pool.quota)
+            return resource_pool.set_quota(quota)
+        else:
+            raise errors.BaseError(
+                message=f"Cannot find a quota for the resource pool with id {resource_pool.id}.",
+                detail="The quota field in the resource pool is expected to be either a string or "
+                f"`models.Quota` but we got {type(resource_pool.quota)}",
+            )

src/renku_crc/app.py

@@ -1,7 +1,7 @@
 """Compute resource control (CRC) app."""
 import asyncio
 from dataclasses import asdict, dataclass
-from typing import Any, Dict, List
+from typing import Any, Dict, List, cast
 
 from sanic import HTTPResponse, Request, Sanic, json
 from sanic_ext import validate
@@ -32,23 +32,8 @@ def get_all(self) -> BlueprintFactoryResponse:
         @authenticate(self.authenticator)
         async def _get_all(request: Request, user: models.APIUser):
             res_filter = query_parameters.ResourceClassesFilter.parse_obj(dict(request.query_args))
-            pool = asyncio.get_running_loop()
-            rps: List[models.ResourcePool]
-            quotas: List[models.Quota]
-            rps, quotas = await asyncio.gather(
-                self.rp_repo.filter_resource_pools(api_user=user, **res_filter.dict()),
-                pool.run_in_executor(None, self.quota_repo.get_quotas),
-            )
-            quotas_dict = {quota.id: quota for quota in quotas}
-            rps_w_quota: List[models.ResourcePool] = []
-            for rp in rps:
-                quota = quotas_dict.get(rp.quota) if isinstance(rp.quota, str) else None
-                if quota is not None:
-                    rp_w_quota = rp.set_quota(quota)
-                    rps_w_quota.append(rp_w_quota)
-                else:
-                    rps_w_quota.append(rp)
-
+            rps = await self.rp_repo.filter_resource_pools(api_user=user, **res_filter.dict())
+            rps_w_quota = [self.quota_repo.hydrate_resource_pool_quota(rp) for rp in rps]
             return json([apispec.ResourcePoolWithIdFiltered.from_orm(r).dict(exclude_none=True) for r in rps_w_quota])
 
         return "/resource_pools", ["GET"], _get_all
@@ -77,22 +62,16 @@ def get_one(self) -> BlueprintFactoryResponse:
 
         @authenticate(self.authenticator)
         async def _get_one(request: Request, resource_pool_id: int, user: models.APIUser):
-            pool = asyncio.get_running_loop()
             rps: List[models.ResourcePool]
-            quotas: List[models.Quota]
-            rps, quotas = await asyncio.gather(
-                self.rp_repo.get_resource_pools(api_user=user, id=resource_pool_id, name=request.args.get("name")),
-                pool.run_in_executor(None, self.quota_repo.get_quotas),
+            rps = await self.rp_repo.get_resource_pools(
+                api_user=user, id=resource_pool_id, name=request.args.get("name")
             )
             if len(rps) < 1:
                 raise errors.MissingResourceError(
                     message=f"The resource pool with id {resource_pool_id} cannot be found."
                 )
             rp = rps[0]
-            quotas = [i for i in quotas if i.id == rp.quota]
-            if len(quotas) >= 1:
-                quota = quotas[0]
-                rp = rp.set_quota(quota)
+            rp = self.quota_repo.hydrate_resource_pool_quota(rp)
             return json(apispec.ResourcePoolWithId.from_orm(rp).dict(exclude_none=True))
 
         return "/resource_pools/<resource_pool_id>", ["GET"], _get_one
@@ -137,26 +116,41 @@ async def _put_patch_resource_pool(
     ):
         body_dict = body.dict(exclude_none=True)
         quota_req = body_dict.pop("quota", None)
-        if quota_req is not None:
-            rps = await self.rp_repo.get_resource_pools(api_user, resource_pool_id)
-            if len(rps) == 0:
-                raise errors.ValidationError(message=f"The resource pool with ID {resource_pool_id} does not exist.")
-            rp = rps[0]
-            if isinstance(rp.quota, str):
-                quota_req["id"] = rp.quota
-            quota_model = models.Quota.from_dict(quota_req)
-            if quota_model.id is None:
-                quota_model = quota_model.generate_id()
-            self.quota_repo.update_quota(quota_model)
-            if rp.quota is None:
-                body_dict["quota"] = quota_model.id
+        rps = await self.rp_repo.get_resource_pools(api_user, resource_pool_id)
+        if len(rps) == 0:
+            raise errors.ValidationError(message=f"The resource pool with ID {resource_pool_id} does not exist.")
+        rp = rps[0]
+        rp = self.quota_repo.hydrate_resource_pool_quota(rp)
+        match rp.quota, quota_req:
+            case _, None:
+                # No quota is specified in request
+                pass
+            case _, dict(quota_req) if not quota_req:
+                # No quota is specified in request
+                pass
+            case models.Quota(id=None), {}:
+                # The quota does not exist, create it
+                quota_req["id"] = None  # ensure that the id is None to make a new Quota
+                quota_req_model = models.Quota.from_dict(quota_req).generate_id()
+                self.quota_repo.create_quota(quota_req_model)
+                rp.set_quota(quota_req_model)
+            case models.Quota(id=_), {}:
+                # The quota exists already, update it
+                if quota_req.get("id") is not None:
+                    raise errors.ValidationError(message="Cannot update the ID of an existing quota.")
+                if not isinstance(rp.quota, models.Quota):
+                    raise errors.BaseError(message=f"Expected quota but got {type(rp.quota)}")
+                new_quota = models.Quota.from_dict({**asdict(rp.quota), **quota_req})
+                self.quota_repo.update_quota(new_quota)
+                rp.set_quota(new_quota)
         res = await self.rp_repo.update_resource_pool(
             api_user=api_user,
             id=resource_pool_id,
             **body_dict,
         )
         if res is None:
             raise errors.MissingResourceError(message=f"The resource pool with ID {resource_pool_id} cannot be found.")
+        res = self.quota_repo.hydrate_resource_pool_quota(res)
         return json(apispec.ResourcePoolWithId.from_orm(res).dict(exclude_none=True))
 
 
@@ -374,20 +368,12 @@ async def _get(_: Request, resource_pool_id: int, user: models.APIUser):
                     message=f"The resource pool with ID {resource_pool_id} cannot be found."
                 )
             rp = rps[0]
+            rp = self.quota_repo.hydrate_resource_pool_quota(rp)
             if rp.quota is None:
                 raise errors.MissingResourceError(
                     message=f"The resource pool with ID {resource_pool_id} does not have a quota."
                 )
-            if not isinstance(rp.quota, str):
-                raise errors.ValidationError(message="The quota in the resource pool should be a string.")
-            quotas = self.quota_repo.get_quotas(name=rp.quota)
-            if len(quotas) < 1:
-                raise errors.MissingResourceError(
-                    message=f"Cannot find the quota with name {rp.quota} "
-                    f"for the resource pool with ID {resource_pool_id}."
-                )
-            quota = quotas[0]
-            return json(apispec.QuotaWithId.from_orm(quota).dict(exclude_none=True))
+            return json(apispec.QuotaWithId.from_orm(rp.quota).dict(exclude_none=True))
 
         return "/resource_pools/<resource_pool_id>/quota", ["GET"], _get
 
@@ -420,18 +406,13 @@ async def _put_patch(
         if len(rps) < 1:
             raise errors.MissingResourceError(message=f"Cannot find the resource pool with ID {resource_pool_id}.")
         rp = rps[0]
+        rp = self.quota_repo.hydrate_resource_pool_quota(rp)
         if rp.quota is None:
             raise errors.MissingResourceError(
                 message=f"The resource pool with ID {resource_pool_id} does not have a quota."
             )
-        if not isinstance(rp.quota, str):
-            raise errors.ValidationError(message="The quota in the resource pool should be a string.")
-        quotas = self.quota_repo.get_quotas(name=rp.quota)
-        if len(quotas) < 1:
-            raise errors.MissingResourceError(
-                message=f"Cannot find the quota with name {rp.quota} for the resource pool with ID {resource_pool_id}."
-            )
-        old_quota = quotas[0]
+        old_quota = rp.quota
+        old_quota = cast(models.Quota, old_quota)
         new_quota = models.Quota.from_dict({**asdict(old_quota), **body.dict(exclude_none=True)})
         self.quota_repo.update_quota(new_quota)
         return json(apispec.QuotaWithId.from_orm(new_quota).dict(exclude_none=True))
@@ -507,6 +488,7 @@ class UserResourcePoolsBP(CustomBlueprint):
     """Handlers for dealing wiht the resource pools of a specific user."""
 
     repo: UserRepository
+    quota_repo: QuotaRepository
     authenticator: models.Authenticator
 
     def get(self) -> BlueprintFactoryResponse:
@@ -516,6 +498,7 @@ def get(self) -> BlueprintFactoryResponse:
         @only_admins
         async def _get(_: Request, user_id: str, user: models.APIUser):
             rps = await self.repo.get_user_resource_pools(keycloak_id=user_id, api_user=user)
+            rps = [self.quota_repo.hydrate_resource_pool_quota(rp) for rp in rps]
             return json([apispec.ResourcePoolWithId.from_orm(rp).dict(exclude_none=True) for rp in rps])
 
         return "/users/<user_id>/resource_pools", ["GET"], _get
@@ -548,6 +531,7 @@ async def _post_put(
         rps = await self.repo.update_user_resource_pools(
             keycloak_id=user_id, resource_pool_ids=resource_pool_ids.__root__, append=post, api_user=api_user
         )
+        rps = [self.quota_repo.hydrate_resource_pool_quota(rp) for rp in rps]
         return json([apispec.ResourcePoolWithId.from_orm(rp).dict(exclude_none=True) for rp in rps])
 
 
@@ -613,7 +597,11 @@ def register_all_handlers(app: Sanic, config: Config) -> Sanic:
         authenticator=config.authenticator,
     )
     user_resource_pools = UserResourcePoolsBP(
-        name="user_resource_pools", url_prefix=url_prefix, repo=config.user_repo, authenticator=config.authenticator
+        name="user_resource_pools",
+        url_prefix=url_prefix,
+        repo=config.user_repo,
+        authenticator=config.authenticator,
+        quota_repo=config.quota_repo,
     )
     misc = MiscBP(name="misc", url_prefix=url_prefix, apispec=config.spec, version=config.version)
     app.blueprint(

src/renku_crc/blueprint.py

@@ -29,8 +29,8 @@ def blueprint(self) -> Blueprint:
         members = getmembers(self, ismethod)
         for name, method in members:
             if name != "blueprint" and not name.startswith("_"):
-                method = cast(BlueprintFactory, method)
-                url, http_methods, handler = method()
+                method_factory = cast(BlueprintFactory, method)
+                url, http_methods, handler = method_factory()
                 bp.add_route(handler=handler, uri=url, methods=http_methods)
         for req_mw in self.request_middlewares:
             bp.middleware("request")(req_mw)