diff --git a/osism/commands/baremetal.py b/osism/commands/baremetal.py index 474df1e70..1ae8e4c8e 100644 --- a/osism/commands/baremetal.py +++ b/osism/commands/baremetal.py @@ -6,10 +6,12 @@ BooleanOptionalAction, RawDescriptionHelpFormatter, ) +from datetime import datetime, timezone from textwrap import dedent import tempfile import os +import re import subprocess import threading from loguru import logger @@ -126,6 +128,59 @@ def _apply_metalbox_vars(play_vars, device): play_vars["docker_insecure_registries"] = ["metalbox:5001"] +LIST_LAST_ERROR_WIDTH = 80 + +# NOTE: C0 and C1 control characters, DEL included. ESC (0x1b) and CSI +# (0x9b) start terminal escape sequences, BEL (0x07) ends OSC ones. +_CONTROL_CHARACTERS = re.compile(r"[\x00-\x1f\x7f-\x9f]") + + +def _single_line_text(value): + """Make a free-text Ironic field safe to print as one table cell. + + Fields such as last_error can carry text from BMCs or the deploy agent. + Control characters become spaces, so an escape sequence prints as + plain text instead of reaching the terminal, and line breaks do not + break the table. + """ + if not value: + return "" + text = _CONTROL_CHARACTERS.sub(" ", value) + return re.sub(r"\s+", " ", text).strip() + + +def _format_state_since(value): + """Render Ironic's provision_updated_at as "YYYY-MM-DD HH:MM" in UTC.""" + if not value: + return "" + try: + timestamp = datetime.fromisoformat(value) + except ValueError: + return value + if timestamp.tzinfo is None: + timestamp = timestamp.replace(tzinfo=timezone.utc) + return timestamp.astimezone(timezone.utc).strftime("%Y-%m-%d %H:%M") + + +def _format_automated_clean(value): + # NOTE: None is not False: the node follows the conductor's + # [conductor]automated_clean setting. + return "default" if value is None else value + + +def _format_maintenance_reason(node): + # NOTE: Ironic sets fault (e.g. "power failure") when it put the node + # into maintenance itself; an operator's reason takes precedence. + return _single_line_text(node["maintenance_reason"] or node["fault"]) + + +def _shorten_last_error(value, width=LIST_LAST_ERROR_WIDTH): + text = _single_line_text(value) + if len(text) <= width: + return text + return text[: width - 3] + "..." + + class BaremetalList(Command): def get_parser(self, prog_name): parser = super(BaremetalList, self).get_parser(prog_name) @@ -138,7 +193,7 @@ def get_parser(self, prog_name): parser.add_argument( "--provision-state", default=None, - choices=["enroll", "managable", "available", "active", "error"], + choices=["enroll", "manageable", "available", "active", "error"], type=str, help="Only list nodes with the given provision state", ) @@ -154,6 +209,16 @@ def get_parser(self, prog_name): action="store_true", help="Include device role from NetBox (slower)", ) + parser.add_argument( + "--details", + default=False, + action="store_true", + help=( + "Also show when each node entered its provision state (UTC), " + "the target state, automated clean, the maintenance reason " + "and the last error" + ), + ) return parser def take_action(self, parsed_args): @@ -161,6 +226,7 @@ def take_action(self, parsed_args): provision_state = parsed_args.provision_state maintenance = parsed_args.maintenance include_netbox = parsed_args.netbox + details = parsed_args.details from osism.tasks.openstack import get_cloud_helpers @@ -181,7 +247,12 @@ def take_action(self, parsed_args): if maintenance: query.update(dict(maintenance=maintenance)) - baremetal = conn.baremetal.nodes(**query) + if details: + from osism.utils.ironic import list_detailed_nodes + + baremetal = list_detailed_nodes(conn, **query) + else: + baremetal = conn.baremetal.nodes(**query) result = [] for b in baremetal: @@ -192,6 +263,14 @@ def take_action(self, parsed_args): b["provision_state"], b["maintenance"], ] + if details: + row += [ + _format_state_since(b["provision_updated_at"]), + b["target_provision_state"] or "", + _format_automated_clean(b["automated_clean"]), + _format_maintenance_reason(b), + _shorten_last_error(b["last_error"]), + ] result.append(row) result.sort(key=lambda x: x[0]) @@ -203,6 +282,14 @@ def take_action(self, parsed_args): "Provision State", "Maintenance", ] + if details: + headers += [ + "State Since (UTC)", + "Target State", + "Automated Clean", + "Maintenance Reason", + "Last Error", + ] if include_netbox and utils.nb: from osism.tasks.openstack import get_baremetal_node_netbox_info diff --git a/osism/utils/ironic.py b/osism/utils/ironic.py new file mode 100644 index 000000000..33a6a62bf --- /dev/null +++ b/osism/utils/ironic.py @@ -0,0 +1,38 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Ironic helpers that need more than openstacksdk exposes. + +Importing this module imports openstacksdk; import it where it is used, +not at the top of a command module. +""" + +from openstack import resource +from openstack.baremetal.v1.node import Node + + +class DetailedNode(Node): + """An Ironic node that also keeps ``provision_updated_at``. + + Ironic returns ``provision_updated_at`` (when the node entered its + current provision state) with every detailed node, but openstacksdk's + ``Node`` has no attribute for it, so the SDK drops the value while + parsing the response. Declaring the field here keeps it. Remove the + declaration once openstacksdk maps the field itself. + """ + + # openstacksdk looks up the alternate id only in the class's own + # __dict__, not in its bases. Without repeating the declaration a + # subclass of Node loses the "uuid" -> id mapping and every node's id + # is None. + id = resource.Body("uuid", alternate_id=True) + + provision_updated_at = resource.Body("provision_updated_at") + + +def list_detailed_nodes(conn, **query): + """List nodes with all fields, as ``conn.baremetal.nodes(details=True)``. + + This goes through the same request and microversion negotiation as the + SDK's own proxy method; only the resource class differs. + """ + return DetailedNode.list(conn.baremetal, details=True, **query) diff --git a/tests/unit/commands/test_baremetal.py b/tests/unit/commands/test_baremetal.py index 4fec10ec4..587ee76fb 100644 --- a/tests/unit/commands/test_baremetal.py +++ b/tests/unit/commands/test_baremetal.py @@ -470,6 +470,15 @@ def test_list_query_flags_forwarded(): ) +def test_list_accepts_manageable_provision_state(): + conn = MagicMock() + conn.baremetal.nodes.return_value = [] + + _run_list(["--provision-state", "manageable"], conn) + + conn.baremetal.nodes.assert_called_once_with(provision_state="manageable") + + def test_list_without_flags_queries_without_kwargs(): conn = MagicMock() conn.baremetal.nodes.return_value = [] @@ -509,6 +518,200 @@ def test_list_netbox_without_connection_skips_extra_column(capsys): assert "Device Role" not in capsys.readouterr().out +def _detailed_node(name, **fields): + from osism.utils.ironic import DetailedNode + + body = { + "uuid": f"uuid-{name}", + "name": name, + "power_state": "power on", + "provision_state": "active", + "maintenance": False, + } + body.update(fields) + return DetailedNode.existing(**body) + + +def _run_list_details(args, nodes): + conn = MagicMock() + with patch( + "osism.utils.ironic.list_detailed_nodes", return_value=nodes + ) as list_detailed: + rc, _ = _run_list(["--details"] + args, conn) + return rc, conn, list_detailed + + +def test_list_without_details_keeps_standard_columns(capsys): + conn = MagicMock() + conn.baremetal.nodes.return_value = [_list_node("node-a")] + + _run_list([], conn) + + out = capsys.readouterr().out + for header in ( + "State Since", + "Target State", + "Automated Clean", + "Maintenance Reason", + "Last Error", + ): + assert header not in out + + +def test_list_details_queries_detailed_nodes_with_filters(): + rc, conn, list_detailed = _run_list_details( + ["--provision-state", "active", "--maintenance"], [] + ) + + list_detailed.assert_called_once_with( + conn, provision_state="active", maintenance=True + ) + conn.baremetal.nodes.assert_not_called() + + +def test_list_details_shows_extra_columns(capsys): + node = _detailed_node( + "node-a", + provision_state="clean failed", + target_provision_state="available", + provision_updated_at="2026-11-10T08:15:42.123456+00:00", + automated_clean=True, + maintenance=True, + maintenance_reason="disk swap", + last_error="Agent did not come back", + ) + + _run_list_details([], [node]) + + out = capsys.readouterr().out + for header in ( + "State Since (UTC)", + "Target State", + "Automated Clean", + "Maintenance Reason", + "Last Error", + ): + assert header in out + assert "uuid-node-a" in out + assert "2026-11-10 08:15" in out + assert "clean failed" in out + assert "available" in out + assert "disk swap" in out + assert "Agent did not come back" in out + + +def test_list_details_state_since_is_converted_to_utc(capsys): + node = _detailed_node("node-a", provision_updated_at="2026-11-10T10:15:00+02:00") + + _run_list_details([], [node]) + + assert "2026-11-10 08:15" in capsys.readouterr().out + + +def test_list_details_unset_automated_clean_shows_default(capsys): + # Ironic's null means "follow the conductor's [conductor]automated_clean". + node = _detailed_node("node-a", automated_clean=None) + + _run_list_details([], [node]) + + assert "default" in capsys.readouterr().out + + +def test_list_details_maintenance_reason_falls_back_to_fault(capsys): + node = _detailed_node("node-a", maintenance=True, fault="power failure") + + _run_list_details([], [node]) + + assert "power failure" in capsys.readouterr().out + + +def test_list_details_last_error_flattened_and_shortened(capsys): + error = "first line\nsecond line " + "x" * 200 + node = _detailed_node("node-a", last_error=error) + + _run_list_details([], [node]) + + shortened = "first line second line " + "x" * 54 + "..." + assert len(shortened) == 80 + out = capsys.readouterr().out + assert f" {shortened} " in out + assert "x" * 55 not in out + + +def test_list_details_free_text_control_characters_neutralised(capsys): + # Free-text node fields must not carry terminal escape sequences (ESC, + # OSC's BEL, C1 CSI) through to the operator's terminal. + node = _detailed_node( + "node-a", + maintenance=True, + maintenance_reason="swap\x1b]0;owned\x07disk", + last_error="boom\x1b[2J\x9b31mred", + ) + + _run_list_details([], [node]) + + out = capsys.readouterr().out + for char in ("\x1b", "\x07", "\x9b"): + assert char not in out + assert "swap ]0;owned disk" in out + assert "boom [2J 31mred" in out + + +def test_detailed_node_keeps_provision_updated_at_and_uuid(): + from osism.utils.ironic import DetailedNode + + node = DetailedNode.existing( + uuid="11111111-2222-3333-4444-555555555555", + provision_updated_at="2026-11-10T08:15:42+00:00", + ) + + assert node["id"] == "11111111-2222-3333-4444-555555555555" + assert node["provision_updated_at"] == "2026-11-10T08:15:42+00:00" + + +def test_list_detailed_nodes_requests_node_detail_endpoint(): + # Runs openstacksdk's real list code against a mocked baremetal proxy: + # the request must go to /nodes/detail (not the summary /nodes), keep + # the filter, negotiate the microversion, and keep both the uuid and + # provision_updated_at of the response. + from openstack.baremetal.v1 import _proxy + + from osism.utils.ironic import list_detailed_nodes + + response = MagicMock(status_code=200, headers={}, links={}) + response.json.return_value = { + "nodes": [ + { + "uuid": "11111111-2222-3333-4444-555555555555", + "name": "node-a", + "provision_updated_at": "2026-11-10T08:15:42+00:00", + } + ] + } + proxy = MagicMock(spec=_proxy.Proxy) + proxy.default_microversion = None + proxy.get_endpoint_data.return_value = MagicMock( + min_microversion="1.1", max_microversion="1.109" + ) + proxy.get.return_value = response + conn = MagicMock() + conn.baremetal = proxy + + nodes = list(list_detailed_nodes(conn, provision_state="active")) + + proxy.get.assert_called_once() + args, kwargs = proxy.get.call_args + assert args == ("/nodes/detail",) + assert kwargs["params"] == {"provision_state": "active"} + assert kwargs["microversion"] == "1.109" + assert [(n["id"], n["provision_updated_at"]) for n in nodes] == [ + ( + "11111111-2222-3333-4444-555555555555", + "2026-11-10T08:15:42+00:00", + ) + ] + + def test_list_cleanup_called_when_listing_fails(): cmd = baremetal.BaremetalList(MagicMock(), MagicMock()) parsed_args = cmd.get_parser("test").parse_args([])