Fix cascade delete flow (pool quota requirements missing)

Also add validation for regular delete to prevent deletion of a
loadbalancer with child objects (which it was doing, incorrectly),
because that is the whole point of cascade_delete.

Change-Id: I0a21c960ea20d56905dd1b5f02dc84326e9791a0
Closes-Bug: #1667161
Closes-Bug: #1671128
This commit is contained in:
Adam Harwell 2017-02-22 16:22:15 -08:00
parent dbd42d63be
commit 7351a4ef71
15 changed files with 96 additions and 25 deletions

View File

@ -231,6 +231,10 @@ class LoadBalancersController(base.BaseController):
db_lb = self._get_db_lb(context.session, id)
self._test_lb_status(context.session, id,
lb_status=constants.PENDING_DELETE)
if (db_lb.listeners or db_lb.pools) and not cascade:
msg = _("Cannot delete Load Balancer %s - it has children") % id
LOG.exception(msg)
raise exceptions.ValidationException(detail=msg)
try:
LOG.info(_LI("Sending deleted Load Balancer %s to the handler"),

View File

@ -80,13 +80,22 @@ class PoolFlows(object):
delete_pool_flow = linear_flow.Flow(constants.DELETE_POOL_FLOW)
# health monitor should cascade
# members should cascade
delete_pool_flow.add(database_tasks.MarkPoolPendingDeleteInDB(
requires=constants.POOL,
rebind={constants.POOL: name}))
delete_pool_flow.add(database_tasks.CountPoolChildrenForQuota(
requires=constants.POOL,
provides=constants.POOL_CHILD_COUNT,
rebind={constants.POOL: name}))
delete_pool_flow.add(model_tasks.DeleteModelObject(
rebind={constants.OBJECT: name}))
delete_pool_flow.add(database_tasks.DeletePoolInDB(
name='delete_pool_in_db_' + name,
requires=constants.POOL,
rebind={constants.POOL: name}))
delete_pool_flow.add(database_tasks.DecrementPoolQuota(
name='decrement_pool_quota_' + name,
requires=constants.POOL,
requires=[constants.POOL, constants.POOL_CHILD_COUNT],
rebind={constants.POOL: name}))
return delete_pool_flow

View File

@ -34,6 +34,7 @@ class BaseAPITest(base_db_test.OctaviaDBTestBase):
QUOTA_DEFAULT_PATH = QUOTAS_PATH + '/{project_id}/default'
LBS_PATH = '/loadbalancers'
LB_PATH = LBS_PATH + '/{lb_id}'
LB_DELETE_CASCADE_PATH = LB_PATH + '/delete_cascade'
LB_STATS_PATH = LB_PATH + '/stats'
LISTENERS_PATH = LB_PATH + '/listeners'
LISTENER_PATH = LISTENERS_PATH + '/{listener_id}'

View File

@ -289,7 +289,8 @@ class TestHealthMonitor(base.BaseAPITest):
self.delete(self.hm_path, status=409)
def test_create_when_lb_pending_delete(self):
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.post(self.hm_path,
body={'type': constants.HEALTH_MONITOR_HTTP,
'delay': 1, 'timeout': 1, 'fall_threshold': 1,
@ -300,12 +301,14 @@ class TestHealthMonitor(base.BaseAPITest):
self.create_health_monitor(self.lb.get('id'), self.pool.get('id'),
constants.HEALTH_MONITOR_HTTP, 1, 1, 1, 1)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.put(self.hm_path, body={'rise_threshold': 2}, status=409)
def test_delete_when_lb_pending_delete(self):
self.create_health_monitor(self.lb.get('id'), self.pool.get('id'),
constants.HEALTH_MONITOR_HTTP, 1, 1, 1, 1)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.delete(self.hm_path, status=409)

View File

@ -396,7 +396,8 @@ class TestL7Policy(base.BaseAPITest):
self.lb.get('id'), self.listener.get('id'),
constants.L7POLICY_ACTION_REJECT)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
new_l7policy = {'action': constants.L7POLICY_ACTION_REDIRECT_TO_URL,
'redirect_url': 'http://www.example.com'}
self.post(self.l7policies_path, body=new_l7policy, status=409)
@ -406,7 +407,8 @@ class TestL7Policy(base.BaseAPITest):
self.lb.get('id'), self.listener.get('id'),
constants.L7POLICY_ACTION_REJECT)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
new_l7policy = {'action': constants.L7POLICY_ACTION_REDIRECT_TO_URL,
'redirect_url': 'http://www.example.com'}
self.put(self.l7policy_path.format(l7policy_id=l7policy.get('id')),
@ -417,6 +419,7 @@ class TestL7Policy(base.BaseAPITest):
self.lb.get('id'), self.listener.get('id'),
constants.L7POLICY_ACTION_REJECT)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.delete(self.l7policy_path.format(l7policy_id=l7policy.get('id')),
status=409)

View File

@ -462,7 +462,8 @@ class TestL7Rule(base.BaseAPITest):
self.l7policy.get('id'), constants.L7RULE_TYPE_PATH,
constants.L7RULE_COMPARE_TYPE_STARTS_WITH, '/api')
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
new_l7rule = {'type': constants.L7RULE_TYPE_HEADER,
'compare_type':
constants.L7RULE_COMPARE_TYPE_STARTS_WITH,
@ -476,7 +477,8 @@ class TestL7Rule(base.BaseAPITest):
self.l7policy.get('id'), constants.L7RULE_TYPE_PATH,
constants.L7RULE_COMPARE_TYPE_STARTS_WITH, '/api')
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
new_l7rule = {'type': constants.L7RULE_TYPE_COOKIE,
'compare_type':
constants.L7RULE_COMPARE_TYPE_ENDS_WITH,
@ -491,6 +493,7 @@ class TestL7Rule(base.BaseAPITest):
self.l7policy.get('id'), constants.L7RULE_TYPE_PATH,
constants.L7RULE_COMPARE_TYPE_STARTS_WITH, '/api')
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.delete(self.l7rule_path.format(l7rule_id=l7rule.get('id')),
status=409)

View File

@ -394,7 +394,7 @@ class TestListener(base.BaseAPITest):
api_listener = self.post(
self.LISTENERS_PATH.format(lb_id=lb.get('id')), lb_listener).json
self.set_lb_status(lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(lb_id=lb.get('id')))
self.put(self.LISTENER_PATH.format(
lb_id=lb.get('id'), listener_id=api_listener.get('id')),
{}, status=409)
@ -411,7 +411,7 @@ class TestListener(base.BaseAPITest):
api_listener = self.post(
self.LISTENERS_PATH.format(lb_id=lb.get('id')), lb_listener).json
self.set_lb_status(lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(lb_id=lb.get('id')))
self.delete(self.LISTENER_PATH.format(
lb_id=lb.get('id'), listener_id=api_listener.get('id')),
status=409)

View File

@ -412,6 +412,32 @@ class TestLoadBalancer(base.BaseAPITest):
api_lb.get('operational_status'))
self.assert_final_lb_statuses(api_lb.get('id'), delete=True)
def test_delete_with_listener(self):
lb = self.create_load_balancer(
{'subnet_id': uuidutils.generate_uuid()},
name='lb1', description='desc1', enabled=False)
lb = self.set_lb_status(lb.get('id'))
self.create_listener(
lb_id=lb.get('id'),
protocol=constants.PROTOCOL_HTTP,
protocol_port=80
)
lb = self.set_lb_status(lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=lb.get('id')), status=400)
def test_delete_with_pool(self):
lb = self.create_load_balancer(
{'subnet_id': uuidutils.generate_uuid()},
name='lb1', description='desc1', enabled=False)
lb = self.set_lb_status(lb.get('id'))
self.create_pool_sans_listener(
lb_id=lb.get('id'),
protocol=constants.PROTOCOL_HTTP,
lb_algorithm=constants.LB_ALGORITHM_ROUND_ROBIN
)
lb = self.set_lb_status(lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=lb.get('id')), status=400)
def test_delete_bad_lb_id(self):
path = self.LB_PATH.format(lb_id='bad_uuid')
self.delete(path, status=404)

View File

@ -377,7 +377,8 @@ class TestMember(base.BaseAPITest):
self.pool.get('id'), ip_address="10.0.0.1",
protocol_port=80)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.post(self.members_path,
body={'ip_address': '10.0.0.2', 'protocol_port': 88,
'project_id': self.project_id},
@ -388,7 +389,8 @@ class TestMember(base.BaseAPITest):
self.pool.get('id'), ip_address="10.0.0.1",
protocol_port=80)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.put(self.member_path.format(member_id=member.get('id')),
body={'protocol_port': 88}, status=409)
@ -397,6 +399,7 @@ class TestMember(base.BaseAPITest):
self.pool.get('id'), ip_address="10.0.0.1",
protocol_port=80)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.delete(self.member_path.format(member_id=member.get('id')),
status=409)

View File

@ -574,7 +574,8 @@ class TestPool(base.BaseAPITest):
self.delete(self.pool_path.format(pool_id=pool.get('id')), status=409)
def test_create_when_lb_pending_delete(self):
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.post(self.pools_path,
body={'protocol': constants.PROTOCOL_HTTP,
'lb_algorithm': constants.LB_ALGORITHM_ROUND_ROBIN,
@ -586,7 +587,8 @@ class TestPool(base.BaseAPITest):
constants.PROTOCOL_HTTP,
constants.LB_ALGORITHM_ROUND_ROBIN)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.put(self.pool_path.format(pool_id=pool.get('id')),
body={'protocol': constants.PROTOCOL_HTTPS},
status=409)
@ -596,5 +598,6 @@ class TestPool(base.BaseAPITest):
constants.PROTOCOL_HTTP,
constants.LB_ALGORITHM_ROUND_ROBIN)
self.set_lb_status(self.lb.get('id'))
self.delete(self.LB_PATH.format(lb_id=self.lb.get('id')))
self.delete(self.LB_DELETE_CASCADE_PATH.format(
lb_id=self.lb.get('id')))
self.delete(self.pool_path.format(pool_id=pool.get('id')), status=409)

View File

@ -24,6 +24,8 @@ class LoadBalancersClient(rest_client.RestClient):
_LOAD_BALANCERS_URL = "v1/loadbalancers"
_LOAD_BALANCER_URL = "{base_url}/{{lb_id}}".format(
base_url=_LOAD_BALANCERS_URL)
_LOAD_BALANCER_CASCADE_DELETE_URL = "{lb_url}/delete_cascade".format(
lb_url=_LOAD_BALANCER_URL)
def list_load_balancers(self, params=None):
"""List all load balancers."""
@ -79,6 +81,13 @@ class LoadBalancersClient(rest_client.RestClient):
self.expected_success(202, resp.status)
return rest_client.ResponseBody(resp, body)
def delete_load_balancer_cascade(self, lb_id):
"""Delete an existing load balancer (cascading)."""
url = self._LOAD_BALANCER_CASCADE_DELETE_URL.format(lb_id=lb_id)
resp, body = self.delete(url)
self.expected_success(202, resp.status)
return rest_client.ResponseBody(resp, body)
def create_load_balancer_over_quota(self, **kwargs):
"""Attempt to build a load balancer over quota."""
url = self._LOAD_BALANCERS_URL

View File

@ -345,6 +345,11 @@ class BaseTestCase(manager.NetworkScenarioTest):
self.load_balancers_client.delete_load_balancer, load_balancer_id)
self._wait_for_load_balancer_status(load_balancer_id, delete=True)
def _delete_load_balancer_cascade(self, load_balancer_id):
self.load_balancers_client.delete_load_balancer_cascade(
load_balancer_id)
self._wait_for_load_balancer_status(load_balancer_id, delete=True)
def _cleanup_listener(self, listener_id, load_balancer_id=None):
test_utils.call_and_ignore_notfound_exc(
self.listeners_client.delete_listener, load_balancer_id,
@ -831,7 +836,7 @@ class BaseTestCase(manager.NetworkScenarioTest):
'pool': pool, 'health_monitor': health_monitor, 'member': member}}
return self.quotas_client.update_quotas(project_id, **body)
def _create_load_balancer_tree(self, ip_version=4):
def _create_load_balancer_tree(self, ip_version=4, cleanup=True):
# TODO(ptoohill): remove or null out project ID when Octavia supports
# keystone auth and automatically populates it for us.
project_id = self.networks_client.tenant_id
@ -855,7 +860,8 @@ class BaseTestCase(manager.NetworkScenarioTest):
.create_load_balancer_graph(create_lb))
load_balancer_id = self.load_balancer['id']
self.addCleanup(self._cleanup_load_balancer, load_balancer_id)
if cleanup:
self.addCleanup(self._cleanup_load_balancer, load_balancer_id)
LOG.info(('Waiting for lb status on create load balancer id: {0}'
.format(load_balancer_id)))
self.load_balancer = self._wait_for_load_balancer_status(

View File

@ -41,5 +41,6 @@ class TestLoadBalancerTreeMinimal(base.BaseTestCase):
self._start_backend_httpd_processes('server1')
self._create_server('server2')
self._start_backend_httpd_processes('server2')
self._create_load_balancer_tree()
self._create_load_balancer_tree(cleanup=False)
self._check_members_balanced(['server1_0', 'server2_0'])
self._delete_load_balancer_cascade(self.load_balancer.get('id'))

View File

@ -119,8 +119,8 @@ class TestLoadBalancerFlows(base.TestCase):
self.assertIn(constants.LOADBALANCER, lb_flow.requires)
self.assertEqual(0, len(lb_flow.provides))
self.assertEqual(5, len(lb_flow.requires))
self.assertEqual(1, len(lb_flow.provides))
self.assertEqual(4, len(lb_flow.requires))
def test_get_new_LB_networking_subflow(self, mock_get_net_driver):

View File

@ -59,8 +59,8 @@ class TestPoolFlows(base.TestCase):
self.assertIsInstance(pool_flow, flow.Flow)
self.assertIn('test', pool_flow.requires)
self.assertEqual(2, len(pool_flow.requires))
self.assertEqual(0, len(pool_flow.provides))
self.assertEqual(1, len(pool_flow.requires))
self.assertEqual(1, len(pool_flow.provides))
def test_get_update_pool_flow(self):