functional: fix OVSFW failure with native OVSDB api
A bunch of functional tests fail because of non implemented x != [] operation in idlutils.condition_match() and wrong condition passed to db_find() in OVSFW test. This patch addresses the issue by implementing lists comparison in native.idlutils and fixing the call to db_find() in OVSFW test. A functional test for OVSDB API's db_find() has been added to ensure that querying a list column gives the same result both with vsctl and native ovsdb_interface; unit test for idlutils.condition_match() with corner cases has been added as well. Change-Id: Ia93fb925b8814210975904a453249f15f3646855 Closes-bug: #1578233
This commit is contained in:
Inessa Vasilevskaya
parent
bb87e58dab
commit
bffc5f062c
|
|
@ -258,6 +258,8 @@ class API(object):
|
|||
:type table: string
|
||||
:param conditions:The conditions to satisfy the query
|
||||
:type conditions: 3-tuples containing (column, operation, match)
|
||||
Type of 'match' parameter MUST be identical to column
|
||||
type
|
||||
Examples:
|
||||
atomic: ('tag', '=', 7)
|
||||
map: ('external_ids' '=', {'iface-id': 'xxx'})
|
||||
|
|
|
|||
|
|
@ -126,6 +126,11 @@ def wait_for_change(_idl, timeout, seqno=None):
|
|||
|
||||
|
||||
def get_column_value(row, col):
|
||||
"""Retrieve column value from the given row.
|
||||
|
||||
If column's type is optional, the value will be returned as a single
|
||||
element instead of a list of length 1.
|
||||
"""
|
||||
if col == '_uuid':
|
||||
val = row.uuid
|
||||
else:
|
||||
|
|
@ -135,8 +140,9 @@ def get_column_value(row, col):
|
|||
if isinstance(val, list) and len(val):
|
||||
if isinstance(val[0], idl.Row):
|
||||
val = [v.uuid for v in val]
|
||||
col_type = row._table.columns[col].type
|
||||
# ovs-vsctl treats lists of 1 as single results
|
||||
if len(val) == 1:
|
||||
if col_type.is_optional():
|
||||
val = val[0]
|
||||
return val
|
||||
|
||||
|
|
@ -147,9 +153,28 @@ def condition_match(row, condition):
|
|||
:param row: An OVSDB Row
|
||||
:param condition: A 3-tuple containing (column, operation, match)
|
||||
"""
|
||||
|
||||
col, op, match = condition
|
||||
val = get_column_value(row, col)
|
||||
|
||||
# both match and val are primitive types, so type can be used for type
|
||||
# equality here.
|
||||
if type(match) is not type(val):
|
||||
# Types of 'val' and 'match' arguments MUST match in all cases with 2
|
||||
# exceptions:
|
||||
# - 'match' is an empty list and column's type is optional;
|
||||
# - 'value' is an empty and column's type is optional
|
||||
if (not all([match, val]) and
|
||||
row._table.columns[col].type.is_optional()):
|
||||
# utilize the single elements comparison logic
|
||||
if match == []:
|
||||
match = None
|
||||
elif val == []:
|
||||
val = None
|
||||
else:
|
||||
# no need to process any further
|
||||
raise ValueError(
|
||||
_("Column type and condition operand do not match"))
|
||||
|
||||
matched = True
|
||||
|
||||
# TODO(twilson) Implement other operators and type comparisons
|
||||
|
|
@ -167,7 +192,25 @@ def condition_match(row, condition):
|
|||
else:
|
||||
raise NotImplementedError()
|
||||
elif isinstance(match, list):
|
||||
raise NotImplementedError()
|
||||
# According to rfc7047, lists support '=' and '!='
|
||||
# (both strict and relaxed). Will follow twilson's dict comparison
|
||||
# and implement relaxed version (excludes/includes as per standart)
|
||||
if op == "=":
|
||||
if not all([val, match]):
|
||||
return val == match
|
||||
for elem in set(match):
|
||||
if elem not in val:
|
||||
matched = False
|
||||
break
|
||||
elif op == '!=':
|
||||
if not all([val, match]):
|
||||
return val != match
|
||||
for elem in set(match):
|
||||
if elem in val:
|
||||
matched = False
|
||||
break
|
||||
else:
|
||||
raise NotImplementedError()
|
||||
else:
|
||||
if op == '=':
|
||||
if val != match:
|
||||
|
|
|
|||
|
|
@ -140,9 +140,8 @@ class BaseFirewallTestCase(base.BaseSudoTestCase):
|
|||
|
||||
def get_not_used_vlan(self):
|
||||
port_vlans = self.firewall.int_br.br.ovsdb.db_find(
|
||||
'Port', ('tag', '!=', '[]'), columns=['tag']).execute()
|
||||
# TODO(jlibosva): Remove 'if val' once bug/1578233 is solved
|
||||
used_vlan_tags = {val['tag'] for val in port_vlans if val['tag']}
|
||||
'Port', ('tag', '!=', []), columns=['tag']).execute()
|
||||
used_vlan_tags = {val['tag'] for val in port_vlans}
|
||||
available_vlans = self.vlan_range - used_vlan_tags
|
||||
return random.choice(list(available_vlans))
|
||||
|
||||
|
|
|
|||
|
|
@ -367,3 +367,29 @@ class OVSLibTestCase(base.BaseOVSLinuxTestCase):
|
|||
self.assertTrue(self.ovs.bridge_exists(name))
|
||||
br.destroy()
|
||||
self.assertFalse(self.ovs.bridge_exists(name))
|
||||
|
||||
def test_db_find_column_type_list(self):
|
||||
"""Fixate output for vsctl/native ovsdb_interface.
|
||||
|
||||
Makes sure that db_find search queries give the same result for both
|
||||
implementations.
|
||||
"""
|
||||
bridge_name = base.get_rand_name(prefix=net_helpers.BR_PREFIX)
|
||||
self.addCleanup(self.ovs.delete_bridge, bridge_name)
|
||||
br = self.ovs.add_bridge(bridge_name)
|
||||
port_name = base.get_rand_name(prefix=net_helpers.PORT_PREFIX)
|
||||
br.add_port(port_name)
|
||||
self.ovs.set_db_attribute('Port', port_name, 'tag', 42)
|
||||
tags = self.ovs.ovsdb.db_list('Port', columns=['tag']).execute()
|
||||
# Make sure that there is data to query.
|
||||
# It should be, but let's be a little paranoid here as otherwise
|
||||
# the test has no sense
|
||||
tags_present = [t for t in tags if t['tag'] != []]
|
||||
self.assertTrue(tags_present)
|
||||
tags_42 = [t for t in tags_present if t['tag'] == 42]
|
||||
single_value = self.ovs.ovsdb.db_find(
|
||||
'Port', ('tag', '=', 42), columns=['tag']).execute()
|
||||
self.assertEqual(tags_42, single_value)
|
||||
len_0_list = self.ovs.ovsdb.db_find(
|
||||
'Port', ('tag', '!=', []), columns=['tag']).execute()
|
||||
self.assertEqual(tags_present, len_0_list)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,100 @@
|
|||
# Copyright 2016, Mirantis Inc.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
|
||||
from neutron.agent.ovsdb.native import idlutils
|
||||
from neutron.tests import base
|
||||
|
||||
|
||||
class MockColumn(object):
|
||||
def __init__(self, name, type, is_optional=False, test_value=None):
|
||||
self.name = name
|
||||
self.type = mock.MagicMock(
|
||||
**{"key.type.name": type,
|
||||
"is_optional": mock.Mock(return_value=is_optional),
|
||||
})
|
||||
# for test purposes only to operate with some values in condition_match
|
||||
# testcase
|
||||
self.test_value = test_value
|
||||
|
||||
|
||||
class MockTable(object):
|
||||
def __init__(self, name, *columns):
|
||||
# columns is a list of tuples (col_name, col_type)
|
||||
self.name = name
|
||||
self.columns = {c.name: c for c in columns}
|
||||
|
||||
|
||||
class MockRow(object):
|
||||
def __init__(self, table):
|
||||
self._table = table
|
||||
|
||||
def __getattr__(self, attr):
|
||||
if attr in self._table.columns:
|
||||
return self._table.columns[attr].test_value
|
||||
return super(MockRow, self).__getattr__(attr)
|
||||
|
||||
|
||||
class TestIdlUtils(base.BaseTestCase):
|
||||
def test_condition_match(self):
|
||||
"""
|
||||
Make sure that the function respects the following:
|
||||
* if column type is_optional and value is a single element, value is
|
||||
transformed to a length-1-list
|
||||
* any other value is returned as it is, no type convertions
|
||||
"""
|
||||
table = MockTable("SomeTable",
|
||||
MockColumn("tag", "integer", is_optional=True,
|
||||
test_value=[42]),
|
||||
MockColumn("num", "integer", is_optional=True,
|
||||
test_value=[]),
|
||||
MockColumn("ids", "integer", is_optional=False,
|
||||
test_value=42),
|
||||
MockColumn("comments", "string",
|
||||
test_value=["a", "b", "c"]),
|
||||
MockColumn("status", "string",
|
||||
test_value="sorry for inconvenience"))
|
||||
row = MockRow(table=table)
|
||||
self.assertTrue(idlutils.condition_match(row, ("tag", "=", 42)))
|
||||
# optional types can be compared only as single elements
|
||||
self.assertRaises(ValueError,
|
||||
idlutils.condition_match, row, ("tag", "!=", [42]))
|
||||
# empty list comparison is ok for optional types though
|
||||
self.assertTrue(idlutils.condition_match(row, ("tag", "!=", [])))
|
||||
self.assertTrue(idlutils.condition_match(row, ("num", "=", [])))
|
||||
# value = [] may be compared to a single elem if optional column type
|
||||
self.assertTrue(idlutils.condition_match(row, ("num", "!=", 42)))
|
||||
# no type conversion for non optional types
|
||||
self.assertTrue(idlutils.condition_match(row, ("ids", "=", 42)))
|
||||
self.assertTrue(idlutils.condition_match(
|
||||
row, ("status", "=", "sorry for inconvenience")))
|
||||
self.assertFalse(idlutils.condition_match(
|
||||
row, ("status", "=", "sorry")))
|
||||
# bad types
|
||||
self.assertRaises(ValueError,
|
||||
idlutils.condition_match, row, ("ids", "=", "42"))
|
||||
self.assertRaises(ValueError,
|
||||
idlutils.condition_match, row, ("ids", "!=", "42"))
|
||||
self.assertRaises(ValueError,
|
||||
idlutils.condition_match, row,
|
||||
("ids", "!=", {"a": "b"}))
|
||||
# non optional list types are kept as they are
|
||||
self.assertTrue(idlutils.condition_match(
|
||||
row, ("comments", "=", ["c", "b", "a"])))
|
||||
# also true because list comparison is relaxed
|
||||
self.assertTrue(idlutils.condition_match(
|
||||
row, ("comments", "=", ["c", "b"])))
|
||||
self.assertTrue(idlutils.condition_match(
|
||||
row, ("comments", "!=", ["d"])))
|
||||
Loading…
Reference in New Issue