x/vmware-nsx
Code Issues Proposed changes

NSX|v: Handle address scope change on subnetpool

Browse Source 
Address scope is mutable on subnetpool after subnets were allocated.
When address scope changes, neutron fires AFTER_CHANGE event. Upon
catching this notification:

1. Log warning if no-nat router resulted with mixed address scopes
2. Recalculate snat rules if needed
3. Recalculate firewall rules

A fix for nsx-v3 was already introduced in commit
I1de25868db0d77fbcb7ebc588c6ab9493a3dadf4

Change-Id: I2696f0ba0edf0409993455b7a9e6e478383f3276
This commit is contained in:
Adit Sarfaty 2017-08-15 11:22:36 +03:00
parent 803a6bffe4
commit 345e5ea10b
5 changed files with 197 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
vmware_nsx/plugins/common/plugin.py
View File

@ -202,6 +202,13 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2,
""" """
pass pass
def recalculate_fw_rules_for_router(self, context, router, subnets):
"""Method to recalculate router FW rules for specific subnets.
Invoked when subnetpool address scope changes.
Implemented in child plugin classes
"""
pass
def _filter_subnets_by_subnetpool(self, subnets, subnetpool_id): def _filter_subnets_by_subnetpool(self, subnets, subnetpool_id):
return [subnet for subnet in subnets return [subnet for subnet in subnets
if subnet['subnetpool_id'] == subnetpool_id] if subnet['subnetpool_id'] == subnetpool_id]
@ -236,7 +243,10 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2,
# (all router subnets were allocated from subnetpool_id) # (all router subnets were allocated from subnetpool_id)
continue continue
# TODO(annak): handle east-west FW rules # Update east-west FW rules
self.recalculate_fw_rules_for_router(context, rtr,
affected_subnets)
if not rtr['external_gateway_info']: if not rtr['external_gateway_info']:
continue continue

21
vmware_nsx/plugins/nsx_v/plugin.py
View File

@ -294,6 +294,11 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
fc_utils.SERVICE_INSERTION_RESOURCE, fc_utils.SERVICE_INSERTION_RESOURCE,
events.AFTER_CREATE) events.AFTER_CREATE)
# Subscribe to subnet pools changes
registry.subscribe(
self.on_subnetpool_address_scope_updated,
resources.SUBNETPOOL_ADDRESS_SCOPE, events.AFTER_UPDATE)
if c_utils.is_nsxv_version_6_2(self.nsx_v.vcns.get_version()): if c_utils.is_nsxv_version_6_2(self.nsx_v.vcns.get_version()):
self.supported_extension_aliases.append("provider-security-group") self.supported_extension_aliases.append("provider-security-group")
@ -3455,6 +3460,22 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
edge_utils.update_nat_rules( edge_utils.update_nat_rules(
self.nsx_v, context, router_id, snat, dnat) self.nsx_v, context, router_id, snat, dnat)
def recalculate_snat_rules_for_router(self, context, router, subnets):
"""Recalculate router snat rules for specific subnets.
Invoked when subnetpool address scope changes.
"""
# Recalculate all nat rules for all subnets of the router
router_db = self._get_router(context, router['id'])
self._update_nat_rules(context, router_db)
def recalculate_fw_rules_for_router(self, context, router, subnets):
"""Recalculate router fw rules for specific subnets.
Invoked when subnetpool address scope changes.
"""
# Recalculate all fw rules for all subnets of the router
router_db = self._get_router(context, router['id'])
self._update_subnets_and_dnat_firewall(context, router_db)
def _check_intf_number_of_router(self, context, router_id): def _check_intf_number_of_router(self, context, router_id):
intf_ports = self._get_port_by_device_id( intf_ports = self._get_port_by_device_id(
context, router_id, l3_db.DEVICE_OWNER_ROUTER_INTF) context, router_id, l3_db.DEVICE_OWNER_ROUTER_INTF)

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

@ -3731,7 +3731,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
net_res[az_ext.AVAILABILITY_ZONES] = [az_name] net_res[az_ext.AVAILABILITY_ZONES] = [az_name]
def recalculate_snat_rules_for_router(self, context, router, subnets): def recalculate_snat_rules_for_router(self, context, router, subnets):
"""Rrecalculate router snat rules for specific subnets. """Recalculate router snat rules for specific subnets.
Invoked when subnetpool address scope changes. Invoked when subnetpool address scope changes.
""" """
nsx_router_id = nsx_db.get_nsx_router_id(context.session, nsx_router_id = nsx_db.get_nsx_router_id(context.session,

278
vmware_nsx/tests/unit/nsx_v/test_plugin.py
View File

@ -3490,6 +3490,26 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
None, None,
err_code) err_code)
def _create_subnet_and_add_to_router(self, subnetpool_id, router_id):
# create a regular network on the given subnet pool
with self.network() as net:
data = {'subnet': {
'network_id': net['network']['id'],
'subnetpool_id': subnetpool_id,
'ip_version': 4,
'tenant_id': net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
int_subnet = self.deserialize(
self.fmt, req.get_response(self.api))
# Add the interface to the router
self._router_interface_action(
'add',
router_id,
int_subnet['subnet']['id'],
None)
return int_subnet
def test_router_no_snat_with_same_address_scope(self): def test_router_no_snat_with_same_address_scope(self):
"""Test that if the router has no snat, you can add an interface """Test that if the router has no snat, you can add an interface
from the same address scope as the gateway. from the same address scope as the gateway.
@ -3514,31 +3534,19 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
ext_subnet = self.deserialize(self.fmt, req.get_response(self.api)) ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
# create a regular network on the same address scope # create a regular network on the same address scope
with self.network() as net: # and create a no snat router with this gateway
data = {'subnet': { with self.router() as r:
'network_id': net['network']['id'], self._add_external_gateway_to_router(
'subnetpool_id': subnetpool_id, r['router']['id'],
'ip_version': 4, ext_subnet['subnet']['network_id'])
'tenant_id': net['network']['tenant_id']}} self._update_router_enable_snat(
req = self.new_create_request('subnets', data) r['router']['id'],
int_subnet = self.deserialize( ext_subnet['subnet']['network_id'],
self.fmt, req.get_response(self.api)) False)
# create a no snat router with this gateway # should succeed adding the interface to the router
with self.router() as r: self._create_subnet_and_add_to_router(
self._add_external_gateway_to_router( subnetpool_id, r['router']['id'])
r['router']['id'],
ext_subnet['subnet']['network_id'])
self._update_router_enable_snat(
r['router']['id'],
ext_subnet['subnet']['network_id'],
False)
# should succeed adding the interface to the router
self._router_interface_action('add',
r['router']['id'],
int_subnet['subnet']['id'],
None)
def test_router_address_scope_snat_rules(self): def test_router_address_scope_snat_rules(self):
"""Test that if the router interface had the same address scope """Test that if the router interface had the same address scope
@ -3564,48 +3572,37 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
ext_subnet = self.deserialize(self.fmt, req.get_response(self.api)) ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
# create a regular network on the same address scope # create a regular network on the same address scope
with self.network() as net: # and create a router with this gateway
data = {'subnet': { with self.router() as r:
'network_id': net['network']['id'], self._add_external_gateway_to_router(
'subnetpool_id': subnetpool_id, r['router']['id'],
'ip_version': 4, ext_subnet['subnet']['network_id'])
'tenant_id': net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
int_subnet = self.deserialize(
self.fmt, req.get_response(self.api))
# create a router with this gateway # Add the interface to the router
with self.router() as r: with mock.patch.object(
self._add_external_gateway_to_router( edge_utils, 'update_nat_rules') as update_nat,\
r['router']['id'], mock.patch.object(
ext_subnet['subnet']['network_id']) edge_utils, 'update_firewall') as update_fw:
# Add the interface to the router int_subnet = self._create_subnet_and_add_to_router(
with mock.patch.object( subnetpool_id, r['router']['id'])
edge_utils, 'update_nat_rules') as update_nat,\
mock.patch.object(
edge_utils, 'update_firewall') as update_fw:
self._router_interface_action(
'add',
r['router']['id'],
int_subnet['subnet']['id'],
None)
# make sure snat rules are not added
update_nat.assert_called_once_with(
mock.ANY, mock.ANY, r['router']['id'], [], [])
# check fw rules # make sure snat rules are not added
fw_rules = update_fw.call_args[0][3][ update_nat.assert_called_once_with(
'firewall_rule_list'] mock.ANY, mock.ANY, r['router']['id'], [], [])
self.assertEqual(2, len(fw_rules))
self.assertEqual('Allocation Pool Rule', # check fw rules
fw_rules[1]['name']) fw_rules = update_fw.call_args[0][3][
self.assertEqual('allow', fw_rules[1]['action']) 'firewall_rule_list']
self.assertEqual( self.assertEqual(2, len(fw_rules))
int_subnet['subnet']['cidr'], self.assertEqual('Allocation Pool Rule',
fw_rules[1]['destination_ip_address'][0]) fw_rules[1]['name'])
self.assertEqual('external', self.assertEqual('allow', fw_rules[1]['action'])
fw_rules[1]['source_vnic_groups'][0]) self.assertEqual(
int_subnet['subnet']['cidr'],
fw_rules[1]['destination_ip_address'][0])
self.assertEqual('external',
fw_rules[1]['source_vnic_groups'][0])
def test_router_address_scope_fw_rules(self): def test_router_address_scope_fw_rules(self):
"""Test that if the router interfaces has different address scope """Test that if the router interfaces has different address scope
@ -3613,9 +3610,8 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
""" """
# create a router, networks, and address scopes # create a router, networks, and address scopes
with self.address_scope(name='as1') as addr_scope1, \ with self.address_scope(name='as1') as addr_scope1, \
self.address_scope(name='as2') as addr_scope2,\ self.address_scope(name='as2') as addr_scope2, \
self.network() as net1, self.network() as net2,\ self.router() as r:
self.network() as net3, self.router() as r:
as1_id = addr_scope1['address_scope']['id'] as1_id = addr_scope1['address_scope']['id']
as2_id = addr_scope2['address_scope']['id'] as2_id = addr_scope2['address_scope']['id']
@ -3630,61 +3626,31 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
subnetpool_id1 = subnetpool1['subnetpool']['id'] subnetpool_id1 = subnetpool1['subnetpool']['id']
subnetpool_id2 = subnetpool2['subnetpool']['id'] subnetpool_id2 = subnetpool2['subnetpool']['id']
# create subnets on the 2 subnet pools
data = {'subnet': {
'network_id': net1['network']['id'],
'subnetpool_id': subnetpool_id1,
'ip_version': 4,
'tenant_id': net1['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
subnet1 = self.deserialize(
self.fmt, req.get_response(self.api))
data = {'subnet': {
'network_id': net2['network']['id'],
'subnetpool_id': subnetpool_id2,
'ip_version': 4,
'tenant_id': net2['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
subnet2 = self.deserialize(
self.fmt, req.get_response(self.api))
data = {'subnet': {
'network_id': net3['network']['id'],
'subnetpool_id': subnetpool_id2,
'ip_version': 4,
'tenant_id': net3['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
subnet3 = self.deserialize(
self.fmt, req.get_response(self.api))
expected_rules = [
{'enabled': True,
'destination_ip_address': [subnet1['subnet']['cidr']],
'action': 'allow',
'name': 'Subnet Rule',
'source_ip_address': [subnet1['subnet']['cidr']]},
{'enabled': True,
'destination_ip_address': [subnet2['subnet']['cidr'],
subnet3['subnet']['cidr']],
'action': 'allow',
'name': 'Subnet Rule',
'source_ip_address': [subnet2['subnet']['cidr'],
subnet3['subnet']['cidr']]}]
# Add the interfaces to the router # Add the interfaces to the router
with mock.patch.object( with mock.patch.object(
edge_utils, 'update_nat_rules'),\ edge_utils, 'update_nat_rules'),\
mock.patch.object(edge_utils, 'update_firewall') as update_fw: mock.patch.object(edge_utils, 'update_firewall') as update_fw:
self._router_interface_action( # create subnets on the 2 subnet pools, and attach to router
'add', r['router']['id'], subnet1 = self._create_subnet_and_add_to_router(
subnet1['subnet']['id'], None) subnetpool_id1, r['router']['id'])
self._router_interface_action( subnet2 = self._create_subnet_and_add_to_router(
'add', r['router']['id'], subnetpool_id2, r['router']['id'])
subnet2['subnet']['id'], None) subnet3 = self._create_subnet_and_add_to_router(
self._router_interface_action( subnetpool_id2, r['router']['id'])
'add', r['router']['id'],
subnet3['subnet']['id'], None) expected_rules = [
{'enabled': True,
'destination_ip_address': [subnet1['subnet']['cidr']],
'action': 'allow',
'name': 'Subnet Rule',
'source_ip_address': [subnet1['subnet']['cidr']]},
{'enabled': True,
'destination_ip_address': [subnet2['subnet']['cidr'],
subnet3['subnet']['cidr']],
'action': 'allow',
'name': 'Subnet Rule',
'source_ip_address': [subnet2['subnet']['cidr'],
subnet3['subnet']['cidr']]}]
# check the final fw rules # check the final fw rules
fw_rules = update_fw.call_args[0][3][ fw_rules = update_fw.call_args[0][3][
@ -3693,6 +3659,84 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
self.assertEqual(self._recursive_sort_list(expected_rules), self.assertEqual(self._recursive_sort_list(expected_rules),
self._recursive_sort_list(fw_rules)) self._recursive_sort_list(fw_rules))
def _prepare_external_subnet_on_address_scope(self,
ext_net,
address_scope):
self._set_net_external(ext_net['network']['id'])
as_id = address_scope['address_scope']['id']
subnet = netaddr.IPNetwork('10.10.10.0/21')
subnetpool = self._test_create_subnetpool(
[subnet.cidr], name='sp1',
min_prefixlen='24', address_scope_id=as_id)
subnetpool_id = subnetpool['subnetpool']['id']
data = {'subnet': {
'network_id': ext_net['network']['id'],
'subnetpool_id': subnetpool_id,
'ip_version': 4,
'enable_dhcp': False,
'tenant_id': ext_net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
return ext_subnet['subnet']
def _test_router_address_scope_change(self, change_gw=False):
"""When subnetpool address scope changes, and router that was
originally under same address scope, results having different
address scopes, relevant snat rules are added.
"""
# create an external network on one address scope
with self.address_scope(name='as1') as addr_scope, \
self.network() as ext_net:
ext_subnet = self._prepare_external_subnet_on_address_scope(
ext_net, addr_scope)
# create a router with this gateway
with self.router() as r:
self._add_external_gateway_to_router(
r['router']['id'],
ext_subnet['network_id'])
# create a regular network on same address scope
# and verify no snat change
as_id = addr_scope['address_scope']['id']
subnet2 = netaddr.IPNetwork('40.10.10.0/24')
subnetpool2 = self._test_create_subnetpool(
[subnet2.cidr], name='sp2',
min_prefixlen='24', address_scope_id=as_id)
subnetpool2_id = subnetpool2['subnetpool']['id']
self._create_subnet_and_add_to_router(
subnetpool2_id, r['router']['id'])
# change address scope of the first subnetpool
with self.address_scope(name='as2') as addr_scope2,\
mock.patch.object(edge_utils, 'update_nat_rules') as update_nat,\
mock.patch.object(edge_utils, 'update_firewall') as update_fw:
as2_id = addr_scope2['address_scope']['id']
data = {'subnetpool': {
'address_scope_id': as2_id}}
if change_gw:
subnetpool_to_update = ext_subnet['subnetpool_id']
else:
subnetpool_to_update = subnetpool2_id
req = self.new_update_request('subnetpools', data,
subnetpool_to_update)
req.get_response(self.api)
# Verify that the snat & fw rule are being updated
update_nat.assert_called_once()
update_fw.assert_called_once()
def test_router_address_scope_change(self):
self._test_router_address_scope_change()
def test_router_address_scope_gw_change(self):
self._test_router_address_scope_change(change_gw=True)
class ExtGwModeTestCase(NsxVPluginV2TestCase, class ExtGwModeTestCase(NsxVPluginV2TestCase,
test_ext_gw_mode.ExtGwModeIntTestCase): test_ext_gw_mode.ExtGwModeIntTestCase):

6
vmware_nsx/tests/unit/nsx_v3/test_plugin.py
View File

@ -1026,7 +1026,7 @@ class TestL3NatTestCase(L3NatTest,
router_id, router_id,
assert_snat_deleted=False, assert_snat_deleted=False,
assert_snat_added=False): assert_snat_added=False):
# create a regular network on the same address scope # create a regular network on the given subnet pool
with self.network() as net: with self.network() as net:
data = {'subnet': { data = {'subnet': {
'network_id': net['network']['id'], 'network_id': net['network']['id'],
@ -1239,8 +1239,8 @@ class TestL3NatTestCase(L3NatTest,
def test_3leg_router_address_scope_change(self): def test_3leg_router_address_scope_change(self):
self._test_3leg_router_address_scope_change() self._test_3leg_router_address_scope_change()
def test_3leg_router_address_scope_change_to_gw(self, change_2gw=True): def test_3leg_router_address_scope_change_to_gw(self):
self._test_3leg_router_address_scope_change() self._test_3leg_router_address_scope_change(change_2gw=True)
def test_3leg_router_gw_address_scope_change(self): def test_3leg_router_gw_address_scope_change(self):
self._test_3leg_router_address_scope_change(change_gw=True) self._test_3leg_router_address_scope_change(change_gw=True)