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,
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:
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):
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)
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,
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):
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,
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
{'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})
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
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):
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)
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)),
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()),
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)
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):
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(
Code Review / openstack-build / neutron-build.git / commitdiff