From 9ed0f90cf7702e87f61cf17008ea34f7c00a308b Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Tue, 22 Oct 2013 17:49:00 +0400 Subject: [PATCH] LBaaS: Fix incorrect pool status change Avoid incorrect status change when deleting the pool. We can check for the conditions prior putting the pool to PENDING_DELETE state, in case delete conditions are met it is safe to change the state to PENDING_DELETE. Also, change create_vip and update_vip operations to respect PENDING_DELETE and avoid race conditions. Change-Id: I9f526901eb85bdb83cf4ff8131460eb592c900f8 Closes-Bug: #1242338 --- neutron/db/loadbalancer/loadbalancer_db.py | 21 ++++++++++----- neutron/services/loadbalancer/plugin.py | 26 +++++++++++++++---- .../db/loadbalancer/test_db_loadbalancer.py | 19 ++++++++++++++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py index 02d2a7b15b..b99a9da13c 100644 --- a/neutron/db/loadbalancer/loadbalancer_db.py +++ b/neutron/db/loadbalancer/loadbalancer_db.py @@ -341,6 +341,9 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, raise loadbalancer.ProtocolMismatch( vip_proto=v['protocol'], pool_proto=pool['protocol']) + if pool['status'] == constants.PENDING_DELETE: + raise loadbalancer.StateInvalid(state=pool['status'], + id=pool['id']) else: pool = None @@ -418,6 +421,10 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, raise loadbalancer.ProtocolMismatch( vip_proto=vip_db['protocol'], pool_proto=new_pool['protocol']) + if new_pool['status'] == constants.PENDING_DELETE: + raise loadbalancer.StateInvalid( + state=new_pool['status'], + id=new_pool['id']) if old_pool_id: old_pool = self._get_resource( @@ -553,15 +560,17 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, return self._make_pool_dict(pool_db) - def delete_pool(self, context, id): + def _ensure_pool_delete_conditions(self, context, pool_id): + if context.session.query(Vip).filter_by(pool_id=pool_id).first(): + raise loadbalancer.PoolInUse(pool_id=pool_id) + + def delete_pool(self, context, pool_id): # Check if the pool is in use - vip = context.session.query(Vip).filter_by(pool_id=id).first() - if vip: - raise loadbalancer.PoolInUse(pool_id=id) + self._ensure_pool_delete_conditions(context, pool_id) with context.session.begin(subtransactions=True): - self._delete_pool_stats(context, id) - pool_db = self._get_resource(context, Pool, id) + self._delete_pool_stats(context, pool_id) + pool_db = self._get_resource(context, Pool, pool_id) context.session.delete(pool_db) def get_pool(self, context, id, fields=None): diff --git a/neutron/services/loadbalancer/plugin.py b/neutron/services/loadbalancer/plugin.py index ea7eff3aac..e773af1a98 100644 --- a/neutron/services/loadbalancer/plugin.py +++ b/neutron/services/loadbalancer/plugin.py @@ -21,6 +21,7 @@ from neutron import context from neutron.db import api as qdbapi from neutron.db.loadbalancer import loadbalancer_db as ldb from neutron.db import servicetype_db as st_db +from neutron.openstack.common import excutils from neutron.openstack.common import log as logging from neutron.plugins.common import constants from neutron.services.loadbalancer import agent_scheduler @@ -168,13 +169,28 @@ class LoadBalancerPlugin(ldb.LoadBalancerPluginDb, def _delete_db_pool(self, context, id): # proxy the call until plugin inherits from DBPlugin # rely on uuid uniqueness: - with context.session.begin(subtransactions=True): - self.service_type_manager.del_resource_associations(context, [id]) - super(LoadBalancerPlugin, self).delete_pool(context, id) + try: + with context.session.begin(subtransactions=True): + self.service_type_manager.del_resource_associations( + context, [id]) + super(LoadBalancerPlugin, self).delete_pool(context, id) + except Exception: + # that should not happen + # if it's still a case - something goes wrong + # log the error and mark the pool as ERROR + LOG.error(_('Failed to delete pool %s, putting it in ERROR state'), + id) + with excutils.save_and_reraise_exception(): + self.update_status(context, ldb.Pool, + id, constants.ERROR) def delete_pool(self, context, id): - self.update_status(context, ldb.Pool, - id, constants.PENDING_DELETE) + # check for delete conditions and update the status + # within a transaction to avoid a race + with context.session.begin(subtransactions=True): + self.update_status(context, ldb.Pool, + id, constants.PENDING_DELETE) + self._ensure_pool_delete_conditions(context, id) p = self.get_pool(context, id) driver = self._get_driver_for_provider(p['provider']) driver.delete_pool(context, p) diff --git a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py index c54c72939b..6ba2b3ff56 100644 --- a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py +++ b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py @@ -679,6 +679,25 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): res = req.get_response(self.ext_api) self.assertEqual(res.status_int, 204) + def test_delete_pool_preserve_state(self): + with self.pool(no_delete=True) as pool: + with self.vip(pool=pool): + req = self.new_delete_request('pools', + pool['pool']['id']) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, 409) + req = self.new_show_request('pools', + pool['pool']['id'], + fmt=self.fmt) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, 200) + res = self.deserialize(self.fmt, + req.get_response(self.ext_api)) + self.assertEqual(res['pool']['status'], + constants.PENDING_CREATE) + req = self.new_delete_request('pools', + pool['pool']['id']) + def test_show_pool(self): name = "pool1" keys = [('name', name),