allocation sync: use physnet, not hostgroup name

The allocation sync currently syncs the host allocations table by using
the name of a hostgroup. This value is almost always the same as the
physical network of the hostgroup, but not always. As new features will
rely on multiple hostgroups belonging to the same physnet we need to fix
this.

While doing so we also make sure that if we have multiple hostgroups for
the same physnet, we merge the vlan ranges we find in the different
hostgroups (that's what the new sorting and grouping is for).

In the config parser, we also now make sure the pyhsical_network config
attribute is always set. For Converged Cloud, this is taken care of by
the helm charts, but one can never be too sure (and this is not marked
as a required attribute).

networking_aci/plugins/ml2/drivers/mech_aci/allocations_manager.py

@@ -12,6 +12,8 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+from itertools import groupby
+from operator import itemgetter
 import os
 import random
 import time
@@ -398,28 +400,32 @@ def _sync_allocations(self):
 
             # sync existing hostgroups (remove out-of-range allocs, add new allocs)
             LOG.debug("Processing hostgroups")
-            for hg_name, hg_config in ACI_CONFIG.hostgroups.items():
-                if hg_config['direct_mode']:
-                    continue
-
-                hg_vlans = self._segmentation_ids(hg_config)
-                if hg_name in allocations:
+            hostgroups = [hg for hg in ACI_CONFIG.hostgroups.values() if not hg['direct_mode']]
+            hostgroups.sort(key=itemgetter('physical_network'))
+            for hg_physnet, hg_configs in groupby(hostgroups, key=itemgetter('physical_network')):
+                hg_vlans = set()
+                for hg_config in hg_configs:
+                    hg_vlans |= self._segmentation_ids(hg_config)
+
+                if hg_physnet in allocations:
                     # delete extra allocs
-                    out_of_range_ids = no_net_allocations[hg_name] - hg_vlans
+                    out_of_range_ids = no_net_allocations[hg_physnet] - hg_vlans
                     if out_of_range_ids:
-                        LOG.debug("Deleting %s out-of-range allocations with no assigned network for hostgroup %s",
-                                  len(out_of_range_ids), hg_name)
+                        LOG.debug("Deleting %s out-of-range allocations with no assigned network for "
+                                  "physical network %s",
+                                  len(out_of_range_ids), hg_physnet)
                         # delete all extra allocations that don't have a network_id referenced
                         del_q = session.query(AllocationsModel)
-                        del_q = del_q.filter_by(level=level, segment_type=segment_type, host=hg_name, network_id=None)
+                        del_q = del_q.filter_by(level=level, segment_type=segment_type, host=hg_physnet,
+                                                network_id=None)
                         del_q = del_q.filter(AllocationsModel.segmentation_id.in_(out_of_range_ids))
                         del_q.delete()
 
-                missing_ids = hg_vlans - allocations.get(hg_name, set())
+                missing_ids = hg_vlans - allocations.get(hg_physnet, set())
                 if missing_ids:
-                    LOG.debug("Adding %s allocations for hostgroup %s", len(missing_ids), hg_name)
+                    LOG.debug("Adding %s allocations for physical network %s", len(missing_ids), hg_physnet)
                     for seg_id in missing_ids:
-                        alloc = AllocationsModel(host=hg_name, level=level, segment_type=segment_type,
+                        alloc = AllocationsModel(host=hg_physnet, level=level, segment_type=segment_type,
                                                  segmentation_id=seg_id, network_id=None)
                         session.add(alloc)
 

networking_aci/plugins/ml2/drivers/mech_aci/config.py

@@ -321,6 +321,8 @@ def _parse_hostgroups(self):
         for hostgroup_name, hostgroup in self._hostgroups.items():
             hostgroup['name'] = hostgroup_name
             hostgroup['child_hostgroups'] = []
+            if 'physical_network' not in hostgroup:
+                hostgroup['physical_network'] = hostgroup_name
 
             # handle segment ranges
             segment_ranges = hostgroup['segment_range']

networking_aci/tests/unit/plugins/ml2/drivers/mech_aci/test_allocations_manager.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import itertools
 import os
 from pathlib import Path
 import tempfile
@@ -77,18 +78,21 @@ def test_sync_allocations(self):
         hostgroups = {
             'seagull': {
                 'direct_mode': False,
+                'physical_network': 'seagull',
                 'hosts': [],
                 'segment_range': [(42, 45)],
                 'segment_type': 'vlan',
             },
             'oystercatcher': {
                 'direct_mode': False,
+                'physical_network': 'oystercatcher',
                 'hosts': [],
                 'segment_range': [(100, 103)],
                 'segment_type': 'vlan',
             },
             'sparrow': {
                 'direct_mode': True,
+                'physical_network': 'sparrow',
                 'hosts': [],
                 'segment_range': [(123, 234)],
                 'segment_type': 'vlan',
@@ -131,3 +135,60 @@ def test_sync_allocations_locking(self):
                 sync_db_mock.assert_not_called()
 
                 self.assertEqual(open(sync_file).read(), str(os.getpid()))
+
+    def test_sync_allocations_split_hg_same_physnet(self):
+        expected_gulls = [('seagull', vid, None)
+                          for vid in itertools.chain(range(42, 46), range(100, 104), range(123, 127))]
+        expected_jays = [('jay', vid, None) for vid in range(200, 203)]
+        expected_result = expected_gulls + expected_jays
+
+        db = common.DBPlugin()
+        ACI_CONFIG.db = db
+
+        hostgroups = {
+            'herring-gull': {
+                'direct_mode': False,
+                'hosts': [],
+                'physical_network': 'seagull',
+                'segment_range': [(42, 45)],
+                'segment_type': 'vlan',
+            },
+            'yellow-legged-gull': {
+                'direct_mode': False,
+                'hosts': [],
+                'physical_network': 'seagull',
+                'segment_range': [(100, 103)],
+                'segment_type': 'vlan',
+            },
+            'seagull': {
+                'direct_mode': False,
+                'physical_network': 'seagull',
+                'hosts': [],
+                'segment_range': [(123, 126)],
+                'segment_type': 'vlan',
+            },
+            'blue-jay': {
+                'direct_mode': False,
+                'hosts': [],
+                'physical_network': 'jay',
+                'segment_range': [(200, 202)],
+                'segment_type': 'vlan',
+            },
+            'diadem-jay': {
+                'direct_mode': False,
+                'hosts': [],
+                'physical_network': 'jay',
+                'segment_range': [(200, 202)],
+                'segment_type': 'vlan',
+            },
+        }
+
+        with mock.patch('networking_aci.plugins.ml2.drivers.mech_aci.config.ACIConfig.hostgroups',
+                        new_callable=mock.PropertyMock, return_value=hostgroups):
+            AllocationsManager(db)
+
+        ctx = context.get_admin_context()
+        with db_api.CONTEXT_READER.using(ctx) as sess:
+            db_result = [(a.host, a.segmentation_id, a.network_id) for a in sess.query(AllocationsModel).all()]
+
+        self.assertEqual(sorted(expected_result), sorted(db_result))