From c8e7e9f619ce3b00ca86d42a3b18b81251d3c815 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 23 Jan 2023 00:32:46 +0100 Subject: [PATCH] Accept HA chassis group commands in HAChassisGroupAdd* Now "HAChassisGroupAddChassisCommand" and "HAChassisGroupDelChassisCommand" accept "HAChassisGroupAddCommand" as input parameter. That allows to add and remove "HA_Chassis" from the "HA_Chassis_Group" register in the same transaction that has created the HA Chassis Group. Related-Bug: #1995078 Change-Id: I443796cc2fcd7eef46f968a8383d068bab6ae670 --- ovsdbapp/api.py | 10 ++++++ ovsdbapp/backend/ovs_idl/__init__.py | 8 ++++- ovsdbapp/backend/ovs_idl/command.py | 10 +++++- ovsdbapp/schema/ovn_northbound/commands.py | 20 ++++++------ .../schema/ovn_northbound/test_impl_idl.py | 31 +++++++++++++++++++ .../unit/backend/ovs_idl/test_idlutils.py | 10 +++++- ovsdbapp/utils.py | 17 ++++++++++ 7 files changed, 94 insertions(+), 12 deletions(-) diff --git a/ovsdbapp/api.py b/ovsdbapp/api.py index 1c43cb34..0439baa6 100644 --- a/ovsdbapp/api.py +++ b/ovsdbapp/api.py @@ -33,6 +33,16 @@ class Command(object, metaclass=abc.ABCMeta): :param transaction_options: Options to pass to the transaction """ + @property + @abc.abstractmethod + def result(self): + """Returned value from the command execution""" + + @result.setter + @abc.abstractmethod + def result(self, value): + """Setter of the result property""" + class Transaction(object, metaclass=abc.ABCMeta): @abc.abstractmethod diff --git a/ovsdbapp/backend/ovs_idl/__init__.py b/ovsdbapp/backend/ovs_idl/__init__.py index f3d99fa6..2ac10687 100644 --- a/ovsdbapp/backend/ovs_idl/__init__.py +++ b/ovsdbapp/backend/ovs_idl/__init__.py @@ -13,8 +13,10 @@ import logging import uuid +from ovs.db import idl from ovsdbapp.backend.ovs_idl import command as cmd from ovsdbapp.backend.ovs_idl import idlutils +from ovsdbapp.backend.ovs_idl import rowview from ovsdbapp.backend.ovs_idl import transaction from ovsdbapp import exceptions @@ -190,7 +192,11 @@ class Backend(object): # Handle commands by simply returning its result if isinstance(record, cmd.BaseCommand): - return record.result + if isinstance(record.result, (rowview.RowView, idl.Row)): + # In case the command (creation) returns an existing record. + return record.result + else: + record = record.result t = self.tables[table] if isinstance(record, uuid.UUID): diff --git a/ovsdbapp/backend/ovs_idl/command.py b/ovsdbapp/backend/ovs_idl/command.py index 358f5b2b..d071a6e5 100644 --- a/ovsdbapp/backend/ovs_idl/command.py +++ b/ovsdbapp/backend/ovs_idl/command.py @@ -30,7 +30,15 @@ class BaseCommand(api.Command): def __init__(self, api): self.api = api - self.result = None + self._result = None + + @property + def result(self): + return self._result + + @result.setter + def result(self, value): + self._result = value def execute(self, check_error=False, log_errors=True, **kwargs): try: diff --git a/ovsdbapp/schema/ovn_northbound/commands.py b/ovsdbapp/schema/ovn_northbound/commands.py index a88e7879..88348328 100644 --- a/ovsdbapp/schema/ovn_northbound/commands.py +++ b/ovsdbapp/schema/ovn_northbound/commands.py @@ -1804,16 +1804,15 @@ class HAChassisGroupAddChassisCommand(cmd.AddCommand): def __init__(self, api, hcg_id, chassis, priority, **columns): super().__init__(api) - self.hcg_id = hcg_id + self.hcg = hcg_id self.chassis = chassis self.priority = priority self.columns = columns def run_idl(self, txn): - hc_group = self.api.lookup('HA_Chassis_Group', self.hcg_id) + hc_group = self.api.lookup('HA_Chassis_Group', self.hcg) found = False - hc = None - for hc in hc_group.ha_chassis: + for hc in getattr(hc_group, 'ha_chassis', []): if hc.chassis_name != self.chassis: continue found = True @@ -1835,26 +1834,29 @@ class HAChassisGroupDelChassisCommand(cmd.BaseCommand): def __init__(self, api, hcg_id, chassis, if_exists=False): super().__init__(api) - self.hcg_id = hcg_id + self.hcg = hcg_id self.chassis = chassis self.if_exists = if_exists def run_idl(self, txn): try: - hc_group = self.api.lookup('HA_Chassis_Group', self.hcg_id) - except idlutils.RowNotFound: + hc_group = self.api.lookup('HA_Chassis_Group', self.hcg) + except idlutils.RowNotFound as exc: if self.if_exists: return + raise RuntimeError('HA Chassis Group %s does not exist' % + utils.get_uuid(self.hcg)) from exc + hc = None - for hc in hc_group.ha_chassis: + for hc in getattr(hc_group, 'ha_chassis', []): if hc.chassis_name == self.chassis: break else: if self.if_exists: return raise RuntimeError( - 'HA Chassis %s does not exist' % self.hcg_id) + 'HA Chassis %s does not exist' % utils.get_uuid(self.hcg)) hc_group.delvalue('ha_chassis', hc) hc.delete() diff --git a/ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py b/ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py index 13ea5d68..17ab31d1 100644 --- a/ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py +++ b/ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py @@ -2192,6 +2192,37 @@ class TestHAChassisGroup(OvnNorthboundTest): check_error=True) self.assertEqual([], hcg.ha_chassis) + def test_ha_chassis_group_add_delete_chassis_within_txn(self): + with self.api.create_transaction(check_error=True) as txn: + hcg_cmd = txn.add(self.api.ha_chassis_group_add(self.hcg_name)) + priority = 20 + txn.add(self.api.ha_chassis_group_add_chassis( + hcg_cmd, self.chassis, priority)) + + # Assert that the HA Chassis entry was created + hcg = self.api.ha_chassis_group_get(self.hcg_name).execute( + check_error=True) + hc = self.api.db_find( + 'HA_Chassis', + ('chassis_name', '=', self.chassis)).execute(check_error=True) + self.assertEqual(priority, hc[0]['priority']) + ha_chassis_uuid_list = [hc.uuid for hc in hcg.ha_chassis] + self.assertEqual(ha_chassis_uuid_list, [hc[0]['_uuid']]) + + with self.api.create_transaction(check_error=True) as txn: + hcg_cmd = txn.add(self.api.ha_chassis_group_add(self.hcg_name, + may_exist=True)) + txn.add(self.api.ha_chassis_group_del_chassis(hcg_cmd, + self.chassis)) + + hcg = self.api.ha_chassis_group_get(self.hcg_name).execute( + check_error=True) + ha = self.api.db_find( + 'HA_Chassis', + ('chassis_name', '=', self.chassis)).execute(check_error=True) + self.assertEqual([], ha) + self.assertEqual([], hcg.ha_chassis) + def test_ha_chassis_group_if_exists(self): self.api.ha_chassis_group_add(self.hcg_name).execute(check_error=True) self.api.ha_chassis_group_add_chassis( diff --git a/ovsdbapp/tests/unit/backend/ovs_idl/test_idlutils.py b/ovsdbapp/tests/unit/backend/ovs_idl/test_idlutils.py index ec432641..d764abd7 100644 --- a/ovsdbapp/tests/unit/backend/ovs_idl/test_idlutils.py +++ b/ovsdbapp/tests/unit/backend/ovs_idl/test_idlutils.py @@ -50,11 +50,19 @@ class MockRow(object): class MockCommand(api.Command): def __init__(self, result): - self.result = result + self._result = result def execute(self, **kwargs): pass + @property + def result(self): + return self._result + + @result.setter + def result(self, value): + self._result = value + class TestIdlUtils(base.TestCase): def test_condition_match(self): diff --git a/ovsdbapp/utils.py b/ovsdbapp/utils.py index c57e5fd9..22338f7a 100644 --- a/ovsdbapp/utils.py +++ b/ovsdbapp/utils.py @@ -13,6 +13,11 @@ import uuid import netaddr +from ovs.db import idl + +from ovsdbapp import api +from ovsdbapp.backend.ovs_idl import rowview + # NOTE(twilson) Clearly these are silly, but they are good enough for now # I'm happy for someone to replace them with better parsing @@ -76,3 +81,15 @@ def is_uuid_like(val): return str(uuid.UUID(val)).replace('-', '') == _format_uuid_string(val) except (TypeError, ValueError, AttributeError): return False + + +def get_uuid(reg_uuid_or_cmd): + """Return the UUID of a UUID itself or a BaseCommand""" + if isinstance(reg_uuid_or_cmd, api.Command): + reg_uuid = reg_uuid_or_cmd.result + if isinstance(reg_uuid, (rowview.RowView, idl.Row)): + reg_uuid = reg_uuid.uuid + else: + reg_uuid = reg_uuid_or_cmd + + return reg_uuid