Merge pull request #86 from knikolla/feature/limits

Feature: Create LimitRange on Project creation

acct_mgt/app.py

@@ -2,12 +2,13 @@
 
 import json
 import os
-from flask import Flask, request, Response
+from flask import Flask, make_response, request, Response
 from flask_httpauth import HTTPBasicAuth
 
 from . import defaults
 from . import kubeclient
 from . import moc_openshift
+from . import exceptions
 
 ENVPREFIX = "ACCT_MGT_"
 
@@ -61,6 +62,11 @@ def verify_password(username, password):
             and password == APP.config["ADMIN_PASSWORD"]
         )
 
+    @APP.errorhandler(exceptions.ApiException)
+    def exception_handler(error):
+        msg = error.message if error.visible else "Internal Server Error"
+        return make_response({"msg": msg}, error.status_code)
+
     @APP.route(
         "/users/<user_name>/projects/<project_name>/roles/<role>", methods=["GET"]
     )
@@ -142,45 +148,23 @@ def get_moc_project(project_uuid):
     def create_moc_project(project_uuid, user_name=None):
         # first check the project_name is a valid openshift project name
         suggested_project_name = shift.cnvt_project_name(project_uuid)
-
         if project_uuid != suggested_project_name:
-            # future work, handel colisons by suggesting a different valid
-            # project name
-            return Response(
-                response=json.dumps(
-                    {
-                        "msg": "ERROR: project name must match regex '[a-z0-9]([-a-z0-9]*[a-z0-9])?'",
-                        "suggested name": suggested_project_name,
-                    }
-                ),
-                status=400,
-                mimetype="application/json",
-            )
-        if not shift.project_exists(project_uuid):
-            payload = request.get_json(silent=True) or {}
-            project_name = payload.pop("displayName", project_uuid)
-            annotations = payload.pop("annotations", {})
-            result = shift.create_project(
-                project_uuid, project_name, user_name, annotations=annotations
-            )
-            if result.status_code in (200, 201):
-                return Response(
-                    response=json.dumps({"msg": f"project created ({project_uuid})"}),
-                    status=200,
-                    mimetype="application/json",
-                )
-            return Response(
-                response=json.dumps(
-                    {"msg": f"unable to create project ({project_uuid})"}
-                ),
-                status=400,
-                mimetype="application/json",
+            raise exceptions.BadRequest(
+                "project name must match regex '[a-z0-9]([-a-z0-9]*[a-z0-9])?'."
+                f" Suggested name: {suggested_project_name}."
             )
-        return Response(
-            response=json.dumps({"msg": f"project already exists ({project_uuid})"}),
-            status=400,
-            mimetype="application/json",
+
+        if shift.project_exists(project_uuid):
+            raise exceptions.Conflict("project already exists.")
+
+        payload = request.get_json(silent=True) or {}
+        project_name = payload.pop("displayName", project_uuid)
+        annotations = payload.pop("annotations", {})
+
+        shift.create_project(
+            project_uuid, project_name, user_name, annotations=annotations
         )
+        return {"msg": f"project created ({project_uuid})"}
 
     @APP.route("/projects/<project_uuid>", methods=["DELETE"])
     @AUTH.login_required

acct_mgt/defaults.py

@@ -2,3 +2,4 @@
 
 ADMIN_USERNAME = "admin"
 QUOTA_DEF_FILE = "quotas.json"
+LIMIT_DEF_FILE = "limits.json"

acct_mgt/exceptions.py

@@ -0,0 +1,32 @@
+"""Defined exceptions used by acct-mgt."""
+
+
+class ApiException(Exception):
+    """Base exception class for errors.
+
+    All exceptions subclassing ApiException will be caught from Flask's
+    error handler and return the appropriate status code and message.
+    The visible parameter controls whether the error message is visible
+    to the end user.
+    """
+
+    status_code = 500
+    visible = True
+    default_message = "Internal Server Error."
+
+    def __init__(self, message=None):
+        self.message = message or self.default_message
+
+
+class BadRequest(ApiException):
+    """Exception class for invalid requests."""
+
+    status_code = 400
+    default_message = "Invalid Request."
+
+
+class Conflict(BadRequest):
+    """Exception class for requests that create already existing resources."""
+
+    status_code = 409
+    default_message = "Resource already exists."

acct_mgt/moc_openshift.py

@@ -4,10 +4,13 @@
 import pprint
 import json
 import re
+import sys
 import time
 
 from flask import Response
 
+from . import exceptions
+
 OPENSHIFT_ROLES = ["admin", "edit", "view"]
 
 
@@ -54,6 +57,11 @@ def __init__(self, client, app):
         self.logger = app.logger
         self.id_provider = app.config["IDENTITY_PROVIDER"]
         self.quotafile = app.config["QUOTA_DEF_FILE"]
+        self.limitfile = app.config["LIMIT_DEF_FILE"]
+
+        if not self.limitfile:
+            self.logger.error("No default limit file provided.")
+            sys.exit(1)
 
     @staticmethod
     def cnvt_project_name(project_name):
@@ -328,6 +336,10 @@ def get_quota_definitions(self):
 
         return quota
 
+    def get_limit_definitions(self):
+        with open(self.limitfile, "r") as file:
+            return json.load(file)
+
     @abc.abstractmethod
     def get_resourcequota_details(self, project_name) -> dict:
         pass
@@ -362,7 +374,11 @@ def create_project(self, project_name, display_name, user_name, annotations=None
             "apiVersion": "project.openshift.io/v1",
             "metadata": {"name": project_name, "annotations": annotations},
         }
-        return self.client.post(url, json=payload)
+        r = self.client.post(url, json=payload)
+        if r.status_code not in [200, 201]:
+            raise exceptions.ApiException(f"unable to create project ({project_name})")
+        self.create_limits(project_name)
+        return r
 
     def delete_project(self, project_name):
         # check project_name
@@ -623,3 +639,23 @@ def get_resourcequota_details(self, project_name) -> dict:
             for rq_info in rq_data["items"]:
                 rq_dict[rq_info["metadata"]["name"]] = rq_info["spec"]
         return rq_dict
+
+    def create_limits(self, namespace, limits=None):
+        """
+        namespace: namespace to create LimitRange
+        limits: dictionary of limits to create, or None for default
+        """
+        url = f"/api/v1/namespaces/{namespace}/limitranges"
+
+        payload = {
+            "apiVersion": "v1",
+            "kind": "LimitRange",
+            "metadata": {"name": f"{namespace.lower()}-limits"},
+            "spec": {"limits": limits or self.get_limit_definitions()},
+        }
+        r = self.client.post(url, json=payload)
+        if r.status_code not in [200, 201]:
+            raise exceptions.ApiException(
+                f"Unable to create default limits on project {namespace}."
+            )
+        return r

k8s/base/deployment.yaml

@@ -14,7 +14,9 @@ spec:
           - name: OPENSHIFT_URL
             value: https://kubernetes.default.svc
           - name: ACCT_MGT_QUOTA_DEF_FILE
-            value: /app/quota/quotas.json
+            value: /app/config/quotas.json
+          - name: ACCT_MGT_LIMIT_DEF_FILE
+            value: /app/config/limits.json
         envFrom:
           - secretRef:
               name: onboarding-credentials
@@ -27,12 +29,12 @@ spec:
             containerPort: 8080
             protocol: TCP
         volumeMounts:
-          - name: quota-vol
-            mountPath: /app/quota
+          - name: config-vol
+            mountPath: /app/config
             readOnly: true
       volumes:
-        - name: quota-vol
+        - name: config-vol
           configMap:
-            name: openshift-quota-definition
+            name: config-files
       serviceAccountName: onboarding-serviceaccount
       automountServiceAccountToken: true

k8s/base/kustomization.yaml

@@ -15,6 +15,7 @@ resources:
 - service.yaml
 
 configMapGenerator:
-  - name: openshift-quota-definition
+  - name: config-files
     files:
+      - limits.json
       - quotas.json

k8s/base/limits.json

@@ -0,0 +1,13 @@
+[
+    {
+        "type": "Container",
+        "default": {
+            "cpu": "200m",
+            "memory": "512Mi"
+        },
+        "defaultRequest": {
+            "cpu": "100m",
+            "memory": "256Mi"
+        }
+    }
+]

tests/functional/test_project.py

@@ -70,16 +70,6 @@ def test_create_project_no_owner(session, suffix):
         session.delete(f"/projects/test-project-{suffix}")
 
 
-# LKS: Cannot differentiate between "conflict with existing project" and
-# "operation failed for some other reason"
-def test_create_project_exists(session, a_project):
-    """Test that we cannot create a project with a conflicting name"""
-
-    res = session.put(f"/projects/{a_project}/owner/test-owner")
-    assert res.status_code == 400
-
-
-@pytest.mark.xfail(reason="not supported by service")
 def test_create_project_exists_409(session, a_project):
     """Test that creating a project with a conflicting name results in
     a 409 CONFLICT error."""

tests/unit/test_app_project.py

@@ -1,6 +1,7 @@
 # pylint: disable=missing-module-docstring
 import json
 
+from acct_mgt.exceptions import ApiException
 from .conftest import fake_200_response, fake_400_response
 
 
@@ -23,7 +24,7 @@ def test_create_moc_project_bad_name(moc, client):
     res = client.put("/projects/Test%20Project")
     assert res.status_code == 400
     assert "project name must match regex" in res.json["msg"]
-    assert res.json["suggested name"] == "test-project"
+    assert "test-project" in res.json["msg"]
 
 
 def test_create_moc_project_no_display_name(moc, client):
@@ -72,17 +73,17 @@ def test_create_moc_project_exists(moc, client):
     moc.cnvt_project_name.return_value = "test-project"
     moc.project_exists.return_value = True
     res = client.put("/projects/test-project")
-    assert res.status_code == 400
+    assert res.status_code == 409
     assert "project already exists" in res.json["msg"]
 
 
 def test_create_moc_project_fails(moc, client):
     moc.cnvt_project_name.return_value = "test-project"
     moc.project_exists.return_value = False
-    moc.create_project.return_value = fake_400_response
+    moc.create_project.side_effect = ApiException("Error")
     res = client.put("/projects/test-project")
-    assert res.status_code == 400
-    assert "unable to create project" in res.json["msg"]
+    assert res.status_code == 500
+    assert "Error" in res.json["msg"]
 
 
 def test_delete_moc_project_exists(moc, client):