Iterate over same port_id if more than one exists
In certain cases where multiple ports can have the same external_ids:iface_id property, the ovs agent will arbitrarily choose one and ignore the rest. In case the chosen port isn't on the integration bridge the ovs agent is managing, an error is returned to the calling function. This is faulty since one of the other ports may belong to the correct bridge and it should be chosen instead. This is interesting for future L3 HA integration tests, where 2 different instances of l3 agents are needed to run on the same machine. In this case, both agents will register ports which have the same iface_id property, but obviously only one of the ports is relevant for each agent. Closes-bug: #1370914 Related-bug: #1374947 Change-Id: I05d70417e0f78ae51a9ce377fc93a3f12046b313
This commit is contained in:
John Schwarz
parent
205162f580
commit
e90d6d6c83
|
|
@ -22,6 +22,7 @@ from neutron.agent.linux import ip_lib
|
|||
from neutron.agent.linux import utils
|
||||
from neutron.common import exceptions
|
||||
from neutron.openstack.common import excutils
|
||||
from neutron.openstack.common.gettextutils import _LI, _LW
|
||||
from neutron.openstack.common import jsonutils
|
||||
from neutron.openstack.common import log as logging
|
||||
from neutron.plugins.common import constants
|
||||
|
|
@ -401,29 +402,28 @@ class OVSBridge(BaseOVS):
|
|||
# an exeception which will be captured in this block.
|
||||
# We won't deal with the possibility of ovs-vsctl return multiple
|
||||
# rows since the interface identifier is unique
|
||||
data = json_result['data'][0]
|
||||
port_name = data[name_idx]
|
||||
switch = get_bridge_for_iface(self.root_helper, port_name)
|
||||
if switch != self.br_name:
|
||||
LOG.info(_("Port: %(port_name)s is on %(switch)s,"
|
||||
" not on %(br_name)s"), {'port_name': port_name,
|
||||
'switch': switch,
|
||||
'br_name': self.br_name})
|
||||
return
|
||||
ofport = data[ofport_idx]
|
||||
# ofport must be integer otherwise return None
|
||||
if not isinstance(ofport, int) or ofport == -1:
|
||||
LOG.warn(_("ofport: %(ofport)s for VIF: %(vif)s is not a "
|
||||
"positive integer"), {'ofport': ofport,
|
||||
'vif': port_id})
|
||||
return
|
||||
# Find VIF's mac address in external ids
|
||||
ext_id_dict = dict((item[0], item[1]) for item in
|
||||
data[ext_ids_idx][1])
|
||||
vif_mac = ext_id_dict['attached-mac']
|
||||
return VifPort(port_name, ofport, port_id, vif_mac, self)
|
||||
except Exception as e:
|
||||
LOG.warn(_("Unable to parse interface details. Exception: %s"), e)
|
||||
for data in json_result['data']:
|
||||
port_name = data[name_idx]
|
||||
switch = get_bridge_for_iface(self.root_helper, port_name)
|
||||
if switch != self.br_name:
|
||||
continue
|
||||
ofport = data[ofport_idx]
|
||||
# ofport must be integer otherwise return None
|
||||
if not isinstance(ofport, int) or ofport == -1:
|
||||
LOG.warn(_LW("ofport: %(ofport)s for VIF: %(vif)s is not a"
|
||||
" positive integer"), {'ofport': ofport,
|
||||
'vif': port_id})
|
||||
return
|
||||
# Find VIF's mac address in external ids
|
||||
ext_id_dict = dict((item[0], item[1]) for item in
|
||||
data[ext_ids_idx][1])
|
||||
vif_mac = ext_id_dict['attached-mac']
|
||||
return VifPort(port_name, ofport, port_id, vif_mac, self)
|
||||
LOG.info(_LI("Port %(port_id)s not present in bridge %(br_name)s"),
|
||||
{'port_id': port_id, 'br_name': self.br_name})
|
||||
except Exception as error:
|
||||
LOG.warn(_LW("Unable to parse interface details. Exception: %s"),
|
||||
error)
|
||||
return
|
||||
|
||||
def delete_ports(self, all_ports=False):
|
||||
|
|
|
|||
|
|
@ -822,8 +822,10 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
with testtools.ExpectedException(Exception):
|
||||
self.br.get_local_port_mac()
|
||||
|
||||
def _test_get_vif_port_by_id(self, iface_id, data, br_name=None):
|
||||
def _test_get_vif_port_by_id(self, iface_id, data, br_name=None,
|
||||
extra_calls_and_values=None):
|
||||
headings = ['external_ids', 'name', 'ofport']
|
||||
|
||||
# Each element is a tuple of (expected mock call, return_value)
|
||||
expected_calls_and_values = [
|
||||
(mock.call(["ovs-vsctl", self.TO, "--format=json",
|
||||
|
|
@ -836,9 +838,15 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
if not br_name:
|
||||
br_name = self.BR_NAME
|
||||
|
||||
# Only the last information list in 'data' is used, so if more
|
||||
# than one vif is described in data, the rest must be declared
|
||||
# in the argument 'expected_calls_and_values'.
|
||||
if extra_calls_and_values:
|
||||
expected_calls_and_values.extend(extra_calls_and_values)
|
||||
|
||||
expected_calls_and_values.append(
|
||||
(mock.call(["ovs-vsctl", self.TO,
|
||||
"iface-to-br", data[0][headings.index('name')]],
|
||||
"iface-to-br", data[-1][headings.index('name')]],
|
||||
root_helper=self.root_helper),
|
||||
br_name))
|
||||
tools.setup_mock_calls(self.execute, expected_calls_and_values)
|
||||
|
|
@ -847,6 +855,15 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
tools.verify_mock_calls(self.execute, expected_calls_and_values)
|
||||
return vif_port
|
||||
|
||||
def _assert_vif_port(self, vif_port, ofport=None, mac=None):
|
||||
if not ofport or ofport == -1 or not mac:
|
||||
self.assertIsNone(vif_port)
|
||||
return
|
||||
self.assertEqual('tap99id', vif_port.vif_id)
|
||||
self.assertEqual(mac, vif_port.vif_mac)
|
||||
self.assertEqual('tap99', vif_port.port_name)
|
||||
self.assertEqual(ofport, vif_port.ofport)
|
||||
|
||||
def _test_get_vif_port_by_id_with_data(self, ofport=None, mac=None):
|
||||
external_ids = [["iface-id", "tap99id"],
|
||||
["iface-status", "active"]]
|
||||
|
|
@ -855,13 +872,7 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
data = [[["map", external_ids], "tap99",
|
||||
ofport if ofport else '["set",[]]']]
|
||||
vif_port = self._test_get_vif_port_by_id('tap99id', data)
|
||||
if not ofport or ofport == -1 or not mac:
|
||||
self.assertIsNone(vif_port)
|
||||
return
|
||||
self.assertEqual(vif_port.vif_id, 'tap99id')
|
||||
self.assertEqual(vif_port.vif_mac, 'aa:bb:cc:dd:ee:ff')
|
||||
self.assertEqual(vif_port.port_name, 'tap99')
|
||||
self.assertEqual(vif_port.ofport, ofport)
|
||||
self._assert_vif_port(vif_port, ofport, mac)
|
||||
|
||||
def test_get_vif_by_port_id_with_ofport(self):
|
||||
self._test_get_vif_port_by_id_with_data(
|
||||
|
|
@ -887,6 +898,22 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
self.assertIsNone(self._test_get_vif_port_by_id('tap99id', data,
|
||||
"br-ext"))
|
||||
|
||||
def test_get_vif_by_port_id_multiple_vifs(self):
|
||||
external_ids = [["iface-id", "tap99id"],
|
||||
["iface-status", "active"],
|
||||
["attached-mac", "de:ad:be:ef:13:37"]]
|
||||
data = [[["map", external_ids], "dummytap", 1],
|
||||
[["map", external_ids], "tap99", 1337]]
|
||||
extra_calls_and_values = [
|
||||
(mock.call(["ovs-vsctl", self.TO,
|
||||
"iface-to-br", "dummytap"],
|
||||
root_helper=self.root_helper),
|
||||
"br-ext")]
|
||||
|
||||
vif_port = self._test_get_vif_port_by_id(
|
||||
'tap99id', data, extra_calls_and_values=extra_calls_and_values)
|
||||
self._assert_vif_port(vif_port, ofport=1337, mac="de:ad:be:ef:13:37")
|
||||
|
||||
|
||||
class TestDeferredOVSBridge(base.BaseTestCase):
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue