Merge "LBaaS: check for associations before deleting health monitor"

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)

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")
 

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,

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):
+    def test_delete_healthmonitor_with_associations_raises(self):
-        with self.health_monitor(type='HTTP', no_delete=True) as monitor:
+        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)
+                self.assertEqual(res.status_int, webob.exc.HTTPConflict.code)
-                # check if all corresponding Pool associations are deleted
+
+                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()
+            self.health_monitor(),
-        ) as (pool, health_mon1, health_mon2):
+            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(),
-                self.health_monitor()
+                self.pool()
-            ) as (pool, hm):
+            ) 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.health_monitor(),
-                self.pool(),
+            self.pool(),
-                self.health_monitor()
+            self.pool()
-        ) as (p1, p2, hm):
+        ) 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 contextlib.nested(
-            with self.health_monitor() as hm:
+            self.health_monitor(),
-                res = self.plugin.create_pool_health_monitor(
+            self.pool(name="pool")
-                    context.get_admin_context(),
+        ) as (hm, pool):
-                    hm, pool['pool']['id'])
+            res = self.plugin.create_pool_health_monitor(
-                self.assertEqual({'health_monitor':
+                context.get_admin_context(),
-                                  [hm['health_monitor']['id']]},
+                hm, pool['pool']['id'])
-                                 res)
+            self.assertEqual({'health_monitor':
+                              [hm['health_monitor']['id']]},
+                             res)
 
-                assoc = self.plugin.get_pool_health_monitor(
+            assoc = self.plugin.get_pool_health_monitor(
-                    context.get_admin_context(),
+                context.get_admin_context(),
-                    hm['health_monitor']['id'],
+                hm['health_monitor']['id'],
-                    pool['pool']['id'])
+                pool['pool']['id'])
-                self.assertEqual(assoc['status'], 'PENDING_CREATE')
+            self.assertEqual(assoc['status'], 'PENDING_CREATE')
-                self.assertIsNone(assoc['status_description'])
+            self.assertIsNone(assoc['status_description'])
 
-                self.plugin.update_pool_health_monitor(
+            self.plugin.update_pool_health_monitor(
-                    context.get_admin_context(),
+                context.get_admin_context(),
-                    hm['health_monitor']['id'],
+                hm['health_monitor']['id'],
-                    pool['pool']['id'],
+                pool['pool']['id'],
-                    'ACTIVE', 'ok')
+                'ACTIVE', 'ok')
-                assoc = self.plugin.get_pool_health_monitor(
+            assoc = self.plugin.get_pool_health_monitor(
-                    context.get_admin_context(),
+                context.get_admin_context(),
-                    hm['health_monitor']['id'],
+                hm['health_monitor']['id'],
-                    pool['pool']['id'])
+                pool['pool']['id'])
-                self.assertEqual(assoc['status'], 'ACTIVE')
+            self.assertEqual(assoc['status'], 'ACTIVE')
-                self.assertEqual(assoc['status_description'], 'ok')
+            self.assertEqual(assoc['status_description'], 'ok')
 
     def test_check_orphan_pool_associations(self):
         with contextlib.nested(

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.health_monitor() as monitor:
-            with self.vip(pool=pool) as vip:
+            with self.pool() as pool:
-                with self.health_monitor() as monitor:
+                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(),
-            self.health_monitor()
+            self.pool()
-        ) 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)
@@ -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 (hm, pool):
-        ) as (pool, hm):
             pool_id = pool['pool']['id']
             ctx = context.get_admin_context()
             self.plugin_instance.create_pool_health_monitor(ctx, hm, pool_id)