Merge pull request #702 from helgi/apps_coverage

ref(models): use DRF APIException as a base for all exceptions

Author: helgi

rootfs/api/models/__init__.py

@@ -13,16 +13,27 @@
 from django.db import models
 from django.db.models.signals import post_delete, post_save
 from django.dispatch import receiver
-from django.core.exceptions import ValidationError
+
+from rest_framework.exceptions import ValidationError, APIException
 from rest_framework.authtoken.models import Token
 
 from api.utils import dict_merge
+from scheduler import KubeException
 
 logger = logging.getLogger(__name__)
 
 
-class AlreadyExists(EnvironmentError):
-    pass
+class DeisException(APIException):
+    status_code = 400
+
+
+class AlreadyExists(APIException):
+    status_code = 409
+
+
+class ServiceUnavailable(APIException):
+    status_code = 503
+    default_detail = 'Service temporarily unavailable, try again later.'
 
 
 def log_event(app, msg, level=logging.INFO):
@@ -57,8 +68,12 @@ def _scheduler(self):
         return mod.SchedulerClient()
 
     def _fetch_service_config(self, app):
-        # Get the service from k8s to attach the domain correctly
-        svc = self._scheduler._get_service(app, app).json()
+        try:
+            # Get the service from k8s to attach the domain correctly
+            svc = self._scheduler._get_service(app, app).json()
+        except KubeException as e:
+            raise ServiceUnavailable(str(e)) from e
+
         # Get minimum structure going if it is missing on the service
         if 'metadata' not in svc or 'annotations' not in svc['metadata']:
             default = {'metadata': {'annotations': {}}}
@@ -90,8 +105,11 @@ def _save_service_config(self, app, component, data):
         data = {"%s%s" % (component, key): value for key, value in list(data.items())}
         svc['metadata']['annotations'].update(morph.flatten(data))
 
-        # Update the k8s service for the application with new domain information
-        self._scheduler._update_service(app, app, svc)
+        # Update the k8s service for the application with new service information
+        try:
+            self._scheduler._update_service(app, app, svc)
+        except KubeException as e:
+            raise ServiceUnavailable(str(e)) from e
 
 
 class UuidAuditedModel(AuditedModel):

rootfs/api/models/app.py

@@ -11,11 +11,12 @@
 
 from django.conf import settings
 from django.db import models
-from django.core.exceptions import ValidationError
+from rest_framework.exceptions import ValidationError, NotFound
 from jsonfield import JSONField
 
 from deis import __version__ as deis_version
-from api.models import UuidAuditedModel, log_event, AlreadyExists
+from api.models import UuidAuditedModel, log_event, AlreadyExists, \
+    DeisException, ServiceUnavailable
 
 from api.utils import generate_app_name, app_build_type
 from api.models.release import Release
@@ -43,7 +44,7 @@ def validate_app_structure(value):
         if any(int(v) < 0 for v in value.values()):
             raise ValueError("Must be greater than or equal to zero")
     except ValueError as err:
-        raise ValidationError(err)
+        raise ValidationError(str(err))
 
 
 def validate_reserved_names(value):
@@ -152,7 +153,10 @@ def create(self, *args, **kwargs):
             )
 
         # create required minimum resources in k8s for the application
-        self._scheduler.create(self.id)
+        try:
+            self._scheduler.create(self.id)
+        except KubeException as e:
+            raise ServiceUnavailable(str(e)) from e
 
         # Attach the platform specific application sub domain to the k8s service
         # Only attach it on first release in case a customer has remove the app domain
@@ -164,8 +168,8 @@ def delete(self, *args, **kwargs):
         try:
             # attempt to remove application from kubernetes
             self._scheduler.destroy(self.id)
-        except KubeException:
-            pass
+        except KubeException as e:
+            raise ServiceUnavailable(str(e)) from e
 
         self._clean_app_logs()
         return super(App, self).delete(*args, **kwargs)
@@ -213,7 +217,7 @@ def restart(self, **kwargs):  # noqa
             while True:
                 # timed out
                 if elapsed >= timeout:
-                    raise KubeException('timeout - 5 minutes have passed and pods are not up')
+                    raise DeisException('timeout - 5 minutes have passed and pods are not up')
 
                 # restarting a single pod behaves differently, fetch the *newest* pod
                 # and hope it is the right one. Comes back sorted
@@ -234,7 +238,6 @@ def restart(self, **kwargs):  # noqa
 
                 elapsed += 5
                 time.sleep(5)
-
         except Exception as e:
             err = "warning, some pods failed to start:\n{}".format(str(e))
             log_event(self, err, logging.WARNING)
@@ -261,18 +264,26 @@ def scale(self, user, structure):  # noqa
         self.create()
 
         if self.release_set.latest().build is None:
-            raise EnvironmentError('No build associated with this release')
+            raise DeisException('No build associated with this release')
 
         release = self.release_set.latest()
 
+        # Validate structure
+        try:
+            for target, count in structure.copy().items():
+                structure[target] = int(count)
+            validate_app_structure(structure)
+        except (TypeError, ValueError) as e:
+            raise DeisException('Invalid scaling format: {}'.format(e))
+
         # test for available process types
         available_process_types = release.build.procfile or {}
         for container_type in structure:
             if container_type == 'cmd':
                 continue  # allow docker cmd types in case we don't have the image source
 
             if container_type not in available_process_types:
-                raise EnvironmentError(
+                raise DeisException(
                     'Container type {} does not exist in application'.format(container_type))
 
         # merge current structure and the new items together
@@ -323,16 +334,15 @@ def _scale_pods(self, scale_types):
                     command=command,
                     **kwargs
                 )
-
             except Exception as e:
                 err = '{} (scale): {}'.format(self._get_job_id(scale_type), e)
                 log_event(self, err, logging.ERROR)
-                raise
+                raise ServiceUnavailable(e) from e
 
     def deploy(self, release):
         """Deploy a new release to this application"""
         if release.build is None:
-            raise EnvironmentError('No build associated with this release')
+            raise DeisException('No build associated with this release')
 
         # use create to make sure minimum resources are created
         self.create()
@@ -386,7 +396,7 @@ def deploy(self, release):
             except Exception as e:
                 err = '{} (app::deploy): {}'.format(self._get_job_id(scale_type), e)
                 log_event(self, err, logging.ERROR)
-                raise
+                raise ServiceUnavailable(err) from e
 
         # cleanup old releases from kubernetes
         release.cleanup_old()
@@ -508,19 +518,20 @@ def logs(self, log_lines=str(settings.LOG_LINES)):
             r = requests.get(url)
         # Handle HTTP request errors
         except requests.exceptions.RequestException as e:
-            logger.error("Error accessing deis-logger using url '{}': {}".format(url, e))
-            raise e
+            msg = "Error accessing deis-logger using url '{}': {}".format(url, e)
+            logger.error(msg)
+            raise ServiceUnavailable(msg) from e
 
         # Handle logs empty or not found
         if r.status_code == 204 or r.status_code == 404:
             logger.info("GET {} returned a {} status code".format(url, r.status_code))
-            raise EnvironmentError('Could not locate logs')
+            raise NotFound('Could not locate logs')
 
         # Handle unanticipated status codes
         if r.status_code != 200:
             logger.error("Error accessing deis-logger: GET {} returned a {} status code"
                          .format(url, r.status_code))
-            raise EnvironmentError('Error accessing deis-logger')
+            raise ServiceUnavailable('Error accessing deis-logger')
 
         # cast content to string since it comes as bytes via the requests object
         return str(r.content)
@@ -532,7 +543,7 @@ def pod_name(size=5, chars=string.ascii_lowercase + string.digits):
         """Run a one-off command in an ephemeral app container."""
         release = self.release_set.latest()
         if release.build is None:
-            raise EnvironmentError('No build associated with this release to run this command')
+            raise DeisException('No build associated with this release to run this command')
 
         # TODO: add support for interactive shell
         # SECURITY: shell-escape user input
@@ -576,7 +587,7 @@ def pod_name(size=5, chars=string.ascii_lowercase + string.digits):
         except Exception as e:
             err = '{} (run): {}'.format(name, e)
             log_event(self, err, logging.ERROR)
-            raise
+            raise ServiceUnavailable(str(e)) from e
 
     def list_pods(self, *args, **kwargs):
         """Used to list basic information about pods running for a given application"""
@@ -627,7 +638,7 @@ def list_pods(self, *args, **kwargs):
         except Exception as e:
             err = '(list pods): {}'.format(e)
             log_event(self, err, logging.ERROR)
-            raise
+            raise ServiceUnavailable(err) from e
 
     def _scheduler_filter(self, **kwargs):
         labels = {'app': self.id}

rootfs/api/models/build.py

@@ -2,7 +2,7 @@
 from django.db import models
 from jsonfield import JSONField
 
-from api.models import UuidAuditedModel
+from api.models import UuidAuditedModel, DeisException
 
 import logging
 logger = logging.getLogger(__name__)
@@ -43,10 +43,11 @@ def create(self, user, *args, **kwargs):
         try:
             self.app.deploy(new_release)
             return new_release
-        except Exception:
+        except Exception as e:
             if 'new_release' in locals():
                 new_release.delete()
-            raise
+
+            raise DeisException(str(e)) from e
 
     def save(self, **kwargs):
         try:

rootfs/api/models/certificate.py

@@ -8,13 +8,14 @@
 from django.shortcuts import get_object_or_404
 from django.db import models
 from django.conf import settings
-from django.core.exceptions import ValidationError, SuspiciousOperation
+from django.core.exceptions import SuspiciousOperation
 from django.contrib.postgres.fields import ArrayField
+from rest_framework.exceptions import ValidationError
 
-from api.models import AuditedModel, validate_label, AlreadyExists
+from api.models import AuditedModel, validate_label, AlreadyExists, ServiceUnavailable
 from api.models.domain import Domain
 
-from scheduler import KubeHTTPException
+from scheduler import KubeException
 
 import logging
 logger = logging.getLogger(__name__)
@@ -179,15 +180,15 @@ def attach_in_kubernetes(self, domain):
             }
 
             secret = self._scheduler._get_secret(app, name).json()['data']
-        except KubeHTTPException:
+        except KubeException:
             self._scheduler._create_secret(app, name, data)
         else:
             # update cert secret to the TLS Ingress format if required
             if secret != data:
                 try:
                     self._scheduler._update_secret(app, name, data)
-                except KubeHTTPException:
-                    raise
+                except KubeException as e:
+                    raise ServiceUnavailable(str(e)) from e
 
         # get config for the service
         config = self._load_service_config(app, 'router')
@@ -220,9 +221,8 @@ def detach(self, *args, **kwargs):
                 # We raise an exception when a secret doesn't exist
                 self._scheduler._get_secret(app, name)
                 self._scheduler._delete_secret(app, name)
-            except KubeHTTPException as e:
-                logger.critical(e)
-                raise EnvironmentError("Could not delete certificate secret {} for application {}".format(name, app))  # noqa
+            except KubeException as e:
+                raise ServiceUnavailable("Could not delete certificate secret {} for application {}".format(name, app)) from e  # noqa
 
         # get config for the service
         config = self._load_service_config(app, 'router')

rootfs/api/models/config.py

@@ -2,7 +2,7 @@
 from django.db import models
 from jsonfield import JSONField
 
-from api.models import UuidAuditedModel
+from api.models import UuidAuditedModel, DeisException
 
 
 class Config(UuidAuditedModel):
@@ -107,7 +107,7 @@ def set_tags(self, previous_config):
             new = set(labels) - set(old)
             message += ' - Addition of {} is the cause'.format(', '.join(new))
 
-        raise EnvironmentError(message)
+        raise DeisException(message)
 
     def save(self, **kwargs):
         """merge the old config with the new"""

rootfs/api/models/key.py

@@ -2,7 +2,7 @@
 
 from django.conf import settings
 from django.db import models
-from django.core.exceptions import ValidationError
+from rest_framework.exceptions import ValidationError
 
 from api.models import UuidAuditedModel
 from api.utils import fingerprint
@@ -13,7 +13,7 @@ def validate_base64(value):
     try:
         base64.b64decode(value.split()[1])
     except Exception as e:
-        raise ValidationError(e)
+        raise ValidationError(str(e))
 
 
 class Key(UuidAuditedModel):