Parse JSON in ovs_lib.get_vif_port_by_id
This patch replaces regex matching of text output with parsing of JSON output in ovs_lib.get_vif_port_by_id. This makes the code more reliable as subtle, possibly even cosmetic, changes in ovs-vsctl output format could cause the regular expression match to fail. Also, this makes the code consistent with ovs_lib.get_vif_port_set which already uses JSON output. Finally this patch slightly changes the behaviour of ovs_lib.get_vif_port_by_id returning None if elements such as mac address or ofport were not available. Change-Id: Ia985a130739c72b5b88414a79b2c6083ca6a0a00 Closes-Bug: #1280827
This commit is contained in:
Salvatore Orlando
parent
6e8aebac95
commit
3d24fe5710
|
|
@ -109,23 +109,9 @@ class OVSBridge(BaseOVS):
|
|||
def __init__(self, br_name, root_helper):
|
||||
super(OVSBridge, self).__init__(root_helper)
|
||||
self.br_name = br_name
|
||||
self.re_id = self.re_compile_id()
|
||||
self.defer_apply_flows = False
|
||||
self.deferred_flows = {'add': '', 'mod': '', 'del': ''}
|
||||
|
||||
def re_compile_id(self):
|
||||
external = 'external_ids\s*'
|
||||
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+|\[\]))'
|
||||
_re = ('%(external)s:\s{ ( %(mac)s,? | %(iface)s,? | . )* }'
|
||||
' \s+ %(name)s \s+ %(port)s' % {'external': external,
|
||||
'mac': mac,
|
||||
'iface': iface, 'name': name,
|
||||
'port': port})
|
||||
return re.compile(_re, re.M | re.X)
|
||||
|
||||
def create(self):
|
||||
self.add_bridge(self.br_name)
|
||||
|
||||
|
|
@ -392,33 +378,39 @@ class OVSBridge(BaseOVS):
|
|||
return edge_ports
|
||||
|
||||
def get_vif_port_by_id(self, port_id):
|
||||
args = ['--', '--columns=external_ids,name,ofport',
|
||||
args = ['--format=json', '--', '--columns=external_ids,name,ofport',
|
||||
'find', 'Interface',
|
||||
'external_ids:iface-id="%s"' % 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)
|
||||
json_result = jsonutils.loads(result)
|
||||
try:
|
||||
vif_mac = match.group('vif_mac')
|
||||
vif_id = match.group('vif_id')
|
||||
port_name = match.group('port_name')
|
||||
# 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)
|
||||
# Retrieve the indexes of the columns we're looking for
|
||||
headings = json_result['headings']
|
||||
ext_ids_idx = headings.index('external_ids')
|
||||
name_idx = headings.index('name')
|
||||
ofport_idx = headings.index('ofport')
|
||||
# If data attribute is missing or empty the line below will raise
|
||||
# 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]
|
||||
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.info(_("Unable to parse regex results. Exception: %s"), e)
|
||||
LOG.warn(_("Unable to parse interface details. Exception: %s"), e)
|
||||
return
|
||||
|
||||
def delete_ports(self, all_ports=False):
|
||||
|
|
|
|||
|
|
@ -517,21 +517,6 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
["ovs-vsctl", self.TO, "clear", "Port", pname, "tag"],
|
||||
root_helper=self.root_helper)
|
||||
|
||||
def test_port_id_regex(self):
|
||||
result = ('external_ids : {attached-mac="fa:16:3e:23:5b:f2",'
|
||||
' iface-id="5c1321a7-c73f-4a77-95e6-9f86402e5c8f",'
|
||||
' iface-status=active}\nname :'
|
||||
' "dhc5c1321a7-c7"\nofport : 2\n')
|
||||
match = self.br.re_id.search(result)
|
||||
vif_mac = match.group('vif_mac')
|
||||
vif_id = match.group('vif_id')
|
||||
port_name = match.group('port_name')
|
||||
ofport = int(match.group('ofport'))
|
||||
self.assertEqual(vif_mac, 'fa:16:3e:23:5b:f2')
|
||||
self.assertEqual(vif_id, '5c1321a7-c73f-4a77-95e6-9f86402e5c8f')
|
||||
self.assertEqual(port_name, 'dhc5c1321a7-c7')
|
||||
self.assertEqual(ofport, 2)
|
||||
|
||||
def _test_iface_to_br(self, exp_timeout=None):
|
||||
iface = 'tap0'
|
||||
br = 'br-int'
|
||||
|
|
@ -623,35 +608,52 @@ class OVS_Lib_Test(base.BaseTestCase):
|
|||
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')
|
||||
|
||||
def _test_get_vif_port_by_id(self, iface_id, data):
|
||||
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,
|
||||
(mock.call(["ovs-vsctl", self.TO, "--format=json",
|
||||
"--", "--columns=external_ids,name,ofport",
|
||||
"find", "Interface",
|
||||
'external_ids:iface-id="tap99id"'],
|
||||
'external_ids:iface-id="%s"' % iface_id],
|
||||
root_helper=self.root_helper),
|
||||
expected_output % {'ofport': ofport or '[]'}),
|
||||
]
|
||||
tools.setup_mock_calls(self.execute, expected_calls_and_values)
|
||||
self._encode_ovs_json(headings, data))]
|
||||
|
||||
tools.setup_mock_calls(self.execute, expected_calls_and_values)
|
||||
vif_port = self.br.get_vif_port_by_id(iface_id)
|
||||
|
||||
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_port_by_id_with_data(self, ofport=None, mac=None):
|
||||
external_ids = [["iface-id", "tap99id"],
|
||||
["iface-status", "active"]]
|
||||
if mac:
|
||||
external_ids.append(["attached-mac", mac])
|
||||
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)
|
||||
|
||||
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)
|
||||
self._test_get_vif_port_by_id_with_data(
|
||||
ofport=1, mac="aa:bb:cc:dd:ee:ff")
|
||||
|
||||
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._test_get_vif_port_by_id_with_data(mac="aa:bb:cc:dd:ee:ff")
|
||||
|
||||
def test_get_vif_by_port_id_with_invalid_ofport(self):
|
||||
self._test_get_vif_port_by_id_with_data(
|
||||
ofport=-1, mac="aa:bb:cc:dd:ee:ff")
|
||||
|
||||
def test_get_vif_by_port_id_without_mac(self):
|
||||
self._test_get_vif_port_by_id_with_data(ofport=1)
|
||||
|
||||
def test_get_vif_by_port_id_with_no_data(self):
|
||||
self.assertIsNone(self._test_get_vif_port_by_id('whatever', []))
|
||||
|
|
|
|||
Loading…
Reference in New Issue