x/group-based-policy
Code Issues Proposed changes

Fix service chain instance deletion on PRS update

Browse Source 
Previously, If there are multiple PRSs (1 redirect and others are allow with
no child PRSs) are associated with provider and consumer PTGs and When user
tries to update allow PRS (say add PR), GBP in its update operation is trying
to update allow PRS and triggers the 'DELETE' service chain instance

In this case, even gbp got update for allow PRS it's not checking whether
redirect action rule is added/removed from original PRS to current PRS. It is
directly considering redirect action rule is removed and triggering 'DELETE'
for service chain instance (it's getting sevice chain instance from provider
PTG) and further it causes deletion of service

Added code which will check redirect action rule is added/removed from original
PRS to current PRS, if yes then trigger create/update/delete action for service
chain instance otherwise, skip it and return

Change-Id: Ie401d33a595c8ee129ff9a4ff176b313732b973d
Closes-Bug: #1585599
(cherry picked from commit 2cfe869dfd)
This commit is contained in:
YogeshRajmane
2016-05-29 22:45:52 +05:30
committed by Yogesh Rajmane
parent 077bf3a072
commit be2bfdf5c4
2 changed files with 152 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
gbpservice/neutron/services/grouppolicy/drivers/chain_mapping.py
View File

@@ -285,8 +285,10 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
@log.log
def update_policy_rule_set_postcommit(self, context):
# Handle any Redirects from the current Policy Rule Set
self._handle_redirect_action(context, [context.current['id']])
if self._is_redirect_rule_updated(context):
# Handle any Redirects from the current Policy Rule Set
self._handle_redirect_action(context, [context.current['id']])
# Handle Update/Delete of Redirects for any child Rule Sets
if (set(context.original['child_policy_rule_sets']) !=
set(context.current['child_policy_rule_sets'])):
@@ -315,6 +317,51 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
def delete_external_policy_postcommit(self, context):
self._handle_prs_removed(context)
def _is_redirect_rule_updated(self, context):
if (not context.original['child_policy_rule_sets']) and (
not context.current['child_policy_rule_sets']):
'''
This method checks if a rule with a REDIRECT action is added to or
removed from the PRS. If no REDIRECT action has been added or
removed, then the processing for REDIRECT does not have to be
performed and existing service chain instances should be
left untouched.
'''
old_redirect_count = self._multiple_pr_redirect_action_number(
context._plugin_context.session,
context.original['policy_rules'])
new_redirect_count = self._multiple_pr_redirect_action_number(
context._plugin_context.session,
context.current['policy_rules'])
if (new_redirect_count == 0) and (old_redirect_count == 0):
return False
elif (new_redirect_count != old_redirect_count):
return True
else:
original_policy_rules = context.original['policy_rules']
current_policy_rules = context.current['policy_rules']
policy_rules = context._plugin.get_policy_rules(
context._plugin_context,
filters={'id': original_policy_rules})
for policy_rule in policy_rules:
original_redirect_policy_action = (
self._get_redirect_action(
context, policy_rule))
if (original_redirect_policy_action) and (
policy_rule['id'] not in current_policy_rules):
return True
policy_rules = context._plugin.get_policy_rules(
context._plugin_context,
filters={'id': current_policy_rules})
for policy_rule in policy_rules:
current_redirect_policy_action = self._get_redirect_action(
context, policy_rule)
if (current_redirect_policy_action) and (
policy_rule['id'] not in original_policy_rules):
return True
return False
return True
def _handle_prs_added(self, context):
# Expecting either a PTG or EP context
if context.current['consumed_policy_rule_sets']:

103
gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py
View File

@@ -3390,6 +3390,109 @@ class TestServiceChain(ResourceMappingTestCase):
self._verify_ptg_delete_cleanup_chain(provider_ptg2_id)
def test_servicechain_deletion_on_prs_update(self):
allow_policy_rule1 = self._create_ssh_allow_rule()
allow_policy_rule_set = self.create_policy_rule_set(
expected_res_status=201,
policy_rules=[allow_policy_rule1['id']],
child_policy_rule_sets=[])['policy_rule_set']
self._verify_prs_rules(allow_policy_rule_set['id'])
scs_id = self._create_servicechain_spec()
_, _, redirect_policy_rule_id = self._create_tcp_redirect_rule(
"20:90", scs_id)
redirect_policy_rule_set = self.create_policy_rule_set(
name='redirect_prs', policy_rules=[redirect_policy_rule_id])
redirect_policy_rule_set_id = redirect_policy_rule_set[
'policy_rule_set']['id']
self._verify_prs_rules(redirect_policy_rule_set_id)
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 0)
provider_ptg = self.create_policy_target_group(
provided_policy_rule_sets={redirect_policy_rule_set_id: None,
allow_policy_rule_set['id']: None})[
'policy_target_group']
consumer_ptg = self.create_policy_target_group(
consumed_policy_rule_sets={redirect_policy_rule_set_id: None,
allow_policy_rule_set['id']: None})[
'policy_target_group']
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 1)
sc_instance = sc_instances['servicechain_instances'][0]
self._assert_proper_chain_instance(sc_instance, provider_ptg['id'],
consumer_ptg['id'], [scs_id])
allow_policy_rule2 = self._create_http_allow_rule()
# Add allow rule in allow policy rule set
self.update_policy_rule_set(allow_policy_rule_set['id'],
expected_res_status=200,
policy_rules=[allow_policy_rule1['id'], allow_policy_rule2['id']])
self._verify_prs_rules(allow_policy_rule_set['id'])
# service chain instance will be there
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 1)
# Remove allow rule from allow policy rule set
self.update_policy_rule_set(allow_policy_rule_set['id'],
expected_res_status=200,
policy_rules=[allow_policy_rule2['id']])
self._verify_prs_rules(allow_policy_rule_set['id'])
# service chain instance will be there
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 1)
# Add allow rule in redirect policy rule set
self.update_policy_rule_set(redirect_policy_rule_set_id,
expected_res_status=200,
policy_rules=[allow_policy_rule2['id'], redirect_policy_rule_id])
self._verify_prs_rules(redirect_policy_rule_set_id)
# service chain instance will be there
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 1)
# Remove redirect rule from redirect policy rule set
self.update_policy_rule_set(redirect_policy_rule_set_id,
expected_res_status=200,
policy_rules=[allow_policy_rule2['id']])
self._verify_prs_rules(redirect_policy_rule_set_id)
# service chain instance will not be there
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 0)
# Add redirect rule in redirect policy rule set
self.update_policy_rule_set(redirect_policy_rule_set_id,
expected_res_status=200,
policy_rules=[allow_policy_rule2['id'], redirect_policy_rule_id])
self._verify_prs_rules(redirect_policy_rule_set_id)
# service chain instance will be there
sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES)
res = sc_node_list_req.get_response(self.ext_api)
sc_instances = self.deserialize(self.fmt, res)
self.assertEqual(len(sc_instances['servicechain_instances']), 1)
class TestServiceChainAdminOwner(TestServiceChain):