x/vmware-nsx
Code Issues Proposed changes

Adopt incremental add/remove member API for NSGroup

Browse Source 
Closes-Bug: #1530627
Change-Id: I74682c7b3732144138c87ca9037772b005df17d7
This commit is contained in:
Roey Chen 2015-12-21 11:59:01 -08:00
parent 9f366641be
commit dd7dddd837
4 changed files with 44 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
vmware_nsx/nsxlib/v3/dfw_api.py
View File

@ -37,11 +37,11 @@ ALLOW = 'ALLOW'
DROP = 'DROP'
REJECT = 'REJECT'
# filtering operators
# filtering operators and expressions
EQUALS = 'EQUALS'
NSGROUP_SIMPLE_EXPRESSION = 'NSGroupSimpleExpression'
NSGROUP = 'NSGroup'
NSGROUP_SIMPLE_EXPRESSION = 'NSGroupSimpleExpression'
LOGICAL_SWITCH = 'LogicalSwitch'
LOGICAL_PORT = 'LogicalPort'
IPV4ADDRESS = 'IPv4Address'
@ -89,7 +89,8 @@ def create_nsgroup(display_name, description, tags):
def list_nsgroups():
return nsxclient.get_resource('ns-groups').get('results', [])
return nsxclient.get_resource(
'ns-groups?populate_references=false').get('results', [])
@utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision)
@ -100,18 +101,20 @@ def update_nsgroup(nsgroup_id, display_name, description):
return nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup)
@utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision)
def get_nsgroup_member_expression(target_type, target_id):
return {'resource_type': NSGROUP_SIMPLE_EXPRESSION,
'target_property': 'id',
'target_type': target_type,
'op': EQUALS,
'value': target_id}
def add_nsgroup_member(nsgroup_id, target_type, target_id):
nsgroup = read_nsgroup(nsgroup_id)
if 'members' not in nsgroup:
nsgroup['members'] = []
nsgroup['members'].append({"resource_type": NSGROUP_SIMPLE_EXPRESSION,
'target_property': 'id',
'target_type': target_type,
'op': EQUALS,
'value': target_id})
member_expr = get_nsgroup_member_expression(target_type, target_id)
data = {'members': [member_expr]}
add_members = 'ns-groups/%s?action=ADD_MEMBERS' % nsgroup_id
try:
nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup)
return nsxclient.create_resource(add_members, data)
except nsx_exc.ManagerError:
# REVISIT(roeyc): A ManagerError might have been raised for a
# different reason, e.g - NSGroup does not exists.
@ -123,34 +126,27 @@ def add_nsgroup_member(nsgroup_id, target_type, target_id):
raise NSGroupIsFull()
@utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision)
def remove_nsgroup_member(nsgroup_id, target_id, verify=False):
nsgroup = read_nsgroup(nsgroup_id)
for i, member in enumerate(nsgroup.get('members', [])):
if target_id == member['value']:
break
else:
def remove_nsgroup_member(nsgroup_id, target_type, target_id, verify=False):
member_expr = get_nsgroup_member_expression(target_type, target_id)
data = {'members': [member_expr]}
remove_members = 'ns-groups/%s?action=REMOVE_MEMBERS' % nsgroup_id
try:
return nsxclient.create_resource(remove_members, data)
except nsx_exc.ManagerError:
if verify:
err_msg = _("Failed to remove member %(tid)s "
"from NSGroup %(nid)s.") % {'tid': target_id,
'nid': nsgroup_id}
raise NSGroupMemberNotFound(err_msg=err_msg)
return
del nsgroup['members'][i]
return nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup)
def read_nsgroup(nsgroup_id):
nsgroup = nsxclient.get_resource('ns-groups/%s' % nsgroup_id)
# NSX API will not specify the 'members' attribute if its empty, but
# requires it on update.
if 'members' not in nsgroup:
nsgroup['members'] = []
return nsgroup
return nsxclient.get_resource(
'ns-groups/%s?populate_references=true' % nsgroup_id)
def delete_nsgroup(nsgroup_id):
return nsxclient.delete_resource('ns-groups/%s' % nsgroup_id)
return nsxclient.delete_resource('ns-groups/%s?force=true' % nsgroup_id)
def _build_section(display_name, description, applied_tos, tags):

4
vmware_nsx/nsxlib/v3/security.py
View File

@ -201,7 +201,7 @@ def update_lport_with_security_groups(context, lport_id, original, updated):
for sg_id in removed:
nsgroup_id, s = get_sg_mappings(context.session, sg_id)
firewall.remove_nsgroup_member(
nsgroup_id, lport_id)
nsgroup_id, firewall.LOGICAL_PORT, lport_id)
def init_nsgroup_manager_and_default_section_rules():
@ -357,7 +357,7 @@ class NSGroupManager(object):
for group in self._suggest_nested_group(nsgroup_id):
try:
firewall.remove_nsgroup_member(
group, nsgroup_id, verify=True)
group, firewall.NSGROUP, nsgroup_id, verify=True)
break
except firewall.NSGroupMemberNotFound:
LOG.warning(_LW("NSGroup %(nsgroup)s was expected to be found "

4
vmware_nsx/plugins/nsx_v3/plugin.py
View File

@ -737,6 +737,8 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
_net_id, nsx_port_id = nsx_db.get_nsx_switch_and_port_id(
context.session, port_id)
self._port_client.delete(nsx_port_id)
security.update_lport_with_security_groups(
context, nsx_port_id, port.get(ext_sg.SECURITYGROUPS, []), [])
self.disassociate_floatingips(context, port_id)
nsx_rpc.handle_port_metadata_access(self, context, port,
is_delete=True)
@ -1520,8 +1522,8 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
nsgroup_id, section_id = security.get_sg_mappings(context.session, id)
super(NsxV3Plugin, self).delete_security_group(context, id)
firewall.delete_section(section_id)
self.nsgroup_manager.remove_nsgroup(nsgroup_id)
firewall.delete_nsgroup(nsgroup_id)
self.nsgroup_manager.remove_nsgroup(nsgroup_id)
def create_security_group_rule(self, context, security_group_rule):
bulk_rule = {'security_group_rules': [security_group_rule]}

20
vmware_nsx/tests/unit/extensions/test_securitygroup.py
View File

@ -85,7 +85,8 @@ class TestSecurityGroups(test_nsxv3.NsxV3PluginTestCaseMixin,
mock.call(NSG_IDS[2], mock.ANY, mock.ANY)]
add_member_mock.assert_has_calls(calls, any_order=True)
remove_member_mock.assert_called_with(NSG_IDS[0], mock.ANY)
remove_member_mock.assert_called_with(
NSG_IDS[0], firewall.LOGICAL_PORT, mock.ANY)
@_mock_create_and_list_nsgroups
@mock.patch.object(firewall, 'remove_nsgroup_member')
@ -96,8 +97,10 @@ class TestSecurityGroups(test_nsxv3.NsxV3PluginTestCaseMixin,
super(TestSecurityGroups,
self).test_update_port_remove_security_group_empty_list()
add_member_mock.assert_called_with(NSG_IDS[1], mock.ANY, mock.ANY)
remove_member_mock.assert_called_with(NSG_IDS[1], mock.ANY)
add_member_mock.assert_called_with(
NSG_IDS[1], firewall.LOGICAL_PORT, mock.ANY)
remove_member_mock.assert_called_with(
NSG_IDS[1], firewall.LOGICAL_PORT, mock.ANY)
class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase):
@ -153,7 +156,7 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase):
add_member_mock.assert_called_once_with(
NSG_IDS[2], firewall.NSGROUP, nsgroup_id)
remove_member_mock.assert_called_once_with(
NSG_IDS[2], nsgroup_id, verify=True)
NSG_IDS[2], firewall.NSGROUP, nsgroup_id, verify=True)
@_mock_create_and_list_nsgroups
@mock.patch.object(firewall, 'remove_nsgroup_member')
@ -166,7 +169,7 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase):
if nsgroup == NSG_IDS[2]:
raise firewall.NSGroupIsFull()
def _remove_member_mock(nsgroup, target_id, verify=False):
def _remove_member_mock(nsgroup, target_type, target_id, verify=False):
if nsgroup == NSG_IDS[2]:
raise firewall.NSGroupMemberNotFound()
@ -191,8 +194,11 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase):
# Since the nsgroup was added to the nested group at index 3, it will
# fail to remove it from the group at index 2, and then will try to
# remove it from the group at index 3.
calls = [mock.call(NSG_IDS[2], nsgroup_id, verify=True),
mock.call(NSG_IDS[3], nsgroup_id, verify=True)]
calls = [
mock.call(
NSG_IDS[2], firewall.NSGROUP, nsgroup_id, verify=True),
mock.call(
NSG_IDS[3], firewall.NSGROUP, nsgroup_id, verify=True)]
remove_member_mock.assert_has_calls(calls)
@_mock_create_and_list_nsgroups