Properly handle segmentation_id in OVS agent

The segmentation_id of a OVS VLAN can be None, but a recent change
assumed that it was always an integer. It highlighted the fact that we
try to store None in the OVS database, which got stored as a string.
This fixes the storage, and handles loading the value while keeping
compatibility.

Change-Id: I6e7df1406c90ddde254467bb87ff1507a4caaadd
Closes-Bug: #1494281

diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
index e63392ddded944365405ab7511913b6f7d392ed3..c051e5d8357bfb08c72076fbdbce529b1ead1b67 100644 (file)
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
@@ -354,11 +354,17 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             net_uuid = local_vlan_map.get('net_uuid')
             if (net_uuid and net_uuid not in self.local_vlan_map
                 and local_vlan != DEAD_VLAN_TAG):
+                segmentation_id = local_vlan_map.get('segmentation_id')
+                if segmentation_id == 'None':
+                    # Backward compatible check when we used to store the
+                    # string 'None' in OVS
+                    segmentation_id = None
+                if segmentation_id is not None:
+                    segmentation_id = int(segmentation_id)
                 self.provision_local_vlan(local_vlan_map['net_uuid'],
                                           local_vlan_map['network_type'],
                                           local_vlan_map['physical_network'],
-                                          int(local_vlan_map[
-                                              'segmentation_id']),
+                                          segmentation_id,
                                           local_vlan)
 
     def setup_rpc(self):
@@ -803,8 +809,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
 
         vlan_mapping = {'net_uuid': net_uuid,
                         'network_type': network_type,
-                        'physical_network': physical_network,
-                        'segmentation_id': segmentation_id}
+                        'physical_network': physical_network}
+        if segmentation_id is not None:
+            vlan_mapping['segmentation_id'] = segmentation_id
         port_other_config.update(vlan_mapping)
         self.int_br.set_db_attribute("Port", port.port_name, "other_config",
                                      port_other_config)

diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
index 1f1db689e1810b2a2ad173d22e841e443d192909..0e5761d254ed56c9749a1687066ec4c3445e3d53 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
@@ -172,32 +172,10 @@ class TestOvsNeutronAgent(object):
         else:
             vlan_mapping = {'net_uuid': net_uuid,
                             'network_type': 'local',
-                            'physical_network': None,
-                            'segmentation_id': None}
+                            'physical_network': None}
             int_br.set_db_attribute.assert_called_once_with(
                 "Port", mock.ANY, "other_config", vlan_mapping)
 
-    def _test_restore_local_vlan_maps(self, tag):
-        port = mock.Mock()
-        port.port_name = 'fake_port'
-        local_vlan_map = {'net_uuid': 'fake_network_id',
-                          'network_type': 'vlan',
-                          'physical_network': 'fake_network',
-                          'segmentation_id': 1}
-        with mock.patch.object(self.agent, 'int_br') as int_br, \
-            mock.patch.object(self.agent, 'provision_local_vlan') as \
-                provision_local_vlan:
-            int_br.get_vif_ports.return_value = [port]
-            int_br.get_ports_attributes.return_value = [{
-                'name': port.port_name, 'other_config': local_vlan_map,
-                'tag': tag
-            }]
-            self.agent._restore_local_vlan_map()
-            if tag:
-                self.assertTrue(provision_local_vlan.called)
-            else:
-                self.assertFalse(provision_local_vlan.called)
-
     def test_datapath_type_system(self):
         # verify kernel datapath is default
         expected = constants.OVS_DATAPATH_SYSTEM
@@ -261,12 +239,40 @@ class TestOvsNeutronAgent(object):
             self.assertEqual(expected,
                              self.agent.agent_state['agent_type'])
 
+    def _test_restore_local_vlan_maps(self, tag, segmentation_id='1'):
+        port = mock.Mock()
+        port.port_name = 'fake_port'
+        local_vlan_map = {'net_uuid': 'fake_network_id',
+                          'network_type': 'vlan',
+                          'physical_network': 'fake_network'}
+        if segmentation_id is not None:
+            local_vlan_map['segmentation_id'] = segmentation_id
+        with mock.patch.object(self.agent, 'int_br') as int_br, \
+            mock.patch.object(self.agent, 'provision_local_vlan') as \
+                provision_local_vlan:
+            int_br.get_vif_ports.return_value = [port]
+            int_br.get_ports_attributes.return_value = [{
+                'name': port.port_name, 'other_config': local_vlan_map,
+                'tag': tag
+            }]
+            self.agent._restore_local_vlan_map()
+            if tag:
+                self.assertTrue(provision_local_vlan.called)
+            else:
+                self.assertFalse(provision_local_vlan.called)
+
     def test_restore_local_vlan_map_with_device_has_tag(self):
         self._test_restore_local_vlan_maps(2)
 
     def test_restore_local_vlan_map_with_device_no_tag(self):
         self._test_restore_local_vlan_maps([])
 
+    def test_restore_local_vlan_map_no_segmentation_id(self):
+        self._test_restore_local_vlan_maps(2, segmentation_id=None)
+
+    def test_restore_local_vlan_map_segmentation_id_compat(self):
+        self._test_restore_local_vlan_maps(2, segmentation_id='None')
+
     def test_check_agent_configurations_for_dvr_raises(self):
         self.agent.enable_distributed_routing = True
         self.agent.enable_tunneling = True