From dbb3769fa3792bf804d5d31197e3776eb4b7c9ff Mon Sep 17 00:00:00 2001 From: lqslan Date: Mon, 6 Apr 2015 10:33:59 +0800 Subject: [PATCH] Destroy the related resources when delete a bay Currently, when we delete a bay, we will delete the stack firstly, then try to detroy the bay record in db. And at this time, a BayNotEmpty exception may raised if some of pods, services or rc are related to this bay. In the meanwhile, the stack should be already deleted, actually, the left bay record is unavailable. This patch destroy the related resources when delete a bay. Change-Id: I56cd8813417bd4a6f6c340deb8afd88d5516f225 Closes-bug: #1440630 --- magnum/common/exception.py | 4 -- magnum/db/sqlalchemy/api.py | 17 +++----- .../tests/unit/api/controllers/v1/test_bay.py | 15 ++----- magnum/tests/unit/common/test_exception.py | 4 -- magnum/tests/unit/db/test_bay.py | 40 +++++++++++++++---- 5 files changed, 41 insertions(+), 39 deletions(-) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 879d664788..41ede11667 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -376,10 +376,6 @@ class BayAlreadyExists(Conflict): message = _("A node with UUID %(uuid)s already exists.") -class BayNotEmpty(Invalid): - message = _("Bay %(bay)s is not empty.") - - class ContainerNotFound(ResourceNotFound): message = _("Container %(container)s could not be found.") diff --git a/magnum/db/sqlalchemy/api.py b/magnum/db/sqlalchemy/api.py index 4e85127342..3fb817eee2 100644 --- a/magnum/db/sqlalchemy/api.py +++ b/magnum/db/sqlalchemy/api.py @@ -189,21 +189,22 @@ class Connection(api.Connection): raise exception.BayNotFound(bay=bay_uuid) def destroy_bay(self, bay_id): - def bay_not_empty(session, bay_uuid): + def destroy_bay_resources(session, bay_uuid): """Checks whether the bay does not have resources.""" query = model_query(models.Pod, session=session) query = self._add_pods_filters(query, {'bay_uuid': bay_uuid}) if query.count() != 0: - return True + query.delete() query = model_query(models.Service, session=session) query = self._add_services_filters(query, {'bay_uuid': bay_uuid}) if query.count() != 0: - return True + query.delete() query = model_query(models.ReplicationController, session=session) query = self._add_rcs_filters(query, {'bay_uuid': bay_uuid}) - return query.count() != 0 + if query.count() != 0: + query.delete() session = get_session() with session.begin(): @@ -215,13 +216,7 @@ class Connection(api.Connection): except NoResultFound: raise exception.BayNotFound(bay=bay_id) - # TODO(hongbin): delete pods and services that attached to the bay. - # We don't do it now because it could cause a long deletion - # time which would block the conductor. It needs either threads - # or coroutines. - if bay_not_empty(session, bay_ref['uuid']): - raise exception.BayNotEmpty(bay=bay_id) - + destroy_bay_resources(session, bay_ref['uuid']) query.delete() def update_bay(self, bay_id, values): diff --git a/magnum/tests/unit/api/controllers/v1/test_bay.py b/magnum/tests/unit/api/controllers/v1/test_bay.py index 333b52d202..17fe372b27 100644 --- a/magnum/tests/unit/api/controllers/v1/test_bay.py +++ b/magnum/tests/unit/api/controllers/v1/test_bay.py @@ -440,28 +440,19 @@ class TestDelete(api_base.FunctionalTest): obj_utils.create_test_pod(self.context, bay_uuid=self.bay.uuid) response = self.delete('/bays/%s' % self.bay.uuid, expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertTrue(response.json['error_message']) - self.assertIn(self.bay.uuid, response.json['error_message']) + self.assertEqual(204, response.status_int) def test_delete_bay_with_services(self): obj_utils.create_test_service(self.context, bay_uuid=self.bay.uuid) response = self.delete('/bays/%s' % self.bay.uuid, expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertTrue(response.json['error_message']) - self.assertIn(self.bay.uuid, response.json['error_message']) + self.assertEqual(204, response.status_int) def test_delete_bay_with_replication_controllers(self): obj_utils.create_test_rc(self.context, bay_uuid=self.bay.uuid) response = self.delete('/bays/%s' % self.bay.uuid, expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertTrue(response.json['error_message']) - self.assertIn(self.bay.uuid, response.json['error_message']) + self.assertEqual(204, response.status_int) def test_delete_bay_with_name_not_found(self): response = self.delete('/bays/not_found', expect_errors=True) diff --git a/magnum/tests/unit/common/test_exception.py b/magnum/tests/unit/common/test_exception.py index c90981bd42..69bd08eb83 100644 --- a/magnum/tests/unit/common/test_exception.py +++ b/magnum/tests/unit/common/test_exception.py @@ -143,10 +143,6 @@ class TestException(base.BaseTestCase): self.assertRaises(exception.BayAlreadyExists, lambda: self.raise_(exception.BayAlreadyExists())) - def test_BayNotEmpty(self): - self.assertRaises(exception.BayNotEmpty, - lambda: self.raise_(exception.BayNotEmpty())) - def test_BayModelNotFound(self): self.assertRaises(exception.BayModelNotFound, lambda: self.raise_(exception.BayModelNotFound())) diff --git a/magnum/tests/unit/db/test_bay.py b/magnum/tests/unit/db/test_bay.py index f3545aa568..64a55fe56a 100644 --- a/magnum/tests/unit/db/test_bay.py +++ b/magnum/tests/unit/db/test_bay.py @@ -144,29 +144,53 @@ class DbBayTestCase(base.DbTestCase): bay = utils.create_test_bay() pod = utils.create_test_pod(bay_uuid=bay.uuid) self.assertEqual(bay.uuid, pod.bay_uuid) - self.assertRaises(exception.BayNotEmpty, - self.dbapi.destroy_bay, bay.id) + self.dbapi.destroy_bay(bay.id) + self.assertRaises(exception.PodNotFound, + self.dbapi.get_pod_by_id, self.context, pod.id) def test_destroy_bay_that_has_pods_by_uuid(self): bay = utils.create_test_bay() pod = utils.create_test_pod(bay_uuid=bay.uuid) self.assertEqual(bay.uuid, pod.bay_uuid) - self.assertRaises(exception.BayNotEmpty, - self.dbapi.destroy_bay, bay.uuid) + self.dbapi.destroy_bay(bay.uuid) + self.assertRaises(exception.PodNotFound, + self.dbapi.get_pod_by_id, self.context, pod.id) def test_destroy_bay_that_has_services(self): bay = utils.create_test_bay() service = utils.create_test_service(bay_uuid=bay.uuid) self.assertEqual(bay.uuid, service.bay_uuid) - self.assertRaises(exception.BayNotEmpty, - self.dbapi.destroy_bay, bay.id) + self.dbapi.destroy_bay(bay.id) + self.assertRaises(exception.ServiceNotFound, + self.dbapi.get_service_by_id, + self.context, service.id) def test_destroy_bay_that_has_services_by_uuid(self): bay = utils.create_test_bay() service = utils.create_test_service(bay_uuid=bay.uuid) self.assertEqual(bay.uuid, service.bay_uuid) - self.assertRaises(exception.BayNotEmpty, - self.dbapi.destroy_bay, bay.uuid) + self.dbapi.destroy_bay(bay.id) + self.assertRaises(exception.ServiceNotFound, + self.dbapi.get_service_by_id, + self.context, service.id) + + def test_destroy_bay_that_has_rc(self): + bay = utils.create_test_bay() + rc = utils.create_test_rc(bay_uuid=bay.uuid) + self.assertEqual(bay.uuid, rc.bay_uuid) + self.dbapi.destroy_bay(bay.id) + self.assertRaises(exception.ReplicationControllerNotFound, + self.dbapi.get_rc_by_id, + self.context, rc.id) + + def test_destroy_bay_that_has_rc_by_uuid(self): + bay = utils.create_test_bay() + rc = utils.create_test_rc(bay_uuid=bay.uuid) + self.assertEqual(bay.uuid, rc.bay_uuid) + self.dbapi.destroy_bay(bay.uuid) + self.assertRaises(exception.ReplicationControllerNotFound, + self.dbapi.get_rc_by_id, + self.context, rc.id) def test_update_bay(self): bay = utils.create_test_bay()