From 616fe642d31c289ceef1db45c6e56dd5429ccb70 Mon Sep 17 00:00:00 2001 From: Doug Goldstein Date: Fri, 28 Aug 2026 16:36:28 -0500 Subject: [PATCH] fix(ironic): stop chassis_model hook from wiping other traits _set_node_traits kept only existing traits not starting with prefix "CUSTOM_", then re-added the chassis trait via TraitList.create, which replaces the whole list. Since every Ironic custom trait starts with CUSTOM_, this dropped all other traits (CUSTOM_*_SWITCH, CUSTOM_FIRMWARE_UPDATE_*, etc.) on every inspection. Move the chassis trait into a reserved CUSTOM_CHASSIS_ namespace (CUSTOM_CHASSIS_{MANUFACTURER}_{MODEL}) so the hook can scope itself: it adds the required trait and prunes only other CUSTOM_CHASSIS_ traits, leaving all other traits untouched. This mirrors the update-baremetal-port hook, which owns the CUSTOM_*_SWITCH namespace. Update the inspection guide and add unit tests for the hook. --- .../openstack-ironic-inspection-guide.md | 7 +- .../inspect_hook_chassis_model.py | 24 +++++-- .../tests/test_inspect_hook_chassis_model.py | 67 +++++++++++++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 python/ironic-understack/ironic_understack/tests/test_inspect_hook_chassis_model.py diff --git a/docs/operator-guide/openstack-ironic-inspection-guide.md b/docs/operator-guide/openstack-ironic-inspection-guide.md index fed8c5f9d..acf668c74 100644 --- a/docs/operator-guide/openstack-ironic-inspection-guide.md +++ b/docs/operator-guide/openstack-ironic-inspection-guide.md @@ -221,9 +221,12 @@ The `chassis_model` hook adds a custom trait identifying the specific hardware c 1. Extracts chassis model from `system_vendor.product_name` 2. Normalizes manufacturer name (handles "DELL", "HP" variants) -3. Creates a trait in the format: `CUSTOM_{MANUFACTURER}_{CHASSIS_MODEL}` +3. Creates a trait in the format: `CUSTOM_CHASSIS_{MANUFACTURER}_{CHASSIS_MODEL}` -**Example**: A Dell PowerEdge R7615 receives the trait `CUSTOM_DELL_POWEREDGE_R7615` +**Example**: A Dell PowerEdge R7615 receives the trait `CUSTOM_CHASSIS_DELL_POWEREDGE_R7615` + +The hook owns the `CUSTOM_CHASSIS_` namespace: on re-inspection it removes any +other `CUSTOM_CHASSIS_` trait and leaves all other traits untouched. This trait enables [flavor definitions](../design-guide/flavors.md) to target specific hardware models. See [hardware traits](../design-guide/hardware-traits.md) for more on how traits work. diff --git a/python/ironic-understack/ironic_understack/inspect_hook_chassis_model.py b/python/ironic-understack/ironic_understack/inspect_hook_chassis_model.py index 399e30c8c..738b8c62e 100644 --- a/python/ironic-understack/ironic_understack/inspect_hook_chassis_model.py +++ b/python/ironic-understack/ironic_understack/inspect_hook_chassis_model.py @@ -7,6 +7,11 @@ LOG = logging.getLogger(__name__) +# Reserved namespace for the chassis-model trait. Owning a dedicated prefix +# lets this hook prune stale chassis traits without touching traits managed +# elsewhere (e.g. CUSTOM_*_SWITCH, CUSTOM_FIRMWARE_UPDATE_*). +CHASSIS_TRAIT_PREFIX = "CUSTOM_CHASSIS_" + class InspectHookChassisModel(base.InspectionHook): """Update baremetal node properties with chassis model number from inventory. @@ -19,16 +24,23 @@ def __call__(self, task, inventory, _plugin_data): chassis_model = _extract_chassis_model(node, inventory) manufacturer = _extract_manufacturer(node, inventory) trait_name = _trait_name(manufacturer, chassis_model) - _set_node_traits(task, "CUSTOM_", trait_name) + _set_node_traits(task, trait_name) + +def _set_node_traits(task, required_trait: str): + """Set the ``CUSTOM_CHASSIS_{MANUFACTURER}_{MODEL}`` trait on the node. -def _set_node_traits(task, prefix: str, required_trait: str): - """Manage the subset of node traits whose names begin with `prefix`.""" + Manages only the ``CUSTOM_CHASSIS_`` namespace: the required trait is + added and any other ``CUSTOM_CHASSIS_`` trait is removed. All other traits + (e.g. ``CUSTOM_*_SWITCH``, ``CUSTOM_FIRMWARE_UPDATE_*``) are left untouched. + """ node = task.node - existing_traits = node.traits.get_trait_names() - required_trait = prefix + required_trait + required_trait = CHASSIS_TRAIT_PREFIX + required_trait - required_traits = {x for x in existing_traits if not x.startswith(prefix)} + existing_traits = set(node.traits.get_trait_names()) + required_traits = { + t for t in existing_traits if not t.startswith(CHASSIS_TRAIT_PREFIX) + } required_traits.add(required_trait) LOG.debug( diff --git a/python/ironic-understack/ironic_understack/tests/test_inspect_hook_chassis_model.py b/python/ironic-understack/ironic_understack/tests/test_inspect_hook_chassis_model.py new file mode 100644 index 000000000..4c8367862 --- /dev/null +++ b/python/ironic-understack/ironic_understack/tests/test_inspect_hook_chassis_model.py @@ -0,0 +1,67 @@ +import logging + +import ironic.objects + +from ironic_understack.inspect_hook_chassis_model import InspectHookChassisModel + +# Populate ironic.objects.TraitList so it can be patched below. +ironic.objects.register_all() + +_INVENTORY = { + "system_vendor": { + "manufacturer": "Dell Inc.", + "product_name": "PowerEdge R7615", + } +} +_PLUGIN_DATA = {} + + +def _mock_task(mocker, existing_traits): + mock_traits = mocker.Mock() + mock_traits.get_trait_names.return_value = list(existing_traits) + mock_context = mocker.Mock() + mock_node = mocker.Mock(id=1234, uuid="node-uuid", traits=mock_traits) + return mocker.Mock(node=mock_node, context=mock_context), mock_node, mock_context + + +def test_preserves_other_custom_traits(mocker, caplog): + """Re-inspection must not wipe traits set by other hooks/rules.""" + caplog.set_level(logging.DEBUG) + + existing = [ + "CUSTOM_FIRMWARE_UPDATE_R7615", + "CUSTOM_NETGROUP_F20_1_NETWORK", + "CUSTOM_NETWORK_SWITCH", + "CUSTOM_STORAGE_SWITCH", + ] + mock_task, mock_node, mock_context = _mock_task(mocker, existing) + trait_create = mocker.patch( + "ironic_understack.inspect_hook_chassis_model.objects.TraitList.create" + ) + + InspectHookChassisModel().__call__(mock_task, _INVENTORY, _PLUGIN_DATA) + + mock_node.save.assert_called_once() + trait_create.assert_called_once_with( + mock_context, + 1234, + set(existing) | {"CUSTOM_CHASSIS_DELL_POWEREDGE_R7615"}, + ) + + +def test_replaces_stale_chassis_trait(mocker): + """A stale CUSTOM_CHASSIS_ trait is removed; other traits are kept.""" + existing = ["CUSTOM_NETWORK_SWITCH", "CUSTOM_CHASSIS_DELL_POWEREDGE_R6615"] + mock_task, mock_node, mock_context = _mock_task(mocker, existing) + trait_create = mocker.patch( + "ironic_understack.inspect_hook_chassis_model.objects.TraitList.create" + ) + + InspectHookChassisModel().__call__(mock_task, _INVENTORY, _PLUGIN_DATA) + + mock_node.save.assert_called_once() + trait_create.assert_called_once_with( + mock_context, + 1234, + {"CUSTOM_NETWORK_SWITCH", "CUSTOM_CHASSIS_DELL_POWEREDGE_R7615"}, + )