Sync nova with oslo DB exception cleanup.

DB exceptions have moved to openstack/common/db/exception module so that
they can be shared with multiple DB implementations.

Deadlock checking was also added to oslo to consolidate with
DuplicateKey checking.  This allows us to clean up our
_retry_on_deadlock decorator in sqlalchemy/api.py

Fixes unrelated pep8 issue with duplicate test in test_compute also.

Change-Id: I7e985b384d1ef345e0d67c919b84b4faff869699

14 files changed, 119 insertions, 81 deletions

diff --git a/bin/nova-manage b/bin/nova-manage
index c4e9841ce..0fde8ba0a 100755
--- a/bin/nova-manage
+++ b/bin/nova-manage
@@ -80,7 +80,7 @@ from nova import db
 from nova.db import migration
 from nova import exception
 from nova.openstack.common import cliutils
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common import importutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import rpc
@@ -861,7 +861,7 @@ class InstanceTypeCommands(object):
         except exception.InstanceTypeNotFound:
             print _("Valid instance type name is required")
             sys.exit(1)
-        except db_session.DBError, e:
+        except db_exc.DBError, e:
             print _("DB Error: %s") % e
             sys.exit(2)
         except Exception:
@@ -878,7 +878,7 @@ class InstanceTypeCommands(object):
                 inst_types = instance_types.get_all_types()
             else:
                 inst_types = instance_types.get_instance_type_by_name(name)
-        except db_session.DBError, e:
+        except db_exc.DBError, e:
             _db_error(e)
         if isinstance(inst_types.values()[0], dict):
             for k, v in inst_types.iteritems():
@@ -909,7 +909,7 @@ class InstanceTypeCommands(object):
                             ext_spec)
             print _("Key %(key)s set to %(value)s on instance"
                     " type %(name)s") % locals()
-        except db_session.DBError, e:
+        except db_exc.DBError, e:
             _db_error(e)
 
     @args('--name', dest='name', metavar='<name>',
@@ -932,7 +932,7 @@ class InstanceTypeCommands(object):
                         key)
 
             print _("Key %(key)s on instance type %(name)s unset") % locals()
-        except db_session.DBError, e:
+        except db_exc.DBError, e:
             _db_error(e)
 
 
diff --git a/nova/compute/instance_types.py b/nova/compute/instance_types.py
index 73105b33f..3060e0bc2 100644
--- a/nova/compute/instance_types.py
+++ b/nova/compute/instance_types.py
@@ -28,7 +28,7 @@ from oslo.config import cfg
 from nova import context
 from nova import db
 from nova import exception
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common import log as logging
 from nova import utils
 
@@ -134,7 +134,7 @@ def create(name, memory, vcpus, root_gb, ephemeral_gb=None, flavorid=None,
 
     try:
         return db.instance_type_create(context.get_admin_context(), kwargs)
-    except db_session.DBError, e:
+    except db_exc.DBError, e:
         LOG.exception(_('DB error: %s') % e)
         raise exception.InstanceTypeCreateFailed()
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 1baa192a3..9efdc47f1 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -30,7 +30,6 @@ import uuid
 from oslo.config import cfg
 from sqlalchemy import and_
 from sqlalchemy import Boolean
-from sqlalchemy import exc as sqla_exc
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.exc import NoSuchTableError
 from sqlalchemy import Integer
@@ -52,6 +51,7 @@ import nova.context
 from nova import db
 from nova.db.sqlalchemy import models
 from nova import exception
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common.db.sqlalchemy import session as db_session
 from nova.openstack.common.db.sqlalchemy import utils as sqlalchemyutils
 from nova.openstack.common import log as logging
@@ -143,30 +143,15 @@ def require_aggregate_exists(f):
 
 def _retry_on_deadlock(f):
     """Decorator to retry a DB API call if Deadlock was received."""
-    def _is_deadlock_exc(dberr_info):
-        deadlock_str = 'Deadlock found when trying to get lock'
-        try:
-            if not isinstance(dberr_info, sqla_exc.OperationalError):
-                return False
-            if deadlock_str in dberr_info.message:
-                LOG.warn(_("Deadlock detected when running "
-                           "'%(func_name)s': Retrying..."),
-                           dict(func_name=f.__name__))
-                return True
-        except Exception:
-            pass
-        return False
-
     @functools.wraps(f)
     def wrapped(*args, **kwargs):
         while True:
             try:
                 return f(*args, **kwargs)
-            except db_session.DBError as db_err:
-                exc_info = sys.exc_info()
-                dberr_info = db_err.inner_exception
-                if not _is_deadlock_exc(dberr_info):
-                    raise exc_info[0], exc_info[1], exc_info[2]
+            except db_exc.DBDeadlock:
+                LOG.warn(_("Deadlock detected when running "
+                           "'%(func_name)s': Retrying..."),
+                           dict(func_name=f.__name__))
                 # Retry!
                 time.sleep(0.5)
                 continue
@@ -1264,7 +1249,7 @@ def virtual_interface_create(context, values):
         vif_ref = models.VirtualInterface()
         vif_ref.update(values)
         vif_ref.save()
-    except db_session.DBError:
+    except db_exc.DBError:
         raise exception.VirtualInterfaceCreateException()
 
     return vif_ref
@@ -2079,7 +2064,7 @@ def network_create_safe(context, values):
     try:
         network_ref.save()
         return network_ref
-    except db_session.DBDuplicateEntry:
+    except db_exc.DBDuplicateEntry:
         raise exception.DuplicateVlan(vlan=values['vlan'])
 
 
@@ -2321,7 +2306,7 @@ def network_update(context, network_id, values):
         network_ref.update(values)
         try:
             network_ref.save(session=session)
-        except db_session.DBDuplicateEntry:
+        except db_exc.DBDuplicateEntry:
             raise exception.DuplicateVlan(vlan=values['vlan'])
         return network_ref
 
@@ -3547,7 +3532,7 @@ def instance_type_create(context, values):
             instance_type_ref.update(values)
             instance_type_ref.save(session=session)
         except Exception, e:
-            raise db_session.DBError(e)
+            raise db_exc.DBError(e)
         return _dict_with_extra_specs(instance_type_ref)
 
 
@@ -4209,7 +4194,7 @@ def s3_image_create(context, image_uuid):
         s3_image_ref.update({'uuid': image_uuid})
         s3_image_ref.save()
     except Exception, e:
-        raise db_session.DBError(e)
+        raise db_exc.DBError(e)
 
     return s3_image_ref
 
@@ -4729,7 +4714,7 @@ def task_log_begin_task(context, task_name, period_beginning, period_ending,
         task.task_items = task_items
     try:
         task.save()
-    except db_session.DBDuplicateEntry:
+    except db_exc.DBDuplicateEntry:
         raise exception.TaskAlreadyRunning(task_name=task_name, host=host)
 
 
diff --git a/nova/openstack/common/db/exception.py b/nova/openstack/common/db/exception.py
new file mode 100644
index 000000000..61ba1b3a9
--- /dev/null
+++ b/nova/openstack/common/db/exception.py
@@ -0,0 +1,45 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""DB related custom exceptions."""
+
+from nova.openstack.common.gettextutils import _
+
+
+class DBError(Exception):
+    """Wraps an implementation specific exception."""
+    def __init__(self, inner_exception=None):
+        self.inner_exception = inner_exception
+        super(DBError, self).__init__(str(inner_exception))
+
+
+class DBDuplicateEntry(DBError):
+    """Wraps an implementation specific exception."""
+    def __init__(self, columns=[], inner_exception=None):
+        self.columns = columns
+        super(DBDuplicateEntry, self).__init__(inner_exception)
+
+
+class DBDeadlock(DBError):
+    def __init__(self, inner_exception=None):
+        super(DBDeadlock, self).__init__(inner_exception)
+
+
+class DBInvalidUnicodeParameter(Exception):
+    message = _("Invalid Parameter: "
+                "Unicode is not supported by the current database.")
diff --git a/nova/openstack/common/db/sqlalchemy/session.py b/nova/openstack/common/db/sqlalchemy/session.py
index fb86d9ca5..cf6713581 100644
--- a/nova/openstack/common/db/sqlalchemy/session.py
+++ b/nova/openstack/common/db/sqlalchemy/session.py
@@ -246,12 +246,13 @@ import time
 
 from eventlet import greenthread
 from oslo.config import cfg
-from sqlalchemy.exc import DisconnectionError, OperationalError, IntegrityError
+from sqlalchemy import exc as sqla_exc
 import sqlalchemy.interfaces
 import sqlalchemy.orm
 from sqlalchemy.pool import NullPool, StaticPool
 from sqlalchemy.sql.expression import literal_column
 
+from nova.openstack.common.db import exception
 from nova.openstack.common import log as logging
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import timeutils
@@ -327,25 +328,6 @@ def get_session(autocommit=True, expire_on_commit=False):
     return session
 
 
-class DBError(Exception):
-    """Wraps an implementation specific exception."""
-    def __init__(self, inner_exception=None):
-        self.inner_exception = inner_exception
-        super(DBError, self).__init__(str(inner_exception))
-
-
-class DBDuplicateEntry(DBError):
-    """Wraps an implementation specific exception."""
-    def __init__(self, columns=[], inner_exception=None):
-        self.columns = columns
-        super(DBDuplicateEntry, self).__init__(inner_exception)
-
-
-class InvalidUnicodeParameter(Exception):
-    message = _("Invalid Parameter: "
-                "Unicode is not supported by the current database.")
-
-
 # note(boris-42): In current versions of DB backends unique constraint
 # violation messages follow the structure:
 #
@@ -364,7 +346,7 @@ class InvalidUnicodeParameter(Exception):
 #               'c1'")
 # N columns - (IntegrityError) (1062, "Duplicate entry 'values joined
 #               with -' for key 'name_of_our_constraint'")
-_RE_DB = {
+_DUP_KEY_RE_DB = {
     "sqlite": re.compile(r"^.*columns?([^)]+)(is|are)\s+not\s+unique$"),
     "postgresql": re.compile(r"^.*duplicate\s+key.*\"([^\"]+)\"\s*\n.*$"),
     "mysql": re.compile(r"^.*\(1062,.*'([^\']+)'\"\)$")
@@ -390,7 +372,7 @@ def raise_if_duplicate_entry_error(integrity_error, engine_name):
     if engine_name not in ["mysql", "sqlite", "postgresql"]:
         return
 
-    m = _RE_DB[engine_name].match(integrity_error.message)
+    m = _DUP_KEY_RE_DB[engine_name].match(integrity_error.message)
     if not m:
         return
     columns = m.group(1)
@@ -399,7 +381,32 @@ def raise_if_duplicate_entry_error(integrity_error, engine_name):
         columns = columns.strip().split(", ")
     else:
         columns = get_columns_from_uniq_cons_or_name(columns)
-    raise DBDuplicateEntry(columns, integrity_error)
+    raise exception.DBDuplicateEntry(columns, integrity_error)
+
+
+# NOTE(comstud): In current versions of DB backends, Deadlock violation
+# messages follow the structure:
+#
+# mysql:
+# (OperationalError) (1213, 'Deadlock found when trying to get lock; try '
+#                     'restarting transaction') <query_str> <query_args>
+_DEADLOCK_RE_DB = {
+    "mysql": re.compile(r"^.*\(1213, 'Deadlock.*")
+}
+
+
+def raise_if_deadlock_error(operational_error, engine_name):
+    """
+    Raise DBDeadlock exception if OperationalError contains a Deadlock
+    condition.
+    """
+    re = _DEADLOCK_RE_DB.get(engine_name)
+    if re is None:
+        return
+    m = re.match(operational_error.message)
+    if not m:
+        return
+    raise exception.DBDeadlock(operational_error)
 
 
 def wrap_db_error(f):
@@ -407,21 +414,26 @@ def wrap_db_error(f):
         try:
             return f(*args, **kwargs)
         except UnicodeEncodeError:
-            raise InvalidUnicodeParameter()
+            raise exception.DBInvalidUnicodeParameter()
         # note(boris-42): We should catch unique constraint violation and
         # wrap it by our own DBDuplicateEntry exception. Unique constraint
         # violation is wrapped by IntegrityError.
-        except IntegrityError, e:
+        except sqla_exc.OperationalError, e:
+            raise_if_deadlock_error(e, get_engine().name)
+            # NOTE(comstud): A lot of code is checking for OperationalError
+            # so let's not wrap it for now.
+            raise
+        except sqla_exc.IntegrityError, e:
             # note(boris-42): SqlAlchemy doesn't unify errors from different
             # DBs so we must do this. Also in some tables (for example
             # instance_types) there are more than one unique constraint. This
             # means we should get names of columns, which values violate
             # unique constraint, from error message.
             raise_if_duplicate_entry_error(e, get_engine().name)
-            raise DBError(e)
+            raise exception.DBError(e)
         except Exception, e:
             LOG.exception(_('DB exception wrapped.'))
-            raise DBError(e)
+            raise exception.DBError(e)
     _wrap.func_name = f.func_name
     return _wrap
 
@@ -471,7 +483,7 @@ def ping_listener(dbapi_conn, connection_rec, connection_proxy):
     except dbapi_conn.OperationalError, ex:
         if ex.args[0] in (2006, 2013, 2014, 2045, 2055):
             LOG.warn(_('Got mysql server has gone away: %s'), ex)
-            raise DisconnectionError("Database server went away")
+            raise sqla_exc.DisconnectionError("Database server went away")
         else:
             raise
 
@@ -532,7 +544,7 @@ def create_engine(sql_connection):
 
     try:
         engine.connect()
-    except OperationalError, e:
+    except sqla_exc.OperationalError, e:
         if not is_db_connection_error(e.args[0]):
             raise
 
@@ -548,7 +560,7 @@ def create_engine(sql_connection):
             try:
                 engine.connect()
                 break
-            except OperationalError, e:
+            except sqla_exc.OperationalError, e:
                 if (remaining != 'infinite' and remaining == 0) or \
                         not is_db_connection_error(e.args[0]):
                     raise
diff --git a/nova/tests/baremetal/db/test_bm_interface.py b/nova/tests/baremetal/db/test_bm_interface.py
index 32beb1ce0..e870ec5e0 100644
--- a/nova/tests/baremetal/db/test_bm_interface.py
+++ b/nova/tests/baremetal/db/test_bm_interface.py
@@ -18,7 +18,7 @@ Bare-metal DB testcase for BareMetalInterface
 """
 
 from nova import exception
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.tests.baremetal.db import base
 from nova.virt.baremetal import db
 
@@ -28,7 +28,7 @@ class BareMetalInterfaceTestCase(base.BMDBTestCase):
     def test_unique_address(self):
         pif1_id = db.bm_interface_create(self.context, 1, '11:11:11:11:11:11',
                                          '0x1', 1)
-        self.assertRaises(db_session.DBError,
+        self.assertRaises(db_exc.DBError,
                           db.bm_interface_create,
                           self.context, 2, '11:11:11:11:11:11', '0x2', 2)
         # succeed after delete pif1
diff --git a/nova/tests/baremetal/db/test_bm_pxe_ip.py b/nova/tests/baremetal/db/test_bm_pxe_ip.py
index 9820f3af0..fe8ba5b3e 100644
--- a/nova/tests/baremetal/db/test_bm_pxe_ip.py
+++ b/nova/tests/baremetal/db/test_bm_pxe_ip.py
@@ -18,7 +18,7 @@ Bare-metal DB testcase for BareMetalPxeIp
 """
 
 from nova import exception
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.tests.baremetal.db import base
 from nova.tests.baremetal.db import utils
 from nova.virt.baremetal import db
@@ -51,14 +51,14 @@ class BareMetalPxeIpTestCase(base.BMDBTestCase):
         # address duplicates
         i = utils.new_bm_pxe_ip(address='10.1.1.1',
                                 server_address='10.1.1.201')
-        self.assertRaises(db_session.DBError,
+        self.assertRaises(db_exc.DBError,
                           db.bm_pxe_ip_create_direct,
                           self.context, i)
 
         # server_address duplicates
         i = utils.new_bm_pxe_ip(address='10.1.1.3',
                                 server_address='10.1.1.101')
-        self.assertRaises(db_session.DBError,
+        self.assertRaises(db_exc.DBError,
                           db.bm_pxe_ip_create_direct,
                           self.context, i)
 
diff --git a/nova/tests/baremetal/test_pxe.py b/nova/tests/baremetal/test_pxe.py
index d9e41bc67..4f4c9f7db 100644
--- a/nova/tests/baremetal/test_pxe.py
+++ b/nova/tests/baremetal/test_pxe.py
@@ -27,7 +27,7 @@ from oslo.config import cfg
 from testtools import matchers
 
 from nova import exception
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.tests.baremetal.db import base as bm_db_base
 from nova.tests.baremetal.db import utils as bm_db_utils
 from nova.tests.image import fake as fake_image
@@ -529,7 +529,7 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
                 AndRaise(exception.NovaException)
         bm_utils.unlink_without_raise(pxe_path)
         self.driver._collect_mac_addresses(self.context, self.node).\
-                AndRaise(db_session.DBError)
+                AndRaise(db_exc.DBError)
         bm_utils.rmtree_without_raise(
                 os.path.join(CONF.baremetal.tftp_root, 'fake-uuid'))
         self.mox.ReplayAll()
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index 1257f67e1..6c40a95e2 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -162,9 +162,6 @@ class ComputeRpcAPITestCase(test.TestCase):
         self._test_compute_api('get_diagnostics', 'call',
                 instance=self.fake_instance)
 
-    def test_get_host_uptime(self):
-        self._test_compute_api('get_host_uptime', 'call')
-
     def test_get_vnc_console(self):
         self._test_compute_api('get_vnc_console', 'call',
                 instance=self.fake_instance, console_type='type')
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index ba997ac9d..92b8e1d91 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -29,7 +29,7 @@ from nova.network import floating_ips
 from nova.network import linux_net
 from nova.network import manager as network_manager
 from nova.network import model as net_model
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common import importutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import rpc
@@ -2157,7 +2157,7 @@ class FloatingIPTestCase(test.TestCase):
         # address column, so fake the collision-avoidance here
         def fake_vif_save(vif):
             if vif.address == crash_test_dummy_vif['address']:
-                raise db_session.DBError("If you're smart, you'll retry!")
+                raise db_exc.DBError("If you're smart, you'll retry!")
         self.stubs.Set(models.VirtualInterface, 'save', fake_vif_save)
 
         # Attempt to add another and make sure that both MACs are consumed
diff --git a/nova/virt/baremetal/db/sqlalchemy/api.py b/nova/virt/baremetal/db/sqlalchemy/api.py
index d64bc4872..d117ad46d 100644
--- a/nova/virt/baremetal/db/sqlalchemy/api.py
+++ b/nova/virt/baremetal/db/sqlalchemy/api.py
@@ -28,6 +28,7 @@ from sqlalchemy.sql.expression import literal_column
 import nova.context
 from nova.db.sqlalchemy import api as sqlalchemy_api
 from nova import exception
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
 from nova.openstack.common import uuidutils
@@ -399,7 +400,7 @@ def bm_interface_set_vif_uuid(context, if_id, vif_uuid):
         try:
             session.add(bm_interface)
             session.flush()
-        except db_session.DBError, e:
+        except db_exc.DBError, e:
             # TODO(deva): clean up when db layer raises DuplicateKeyError
             if str(e).find('IntegrityError') != -1:
                 raise exception.NovaException(_("Baremetal interface %s "
diff --git a/nova/virt/baremetal/db/sqlalchemy/session.py b/nova/virt/baremetal/db/sqlalchemy/session.py
index 585096c94..fc045d5ca 100644
--- a/nova/virt/baremetal/db/sqlalchemy/session.py
+++ b/nova/virt/baremetal/db/sqlalchemy/session.py
@@ -44,8 +44,6 @@ CONF.import_opt('sqlite_db', 'nova.openstack.common.db.sqlalchemy.session')
 _ENGINE = None
 _MAKER = None
 
-DBError = nova_session.DBError
-
 
 def get_session(autocommit=True, expire_on_commit=False):
     """Return a SQLAlchemy session."""
diff --git a/nova/virt/baremetal/pxe.py b/nova/virt/baremetal/pxe.py
index 813f95c05..1e98126e2 100644
--- a/nova/virt/baremetal/pxe.py
+++ b/nova/virt/baremetal/pxe.py
@@ -27,7 +27,7 @@ from oslo.config import cfg
 
 from nova.compute import instance_types
 from nova import exception
-from nova.openstack.common.db.sqlalchemy import session as db_session
+from nova.openstack.common.db import exception as db_exc
 from nova.openstack.common import fileutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
@@ -428,7 +428,7 @@ class PXE(base.NodeDriver):
         bm_utils.unlink_without_raise(get_pxe_config_file_path(instance))
         try:
             macs = self._collect_mac_addresses(context, node)
-        except db_session.DBError:
+        except db_exc.DBError:
             pass
         else:
             for mac in macs:
diff --git a/openstack-common.conf b/openstack-common.conf
index 36abed038..a2688fa45 100644
--- a/openstack-common.conf
+++ b/openstack-common.conf
@@ -1,7 +1,7 @@
 [DEFAULT]
 
 # The list of modules to copy from openstack-common
-modules=cliutils,context,db,db.api,db.sqlalchemy,excutils,eventlet_backdoor,fileutils,gettextutils,importutils,jsonutils,local,lockutils,log,network_utils,notifier,plugin,policy,rootwrap,setup,timeutils,rpc,uuidutils,install_venv_common,flakes,version,processutils
+modules=cliutils,context,db,excutils,eventlet_backdoor,fileutils,gettextutils,importutils,jsonutils,local,lockutils,log,network_utils,notifier,plugin,policy,rootwrap,setup,timeutils,rpc,uuidutils,install_venv_common,flakes,version,processutils
 
 # The base module to hold the copy of openstack.common
 base=nova