From f62890527974e1fea286373ba47b463671360947 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 16 Mar 2021 10:09:27 +0000 Subject: [PATCH] Fix VLAN driver tests Since [1], the physical network VLAN ranges parser populates the ranges for those entries without a defined range, allowing all valid VLAN ranges ([1, 4094]). Some VLAN driver tests, relying on the previous implementation, considered that the physical network without a defined VLAN range does not have segments to allocated (those segments are created on the fly by "SegmentTypeDriverallocate_fully_specified_segment). Since [1], all physical network segments are stored in the "ml2_vlan_allocations" table and set as non allocated. This patch also reverts [2]. When the physical networks are defined in "network_vlan_ranges", there is no distinction between tenant and provided networks; the physical network segments are assigned by the user. It is possible to create a provider network without defining the segmentation ID, it will be provided by the Neutron VLAN driver, if there are free segments for the required physical network. [1]https://review.opendev.org/c/openstack/neutron-lib/+/779515 [2]1376df7873c2ac77c256ab2fed928de41a2c1d58 Closes-Bug: #1919280 Related-Bug: #1918274 Related-Bug: #1649750 Change-Id: I191e020ddb97dcf8fb41139d35bfd699e125379b --- lower-constraints.txt | 2 +- neutron/plugins/ml2/drivers/type_vlan.py | 6 -- .../unit/plugins/ml2/drivers/test_helpers.py | 2 +- .../plugins/ml2/drivers/test_type_vlan.py | 80 ++++++++++--------- requirements.txt | 2 +- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index b48d70c980e..cd0f06d5307 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -50,7 +50,7 @@ msgpack-python==0.4.0 munch==2.1.0 netaddr==0.7.18 netifaces==0.10.4 -neutron-lib==2.9.0 +neutron-lib==2.10.0 openstacksdk==0.31.2 os-client-config==1.28.0 os-ken==0.3.0 diff --git a/neutron/plugins/ml2/drivers/type_vlan.py b/neutron/plugins/ml2/drivers/type_vlan.py index af49d019d49..b71447b6501 100644 --- a/neutron/plugins/ml2/drivers/type_vlan.py +++ b/neutron/plugins/ml2/drivers/type_vlan.py @@ -226,12 +226,6 @@ class VlanTypeDriver(helpers.SegmentTypeDriver): {'min': p_const.MIN_VLAN_TAG, 'max': p_const.MAX_VLAN_TAG}) raise exc.InvalidInput(error_message=msg) - else: - if not ranges.get(physical_network): - msg = (_("Physical network %s requires segmentation_id " - "to be specified when creating a provider " - "network") % physical_network) - raise exc.InvalidInput(error_message=msg) elif segmentation_id is not None: msg = _("segmentation_id requires physical_network for VLAN " "provider network") diff --git a/neutron/tests/unit/plugins/ml2/drivers/test_helpers.py b/neutron/tests/unit/plugins/ml2/drivers/test_helpers.py index 5f69df11af7..8714e96b477 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/test_helpers.py +++ b/neutron/tests/unit/plugins/ml2/drivers/test_helpers.py @@ -32,7 +32,7 @@ VLAN_OUTSIDE = 100 NETWORK_VLAN_RANGES = { TENANT_NET: [(VLAN_MIN, VLAN_MAX)], } -NETWORK_VLAN_RANGES_CFG_ENTRIES = [TENANT_NET, "%s:%s:%s" % +NETWORK_VLAN_RANGES_CFG_ENTRIES = ["%s:%s:%s" % (TENANT_NET, VLAN_MIN, VLAN_MAX)] SERVICE_PLUGIN_KLASS = ('neutron.services.network_segment_range.plugin.' 'NetworkSegmentRangePlugin') diff --git a/neutron/tests/unit/plugins/ml2/drivers/test_type_vlan.py b/neutron/tests/unit/plugins/ml2/drivers/test_type_vlan.py index d5a1c779a93..b9b4de5c24d 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/test_type_vlan.py +++ b/neutron/tests/unit/plugins/ml2/drivers/test_type_vlan.py @@ -34,17 +34,17 @@ TENANT_NET = 'phys_net2' UNCONFIGURED_NET = 'no_net' VLAN_MIN = 200 VLAN_MAX = 209 -NETWORK_VLAN_RANGES = [PROVIDER_NET, "%s:%s:%s" % - (TENANT_NET, VLAN_MIN, VLAN_MAX)] +TENANT_VLAN_RANGES = ["%s:%s:%s" % (TENANT_NET, VLAN_MIN, VLAN_MAX)] +NETWORK_VLAN_RANGES = [PROVIDER_NET] + TENANT_VLAN_RANGES UPDATED_VLAN_RANGES = { - PROVIDER_NET: [], + PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)], TENANT_NET: [(VLAN_MIN + 5, VLAN_MAX + 5)], } EMPTY_VLAN_RANGES = { - PROVIDER_NET: [] + PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)] } NETWORK_VLAN_RANGES_WITH_UNCONFIG = { - PROVIDER_NET: [], + PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)], TENANT_NET: [(VLAN_MIN + 5, VLAN_MAX + 5)], UNCONFIGURED_NET: [(VLAN_MIN, VLAN_MAX)] } @@ -151,13 +151,6 @@ class VlanTypeTest(testlib_api.SqlTestCase): self.driver.validate_provider_segment, segment) - def test_validate_provider_segment_with_physical_network_only(self): - segment = {api.NETWORK_TYPE: p_const.TYPE_VLAN, - api.PHYSICAL_NETWORK: PROVIDER_NET} - self.assertRaises(exc.InvalidInput, - self.driver.validate_provider_segment, - segment) - def test_sync_vlan_allocations(self): def check_in_ranges(network_vlan_ranges): vlan_min, vlan_max = network_vlan_ranges[TENANT_NET][0] @@ -212,7 +205,9 @@ class VlanTypeTest(testlib_api.SqlTestCase): api.PHYSICAL_NETWORK: PROVIDER_NET, api.SEGMENTATION_ID: 101} alloc = self._get_allocation(self.context, segment) - self.assertIsNone(alloc) + expected = vlan_alloc_obj.VlanAllocation( + allocated=False, physical_network=PROVIDER_NET, vlan_id=101) + self.assertEqual(expected.__repr__(), alloc.__repr__()) observed = self.driver.reserve_provider_segment(self.context, segment) alloc = self._get_allocation(self.context, observed) self.assertTrue(alloc.allocated) @@ -252,19 +247,11 @@ class VlanTypeTest(testlib_api.SqlTestCase): observed = self.driver.reserve_provider_segment(self.context, segment) alloc = self._get_allocation(self.context, observed) self.assertTrue(alloc.allocated) - vlan_id = observed[api.SEGMENTATION_ID] - self.assertThat(vlan_id, matchers.GreaterThan(VLAN_MIN - 1)) - self.assertThat(vlan_id, matchers.LessThan(VLAN_MAX + 1)) - self.assertEqual(TENANT_NET, observed[api.PHYSICAL_NETWORK]) - - def test_reserve_provider_segment_all_allocateds(self): - for __ in range(VLAN_MIN, VLAN_MAX + 1): - self.driver.allocate_tenant_segment(self.context) - segment = {api.NETWORK_TYPE: p_const.TYPE_VLAN} - self.assertRaises(exc.NoNetworkAvailable, - self.driver.reserve_provider_segment, - self.context, - segment) + # NOTE(ralonsoh): first network provided in the configuration is + # PROVIDER_NET. + self.assertEqual(PROVIDER_NET, observed[api.PHYSICAL_NETWORK]) + self.assertIn(observed[api.SEGMENTATION_ID], + range(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG + 1)) def test_get_mtu(self): cfg.CONF.set_override('global_physnet_mtu', 1475) @@ -288,20 +275,27 @@ class VlanTypeTest(testlib_api.SqlTestCase): self.assertEqual(0, self.driver.get_mtu('physnet1')) def test_allocate_tenant_segment(self): + cfg.CONF.set_override('network_vlan_ranges', TENANT_VLAN_RANGES, + group='ml2_type_vlan') + driver = type_vlan.VlanTypeDriver() + driver._sync_vlan_allocations() for __ in range(VLAN_MIN, VLAN_MAX + 1): segment = self.driver.allocate_tenant_segment(self.context) alloc = self._get_allocation(self.context, segment) self.assertTrue(alloc.allocated) vlan_id = segment[api.SEGMENTATION_ID] - self.assertThat(vlan_id, matchers.GreaterThan(VLAN_MIN - 1)) - self.assertThat(vlan_id, matchers.LessThan(VLAN_MAX + 1)) + self.assertGreater(vlan_id, VLAN_MIN - 1) + self.assertLess(vlan_id, VLAN_MAX + 1) self.assertEqual(TENANT_NET, segment[api.PHYSICAL_NETWORK]) def test_allocate_tenant_segment_no_available(self): + cfg.CONF.set_override('network_vlan_ranges', TENANT_VLAN_RANGES, + group='ml2_type_vlan') + driver = type_vlan.VlanTypeDriver() + driver._sync_vlan_allocations() for __ in range(VLAN_MIN, VLAN_MAX + 1): - self.driver.allocate_tenant_segment(self.context) - segment = self.driver.allocate_tenant_segment(self.context) - self.assertIsNone(segment) + driver.allocate_tenant_segment(self.context) + self.assertIsNone(driver.allocate_tenant_segment(self.context)) def test_release_segment(self): segment = self.driver.allocate_tenant_segment(self.context) @@ -311,14 +305,14 @@ class VlanTypeTest(testlib_api.SqlTestCase): def test_release_segment_unallocated(self): segment = {api.NETWORK_TYPE: p_const.TYPE_VLAN, - api.PHYSICAL_NETWORK: PROVIDER_NET, + api.PHYSICAL_NETWORK: 'non_existing_physnet', api.SEGMENTATION_ID: 101} with mock.patch.object(type_vlan.LOG, 'warning') as log_warn: self.driver.release_segment(self.context, segment) log_warn.assert_called_once_with( "No vlan_id %(vlan_id)s found on physical network " "%(physical_network)s", - {'vlan_id': 101, 'physical_network': PROVIDER_NET}) + {'vlan_id': 101, 'physical_network': 'non_existing_physnet'}) class VlanTypeAllocationTest(testlib_api.SqlTestCase): @@ -347,11 +341,15 @@ class VlanTypeAllocationTest(testlib_api.SqlTestCase): for vlan in range(10): # then physnet2 self.assertEqual( - {'network_type': 'vlan', 'physical_network': 'phys_net2', + {'network_type': 'vlan', 'physical_network': TENANT_NET, 'segmentation_id': mock.ANY, 'mtu': 1500}, driver.allocate_tenant_segment(ctx)) - # then nothing - self.assertFalse(driver.allocate_tenant_segment(ctx)) + # NOTE(ralonsoh): to save time, this test won't allocate 4094 segments + # for PROVIDER_NET. + self.assertEqual( + {'network_type': 'vlan', 'physical_network': PROVIDER_NET, + 'segmentation_id': p_const.MIN_VLAN_TAG, 'mtu': 1500}, + driver.allocate_tenant_segment(ctx)) class VlanTypeTestWithNetworkSegmentRange(testlib_api.SqlTestCase): @@ -375,12 +373,20 @@ class VlanTypeTestWithNetworkSegmentRange(testlib_api.SqlTestCase): # one of the `service_plugins` ret = obj_network_segment_range.NetworkSegmentRange.get_objects( self.context) - self.assertEqual(1, len(ret)) + self.assertEqual(2, len(ret)) network_segment_range = ret[0] self.assertTrue(network_segment_range.default) self.assertTrue(network_segment_range.shared) self.assertIsNone(network_segment_range.project_id) self.assertEqual(p_const.TYPE_VLAN, network_segment_range.network_type) + self.assertEqual(PROVIDER_NET, network_segment_range.physical_network) + self.assertEqual(p_const.MIN_VLAN_TAG, network_segment_range.minimum) + self.assertEqual(p_const.MAX_VLAN_TAG, network_segment_range.maximum) + network_segment_range = ret[1] + self.assertTrue(network_segment_range.default) + self.assertTrue(network_segment_range.shared) + self.assertIsNone(network_segment_range.project_id) + self.assertEqual(p_const.TYPE_VLAN, network_segment_range.network_type) self.assertEqual(TENANT_NET, network_segment_range.physical_network) self.assertEqual(VLAN_MIN, network_segment_range.minimum) self.assertEqual(VLAN_MAX, network_segment_range.maximum) diff --git a/requirements.txt b/requirements.txt index e7554eb2c74..20fdcb2a518 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,7 @@ Jinja2>=2.10 # BSD License (3 clause) keystonemiddleware>=5.1.0 # Apache-2.0 netaddr>=0.7.18 # BSD netifaces>=0.10.4 # MIT -neutron-lib>=2.9.0 # Apache-2.0 +neutron-lib>=2.10.0 # Apache-2.0 python-neutronclient>=6.7.0 # Apache-2.0 tenacity>=6.0.0 # Apache-2.0 SQLAlchemy>=1.2.0 # MIT