From d1f2364091ff0f0a6a1ffac5cb51a618169c6e2a Mon Sep 17 00:00:00 2001 From: Clif Houck Date: Mon, 26 Jan 2026 09:34:13 -0600 Subject: [PATCH] Update TBN config file to improve trait structure Adds an explicit 'actions' member which is the list of actions belonging to a trait. Also add an 'order' member to aid in more explicit ordering of traits if necessary. Change-Id: I35a80f710d6f79ac90649fcc5120a25acd1ea989 Signed-off-by: Clif Houck --- etc/ironic/trait_based_networks.yaml.sample | 38 +++++---- ironic/common/trait_based_networking/base.py | 5 +- .../trait_based_networking/config_file.py | 51 +++++++----- .../test_config_file.py | 79 +++++++++++-------- .../trait_based_networking/test_loader.py | 21 ++--- .../drivers/modules/network/test_common.py | 26 +++--- 6 files changed, 129 insertions(+), 91 deletions(-) diff --git a/etc/ironic/trait_based_networks.yaml.sample b/etc/ironic/trait_based_networks.yaml.sample index ee75f4f677..eaa50c428d 100644 --- a/etc/ironic/trait_based_networks.yaml.sample +++ b/etc/ironic/trait_based_networks.yaml.sample @@ -1,21 +1,27 @@ CUSTOM_TRAIT_NAME: - - action: bond_ports - filter: port.vendor == 'vendor_string' - min_count: 2 + order: 1 + actions: + - action: bond_ports + filter: port.vendor == 'vendor_string' + min_count: 2 CUSTOM_DIRECT_ATTACH_A_PURPLE_TO_STORAGE: - - action: attach_port - filter: port.vendor == 'purple' && network.name == 'storage' + actions: + - action: attach_port + filter: port.vendor == 'purple' && network.name == 'storage' CUSTOM_BOND_PURPLE_BY_2: - - action: group_and_attach_ports - filter: port.vendor == 'purple' - max_count: 2 + actions: + - action: group_and_attach_ports + filter: port.vendor == 'purple' + max_count: 2 CUSTOM_BOND_GREEN_STORAGE_TO_STORAGE_BY_2: - - action: group_and_attach_ports - filter: port.vendor == 'green' && port.category == 'storage' && ( network.name =~ 'storage' || network.tags =~ 'storage' ) - max_count: 2 - min_count: 2 + actions: + - action: group_and_attach_ports + filter: port.vendor == 'green' && port.category == 'storage' && ( network.name =~ 'storage' || network.tags =~ 'storage' ) + max_count: 2 + min_count: 2 CUSTOM_USE_PHYSNET_A_OR_B: - - action: attach_port - filter: port.physical_network == 'fabric_a' && network.tags == 'a' - - action: attach_port - filter: port.physical_network == 'fabric_b' && network.tags == 'b' + actions: + - action: attach_port + filter: port.physical_network == 'fabric_a' && network.tags == 'a' + - action: attach_port + filter: port.physical_network == 'fabric_b' && network.tags == 'b' diff --git a/ironic/common/trait_based_networking/base.py b/ironic/common/trait_based_networking/base.py index 6835ddaf00..527eb37a15 100644 --- a/ironic/common/trait_based_networking/base.py +++ b/ironic/common/trait_based_networking/base.py @@ -220,9 +220,10 @@ class TraitAction(object): class NetworkTrait(object): - def __init__(self, name, actions): + def __init__(self, name, actions, order=1): self.name = name self.actions = actions + self.order = order def __eq__(self, other): if self.name != other.name: @@ -238,7 +239,7 @@ class NetworkTrait(object): if not match_found: return False - return True + return self.order == other.order class PrimordialPort(object): diff --git a/ironic/common/trait_based_networking/config_file.py b/ironic/common/trait_based_networking/config_file.py index 22b555f4fe..30e990c73f 100644 --- a/ironic/common/trait_based_networking/config_file.py +++ b/ironic/common/trait_based_networking/config_file.py @@ -20,6 +20,7 @@ import yaml class ConfigFile(object): def __init__(self, filename): self._filename = filename + self._traits = [] # TODO(clif): Do this here, or defer to clients of class calling these? self.read() @@ -31,65 +32,75 @@ class ConfigFile(object): """Check that contents conform to TBN expectations.""" reasons = [] valid = True - for key, value_list in self._contents.items(): - if not isinstance(value_list, list): + for trait_name, trait_members in self._contents.items(): + if 'actions' not in trait_members: + valid = False reasons.append( - _(f"'{key}' trait does not consist of a list of actions")) + _(f"'{trait_name}' trait does not include an 'actions' " + "key ")) + continue + if not isinstance(trait_members['actions'], list): + reasons.append( + _(f"'{trait_name}.actions' does not consist of a list of " + "actions")) valid = False continue - for v in value_list: + for trait_action in trait_members['actions']: # Check necessary keys are present. for n in base.TraitAction.NECESSARY_KEYS: - if n not in v: + if n not in trait_action.keys(): reasons.append( - _(f"'{key}' trait is missing '{n}' key")) + _(f"'{trait_name}' trait is missing '{n}' key")) valid = False # Check for errant keys. - for sub_key in v.keys(): + for sub_key in trait_action.keys(): if sub_key not in base.TraitAction.ALL_KEYS: reasons.append( - _(f"'{key}' trait action has unrecognized key " - f"'{sub_key}'")) + _(f"'{trait_name}' trait action has unrecognized " + f"key '{sub_key}'")) valid = False # Make sure action is valid - if 'action' in v: - action = v['action'] + if 'action' in trait_action.keys(): + action = trait_action['action'] try: base.Actions(action) except Exception: valid = False reasons.append( - _(f"'{key}' trait action has unrecognized action " - f"'{action}'")) + _(f"'{trait_name}' trait action has unrecognized " + f"action '{action}'")) # Does the filter parse? - if 'filter' in v: + if 'filter' in trait_action.keys(): try: - base.FilterExpression.parse(v['filter']) + base.FilterExpression.parse(trait_action['filter']) except Exception: valid = False # TODO(clif): Surface exception text in reason below? reasons.append( - _(f"'{key}' trait action has malformed " - f"filter expression: '{v['filter']}'")) + _(f"'{trait_name}' trait action has malformed " + "filter expression: " + f"'{trait_action['filter']}'")) return valid, reasons def parse(self): """Render contents of configuration file as TBN objects""" self._traits = [] - for trait_name, actions in self._contents.items(): + for trait_name, trait_members in self._contents.items(): parsed_actions = [] - for action in actions: + for action in trait_members['actions']: parsed_actions.append(base.TraitAction( trait_name, base.Actions(action['action']), base.FilterExpression.parse(action['filter']), min_count=action.get('min_count', None), max_count=action.get('max_count', None))) - self._traits.append(base.NetworkTrait(trait_name, parsed_actions)) + order = trait_members.get('order', 1) + self._traits.append(base.NetworkTrait(trait_name, parsed_actions, + order)) def traits(self): return self._traits diff --git a/ironic/tests/unit/common/trait_based_networking/test_config_file.py b/ironic/tests/unit/common/trait_based_networking/test_config_file.py index 7c1f7a14f8..744c64ce46 100644 --- a/ironic/tests/unit/common/trait_based_networking/test_config_file.py +++ b/ironic/tests/unit/common/trait_based_networking/test_config_file.py @@ -50,32 +50,38 @@ class TraitBasedNetworkingConfigFileTestCase(base.TestCase): SubTestCase( "Valid - single trait", ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n"), + " order: 1\n" + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n"), True, [], ), SubTestCase( "Valid - Several traits", ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n" + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n" "CUSTOM_TRAIT_2:\n" - " - action: attach_port\n" - " filter: port.vendor != 'vendor_string'\n" - " max_count: 2\n" + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor != 'vendor_string'\n" + " max_count: 2\n" "CUSTOM_TRAIT_3:\n" - " - action: attach_port\n" - " filter: port.vendor != 'vendor_string'\n" - " max_count: 2\n"), + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor != 'vendor_string'\n" + " max_count: 2\n"), True, [], ), SubTestCase( "Invalid - Missing trait has required entry missing", ("trait_name:\n" + " actions:\n" " - action: bond_ports\n"), False, ["'trait_name' trait is missing 'filter' key"], @@ -83,10 +89,11 @@ class TraitBasedNetworkingConfigFileTestCase(base.TestCase): SubTestCase( "Invalid - Unrecognized trait entry", ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n" - " wrong: hi\n"), + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n" + " wrong: hi\n"), False, [("'CUSTOM_TRAIT_NAME' trait action has unrecognized key " "'wrong'")], @@ -94,29 +101,33 @@ class TraitBasedNetworkingConfigFileTestCase(base.TestCase): SubTestCase( "Invalid - Unrecognized action", ("CUSTOM_TRAIT_NAME:\n" - " - action: invalid\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n"), + " actions:\n" + " - action: invalid\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n"), False, ["'CUSTOM_TRAIT_NAME' trait action has unrecognized action " "'invalid'"], ), SubTestCase( - "Invalid - trait does not consist of a list of actions", + ("Invalid - trait actions does not consist of a list of " + "actions"), ("CUSTOM_TRAIT_NAME:\n" - " action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n"), + " actions:\n" + " action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n"), False, - [("'CUSTOM_TRAIT_NAME' trait does not consist of a list " + [("'CUSTOM_TRAIT_NAME.actions' does not consist of a list " "of actions")], ), SubTestCase( "Invalid - trait action has malformed filter expression", ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor &= 'vendor_string'\n" - " min_count: 2\n"), + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor &= 'vendor_string'\n" + " min_count: 2\n"), False, [("'CUSTOM_TRAIT_NAME' trait action has malformed filter " "expression: 'port.vendor &= 'vendor_string''")], @@ -124,9 +135,10 @@ class TraitBasedNetworkingConfigFileTestCase(base.TestCase): SubTestCase( "Invalid - several things wrong", ("CUSTOM_TRAIT_NAME:\n" - " - filter: port.vendor &= 'vendor_string'\n" - " min_count: 2\n" - " wrong: oops\n"), + " actions:\n" + " - filter: port.vendor &= 'vendor_string'\n" + " min_count: 2\n" + " wrong: oops\n"), False, [("'CUSTOM_TRAIT_NAME' trait action has malformed filter " "expression: 'port.vendor &= 'vendor_string''"), @@ -152,9 +164,10 @@ class TraitBasedNetworkingConfigFileTestCase(base.TestCase): def test_parse(self): contents = ( "CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n") + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n") with tempfile.NamedTemporaryFile( mode='w', diff --git a/ironic/tests/unit/common/trait_based_networking/test_loader.py b/ironic/tests/unit/common/trait_based_networking/test_loader.py index 5cd1853c4c..dcfc56bc60 100644 --- a/ironic/tests/unit/common/trait_based_networking/test_loader.py +++ b/ironic/tests/unit/common/trait_based_networking/test_loader.py @@ -22,9 +22,10 @@ class TraitBasedNetworkingConfigLoaderTestCase(base.TestCase): def test_config_loader(self): self.tmpdir = tempfile.TemporaryDirectory() contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n") + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n") with tempfile.NamedTemporaryFile( mode='w', dir=self.tmpdir.name, @@ -42,9 +43,10 @@ class TraitBasedNetworkingConfigLoaderTestCase(base.TestCase): def test_config_loader_refresh(self): self.tmpdir = tempfile.TemporaryDirectory() contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n") + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n") with tempfile.NamedTemporaryFile( mode='w', dir=self.tmpdir.name, @@ -72,9 +74,10 @@ class TraitBasedNetworkingConfigLoaderTestCase(base.TestCase): with open(tmpfile.name, mode='w') as newfile: contents = ("CUSTOM_TRAIT_NAME_CHANGED:\n" - " - action: bond_ports\n" - " filter: port.vendor == 'vendor_string'\n" - " min_count: 2\n") + " actions:\n" + " - action: bond_ports\n" + " filter: port.vendor == 'vendor_string'\n" + " min_count: 2\n") newfile.write(contents) newfile.close() diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index 1e5bb5984b..445baad741 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -1061,8 +1061,9 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): vif_info = {'id': 'fake_vif_id'} contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: attach_port\n" - " filter: port.vendor == 'fake_vendor'\n") + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor == 'fake_vendor'\n") self.tmpdir = tempfile.TemporaryDirectory() with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir.name, @@ -1109,8 +1110,9 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): vif_info = {'id': 'fake_vif_id'} contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: attach_port\n" - " filter: port.vendor == 'fake_vendor'\n") + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor == 'fake_vendor'\n") self.tmpdir = tempfile.TemporaryDirectory() with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir.name, @@ -1161,9 +1163,10 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): vif_info = {'id': 'fake_vif_id'} contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: attach_port\n" - " filter: port.vendor == 'fake_vendor'\n" - " min_count: 2") + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor == 'fake_vendor'\n" + " min_count: 2") self.tmpdir = tempfile.TemporaryDirectory() with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir.name, @@ -1221,10 +1224,11 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): # the other will generate a NoMatch action. # Make sure things don't blow up. contents = ("CUSTOM_TRAIT_NAME:\n" - " - action: attach_port\n" - " filter: port.vendor == 'other_vendor'\n" - " - action: attach_port\n" - " filter: port.vendor == 'fake_vendor'\n") + " actions:\n" + " - action: attach_port\n" + " filter: port.vendor == 'other_vendor'\n" + " - action: attach_port\n" + " filter: port.vendor == 'fake_vendor'\n") self.tmpdir = tempfile.TemporaryDirectory() with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir.name,