baremetal list: add --details

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,13 +209,24 @@ 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):
         cloud = parsed_args.cloud
         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

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)