Merge pull request #24 from UpCloudLtd/0.3.6-test

Elias Nygren · web-flow · commit 28d0c6a837fc · 2016-07-18T19:25:35.000+03:00
0.3.6 final improvements
diff --git a/test/conftest.py b/test/conftest.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import object, open
 import json
 import os
 import pytest
diff --git a/test/test_integration/test_integration_test.py b/test/test_integration/test_integration_test.py
@@ -34,26 +34,21 @@
 def teardown_module(module):
     manager = CloudManager(USERNAME, PASSWORD, timeout=120)
 
-    try:
-        # if we are at CIRCLECI, clean up everything
-        # useful as we don't want to manually clean anything after erroneous builds
-        if os.environ.get('CIRCLECI', False):
-            for server in manager.get_servers():
-                server.stop_and_destroy()
-
-            for tag in manager.get_tags():
-                tag.destroy()
-        else:
-            print('removing {}'.format(CREATED_SERVERS))
-            for server in CREATED_SERVERS:
-                server.stop_and_destroy()
-
-            print('removing {}'.format(CREATED_TAGS))
-            for tag in CREATED_TAGS:
-                manager.delete_tag(tag)
-
-    except Exception as e:
-        raise Exception('Cleanup failed due to: {}'.format(e))
+    # if we are at CIRCLECI, clean up everything
+    if os.environ.get('CIRCLECI', False):
+        for server in manager.get_servers():
+            server.stop_and_destroy()
+
+        for tag in manager.get_tags():
+            tag.destroy()
+    else:
+        print('removing {}'.format(CREATED_SERVERS))
+        for server in CREATED_SERVERS:
+            server.stop_and_destroy()
+
+        print('removing {}'.format(CREATED_TAGS))
+        for tag in CREATED_TAGS:
+            manager.delete_tag(tag)
 
 
 @integration_test
diff --git a/test/test_server.py b/test/test_server.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import str
 
 from conftest import Mock
 import pytest
diff --git a/test/test_storage.py b/test/test_storage.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import object
 
 from conftest import Mock
 import responses
diff --git a/test/test_tags.py b/test/test_tags.py
@@ -2,7 +2,8 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import list, str, dict
+
+import six
 
 from upcloud_api import Tag
 
@@ -21,10 +22,10 @@ def tag_post_callback(request):
     if 'servers' in request_body['tag']:
         assert isinstance(request_body['tag']['servers'], dict)
         assert isinstance(request_body['tag']['servers']['server'], list)
-        assert isinstance(request_body['tag']['servers']['server'][0], str)
+        assert isinstance(request_body['tag']['servers']['server'][0], six.string_types)
 
     if 'description' in request_body['tag']:
-        assert isinstance(request_body['tag']['description'], str)
+        assert isinstance(request_body['tag']['description'], six.string_types)
 
     return(201, {}, json.dumps(request_body))
 
diff --git a/upcloud_api/__init__.py b/upcloud_api/__init__.py
@@ -13,6 +13,7 @@
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) 2015 Elias Nygren'
 
+from upcloud_api.errors import UpCloudAPIError
 from upcloud_api.storage import Storage
 from upcloud_api.ip_address import IP_address
 from upcloud_api.server import Server, login_user_block
diff --git a/upcloud_api/base.py b/upcloud_api/base.py
@@ -1,7 +1,10 @@
 from __future__ import unicode_literals
+
 import json
 import requests
 
+from upcloud_api import UpCloudAPIError
+
 
 class BaseAPI(object):
     """
@@ -65,6 +68,8 @@ def __error_middleware(self, res, res_json):
         Middleware that raises an exception when HTTP statuscode is an error code.
         """
         if(res.status_code in [400, 401, 402, 403, 404, 405, 406, 409]):
-            raise Exception(res_json)
+            err_dict = res_json.get('error', {})
+            raise UpCloudAPIError(error_code=err_dict.get('error_code'),
+                                  error_message=err_dict.get('error_message'))
 
         return res_json
diff --git a/upcloud_api/cloud_manager/ip_address_mixin.py b/upcloud_api/cloud_manager/ip_address_mixin.py
@@ -2,7 +2,8 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import str
+
+import six
 
 from upcloud_api import IP_address
 
@@ -33,7 +34,7 @@ def attach_IP(self, server, family='IPv4'):
         """
         Attach a new (random) IP_address to the given server (object or UUID).
         """
-        if not isinstance(server, str):
+        if not isinstance(server, six.string_types):
             server = server.uuid
 
         body = {
@@ -52,7 +53,7 @@ def modify_IP(self, IP_addr, ptr_record):
 
         Accepts an IP_address instance (object) or its address (string).
         """
-        if not isinstance(IP_addr, str):
+        if not isinstance(IP_addr, six.string_types):
             IP_addr = IP_addr.address
 
         body = {
@@ -70,7 +71,7 @@ def release_IP(self, IP_addr):
 
         Accepts an IP_address instance (object) or its address (string).
         """
-        if not isinstance(IP_addr, str):
+        if not isinstance(IP_addr, six.string_types):
             IP_addr = IP_addr.address
 
         return self.request('DELETE', '/ip_address/' + IP_addr)
diff --git a/upcloud_api/cloud_manager/server_mixin.py b/upcloud_api/cloud_manager/server_mixin.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import str
 
 from upcloud_api import IP_address, Server, Storage
 from upcloud_api.tools import assignIfExists
diff --git a/upcloud_api/cloud_manager/tag_mixin.py b/upcloud_api/cloud_manager/tag_mixin.py
@@ -2,7 +2,9 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import str
+
+import six
+
 from upcloud_api import Tag
 
 
@@ -80,7 +82,7 @@ def remove_tags(self, server, tags):
 
     def delete_tag(self, tag):
         """Delete the Tag. Returns and empty object."""
-        if not isinstance(tag, str):
+        if not isinstance(tag, six.string_types):
             tag = tag.name
 
         return self.request('DELETE', '/tag/' + tag)
diff --git a/upcloud_api/errors.py b/upcloud_api/errors.py
@@ -0,0 +1,13 @@
+
+class UpCloudAPIError(Exception):
+    """Custom Error class for UpCloud API error responses.
+
+    Each API call returns an `error_code` and `error_message` that
+    are available as attributes via instances of this class.
+    """
+    def __init__(self, error_code, error_message):
+        self.error_code = error_code
+        self.error_message = error_message
+
+    def __str__(self):
+        return '{0} {1}'.format(self.error_code, self.error_message)
diff --git a/upcloud_api/server.py b/upcloud_api/server.py
@@ -1,10 +1,11 @@
 from __future__ import unicode_literals
 from __future__ import absolute_import
-from builtins import dict
-from builtins import str
+
+import itertools
 from time import sleep
 
-from upcloud_api import Storage, IP_address
+from upcloud_api import Storage, IP_address, UpCloudAPIError
+from upcloud_api.utils import try_it_n_times
 
 
 def login_user_block(username, ssh_keys, create_password=True):
@@ -431,7 +432,7 @@ def _wait_for_state_change(self, target_states, update_interval=10):
 
     def ensure_started(self):
         """
-        Start a server and waits (blocking wait) until a it is fully started.
+        Start a server and waits (blocking wait) until it is fully started.
         """
         # server is either starting or stopping (or error)
         if self.state in ['maintenance', 'error']:
@@ -447,35 +448,45 @@ def ensure_started(self):
             # something went wrong, fail explicitly
             raise Exception('unknown server state: ' + self.state)
 
-    def stop_and_destroy(self):
+    def stop_and_destroy(self, sync=True):
         """
-        Destroy a server and its storages.
+        Destroy a server and its storages. Stops the server before destroying.
 
-        Stops the server (blocking wait) before destroying.
+        Syncs the server state from the API, use sync=False to disable.
         """
-        def destroy_storages():
-            # list view does not return all server info, populate if necessary
-            if not hasattr(self, 'storage_devices'):
-                self.populate()
+        def _self_destruct():
+            """destroy the server and all storages attached to it."""
+
+            # try_it_n_times util is used as a convenience because
+            # Servers and Storages can fluctuate between "maintenance" and their
+            # original state due to several different reasons especially when
+            # destroying infrastructure.
 
-            # destroy the server and all storages attached to it
-            self.destroy()
+            # first destroy server
+            try_it_n_times(operation=self.destroy,
+                           expected_error_codes=['SERVER_STATE_ILLEGAL'],
+                           custom_error='stopping server failed')
+
+            # storages may be deleted instantly after server DELETE
             for storage in self.storage_devices:
-                storage.destroy()
+                try_it_n_times(operation=storage.destroy,
+                               expected_error_codes=['STORAGE_STATE_ILLEGAL'],
+                               custom_error='destroying storage failed')
+
+        if sync:
+            self.populate()
 
         # server is either starting or stopping (or error)
         if self.state in ['maintenance', 'error']:
             self._wait_for_state_change(['stopped', 'started'])
 
-        # server is started
-        if self.state != 'stopped':
+        if self.state == 'started':
             self.stop()
             self._wait_for_state_change(['stopped'])
 
         if self.state == 'stopped':
-            destroy_storages()
+            _self_destruct()
         else:
-            # something went wrong, fail explicitly
             raise Exception('unknown server state: ' + self.state)
 
     @classmethod
diff --git a/upcloud_api/storage.py b/upcloud_api/storage.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
-from builtins import dict, object, str
 
 from upcloud_api.tools import assignIfExists, OperatingSystems
 
diff --git a/upcloud_api/tag.py b/upcloud_api/tag.py
@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 from __future__ import absolute_import
-from builtins import dict, str
+
+import six
 
 from upcloud_api import Server
 
@@ -42,7 +43,7 @@ def __reset(self, **kwargs):
             servers = kwargs['servers']
 
         # convert UUIDs into server objects
-        if servers and isinstance(servers[0], str):
+        if servers and isinstance(servers[0], six.string_types):
             kwargs['servers'] = [Server(uuid=server, populated=False) for server in servers]
         else:
             kwargs['servers'] = servers
diff --git a/upcloud_api/utils.py b/upcloud_api/utils.py
@@ -0,0 +1,24 @@
+import itertools
+from time import sleep
+
+from upcloud_api import UpCloudAPIError
+
+
+def try_it_n_times(operation, expected_error_codes, custom_error='operation failed', n=5):
+    """
+    Try a given operation (API call) n times.
+
+    Raises if the API call fails with an error_code that is not expected.
+    Raises if the API call has not succeeded within n attempts.
+    Waits 3 seconds betwee each attempt.
+    """
+    for i in itertools.count():
+        try:
+            operation()
+            break
+        except UpCloudAPIError as e:
+            if e.error_code not in expected_error_codes:
+                raise e
+            sleep(3)
+        if i >= n - 1:
+            raise Exception(custom_error)
