From e002d1c712a17c7c08eefd0e11e519f0cf35bd20 Mon Sep 17 00:00:00 2001 From: Jay Dobies Date: Mon, 10 Feb 2014 14:58:06 -0500 Subject: [PATCH] Fix delete cascade for attribute and counts Change-Id: Ic355a215d576565688c1b9ef1976ff998c799d8e Closes-Bug: #1278396 --- tuskar/common/exception.py | 4 ++++ tuskar/db/sqlalchemy/api.py | 44 +++++++++++++++++++--------------- tuskar/db/sqlalchemy/models.py | 11 +++++---- tuskar/tests/db/test_api.py | 11 +++++++++ 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/tuskar/common/exception.py b/tuskar/common/exception.py index 440a5b05..ecd73ea2 100644 --- a/tuskar/common/exception.py +++ b/tuskar/common/exception.py @@ -151,6 +151,10 @@ class OvercloudRoleExists(DuplicateEntry): message = _("Overcloud role with name %(name)s already exists.") +class OvercloudRoleInUse(TuskarException): + message = _('Role %(name)s is in use by an overcloud.') + + class OvercloudRoleCountExists(DuplicateEntry): message = _("Count for overcloud %(cloud)s and " "role %(role)s already exists.") diff --git a/tuskar/db/sqlalchemy/api.py b/tuskar/db/sqlalchemy/api.py index 2f59ea6c..b210aa3f 100644 --- a/tuskar/db/sqlalchemy/api.py +++ b/tuskar/db/sqlalchemy/api.py @@ -20,6 +20,7 @@ from oslo.config import cfg # TODO(deva): import MultipleResultsFound and handle it appropriately +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm import subqueryload @@ -54,6 +55,10 @@ def model_query(model, *args, **kwargs): return query +def get_session(): + return db_session.get_session(sqlite_fk=True) + + class Connection(api.Connection): """SqlAlchemy connection.""" @@ -64,21 +69,19 @@ class Connection(api.Connection): # here. pass - @staticmethod - def get_overcloud_roles(): + def get_overcloud_roles(self): """Returns all overcloud roles known to Tuskar. :return: list of roles; empty list if none are found :rtype: list of tuskar.db.sqlalchemy.models.OvercloudRole """ - session = db_session.get_session() + session = get_session() roles = session.query(models.OvercloudRole).all() session.close() return roles - @staticmethod - def get_overcloud_role_by_id(role_id): + def get_overcloud_role_by_id(self, role_id): """Single overcloud role query. :return: role if one exists with the given ID @@ -88,7 +91,7 @@ class Connection(api.Connection): role with the given ID exists """ - session = db_session.get_session() + session = get_session() try: query = session.query(models.OvercloudRole).filter_by( id=role_id) @@ -102,8 +105,7 @@ class Connection(api.Connection): return result - @staticmethod - def create_overcloud_role(overcloud_role): + def create_overcloud_role(self, overcloud_role): """Creates a new overcloud role in the database. :param overcloud_role: role instance to save @@ -116,7 +118,7 @@ class Connection(api.Connection): :raises: tuskar.common.exception.OvercloudRoleExists: if a role with the given name exists """ - session = db_session.get_session() + session = get_session() session.begin() try: @@ -161,25 +163,30 @@ class Connection(api.Connection): """ role = self.get_overcloud_role_by_id(role_id) - session = db_session.get_session() + session = get_session() session.begin() try: session.delete(role) session.commit() + except db_exception.DBError as e: + if isinstance(e.inner_exception, IntegrityError): + raise exception.OvercloudRoleInUse(name=role.name) + else: + raise + finally: session.close() - @staticmethod - def get_overclouds(): + def get_overclouds(self): """Returns all overcloud instances from the database. :return: list of overcloud instances; empty list if none are found :rtype: list of tuskar.db.sqlalchemy.models.Overcloud """ - session = db_session.get_session() + session = get_session() overclouds = session.query(models.Overcloud).\ options(subqueryload(models.Overcloud.attributes)).\ options(subqueryload(models.Overcloud.counts)).\ @@ -187,8 +194,7 @@ class Connection(api.Connection): session.close() return overclouds - @staticmethod - def get_overcloud_by_id(overcloud_id): + def get_overcloud_by_id(self, overcloud_id): """Returns a specific overcloud instance. :return: overcloud if one exists with the given ID @@ -198,7 +204,7 @@ class Connection(api.Connection): overcloud with the given ID exists """ - session = db_session.get_session() + session = get_session() try: query = session.query(models.Overcloud).\ options(subqueryload(models.Overcloud.attributes)).\ @@ -227,7 +233,7 @@ class Connection(api.Connection): :raises: tuskar.common.exception.OvercloudExists: if an overcloud role with the given name exists """ - session = db_session.get_session() + session = get_session() session.begin() try: @@ -277,7 +283,7 @@ class Connection(api.Connection): existing = self.get_overcloud_by_id(updated.id) - session = db_session.get_session() + session = get_session() session.begin() try: @@ -384,7 +390,7 @@ class Connection(api.Connection): """ overcloud = self.get_overcloud_by_id(overcloud_id) - session = db_session.get_session() + session = get_session() session.begin() try: diff --git a/tuskar/db/sqlalchemy/models.py b/tuskar/db/sqlalchemy/models.py index 47a5502e..3f7d2d7e 100644 --- a/tuskar/db/sqlalchemy/models.py +++ b/tuskar/db/sqlalchemy/models.py @@ -122,7 +122,7 @@ class OvercloudRoleCount(Base): # Overcloud in which the role is being deployed overcloud_id = \ Column(Integer, - ForeignKey('%s.id' % TABLE_OVERCLOUD), + ForeignKey('%s.id' % TABLE_OVERCLOUD, ondelete='CASCADE'), nullable=False) # Number of nodes of this configuration that should be deployed @@ -148,7 +148,8 @@ class OvercloudAttribute(Base): # Reference back to the overcloud being configured overcloud_id = Column(Integer, - ForeignKey('%s.id' % TABLE_OVERCLOUD), + ForeignKey('%s.id' % TABLE_OVERCLOUD, + ondelete='CASCADE'), nullable=False) # Identifier and value of the configuration attribute @@ -182,10 +183,12 @@ class Overcloud(Base): description = Column(String(length=LENGTH_DESCRIPTION)) # List of configuration attributes for the overcloud - attributes = relationship(OvercloudAttribute.__name__) + attributes = relationship(OvercloudAttribute.__name__, + cascade='all,delete') # List of counts of overcloud roles to deploy - counts = relationship(OvercloudRoleCount.__name__) + counts = relationship(OvercloudRoleCount.__name__, + cascade='all,delete') def __eq__(self, other): return self.name == other.name diff --git a/tuskar/tests/db/test_api.py b/tuskar/tests/db/test_api.py index dc2094af..77422ea7 100644 --- a/tuskar/tests/db/test_api.py +++ b/tuskar/tests/db/test_api.py @@ -363,6 +363,17 @@ class OvercloudTests(db_base.DbTestCase): found = self.connection.get_overclouds() self.assertEqual(0, len(found)) + # Ensure the joined tables are clear too + session = dbapi.get_session() + all_counts = session.query(models.OvercloudAttribute).all() + session.close() + self.assertEqual(0, len(all_counts)) + + session = dbapi.get_session() + all_counts = session.query(models.OvercloudRoleCount).all() + session.close() + self.assertEqual(0, len(all_counts)) + def test_delete_nonexistent_overcloud(self): self.assertRaises(exception.OvercloudNotFound, self.connection.delete_overcloud_by_id,