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

diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py
index 7c711b6b61332af1c035aa3ecd4e01dfbf06de5b..e18a1b2ed041eb2edb1434cae5c5f9ee1f005220 100644 (file)
--- 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 7f23704d8f619b15cadad7e4b615ef90e9852d34..67c231490b22ccdc083111af739d98794fef04e4 100644 (file)
--- 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 29813579ae6eeab54b9ec5179ca75326310af63a..b87b7a85aab782f3cf8c52d27fdb7fb020dfe7ec 100644 (file)
--- 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 d3a4db16c080b648e8f7f6f7e0d543fd31e966bb..691b9615697afd1e1b1c8f39747ab11e5bfedd24 100644 (file)
--- 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 692ed0029980d73992a9f9e7b0cbe209dd965ac4..de9e8fbd654cc79f56d2d6ddc75194c0801e928b 100644 (file)
--- 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)