diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py index 7c711b6b61..e18a1b2ed0 100644 --- a/neutron/db/loadbalancer/loadbalancer_db.py +++ b/neutron/db/loadbalancer/loadbalancer_db.py @@ -770,6 +770,15 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, return self._make_health_monitor_dict(monitor_db) def delete_health_monitor(self, context, id): + """Delete health monitor object from DB + + Raises an error if the monitor has associations with pools + """ + query = self._model_query(context, PoolMonitorAssociation) + has_associations = query.filter_by(monitor_id=id).first() + if has_associations: + raise loadbalancer.HealthMonitorInUse(monitor_id=id) + with context.session.begin(subtransactions=True): monitor_db = self._get_resource(context, HealthMonitor, id) context.session.delete(monitor_db) diff --git a/neutron/extensions/loadbalancer.py b/neutron/extensions/loadbalancer.py index 7f23704d8f..67c231490b 100644 --- a/neutron/extensions/loadbalancer.py +++ b/neutron/extensions/loadbalancer.py @@ -69,6 +69,11 @@ class PoolInUse(qexception.InUse): message = _("Pool %(pool_id)s is still in use") +class HealthMonitorInUse(qexception.InUse): + message = _("Health monitor %(monitor_id)s still has associations with " + "pools") + + class PoolStatsNotFound(qexception.NotFound): message = _("Statistics of Pool %(pool_id)s could not be found") diff --git a/neutron/services/loadbalancer/plugin.py b/neutron/services/loadbalancer/plugin.py index 29813579ae..b87b7a85aa 100644 --- a/neutron/services/loadbalancer/plugin.py +++ b/neutron/services/loadbalancer/plugin.py @@ -254,19 +254,6 @@ class LoadBalancerPlugin(ldb.LoadBalancerPluginDb, def _delete_db_health_monitor(self, context, id): super(LoadBalancerPlugin, self).delete_health_monitor(context, id) - def delete_health_monitor(self, context, id): - with context.session.begin(subtransactions=True): - hm = self.get_health_monitor(context, id) - qry = context.session.query( - ldb.PoolMonitorAssociation - ).filter_by(monitor_id=id).join(ldb.Pool) - for assoc in qry: - driver = self._get_driver_for_pool(context, assoc['pool_id']) - driver.delete_pool_health_monitor(context, - hm, - assoc['pool_id']) - super(LoadBalancerPlugin, self).delete_health_monitor(context, id) - def create_pool_health_monitor(self, context, health_monitor, pool_id): retval = super(LoadBalancerPlugin, self).create_pool_health_monitor( context, diff --git a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py index d3a4db16c0..691b961569 100644 --- a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py +++ b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py @@ -1022,8 +1022,8 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): qry = qry.filter_by(id=monitor['health_monitor']['id']) self.assertIsNone(qry.first()) - def test_delete_healthmonitor_cascade_deletion_of_associations(self): - with self.health_monitor(type='HTTP', no_delete=True) as monitor: + def test_delete_healthmonitor_with_associations_raises(self): + with self.health_monitor(type='HTTP') as monitor: with self.pool() as pool: data = { 'health_monitor': { @@ -1046,17 +1046,21 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): qry = ctx.session.query(ldb.PoolMonitorAssociation) qry = qry.filter_by(monitor_id=monitor['health_monitor']['id']) self.assertTrue(qry.all()) - # delete the HealthMonitor instance + # try to delete the HealthMonitor instance req = self.new_delete_request( 'health_monitors', monitor['health_monitor']['id'] ) res = req.get_response(self.ext_api) - self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) - # check if all corresponding Pool associations are deleted + self.assertEqual(res.status_int, webob.exc.HTTPConflict.code) + + qry = ctx.session.query(ldb.HealthMonitor) + qry = qry.filter_by(id=monitor['health_monitor']['id']) + self.assertIsNotNone(qry.first()) + # check if all corresponding Pool associations are not deleted qry = ctx.session.query(ldb.PoolMonitorAssociation) qry = qry.filter_by(monitor_id=monitor['health_monitor']['id']) - self.assertFalse(qry.all()) + self.assertTrue(qry.all()) def test_show_healthmonitor(self): with self.health_monitor() as monitor: @@ -1363,6 +1367,15 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): pool_updated['pool']['id']) # clean up + # disassociate the health_monitor from the pool first + req = self.new_delete_request( + "pools", + fmt=self.fmt, + id=pool['pool']['id'], + subresource="health_monitors", + sub_id=health_monitor['health_monitor']['id']) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) self._delete('health_monitors', health_monitor['health_monitor']['id']) self._delete('members', member1['member']['id']) @@ -1370,10 +1383,10 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): def test_create_pool_health_monitor(self): with contextlib.nested( - self.pool(name="pool"), self.health_monitor(), - self.health_monitor() - ) as (pool, health_mon1, health_mon2): + self.health_monitor(), + self.pool(name="pool") + ) as (health_mon1, health_mon2, pool): res = self.plugin.create_pool_health_monitor( context.get_admin_context(), health_mon1, pool['pool']['id'] @@ -1401,9 +1414,9 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): with mock.patch.object(self.plugin.drivers['lbaas'], 'create_pool_health_monitor') as driver_call: with contextlib.nested( - self.pool(), - self.health_monitor() - ) as (pool, hm): + self.health_monitor(), + self.pool() + ) as (hm, pool): data = {'health_monitor': { 'id': hm['health_monitor']['id'], 'tenant_id': self._tenant_id}} @@ -1420,10 +1433,10 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): def test_pool_monitor_list_of_pools(self): with contextlib.nested( - self.pool(), - self.pool(), - self.health_monitor() - ) as (p1, p2, hm): + self.health_monitor(), + self.pool(), + self.pool() + ) as (hm, p1, p2): ctx = context.get_admin_context() data = {'health_monitor': { 'id': hm['health_monitor']['id'], @@ -1455,9 +1468,9 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): def test_create_pool_health_monitor_already_associated(self): with contextlib.nested( - self.pool(name="pool"), self.health_monitor(), - ) as (pool, hm): + self.pool(name="pool") + ) as (hm, pool): res = self.plugin.create_pool_health_monitor( context.get_admin_context(), hm, pool['pool']['id'] @@ -1501,33 +1514,35 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): self.assertFalse(updated_pool['status_description']) def test_update_pool_health_monitor(self): - with self.pool() as pool: - with self.health_monitor() as hm: - res = self.plugin.create_pool_health_monitor( - context.get_admin_context(), - hm, pool['pool']['id']) - self.assertEqual({'health_monitor': - [hm['health_monitor']['id']]}, - res) + with contextlib.nested( + self.health_monitor(), + self.pool(name="pool") + ) as (hm, pool): + res = self.plugin.create_pool_health_monitor( + context.get_admin_context(), + hm, pool['pool']['id']) + self.assertEqual({'health_monitor': + [hm['health_monitor']['id']]}, + res) - assoc = self.plugin.get_pool_health_monitor( - context.get_admin_context(), - hm['health_monitor']['id'], - pool['pool']['id']) - self.assertEqual(assoc['status'], 'PENDING_CREATE') - self.assertIsNone(assoc['status_description']) + assoc = self.plugin.get_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id']) + self.assertEqual(assoc['status'], 'PENDING_CREATE') + self.assertIsNone(assoc['status_description']) - self.plugin.update_pool_health_monitor( - context.get_admin_context(), - hm['health_monitor']['id'], - pool['pool']['id'], - 'ACTIVE', 'ok') - assoc = self.plugin.get_pool_health_monitor( - context.get_admin_context(), - hm['health_monitor']['id'], - pool['pool']['id']) - self.assertEqual(assoc['status'], 'ACTIVE') - self.assertEqual(assoc['status_description'], 'ok') + self.plugin.update_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id'], + 'ACTIVE', 'ok') + assoc = self.plugin.get_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id']) + self.assertEqual(assoc['status'], 'ACTIVE') + self.assertEqual(assoc['status_description'], 'ok') def test_check_orphan_pool_associations(self): with contextlib.nested( diff --git a/neutron/tests/unit/services/loadbalancer/drivers/test_agent_driver_base.py b/neutron/tests/unit/services/loadbalancer/drivers/test_agent_driver_base.py index 692ed00299..de9e8fbd65 100644 --- a/neutron/tests/unit/services/loadbalancer/drivers/test_agent_driver_base.py +++ b/neutron/tests/unit/services/loadbalancer/drivers/test_agent_driver_base.py @@ -282,9 +282,9 @@ class TestLoadBalancerCallbacks(TestLoadBalancerPluginBase): self.assertEqual([member], logical_config['members']) def test_get_logical_device_pending_create_health_monitor(self): - with self.pool() as pool: - with self.vip(pool=pool) as vip: - with self.health_monitor() as monitor: + with self.health_monitor() as monitor: + with self.pool() as pool: + with self.vip(pool=pool) as vip: ctx = context.get_admin_context() self.plugin_instance.update_status(ctx, ldb.Pool, pool['pool']['id'], @@ -408,9 +408,9 @@ class TestLoadBalancerCallbacks(TestLoadBalancerPluginBase): def test_update_status_health_monitor(self): with contextlib.nested( - self.pool(), - self.health_monitor() - ) as (pool, hm): + self.health_monitor(), + self.pool() + ) as (hm, pool): pool_id = pool['pool']['id'] ctx = context.get_admin_context() self.plugin_instance.create_pool_health_monitor(ctx, hm, pool_id) @@ -671,9 +671,9 @@ class TestLoadBalancerPluginNotificationWrapper(TestLoadBalancerPluginBase): def test_create_pool_health_monitor(self): with contextlib.nested( + self.health_monitor(), self.pool(), - self.health_monitor() - ) as (pool, hm): + ) as (hm, pool): pool_id = pool['pool']['id'] ctx = context.get_admin_context() self.plugin_instance.create_pool_health_monitor(ctx, hm, pool_id)