Merge "Keep allocated VLAN registers when purging a physical network"

neutron/objects/plugins/ml2/vlanallocation.py

@@ -15,6 +15,7 @@
 from neutron_lib import constants as n_const
 from neutron_lib.objects import common_types
 from oslo_versionedobjects import fields as obj_fields
+from sqlalchemy import sql
 
 from neutron.db.models.plugins.ml2 import vlanallocation as vlan_alloc_model
 from neutron.objects import base
@@ -47,9 +48,16 @@ class VlanAllocation(base.NeutronDbObject, ml2_base.SegmentAllocation):
 
     @staticmethod
     def delete_physical_networks(context, physical_networks):
+        """Delete the unallocated registers of the given physical networks
+
+        Allocated registers are kept: they are the only record that the
+        segmentation ID is in use.
+        """
         column = VlanAllocation.db_model.physical_network
         context.session.query(VlanAllocation.db_model).filter(
-            column.in_(physical_networks)).delete(synchronize_session=False)
+            column.in_(physical_networks),
+            VlanAllocation.db_model.allocated == sql.false()).delete(
+                synchronize_session=False)
 
     @staticmethod
     def bulk_create(ctx, physical_network, vlan_ids):

neutron/plugins/ml2/common/exceptions.py

@@ -40,3 +40,12 @@ class ExtensionDriverNotFound(exceptions.InvalidConfigurationOption):
     """Required extension driver not found in ML2 config."""
     message = _("Extension driver %(driver)s required for "
                 "service plugin %(service_plugin)s not found.")
+
+
+class VlanAllocationOutsideConfiguredRanges(exceptions.NeutronException):
+    """Config file VLAN ranges no longer cover an allocated segment."""
+    message = _("Network segments still hold VLAN IDs that "
+                "'[ml2_type_vlan] network_vlan_ranges' no longer covers "
+                "(%(allocations)s). Restore those ranges in the "
+                "configuration file, or delete the networks holding those "
+                "VLAN IDs, before starting Neutron again.")

neutron/plugins/ml2/drivers/type_vlan.py

@@ -33,6 +33,7 @@ from neutron.conf.plugins.ml2.drivers import driver_type
 from neutron.db.models.plugins.ml2 import vlanallocation as vlan_alloc_model
 from neutron.objects import network_segment_range as range_obj
 from neutron.objects.plugins.ml2 import vlanallocation as vlanalloc
+from neutron.plugins.ml2.common import exceptions as ml2_exc
 from neutron.plugins.ml2.drivers import helpers
 from neutron.services.network_segment_range import plugin as range_plugin
 
@@ -87,8 +88,8 @@ class VlanTypeDriver(helpers.SegmentTypeDriver):
             physnets = vlanalloc.VlanAllocation.get_physical_networks(ctx)
             physnets_unconfigured = physnets - set(ranges)
             if physnets_unconfigured:
-                LOG.debug('Removing any VLAN register on physical networks %s',
-                          physnets_unconfigured)
+                LOG.debug('Removing any unallocated VLAN register on '
+                          'physical networks %s', physnets_unconfigured)
                 vlanalloc.VlanAllocation.delete_physical_networks(
                     ctx, physnets_unconfigured)
 
@@ -169,15 +170,92 @@ class VlanTypeDriver(helpers.SegmentTypeDriver):
             self._sync_vlan_allocations()
         LOG.info("VlanTypeDriver initialization complete")
 
+    @staticmethod
+    def _network_segment_ranges(ctx, default=None):
+        """VLAN ranges per physical network, as stored in the DB
+
+        With ``default`` set to True only the registers populated from
+        ``[ml2_type_vlan] network_vlan_ranges`` are returned, with False only
+        those created through the API, and with None every one of them.
+        """
+        filters = {'network_type': p_const.TYPE_VLAN}
+        if default is not None:
+            filters['default'] = default
+        ranges = collections.defaultdict(list)
+        for obj in range_obj.NetworkSegmentRange.get_objects(ctx, **filters):
+            ranges[obj['physical_network']].append(
+                (obj['minimum'], obj['maximum']))
+        return ranges
+
+    @staticmethod
+    def _covered_by(vlan_id, vlan_ranges):
+        return any(vlan_min <= vlan_id <= vlan_max
+                   for vlan_min, vlan_max in vlan_ranges)
+
+    def _validate_config_ranges_cover_allocations(self, ctx,
+                                                  previous_default_ranges):
+        """Refuse to start when a config file edit orphaned an allocation
+
+        ``network_vlan_ranges`` has been narrowed, or a physical network
+        dropped from it, while segments still hold VLAN IDs that the new
+        ranges no longer cover. Restoring the previous ranges in the
+        configuration file lets Neutron start again, so failing here is
+        recoverable and points at the change that caused it.
+
+        Only the VLAN IDs the ``default`` registers used to cover are
+        considered. Ranges created through the API are left to
+        ``_sync_vlan_allocations``, which keeps their allocated registers:
+        refusing to start over those would leave an administrator unable to
+        reach the API that would undo the change.
+        """
+        if not previous_default_ranges:
+            return
+
+        effective_ranges = self._network_segment_ranges(ctx)
+        orphaned = collections.defaultdict(set)
+        allocations = vlanalloc.VlanAllocation.get_objects(
+            ctx, allocated=True,
+            physical_network=list(previous_default_ranges))
+        for alloc in allocations:
+            physical_network = alloc.physical_network
+            if self._covered_by(alloc.vlan_id,
+                                effective_ranges[physical_network]):
+                continue
+            if not self._covered_by(
+                    alloc.vlan_id,
+                    previous_default_ranges[physical_network]):
+                continue
+            orphaned[physical_network].add(alloc.vlan_id)
+
+        if orphaned:
+            raise ml2_exc.VlanAllocationOutsideConfiguredRanges(
+                allocations='; '.join(
+                    '{}: {}'.format(
+                        physical_network,
+                        ', '.join(str(v) for v in sorted(vlan_ids)))
+                    for physical_network, vlan_ids in sorted(
+                        orphaned.items())))
+
     @db_api.retry_db_errors
     def initialize_network_segment_range_support(self, start_time):
         admin_context = context.get_admin_context()
         try:
             with db_api.CONTEXT_WRITER.using(admin_context):
+                # Read before the expired registers go away: they are the
+                # only record of what the configuration file asked for the
+                # last time Neutron started.
+                previous_default_ranges = self._network_segment_ranges(
+                    admin_context, default=True)
                 self._delete_expired_default_network_segment_ranges(
                     admin_context, start_time)
                 self._populate_new_default_network_segment_ranges(
                     admin_context, start_time)
+                # Raising inside the transaction rolls the two calls above
+                # back, so the previous default registers survive and the
+                # next start reaches the same verdict instead of quietly
+                # accepting the new ranges.
+                self._validate_config_ranges_cover_allocations(
+                    admin_context, previous_default_ranges)
         except o_exc.NeutronDbObjectDuplicateEntry:
             pass
 

neutron/tests/unit/objects/plugins/ml2/test_vlanallocation.py

@@ -28,3 +28,22 @@ class VlanAllocationDbObjTestCase(
         ml2_test_base.SegmentAllocationDbObjTestCase):
 
     _test_class = vlanallocation.VlanAllocation
+
+    def test_delete_physical_networks_keeps_allocated(self):
+        for vlan_id, allocated in ((100, False), (101, True)):
+            vlanallocation.VlanAllocation(
+                self.context, physical_network='physnet1', vlan_id=vlan_id,
+                allocated=allocated).create()
+        vlanallocation.VlanAllocation(
+            self.context, physical_network='physnet2', vlan_id=100,
+            allocated=False).create()
+
+        vlanallocation.VlanAllocation.delete_physical_networks(
+            self.context, ['physnet1'])
+
+        remaining = {(alloc.physical_network, alloc.vlan_id)
+                     for alloc in vlanallocation.VlanAllocation.get_objects(
+                         self.context)}
+        # the unallocated physnet1 register is gone, the allocated one is
+        # kept, and other physical networks are untouched
+        self.assertEqual({('physnet1', 101), ('physnet2', 100)}, remaining)

neutron/tests/unit/plugins/ml2/drivers/test_type_vlan.py

@@ -29,6 +29,7 @@ from neutron.common import wsgi_utils
 from neutron.conf.plugins.ml2 import config as ml2_config
 from neutron.objects import network_segment_range as obj_network_segment_range
 from neutron.objects.plugins.ml2 import vlanallocation as vlan_alloc_obj
+from neutron.plugins.ml2.common import exceptions as ml2_exc
 from neutron.plugins.ml2.drivers import type_vlan
 from neutron.tests.unit import testlib_api
 
@@ -187,7 +188,8 @@ class VlanTypeTest(testlib_api.SqlTestCase):
         with mock.patch.object(type_vlan.LOG, 'debug') as mock_debug:
             self.driver._sync_vlan_allocations()
             mock_debug.assert_called_once_with(
-                'Removing any VLAN register on physical networks %s',
+                'Removing any unallocated VLAN register on '
+                'physical networks %s',
                 {UNCONFIGURED_NET})
         check_in_ranges(UPDATED_VLAN_RANGES)
 
@@ -404,6 +406,82 @@ class VlanTypeTestWithNetworkSegmentRange(testlib_api.SqlTestCase):
         self.assertEqual(VLAN_MIN, network_segment_range.minimum)
         self.assertEqual(VLAN_MAX, network_segment_range.maximum)
 
+    def _allocate(self, physical_network, vlan_id):
+        # the register already exists for a configured physical network,
+        # created unallocated by the sync in setUp
+        updated = vlan_alloc_obj.VlanAllocation.update_objects(
+            self.context, values={'allocated': True},
+            physical_network=physical_network, vlan_id=vlan_id)
+        if not updated:
+            vlan_alloc_obj.VlanAllocation(
+                self.context, physical_network=physical_network,
+                vlan_id=vlan_id, allocated=True).create()
+
+    def _default_ranges_for(self, physical_network):
+        return sorted(
+            (obj.minimum, obj.maximum)
+            for obj in obj_network_segment_range.NetworkSegmentRange.
+            get_objects(self.context, network_type=p_const.TYPE_VLAN,
+                        default=True, physical_network=physical_network))
+
+    def test_narrowing_config_ranges_over_an_allocation_refuses_to_start(self):
+        # phys_net2 is configured as 200:209 and a segment holds 205; the
+        # operator narrows the config file to 200:204
+        self._allocate(TENANT_NET, VLAN_MAX)
+        self.driver._network_vlan_ranges = {
+            PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)],
+            TENANT_NET: [(VLAN_MIN, VLAN_MAX - 5)]}
+
+        exc_raised = self.assertRaises(
+            ml2_exc.VlanAllocationOutsideConfiguredRanges,
+            self.driver.initialize_network_segment_range_support,
+            self.start_time)
+
+        self.assertIn(TENANT_NET, str(exc_raised))
+        self.assertIn(str(VLAN_MAX), str(exc_raised))
+        # the raise happened inside the transaction, so the previous default
+        # registers survive and the next start reaches the same verdict
+        self.assertEqual([(VLAN_MIN, VLAN_MAX)],
+                         self._default_ranges_for(TENANT_NET))
+
+    def test_dropping_a_physnet_from_config_over_an_allocation_refuses(self):
+        self._allocate(TENANT_NET, VLAN_MIN)
+        self.driver._network_vlan_ranges = {
+            PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)]}
+
+        self.assertRaises(
+            ml2_exc.VlanAllocationOutsideConfiguredRanges,
+            self.driver.initialize_network_segment_range_support,
+            self.start_time)
+
+        self.assertEqual([(VLAN_MIN, VLAN_MAX)],
+                         self._default_ranges_for(TENANT_NET))
+
+    def test_allocation_still_covered_by_config_starts(self):
+        self._allocate(TENANT_NET, VLAN_MIN)
+        self.driver._network_vlan_ranges = {
+            PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)],
+            TENANT_NET: [(VLAN_MIN, VLAN_MAX - 5)]}
+
+        self.driver.initialize_network_segment_range_support(self.start_time)
+
+        self.assertEqual([(VLAN_MIN, VLAN_MAX - 5)],
+                         self._default_ranges_for(TENANT_NET))
+
+    def test_allocation_on_an_api_managed_range_does_not_refuse(self):
+        # an operator shrinking a range through the API must not be able to
+        # stop Neutron from starting: they could not reach the API to undo it
+        obj_network_segment_range.NetworkSegmentRange(
+            self.context, network_type=p_const.TYPE_VLAN,
+            physical_network=UNCONFIGURED_NET, minimum=VLAN_MIN,
+            maximum=VLAN_MAX, default=False, shared=True).create()
+        self._allocate(UNCONFIGURED_NET, VLAN_MIN)
+        self.driver._network_vlan_ranges = {
+            PROVIDER_NET: [(p_const.MIN_VLAN_TAG, p_const.MAX_VLAN_TAG)],
+            TENANT_NET: [(VLAN_MIN, VLAN_MAX)]}
+
+        self.driver.initialize_network_segment_range_support(self.start_time)
+
     def test__delete_expired_default_network_segment_ranges(self):
         self.driver._delete_expired_default_network_segment_ranges(
             self.context, self.start_time)

releasenotes/notes/ml2-keep-allocated-vlan-registers-d3ba3e62590eb72b.yaml

@@ -0,0 +1,23 @@
+---
+upgrade:
+  - |
+    Neutron now refuses to start when ``[ml2_type_vlan]
+    network_vlan_ranges`` no longer covers a VLAN ID that a network segment
+    still holds, either because a range was narrowed or because a physical
+    network was dropped from it. The error names the physical networks and
+    VLAN IDs involved. Restore the ranges in the configuration file, or
+    delete the networks holding those VLAN IDs, to start again. Network
+    segment ranges created through the API are not affected: their allocated
+    registers are kept and Neutron starts normally, since an administrator
+    could not otherwise reach the API to undo such a change.
+fixes:
+  - |
+    Synchronizing the VLAN allocations no longer deletes allocated registers
+    when it removes the registers of a physical network that is no longer
+    configured. Deleting them discarded the only record that a segmentation
+    ID was in use while the network segment referencing it still existed.
+    Allocated registers are now kept and removed when their segment is
+    released, which matches both the per-VLAN removal in the same routine
+    and the behaviour of the tunnel type drivers.
+    See bug: `2167266
+    <https://bugs.launchpad.net/neutron/+bug/2167266>`_.