openstack
/
ovsdbapp
Code Issues Proposed changes

Update QoS register in "QoSAddCommand" if exists 

"QoSAddCommand" can return an existing QoS register if "may_exist" is
enabled. Now this register can be updated if the arguments passed to
the creation method differ.

This patch also adds the ability to retrieve a QoS rule by its
"external_ids".

Related-Bug: #1947334
Change-Id: I21b6e8dd603847b9a757799f24b8a7dd63d76a36
(cherry picked from commit 3ad4a1db97)
This commit is contained in:
Rodolfo Alonso Hernandez 2021-12-17 15:03:49 +00:00
parent 62ee947412
commit 4d4402ce06
3 changed files with 130 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
ovsdbapp/schema/ovn_northbound/commands.py
View File

@ -218,7 +218,8 @@ class QoSAddCommand(cmd.AddCommand):
table_name = 'QoS'
def __init__(self, api, switch, direction, priority, match, rate=None,
burst=None, dscp=None, may_exist=False, **columns):
burst=None, dscp=None, external_ids_match=None,
may_exist=False, **columns):
if direction not in ('from-lport', 'to-lport'):
raise TypeError("direction must be either from-lport or to-lport")
if not 0 <= priority <= const.ACL_PRIORITY_MAX:
@ -235,6 +236,9 @@ class QoSAddCommand(cmd.AddCommand):
dscp, const.QOS_DSCP_MAX)
if rate is None and dscp is None:
raise ValueError("One of the rate or dscp must be configured")
if (external_ids_match is not None and
not isinstance(external_ids_match, dict)):
raise ValueError("external_ids_match must be None or a dictionary")
super().__init__(api)
self.switch = switch
self.direction = direction
@ -245,8 +249,12 @@ class QoSAddCommand(cmd.AddCommand):
self.dscp = dscp
self.may_exist = may_exist
self.columns = columns
self.external_ids_match = external_ids_match
def qos_match(self, row):
if self.external_ids_match:
return self.external_ids_match.items() <= row.external_ids.items()
return (self.direction == row.direction and
self.priority == row.priority and
self.match == row.match)
@ -255,10 +263,13 @@ class QoSAddCommand(cmd.AddCommand):
ls = self.api.lookup('Logical_Switch', self.switch)
for qos_rule in iter(r for r in ls.qos_rules if self.qos_match(r)):
if self.may_exist:
self.result = rowview.RowView(qos_rule)
return
return self._update_qos(qos_rule)
raise RuntimeError("QoS (%s, %s, %s) already exists" % (
self.direction, self.priority, self.match))
self._create_qos(txn, ls)
def _create_qos(self, txn, ls):
row = txn.insert(self.api.tables[self.table_name])
row.direction = self.direction
row.priority = self.priority
@ -273,6 +284,30 @@ class QoSAddCommand(cmd.AddCommand):
ls.addvalue('qos_rules', row)
self.result = row.uuid
def _update_qos(self, qos_rule):
qos_rule.direction = self.direction
qos_rule.priority = self.priority
qos_rule.match = self.match
if self.rate:
qos_rule.setkey('bandwidth', 'rate', self.rate)
if self.burst:
qos_rule.setkey('bandwidth', 'burst', self.burst)
else:
qos_rule.delkey('bandwidth', 'burst')
else:
qos_rule.delkey('bandwidth', 'rate')
qos_rule.delkey('bandwidth', 'burst')
if self.dscp:
qos_rule.setkey('action', 'dscp', self.dscp)
else:
qos_rule.delkey('action', 'dscp')
for column, value in self.columns.items():
if getattr(qos_rule, column) != value:
self.set_column(qos_rule, column, value)
self.result = qos_rule.uuid
class QoSDelCommand(cmd.BaseCommand):
def __init__(self, api, switch, direction=None,

6
ovsdbapp/schema/ovn_northbound/impl_idl.py
View File

@ -81,9 +81,11 @@ class OvnNbApiIdlImpl(ovs_idl.Backend, api.API):
return cmd.PgAclListCommand(self, port_group)
def qos_add(self, switch, direction, priority, match, rate=None,
burst=None, dscp=None, may_exist=False, **columns):
burst=None, dscp=None, external_ids_match=None,
may_exist=False, **columns):
return cmd.QoSAddCommand(self, switch, direction, priority, match,
rate, burst, dscp, may_exist, **columns)
rate, burst, dscp, external_ids_match,
may_exist, **columns)
def qos_del(self, switch, direction=None, priority=None, match=None,
if_exists=True):

95
ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py
View File

@ -12,6 +12,7 @@
import netaddr
import testscenarios
import uuid
from ovsdbapp.backend.ovs_idl import idlutils
from ovsdbapp import constants as const
@ -252,7 +253,7 @@ class TestQoSOps(OvnNorthboundTest):
self.assertEqual(cmd.rate, row.bandwidth.get('rate', None))
self.assertEqual(cmd.burst, row.bandwidth.get('burst', None))
self.assertEqual(cmd.dscp, row.action.get('dscp', None))
return row
return idlutils.frozen_row(row)
def test_qos_add_dscp(self):
self._qos_add('from-lport', 0, 'output == "fake_port" && ip', dscp=33)
@ -287,10 +288,81 @@ class TestQoSOps(OvnNorthboundTest):
def test_qos_add_may_exist(self):
args = ('from-lport', 0, 'output == "fake_port" && ip', 1000)
row = self._qos_add(*args)
row2 = self._qos_add(*args, may_exist=True)
row = self._qos_add(*args, external_ids={'port_id': '1'})
row2 = self._qos_add(*args, external_ids={'port_id': '1'},
may_exist=True)
self.assertEqual(row, row2)
def test_qos_add_may_exist_update_using_external_ids_match(self):
args = ('from-lport', 0, 'output == "fake_port" && ip')
kwargs = {'rate': 1000, 'burst': 800, 'dscp': 16,
'external_ids': {'port_id': '1'}}
row = self._qos_add(*args, **kwargs)
# Update QoS parameters: rate, burst and DSCP.
kwargs = {'rate': 1200, 'burst': 900, 'dscp': 24}
row2 = self._qos_add(*args, external_ids_match={'port_id': '1'},
may_exist=True, **kwargs)
self.assertEqual(row.uuid, row2.uuid)
self.assertEqual(row2.bandwidth, {'rate': 1200, 'burst': 900})
self.assertEqual(row2.action, {'dscp': 24})
# Remove QoS parameters.
kwargs = {'rate': 1500, 'burst': 1100}
row3 = self._qos_add(*args, external_ids_match={'port_id': '1'},
may_exist=True, **kwargs)
self.assertEqual(row.uuid, row3.uuid)
self.assertEqual(row3.bandwidth, {'rate': 1500, 'burst': 1100})
self.assertEqual(row3.action, {})
kwargs = {'rate': 2000}
row4 = self._qos_add(*args, external_ids_match={'port_id': '1'},
may_exist=True, **kwargs)
self.assertEqual(row.uuid, row4.uuid)
self.assertEqual(row4.bandwidth, {'rate': 2000})
self.assertEqual(row4.action, {})
kwargs = {'dscp': 16}
row5 = self._qos_add(*args, external_ids_match={'port_id': '1'},
may_exist=True, **kwargs)
self.assertEqual(row.uuid, row5.uuid)
self.assertEqual(row5.bandwidth, {})
self.assertEqual(row5.action, {'dscp': 16})
def test_qos_add_may_exist_using_external_ids_match(self):
_uuid = str(uuid.uuid4())
_uuid2 = str(uuid.uuid4())
for key in ('neutron:port_id', 'neutron:fip_id'):
args = ('from-lport', 0, 'output == "fake_port" && ip')
kwargs = {'rate': 1000, 'burst': 800, 'dscp': 16}
self._qos_add(*args, external_ids={key: _uuid}, **kwargs)
# "args" in this second call are different, "QoSAddCommand" will
# use the "external_ids_match" reference passed instead to match
# the QoS rules.
args = ('from-lport', 1, 'output == "fake_port" && ip')
self._qos_add(*args, external_ids_match={key: _uuid},
may_exist=True, **kwargs)
qos_rules = self.api.qos_list(self.switch.uuid).execute(
check_error=True)
self.assertEqual(1, len(qos_rules))
# This call will update the "external_ids" to "_uuid2". Before
# changing it, "_uuid" will be used to find the register.
self._qos_add(*args, external_ids_match={key: _uuid},
external_ids={key: _uuid2}, may_exist=True, **kwargs)
qos_rules = self.api.qos_list(self.switch.uuid).execute(
check_error=True)
self.assertEqual(1, len(qos_rules))
# The deletion call uses "_uuid2" because it was changed in the
# previous call.
self.api.qos_del_ext_ids(self.switch.name,
{key: _uuid2}).execute(check_error=True)
qos_rules = self.api.qos_list(self.switch.uuid).execute(
check_error=True)
self.assertEqual(0, len(qos_rules))
def test_qos_add_extids(self):
external_ids = {'mykey': 'myvalue', 'yourkey': 'yourvalue'}
qos = self._qos_add('from-lport', 0, 'output == "fake_port" && ip',
@ -308,8 +380,9 @@ class TestQoSOps(OvnNorthboundTest):
r2 = self._qos_add('to-lport', 0, 'output == "fake_port"', 1000)
self.api.qos_del(self.switch.uuid, 'from-lport').execute(
check_error=True)
self.assertNotIn(r1, self.switch.qos_rules)
self.assertIn(r2, self.switch.qos_rules)
qos_rules = [idlutils.frozen_row(row) for row in self.switch.qos_rules]
self.assertNotIn(r1, qos_rules)
self.assertIn(r2, qos_rules)
def test_qos_del_direction_priority_match(self):
r1 = self._qos_add('from-lport', 0, 'output == "fake_port"', 1000)
@ -317,8 +390,9 @@ class TestQoSOps(OvnNorthboundTest):
cmd = self.api.qos_del(self.switch.uuid,
'from-lport', 0, 'output == "fake_port"')
cmd.execute(check_error=True)
self.assertNotIn(r1, self.switch.qos_rules)
self.assertIn(r2, self.switch.qos_rules)
qos_rules = [idlutils.frozen_row(row) for row in self.switch.qos_rules]
self.assertNotIn(r1, qos_rules)
self.assertIn(r2, qos_rules)
def test_qos_del_priority_without_match(self):
self.assertRaises(TypeError, self.api.qos_del, self.switch.uuid,
@ -340,6 +414,7 @@ class TestQoSOps(OvnNorthboundTest):
r2 = self._qos_add('from-lport', 1, 'output == "fake_port2"', 1000)
qos_rules = self.api.qos_list(self.switch.uuid).execute(
check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertIn(r1, qos_rules)
self.assertIn(r2, qos_rules)
@ -359,6 +434,7 @@ class TestQoSOps(OvnNorthboundTest):
{'key1': 'value1'}).execute(check_error=True)
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_2, self.qos_3], qos_rules)
self.api.qos_del_ext_ids(
@ -366,6 +442,7 @@ class TestQoSOps(OvnNorthboundTest):
{'key3': 'value3', 'key4': 'value4'}).execute(check_error=True)
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_3], qos_rules)
def test_qos_delete_external_ids_wrong_keys_or_values(self):
@ -374,12 +451,14 @@ class TestQoSOps(OvnNorthboundTest):
{'key_z': 'value1'}).execute(check_error=True)
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_1, self.qos_2, self.qos_3], qos_rules)
self.api.qos_del_ext_ids(self.switch.name,
{'key1': 'value_z'}).execute(check_error=True)
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_1, self.qos_2, self.qos_3], qos_rules)
self.api.qos_del_ext_ids(
@ -387,6 +466,7 @@ class TestQoSOps(OvnNorthboundTest):
{'key3': 'value3', 'key4': 'value_z'}).execute(check_error=True)
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_1, self.qos_2, self.qos_3], qos_rules)
def test_qos_delete_external_ids_empty_dict(self):
@ -404,6 +484,7 @@ class TestQoSOps(OvnNorthboundTest):
# No qos rule has been deleted from the correct logical switch.
qos_rules = self.api.qos_list(
self.switch.uuid).execute(check_error=True)
qos_rules = [idlutils.frozen_row(row) for row in qos_rules]
self.assertCountEqual([self.qos_1, self.qos_2, self.qos_3], qos_rules)