diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 26df4984688..1cb5f4cd180 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -141,6 +141,12 @@ class BaseOVS(object): return self.ovsdb.db_get(table, record, column).execute( check_error=check_error, log_errors=log_errors) + def db_list(self, table, records=None, columns=None, + check_error=True, log_errors=True, if_exists=False): + return (self.ovsdb.db_list(table, records=records, columns=columns, + if_exists=if_exists). + execute(check_error=check_error, log_errors=log_errors)) + class OVSBridge(BaseOVS): def __init__(self, br_name): @@ -319,11 +325,12 @@ class OVSBridge(BaseOVS): def get_vif_ports(self): edge_ports = [] port_names = self.get_port_name_list() + port_info = self.db_list( + 'Interface', columns=['name', 'external_ids', 'ofport']) + by_name = {x['name']: x for x in port_info} for name in port_names: - external_ids = self.db_get_val("Interface", name, "external_ids", - check_error=True) - ofport = self.db_get_val("Interface", name, "ofport", - check_error=True) + external_ids = by_name[name]['external_ids'] + ofport = by_name[name]['ofport'] if "iface-id" in external_ids and "attached-mac" in external_ids: p = VifPort(name, ofport, external_ids["iface-id"], external_ids["attached-mac"], self) @@ -341,10 +348,9 @@ class OVSBridge(BaseOVS): def get_vif_port_to_ofport_map(self): port_names = self.get_port_name_list() - cmd = self.ovsdb.db_list( - 'Interface', port_names, - columns=['name', 'external_ids', 'ofport'], if_exists=True) - results = cmd.execute(check_error=True) + results = self.db_list( + 'Interface', port_names, ['name', 'external_ids', 'ofport'], + if_exists=True) port_map = {} for r in results: # fall back to basic interface name @@ -359,10 +365,9 @@ class OVSBridge(BaseOVS): def get_vif_port_set(self): edge_ports = set() port_names = self.get_port_name_list() - cmd = self.ovsdb.db_list( - 'Interface', port_names, - columns=['name', 'external_ids', 'ofport'], if_exists=True) - results = cmd.execute(check_error=True) + results = self.db_list( + 'Interface', port_names, ['name', 'external_ids', 'ofport'], + if_exists=True) for result in results: if result['ofport'] == UNASSIGNED_OFPORT: LOG.warn(_LW("Found not yet ready openvswitch port: %s"), @@ -400,11 +405,42 @@ class OVSBridge(BaseOVS): """ port_names = self.get_port_name_list() - cmd = self.ovsdb.db_list('Port', port_names, columns=['name', 'tag'], - if_exists=True) - results = cmd.execute(check_error=True) + results = self.db_list('Port', port_names, ['name', 'tag'], + if_exists=True) return {p['name']: p['tag'] for p in results} + def get_vifs_by_ids(self, port_ids): + interface_info = self.db_list( + "Interface", columns=["name", "external_ids", "ofport"]) + by_id = {x['external_ids'].get('iface-id'): x for x in interface_info} + intfs_on_bridge = self.ovsdb.list_ports(self.br_name).execute( + check_error=True) + result = {} + for port_id in port_ids: + result[port_id] = None + if (port_id not in by_id or + by_id[port_id]['name'] not in intfs_on_bridge): + LOG.info(_LI("Port %(port_id)s not present in bridge " + "%(br_name)s"), + {'port_id': port_id, 'br_name': self.br_name}) + continue + pinfo = by_id[port_id] + if not self._check_ofport(port_id, pinfo): + continue + mac = pinfo['external_ids'].get('attached-mac') + result[port_id] = VifPort(pinfo['name'], pinfo['ofport'], + port_id, mac, self) + return result + + @staticmethod + def _check_ofport(port_id, port_info): + if port_info['ofport'] in [UNASSIGNED_OFPORT, INVALID_OFPORT]: + LOG.warn(_LW("ofport: %(ofport)s for VIF: %(vif)s is not a" + " positive integer"), + {'ofport': port_info['ofport'], 'vif': port_id}) + return False + return True + def get_vif_port_by_id(self, port_id): ports = self.ovsdb.db_find( 'Interface', ('external_ids', '=', {'iface-id': port_id}), @@ -413,10 +449,7 @@ class OVSBridge(BaseOVS): for port in ports: if self.br_name != self.get_bridge_for_iface(port['name']): continue - if port['ofport'] in [UNASSIGNED_OFPORT, INVALID_OFPORT]: - LOG.warn(_LW("ofport: %(ofport)s for VIF: %(vif)s is not a" - " positive integer"), - {'ofport': port['ofport'], 'vif': port_id}) + if not self._check_ofport(port_id, port): continue mac = port['external_ids'].get('attached-mac') return VifPort(port['name'], port['ofport'], port_id, mac, self) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index d0f2aa8f156..48dea6bf6d0 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -391,7 +391,8 @@ class Controller(object): self._notifier.info(request.context, self._resource + '.create.start', body) - body = Controller.prepare_request_body(request.context, body, True, + body = Controller.prepare_request_body(request.context, + copy.deepcopy(body), True, self._resource, self._attr_info, allow_bulk=self._allow_bulk) action = self._plugin_handlers[self.CREATE] diff --git a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py index 69da70c79bd..b359dac62ec 100644 --- a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py @@ -948,7 +948,6 @@ class LinuxBridgeNeutronAgentRPC(service.Service): LOG.info(_LI("Port %s updated."), device) else: LOG.debug("Device %s not defined on plugin", device) - self.br_mgr.remove_empty_bridges() return resync def scan_devices(self, previous, sync): 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 35bfe645e2f..1f4a4119c68 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -312,10 +312,17 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, def _restore_local_vlan_map(self): cur_ports = self.int_br.get_vif_ports() + port_info = self.int_br.db_list( + "Port", columns=["name", "other_config", "tag"]) + by_name = {x['name']: x for x in port_info} for port in cur_ports: - local_vlan_map = self.int_br.db_get_val("Port", port.port_name, - "other_config") - local_vlan = self.int_br.db_get_val("Port", port.port_name, "tag") + # if a port was deleted between get_vif_ports and db_lists, we + # will get a KeyError + try: + local_vlan_map = by_name[port.port_name]['other_config'] + local_vlan = by_name[port.port_name]['tag'] + except KeyError: + continue if not local_vlan: continue net_uuid = local_vlan_map.get('net_uuid') @@ -730,6 +737,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, port_other_config) def _bind_devices(self, need_binding_ports): + port_info = self.int_br.db_list( + "Port", columns=["name", "tag"]) + tags_by_name = {x['name']: x['tag'] for x in port_info} for port_detail in need_binding_ports: lvm = self.local_vlan_map.get(port_detail['network_id']) if not lvm: @@ -739,7 +749,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, port = port_detail['vif_port'] device = port_detail['device'] # Do not bind a port if it's already bound - cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag") + cur_tag = tags_by_name.get(port.port_name) if cur_tag != lvm.vlan: self.int_br.set_db_attribute( "Port", port.port_name, "tag", lvm.vlan) @@ -1196,10 +1206,12 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.conf.host) except Exception as e: raise DeviceListRetrievalError(devices=devices, error=e) + vif_by_id = self.int_br.get_vifs_by_ids( + [vif['device'] for vif in devices_details_list]) for details in devices_details_list: device = details['device'] LOG.debug("Processing port: %s", device) - port = self.int_br.get_vif_port_by_id(device) + port = vif_by_id.get(device) if not port: # The port disappeared and cannot be processed LOG.info(_LI("Port %s was not found on the integration bridge " diff --git a/neutron/plugins/plumgrid/README b/neutron/plugins/plumgrid/README index e7118307d3e..5fc4050e4cb 100644 --- a/neutron/plugins/plumgrid/README +++ b/neutron/plugins/plumgrid/README @@ -1,8 +1,14 @@ -PLUMgrid Neutron Plugin for Virtual Network Infrastructure (VNI) +PLUMgrid Neutron Plugin +======================== -This plugin implements Neutron v2 APIs and helps configure -L2/L3 virtual networks consisting of PLUMgrid Platform. -Implements External Networks and Port Binding Extension +PLUMgrid Neutron Plugin for PLUMgrid Open Networking Suite -For more details on use please refer to: -http://wiki.openstack.org/PLUMgrid-Neutron +* Full plugin code is available at: + * https://github.com/stackforge/networking-plumgrid + +* PyPI package location: + * https://pypi.python.org/pypi/networking-plumgrid + +* For config, install and other details, please refer to + wiki page: + * http://wiki.openstack.org/PLUMgrid-Neutron diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index 75614b20f3c..6deaab64e2e 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -60,8 +60,8 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase): self.br_tun = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN, prefix='br-tun') patch_name_len = n_const.DEVICE_NAME_MAX_LEN - len("-patch-tun") - self.patch_tun = "%s-patch-tun" % self.br_int[:patch_name_len] - self.patch_int = "%s-patch-int" % self.br_tun[:patch_name_len] + self.patch_tun = "%s-patch-tun" % self.br_int[patch_name_len:] + self.patch_int = "%s-patch-int" % self.br_tun[patch_name_len:] self.ovs = ovs_lib.BaseOVS() self.config = self._configure_agent() self.driver = interface.OVSInterfaceDriver(self.config) diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index be71f2ef8a2..863a15ab0d0 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -209,6 +209,15 @@ class OVSBridgeTestCase(OVSBridgeTestBase): self.assertEqual(self.br.get_vif_port_by_id(vif.vif_id).vif_id, vif.vif_id) + def test_get_vifs_by_ids(self): + for i in range(2): + self.create_ovs_port() + vif_ports = [self.create_ovs_vif_port() for i in range(3)] + by_id = self.br.get_vifs_by_ids([v.vif_id for v in vif_ports]) + # convert to str for comparison of VifPorts + by_id = {vid: str(vport) for vid, vport in by_id.items()} + self.assertEqual({v.vif_id: str(v) for v in vif_ports}, by_id) + def test_delete_ports(self): # TODO(twilson) I intensely dislike the current delete_ports function # as the default behavior is really delete_vif_ports(), then it acts diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index 9e9c514dcd8..3f0b12b3b85 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -470,23 +470,22 @@ class OVS_Lib_Test(base.BaseTestCase): def _test_get_vif_ports(self, is_xen=False): pname = "tap99" ofport = 6 - ofport_data = self._encode_ovs_json(['ofport'], [[ofport]]) vif_id = uuidutils.generate_uuid() mac = "ca:fe:de:ad:be:ef" id_field = 'xs-vif-uuid' if is_xen else 'iface-id' external_ids = ('{"data":[[["map",[["attached-mac","%(mac)s"],' '["%(id_field)s","%(vif)s"],' - '["iface-status","active"]]]]],' - '"headings":["external_ids"]}' % { - 'mac': mac, 'vif': vif_id, 'id_field': id_field}) + '["iface-status","active"]]], ' + '"%(name)s", %(ofport)s]],' + '"headings":["external_ids", "name", "ofport"]}' % { + 'mac': mac, 'vif': vif_id, 'id_field': id_field, + 'name': pname, 'ofport': ofport}) # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ (self._vsctl_mock("list-ports", self.BR_NAME), "%s\n" % pname), - (self._vsctl_mock("--columns=external_ids", "list", - "Interface", pname), external_ids), - (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), - ofport_data), + (self._vsctl_mock("--columns=name,external_ids,ofport", "list", + "Interface"), external_ids), ] if is_xen: expected_calls_and_values.append( @@ -724,6 +723,35 @@ class OVS_Lib_Test(base.BaseTestCase): with testtools.ExpectedException(Exception): self.br.get_local_port_mac() + def test_get_vifs_by_ids(self): + db_list_res = [ + {'name': 'qvo1', 'ofport': 1, + 'external_ids': {'iface-id': 'pid1', 'attached-mac': '11'}}, + {'name': 'qvo2', 'ofport': 2, + 'external_ids': {'iface-id': 'pid2', 'attached-mac': '22'}}, + {'name': 'qvo3', 'ofport': 3, + 'external_ids': {'iface-id': 'pid3', 'attached-mac': '33'}}, + {'name': 'qvo4', 'ofport': -1, + 'external_ids': {'iface-id': 'pid4', 'attached-mac': '44'}}, + ] + self.br.db_list = mock.Mock(return_value=db_list_res) + self.br.ovsdb = mock.Mock() + self.br.ovsdb.list_ports.return_value.execute.return_value = [ + 'qvo1', 'qvo2', 'qvo4'] + by_id = self.br.get_vifs_by_ids(['pid1', 'pid2', 'pid3', + 'pid4', 'pid5']) + # pid3 isn't on bridge and pid4 doesn't have a valid ofport and pid5 + # isn't present in the db + self.assertIsNone(by_id['pid3']) + self.assertIsNone(by_id['pid4']) + self.assertIsNone(by_id['pid5']) + self.assertEqual('pid1', by_id['pid1'].vif_id) + self.assertEqual('qvo1', by_id['pid1'].port_name) + self.assertEqual(1, by_id['pid1'].ofport) + self.assertEqual('pid2', by_id['pid2'].vif_id) + self.assertEqual('qvo2', by_id['pid2'].port_name) + self.assertEqual(2, by_id['pid2'].ofport) + def _test_get_vif_port_by_id(self, iface_id, data, br_name=None, extra_calls_and_values=None): headings = ['external_ids', 'name', 'ofport'] 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 d54b5b04ecf..9b4cf011038 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 @@ -114,6 +114,8 @@ class TestOvsNeutronAgent(object): cfg.CONF.set_default('quitting_rpc_timeout', 10, 'AGENT') cfg.CONF.set_default('prevent_arp_spoofing', False, 'AGENT') kwargs = self.mod_agent.create_agent_config_map(cfg.CONF) + mock.patch('neutron.agent.common.ovs_lib.OVSBridge.db_list', + return_value=[]).start() with mock.patch.object(self.mod_agent.OVSNeutronAgent, 'setup_integration_br'),\ @@ -171,7 +173,10 @@ class TestOvsNeutronAgent(object): mock.patch.object(self.agent, 'provision_local_vlan') as \ provision_local_vlan: int_br.get_vif_ports.return_value = [port] - int_br.db_get_val.side_effect = [local_vlan_map, tag] + int_br.db_list.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) @@ -340,8 +345,8 @@ class TestOvsNeutronAgent(object): 'get_devices_details_list', return_value=[details]),\ mock.patch.object(self.agent.int_br, - 'get_vif_port_by_id', - return_value=port),\ + 'get_vifs_by_ids', + return_value={details['device']: port}),\ mock.patch.object(self.agent, func_name) as func: skip_devs, need_bound_devices = ( self.agent.treat_devices_added_or_updated([{}], False)) @@ -382,8 +387,8 @@ class TestOvsNeutronAgent(object): 'get_devices_details_list', return_value=[dev_mock]),\ mock.patch.object(self.agent.int_br, - 'get_vif_port_by_id', - return_value=None),\ + 'get_vifs_by_ids', + return_value={}),\ mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: skip_devs = self.agent.treat_devices_added_or_updated([{}], False) @@ -409,8 +414,8 @@ class TestOvsNeutronAgent(object): 'get_devices_details_list', return_value=[fake_details_dict]),\ mock.patch.object(self.agent.int_br, - 'get_vif_port_by_id', - return_value=mock.MagicMock()),\ + 'get_vifs_by_ids', + return_value={'xxx': mock.MagicMock()}),\ mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: skip_devs, need_bound_devices = ( @@ -448,6 +453,8 @@ class TestOvsNeutronAgent(object): self.agent, "treat_devices_added_or_updated", return_value=([], [])) as device_added_updated,\ + mock.patch.object(self.agent.int_br, "db_list", + return_value=[]),\ mock.patch.object(self.agent, "treat_devices_removed", return_value=False) as device_removed: @@ -1178,6 +1185,9 @@ class AncillaryBridgesTest(object): mock.patch('neutron.agent.common.ovs_lib.BaseOVS.' 'get_bridge_external_bridge_id', side_effect=pullup_side_effect),\ + mock.patch( + 'neutron.agent.common.ovs_lib.OVSBridge.' 'db_list', + return_value=[]),\ mock.patch( 'neutron.agent.common.ovs_lib.OVSBridge.' 'get_vif_ports', return_value=[]): @@ -1267,6 +1277,9 @@ class TestOvsDvrNeutronAgent(object): mock.patch('oslo_service.loopingcall.' 'FixedIntervalLoopingCall', new=MockFixedIntervalLoopingCall),\ + mock.patch( + 'neutron.agent.common.ovs_lib.OVSBridge.' 'db_list', + return_value=[]),\ mock.patch( 'neutron.agent.common.ovs_lib.OVSBridge.' 'get_vif_ports', return_value=[]): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 6973842aab6..e0c8df7ef66 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -121,6 +121,7 @@ class TunnelTest(object): self.mock_int_bridge.add_patch_port.side_effect = ( lambda tap, peer: self.ovs_int_ofports[tap]) self.mock_int_bridge.get_vif_ports.return_value = [] + self.mock_int_bridge.db_list.return_value = [] self.mock_int_bridge.db_get_val.return_value = {} self.mock_map_tun_bridge = self.ovs_bridges[self.MAP_TUN_BRIDGE] @@ -208,6 +209,7 @@ class TunnelTest(object): ] self.mock_int_bridge_expected += [ mock.call.get_vif_ports(), + mock.call.db_list('Port', columns=['name', 'other_config', 'tag']) ] self.mock_tun_bridge_expected += [ @@ -246,7 +248,7 @@ class TunnelTest(object): def _verify_mock_call(self, mock_obj, expected): mock_obj.assert_has_calls(expected) - self.assertEqual(len(mock_obj.mock_calls), len(expected)) + self.assertEqual(expected, mock_obj.mock_calls) def _verify_mock_calls(self): self._verify_mock_call(self.mock_int_bridge_cls, @@ -599,6 +601,7 @@ class TunnelTestUseVethInterco(TunnelTest): ] self.mock_int_bridge_expected += [ mock.call.get_vif_ports(), + mock.call.db_list('Port', columns=['name', 'other_config', 'tag']) ] self.mock_tun_bridge_expected += [ mock.call.delete_flows(),