Merge "Avoid processing ports which are not yet ready"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
2343e47d94
@ -118,7 +118,7 @@ class OVSBridge(BaseOVS):
|
||||
mac = 'attached-mac="(?P<vif_mac>([a-fA-F\d]{2}:){5}([a-fA-F\d]{2}))"'
|
||||
iface = 'iface-id="(?P<vif_id>[^"]+)"'
|
||||
name = 'name\s*:\s"(?P<port_name>[^"]*)"'
|
||||
port = 'ofport\s*:\s(?P<ofport>-?\d+)'
|
||||
port = 'ofport\s*:\s(?P<ofport>(-?\d+|\[\]))'
|
||||
_re = ('%(external)s:\s{ ( %(mac)s,? | %(iface)s,? | . )* }'
|
||||
' \s+ %(name)s \s+ %(port)s' % {'external': external,
|
||||
'mac': mac,
|
||||
@ -356,7 +356,7 @@ class OVSBridge(BaseOVS):
|
||||
def get_vif_port_set(self):
|
||||
port_names = self.get_port_name_list()
|
||||
edge_ports = set()
|
||||
args = ['--format=json', '--', '--columns=name,external_ids',
|
||||
args = ['--format=json', '--', '--columns=name,external_ids,ofport',
|
||||
'list', 'Interface']
|
||||
result = self.run_vsctl(args, check_error=True)
|
||||
if not result:
|
||||
@ -366,14 +366,29 @@ class OVSBridge(BaseOVS):
|
||||
if name not in port_names:
|
||||
continue
|
||||
external_ids = dict(row[1][1])
|
||||
if "iface-id" in external_ids and "attached-mac" in external_ids:
|
||||
edge_ports.add(external_ids['iface-id'])
|
||||
elif ("xs-vif-uuid" in external_ids and
|
||||
"attached-mac" in external_ids):
|
||||
# if this is a xenserver and iface-id is not automatically
|
||||
# synced to OVS from XAPI, we grab it from XAPI directly
|
||||
iface_id = self.get_xapi_iface_id(external_ids["xs-vif-uuid"])
|
||||
edge_ports.add(iface_id)
|
||||
# Do not consider VIFs which aren't yet ready
|
||||
# This can happen when ofport values are either [] or ["set", []]
|
||||
# We will therefore consider only integer values for ofport
|
||||
ofport = row[2]
|
||||
try:
|
||||
int_ofport = int(ofport)
|
||||
except (ValueError, TypeError):
|
||||
LOG.warn(_("Found not yet ready openvswitch port: %s"), row)
|
||||
else:
|
||||
if int_ofport > 0:
|
||||
if ("iface-id" in external_ids and
|
||||
"attached-mac" in external_ids):
|
||||
edge_ports.add(external_ids['iface-id'])
|
||||
elif ("xs-vif-uuid" in external_ids and
|
||||
"attached-mac" in external_ids):
|
||||
# if this is a xenserver and iface-id is not
|
||||
# automatically synced to OVS from XAPI, we grab it
|
||||
# from XAPI directly
|
||||
iface_id = self.get_xapi_iface_id(
|
||||
external_ids["xs-vif-uuid"])
|
||||
edge_ports.add(iface_id)
|
||||
else:
|
||||
LOG.warn(_("Found failed openvswitch port: %s"), row)
|
||||
return edge_ports
|
||||
|
||||
def get_vif_port_by_id(self, port_id):
|
||||
@ -383,12 +398,24 @@ class OVSBridge(BaseOVS):
|
||||
result = self.run_vsctl(args)
|
||||
if not result:
|
||||
return
|
||||
# TODO(salv-orlando): consider whether it would be possible to use
|
||||
# JSON formatting rather than doing regex parsing.
|
||||
match = self.re_id.search(result)
|
||||
try:
|
||||
vif_mac = match.group('vif_mac')
|
||||
vif_id = match.group('vif_id')
|
||||
port_name = match.group('port_name')
|
||||
ofport = int(match.group('ofport'))
|
||||
# Tolerate ports which might not have an ofport as they are not
|
||||
# ready yet
|
||||
# NOTE(salv-orlando): Consider returning None when ofport is not
|
||||
# available.
|
||||
try:
|
||||
ofport = int(match.group('ofport'))
|
||||
except ValueError:
|
||||
LOG.warn(_("ofport for vif: %s is not a valid integer. "
|
||||
"The port has not yet been configured by OVS"),
|
||||
vif_id)
|
||||
ofport = None
|
||||
return VifPort(port_name, ofport, vif_id, vif_mac, self)
|
||||
except Exception as e:
|
||||
LOG.info(_("Unable to parse regex results. Exception: %s"), e)
|
||||
|
||||
@ -72,7 +72,7 @@ class SimpleInterfaceMonitor(OvsdbMonitor):
|
||||
def __init__(self, root_helper=None, respawn_interval=None):
|
||||
super(SimpleInterfaceMonitor, self).__init__(
|
||||
'Interface',
|
||||
columns=['name'],
|
||||
columns=['name', 'ofport'],
|
||||
format='json',
|
||||
root_helper=root_helper,
|
||||
respawn_interval=respawn_interval,
|
||||
|
||||
@ -599,7 +599,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||
if cur_tag != str(lvm.vlan):
|
||||
self.int_br.set_db_attribute("Port", port.port_name, "tag",
|
||||
str(lvm.vlan))
|
||||
if int(port.ofport) != -1:
|
||||
if port.ofport != -1:
|
||||
self.int_br.delete_flows(in_port=port.ofport)
|
||||
|
||||
def port_unbound(self, vif_id, net_uuid=None):
|
||||
@ -854,6 +854,13 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||
|
||||
def treat_vif_port(self, vif_port, port_id, network_id, network_type,
|
||||
physical_network, segmentation_id, admin_state_up):
|
||||
# When this function is called for a port, the port should have
|
||||
# an OVS ofport configured, as only these ports were considered
|
||||
# for being treated. If that does not happen, it is a potential
|
||||
# error condition of which operators should be aware
|
||||
if not vif_port.ofport:
|
||||
LOG.warn(_("VIF port: %s has no ofport configured, and might not "
|
||||
"be able to transmit"), vif_port.vif_id)
|
||||
if vif_port:
|
||||
if admin_state_up:
|
||||
self.port_bound(vif_port, network_id, network_type,
|
||||
@ -918,7 +925,15 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||
def treat_devices_added_or_updated(self, devices):
|
||||
resync = False
|
||||
for device in devices:
|
||||
LOG.debug(_("Processing port:%s"), device)
|
||||
LOG.debug(_("Processing port %s"), device)
|
||||
port = self.int_br.get_vif_port_by_id(device)
|
||||
if not port:
|
||||
# The port has disappeared and should not be processed
|
||||
# There is no need to put the port DOWN in the plugin as
|
||||
# it never went up in the first place
|
||||
LOG.info(_("Port %s was not found on the integration bridge "
|
||||
"and will therefore not be processed"), device)
|
||||
continue
|
||||
try:
|
||||
# TODO(salv-orlando): Provide bulk API for retrieving
|
||||
# details for all devices in one call
|
||||
@ -931,7 +946,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||
{'device': device, 'e': e})
|
||||
resync = True
|
||||
continue
|
||||
port = self.int_br.get_vif_port_by_id(details['device'])
|
||||
if 'port_id' in details:
|
||||
LOG.info(_("Port %(device)s updated. Details: %(details)s"),
|
||||
{'device': device, 'details': details})
|
||||
@ -950,9 +964,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||
LOG.debug(_("Setting status for %s to DOWN"), device)
|
||||
self.plugin_rpc.update_device_down(
|
||||
self.context, device, self.agent_id, cfg.CONF.host)
|
||||
LOG.info(_("Configuration for device %s completed."), device)
|
||||
else:
|
||||
LOG.debug(_("Device %s not defined on plugin"), device)
|
||||
if (port and int(port.ofport) != -1):
|
||||
LOG.warn(_("Device %s not defined on plugin"), device)
|
||||
if (port and port.ofport != -1):
|
||||
self.port_dead(port)
|
||||
return resync
|
||||
|
||||
|
||||
@ -408,12 +408,13 @@ class OVS_Lib_Test(base.BaseTestCase):
|
||||
ovs_row = []
|
||||
r["data"].append(ovs_row)
|
||||
for cell in row:
|
||||
if isinstance(cell, str):
|
||||
if isinstance(cell, (str, int, list)):
|
||||
ovs_row.append(cell)
|
||||
elif isinstance(cell, dict):
|
||||
ovs_row.append(["map", cell.items()])
|
||||
else:
|
||||
raise TypeError('%r not str or dict' % type(cell))
|
||||
raise TypeError('%r not int, str, list or dict' %
|
||||
type(cell))
|
||||
return jsonutils.dumps(r)
|
||||
|
||||
def _test_get_vif_port_set(self, is_xen):
|
||||
@ -425,11 +426,17 @@ class OVS_Lib_Test(base.BaseTestCase):
|
||||
headings = ['name', 'external_ids']
|
||||
data = [
|
||||
# A vif port on this bridge:
|
||||
['tap99', {id_key: 'tap99id', 'attached-mac': 'tap99mac'}],
|
||||
['tap99', {id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1],
|
||||
# A vif port on this bridge not yet configured
|
||||
['tap98', {id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []],
|
||||
# Another vif port on this bridge not yet configured
|
||||
['tap97', {id_key: 'tap97id', 'attached-mac': 'tap97mac'},
|
||||
['set', []]],
|
||||
|
||||
# A vif port on another bridge:
|
||||
['tap88', {id_key: 'tap88id', 'attached-mac': 'tap88id'}],
|
||||
['tap88', {id_key: 'tap88id', 'attached-mac': 'tap88id'}, 1],
|
||||
# Non-vif port on this bridge:
|
||||
['tun22', {}],
|
||||
['tun22', {}, 2],
|
||||
]
|
||||
|
||||
# Each element is a tuple of (expected mock call, return_value)
|
||||
@ -438,7 +445,7 @@ class OVS_Lib_Test(base.BaseTestCase):
|
||||
root_helper=self.root_helper),
|
||||
'tap99\ntun22'),
|
||||
(mock.call(["ovs-vsctl", self.TO, "--format=json",
|
||||
"--", "--columns=name,external_ids",
|
||||
"--", "--columns=name,external_ids,ofport",
|
||||
"list", "Interface"],
|
||||
root_helper=self.root_helper),
|
||||
self._encode_ovs_json(headings, data)),
|
||||
@ -494,7 +501,7 @@ class OVS_Lib_Test(base.BaseTestCase):
|
||||
root_helper=self.root_helper),
|
||||
'tap99\n'),
|
||||
(mock.call(["ovs-vsctl", self.TO, "--format=json",
|
||||
"--", "--columns=name,external_ids",
|
||||
"--", "--columns=name,external_ids,ofport",
|
||||
"list", "Interface"],
|
||||
root_helper=self.root_helper),
|
||||
RuntimeError()),
|
||||
@ -615,3 +622,36 @@ class OVS_Lib_Test(base.BaseTestCase):
|
||||
return_value=mock.Mock(address=None)):
|
||||
with testtools.ExpectedException(Exception):
|
||||
self.br.get_local_port_mac()
|
||||
|
||||
def _test_get_vif_port_by_id(self, ofport=None):
|
||||
expected_output = ('external_ids : {attached-mac="aa:bb:cc:dd:ee:ff", '
|
||||
'iface-id="tap99id",'
|
||||
'iface-status=active, '
|
||||
'vm-uuid="tap99vm"}'
|
||||
'\nname : "tap99"\nofport : %(ofport)s\n')
|
||||
|
||||
# Each element is a tuple of (expected mock call, return_value)
|
||||
expected_calls_and_values = [
|
||||
(mock.call(["ovs-vsctl", self.TO,
|
||||
"--", "--columns=external_ids,name,ofport",
|
||||
"find", "Interface",
|
||||
'external_ids:iface-id="tap99id"'],
|
||||
root_helper=self.root_helper),
|
||||
expected_output % {'ofport': ofport or '[]'}),
|
||||
]
|
||||
tools.setup_mock_calls(self.execute, expected_calls_and_values)
|
||||
|
||||
vif_port = self.br.get_vif_port_by_id('tap99id')
|
||||
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')
|
||||
tools.verify_mock_calls(self.execute, expected_calls_and_values)
|
||||
return vif_port
|
||||
|
||||
def test_get_vif_by_port_id_with_ofport(self):
|
||||
vif_port = self._test_get_vif_port_by_id('1')
|
||||
self.assertEqual(vif_port.ofport, 1)
|
||||
|
||||
def test_get_vif_by_port_id_without_ofport(self):
|
||||
vif_port = self._test_get_vif_port_by_id()
|
||||
self.assertIsNone(vif_port.ofport)
|
||||
|
||||
@ -248,8 +248,11 @@ class TestOvsNeutronAgent(base.BaseTestCase):
|
||||
self.assertEqual(expected, actual)
|
||||
|
||||
def test_treat_devices_added_returns_true_for_missing_device(self):
|
||||
with mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
|
||||
side_effect=Exception()):
|
||||
with contextlib.nested(
|
||||
mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
|
||||
side_effect=Exception()),
|
||||
mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
|
||||
return_value=mock.Mock())):
|
||||
self.assertTrue(self.agent.treat_devices_added_or_updated([{}]))
|
||||
|
||||
def _mock_treat_devices_added_updated(self, details, port, func_name):
|
||||
@ -284,7 +287,15 @@ class TestOvsNeutronAgent(base.BaseTestCase):
|
||||
self.assertTrue(self._mock_treat_devices_added_updated(
|
||||
mock.MagicMock(), port, 'port_dead'))
|
||||
|
||||
def test_treat_devices_added_updated_updates_known_port(self):
|
||||
def test_treat_devices_added_does_not_process_missing_port(self):
|
||||
with contextlib.nested(
|
||||
mock.patch.object(self.agent.plugin_rpc, 'get_device_details'),
|
||||
mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
|
||||
return_value=None)
|
||||
) as (get_dev_fn, get_vif_func):
|
||||
self.assertFalse(get_dev_fn.called)
|
||||
|
||||
def test_treat_devices_added__updated_updates_known_port(self):
|
||||
details = mock.MagicMock()
|
||||
details.__contains__.side_effect = lambda x: True
|
||||
self.assertTrue(self._mock_treat_devices_added_updated(
|
||||
|
||||
Loading…
Reference in New Issue
Block a user