From a1de76d1407952572cfe081c4872d7a6127995b3 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Thu, 24 Oct 2013 16:53:55 +0400 Subject: [PATCH] LBaaS: check for associations before deleting health monitor Need to prohibit health monitor deletion if it has associations with pools. Given that pools may belong to different lbaas drivers the process of monitor deletion becomes complex and unreliable since association deletion may fail on any single driver. DocImpact Closes-Bug: #1243129 Change-Id: I27c20e7a5be8433f90569534ecf838e33027cb00 --- neutron/db/loadbalancer/loadbalancer_db.py | 9 ++ neutron/extensions/loadbalancer.py | 5 + neutron/services/loadbalancer/plugin.py | 13 --- .../db/loadbalancer/test_db_loadbalancer.py | 101 ++++++++++-------- .../drivers/test_agent_driver_base.py | 16 +-- 5 files changed, 80 insertions(+), 64 deletions(-) diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py index 7c711b6b6..e18a1b2ed 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 7f23704d8..67c231490 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 29813579a..b87b7a85a 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 d3a4db16c..691b96156 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 692ed0029..de9e8fbd6 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) -- 2.45.2