From 10e07503524cc244d5c8f1f285db4a4f06dd12e7 Mon Sep 17 00:00:00 2001 From: Thomas Herve Date: Tue, 20 Oct 2015 15:42:59 +0200 Subject: [PATCH] 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 (cherry picked from commit 51f6b2e1c9c2f5f5106b9ae8316e57750f09d7c9) --- .../openvswitch/agent/ovs_neutron_agent.py | 15 ++++-- .../agent/test_ovs_neutron_agent.py | 52 +++++++++++-------- 2 files changed, 40 insertions(+), 27 deletions(-) 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 a305a767f29..d3795eb4641 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -353,11 +353,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): @@ -802,8 +808,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 d9794f12b45..f3b7f6f7b38 100644 --- 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