diff --git a/python/understack-workflows/README.md b/python/understack-workflows/README.md index 29c1efa05..1456356f1 100644 --- a/python/understack-workflows/README.md +++ b/python/understack-workflows/README.md @@ -10,20 +10,29 @@ operation happens against the Nautobot Tenants. Operations include create, update and delete. -## enroll-server post-inspection runbooks +## Ironic runbooks -Optionally pass an ordered JSON array of runbook names or UUIDs: +`apply-node-runbook` runs configured Ironic runbooks for a node, handling the +required state transitions. Runbooks execute in order; a failure stops the list. ```sh -enroll-server --ip-address 192.0.2.10 \ - --post-inspection-runbooks '["first-runbook", "second-runbook"]' +apply-node-runbook --node NODE_UUID \ + --runbooks '["bmc-configure-snmpv3-user"]' ``` -For Argo, use the `enroll-server` WorkflowTemplate parameter -`post_inspection_runbooks` with the same JSON array. It defaults to `[]`. - -Runbooks execute sequentially after agent inspection and final BIOS -configuration, before RAID configuration, firmware updates, and `provide`. -Each requires at least one matching node/runbook trait (Ironic API 1.112 or -newer), uses the runbook's ramdisk setting, and waits for `manageable`. -A failed lookup, trait check, or cleaning operation stops enrollment. +For `active` nodes, the runner uses Ironic servicing and waits for the node to +return to `active` after each runbook. All requested runbooks must explicitly +set `disable_ramdisk=True`; Ironic also validates their service steps. This +prevents booting IPA, but the steps themselves must still be appropriate for a +running server. The SNMPv3 runbook configures the BMC account out of band. +Instance and allocation IDs are allowed on active nodes. Maintenance, conductor +reservations, and pending state transitions are rejected for all nodes. + +For `available` nodes, the runner transitions to `manageable`, runs manual +cleaning, then requests `provide` to return to `available`. Nodes starting in +`manageable` remain there. Failures stop the sequence without restoring the +starting state; inspect the Ironic error before retrying. + +For automatic execution after enrollment, configure `post_enroll_workflows` +with WorkflowTemplates that accept `node_uuid`. Enrollment still uses the idle +node cleaning path; active-node servicing does not change that behavior. diff --git a/python/understack-workflows/pyproject.toml b/python/understack-workflows/pyproject.toml index 3e122c907..eedc7b6ed 100644 --- a/python/understack-workflows/pyproject.toml +++ b/python/understack-workflows/pyproject.toml @@ -30,6 +30,7 @@ dependencies = [ ] [project.scripts] +apply-node-runbook = "understack_workflows.main.apply_node_runbook:main" bmc-kube-password = "understack_workflows.main.bmc_display_password:main" bmc-password = "understack_workflows.main.print_bmc_password:main" enroll-fw = "understack_workflows.main.enroll_fw:main" diff --git a/python/understack-workflows/tests/test_apply_node_runbook.py b/python/understack-workflows/tests/test_apply_node_runbook.py new file mode 100644 index 000000000..f85d5e41d --- /dev/null +++ b/python/understack-workflows/tests/test_apply_node_runbook.py @@ -0,0 +1,358 @@ +import argparse +from types import SimpleNamespace +from typing import cast +from unittest.mock import call + +import pytest +from ironicclient.v1.node import Node + +from understack_workflows import ironic_node +from understack_workflows.main import apply_node_runbook + +# --- ironic_node.run_runbook_standalone (state bracketing) ------------------- + + +def _patch_client( + mocker, + *, + provision_state, + runbook_traits=("CUSTOM_DELL",), + node_traits=("CUSTOM_DELL",), +): + client = mocker.patch.object(ironic_node, "IronicClient").return_value + client.get_runbook.return_value = SimpleNamespace( + uuid="runbook-123", traits=list(runbook_traits), disable_ramdisk=True + ) + client.get_node_traits.return_value = list(node_traits) + mocker.patch.object( + ironic_node, + "refresh", + side_effect=[SimpleNamespace(uuid="node-123", provision_state=provision_state)] + + [ + SimpleNamespace( + uuid="node-123", + provision_state="active" + if provision_state == "active" + else "manageable", + ) + ] + * 10, + ) + return client + + +def test_standalone_available_node_is_bracketed_manage_and_provide(mocker): + client = _patch_client(mocker, provision_state="available") + node = cast(Node, SimpleNamespace(uuid="node-123")) + + ironic_node.run_runbook_standalone(node, "bmc-configure-snmpv3-user") + + # manage -> clean(runbook) -> provide, in that order. + assert client.set_node_provision_state.mock_calls == [ + call( + "node-123", "manage", clean_steps=None, runbook=None, disable_ramdisk=None + ), + call( + "node-123", + "clean", + clean_steps=None, + runbook="runbook-123", + disable_ramdisk=None, + ), + call( + "node-123", "provide", clean_steps=None, runbook=None, disable_ramdisk=None + ), + ] + assert client.wait_for_node_provision_state.mock_calls == [ + call("node-123", "manageable", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS), + call("node-123", "manageable", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS), + call("node-123", "available", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS), + ] + + +def test_standalone_manageable_node_is_not_provided(mocker): + client = _patch_client(mocker, provision_state="manageable") + node = cast(Node, SimpleNamespace(uuid="node-123")) + + ironic_node.run_runbook_standalone(node, "bmc-configure-snmpv3-user") + + # No manage, no provide: only the clean runs; node stays manageable. + assert client.set_node_provision_state.mock_calls == [ + call( + "node-123", + "clean", + clean_steps=None, + runbook="runbook-123", + disable_ramdisk=None, + ), + ] + + +def test_standalone_active_node_uses_service_and_waits_for_active(mocker): + client = _patch_client(mocker, provision_state="active") + node = cast(Node, SimpleNamespace(uuid="node-123")) + + ironic_node.run_runbook_standalone(node, "bmc-configure-snmpv3-user") + + client.set_node_provision_state.assert_called_once_with( + "node-123", + "service", + clean_steps=None, + runbook="runbook-123", + disable_ramdisk=None, + ) + client.wait_for_node_provision_state.assert_called_once_with( + "node-123", "active", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS + ) + + +def test_active_node_allows_existing_instance_and_allocation(mocker): + client = _patch_client(mocker, provision_state="active") + node = cast( + Node, + SimpleNamespace( + uuid="node-123", + provision_state="active", + instance_uuid="instance-123", + allocation_id="allocation-123", + ), + ) + mocker.patch.object(ironic_node, "refresh", return_value=node) + + ironic_node.run_runbooks_standalone(node, ["first", "second"]) + + assert [c.args[1] for c in client.set_node_provision_state.call_args_list] == [ + "service", + "service", + ] + assert client.wait_for_node_provision_state.mock_calls == [ + call("node-123", "active", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS), + call("node-123", "active", timeout=ironic_node.NODE_STATE_TIMEOUT_SECS), + ] + + +@pytest.mark.parametrize("disable_ramdisk", [False, None]) +def test_active_sequence_preflights_ramdisk_setting_before_service( + mocker, disable_ramdisk +): + client = _patch_client(mocker, provision_state="active") + client.get_runbook.side_effect = [ + SimpleNamespace(uuid="r-1", traits=["CUSTOM_DELL"], disable_ramdisk=True), + SimpleNamespace( + uuid="r-2", traits=["CUSTOM_DELL"], disable_ramdisk=disable_ramdisk + ), + ] + with pytest.raises(ValueError, match="disable_ramdisk=True"): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first", "second"] + ) + client.set_node_provision_state.assert_not_called() + + +def test_active_sequence_stops_on_service_failure_without_state_restoration(mocker): + client = _patch_client(mocker, provision_state="active") + client.wait_for_node_provision_state.side_effect = RuntimeError("service failed") + with pytest.raises(RuntimeError, match="service failed"): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first", "second"] + ) + assert [c.args[1] for c in client.set_node_provision_state.call_args_list] == [ + "service" + ] + + +def test_active_sequence_refuses_state_change_to_idle_before_service(mocker): + client = _patch_client(mocker, provision_state="active") + mocker.patch.object( + ironic_node, + "refresh", + side_effect=[ + SimpleNamespace(uuid="node-123", provision_state=state) + for state in ["active", "available"] + ], + ) + with pytest.raises(ValueError, match="Expected active"): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first"] + ) + client.set_node_provision_state.assert_not_called() + + +def test_standalone_unexpected_state_is_refused(mocker): + client = _patch_client(mocker, provision_state="deploying") + node = cast(Node, SimpleNamespace(uuid="node-123")) + + with pytest.raises(ValueError, match="refusing to run runbook"): + ironic_node.run_runbook_standalone(node, "bmc-configure-snmpv3-user") + + client.set_node_provision_state.assert_not_called() + + +@pytest.mark.parametrize("runbook_traits", [[], ["CUSTOM_OTHER"], None]) +def test_standalone_rejects_ineligible_runbook(mocker, runbook_traits): + client = _patch_client( + mocker, + provision_state="manageable", + node_traits=("CUSTOM_DELL",), + ) + client.get_runbook.return_value.traits = runbook_traits + node = cast(Node, SimpleNamespace(uuid="node-123")) + + with pytest.raises(ValueError, match=r"traits metadata|matching node trait"): + ironic_node.run_runbook_standalone(node, "bmc-configure-snmpv3-user") + + client.set_node_provision_state.assert_not_called() + + +# --- CLI --------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "value", ["invalid", '"first"', "{}", "null", "[null]", "[1]", '[""]', '[" "]'] +) +def test_parse_runbooks_rejects_invalid(value): + with pytest.raises(argparse.ArgumentTypeError, match="JSON array"): + apply_node_runbook.parse_runbooks(value) + + +def test_main_single_node(mocker): + mocker.patch( + "sys.argv", + [ + "apply-node-runbook", + "--node", + "node-123", + "--runbooks", + '["first", "second"]', + ], + ) + mocker.patch.object(apply_node_runbook.helpers, "setup_logger") + client = mocker.patch.object(apply_node_runbook, "IronicClient").return_value + run = mocker.patch.object(ironic_node, "run_runbooks_standalone") + + apply_node_runbook.main() + + client.get_node.assert_called_once_with("node-123") + run.assert_called_once_with(client.get_node.return_value, ["first", "second"]) + + +def test_main_propagates_runbook_failure(mocker): + mocker.patch( + "sys.argv", + ["apply-node-runbook", "--node", "node-123", "--runbooks", '["first"]'], + ) + mocker.patch.object(apply_node_runbook.helpers, "setup_logger") + mocker.patch.object(apply_node_runbook, "IronicClient") + mocker.patch.object( + ironic_node, "run_runbooks_standalone", side_effect=RuntimeError("clean failed") + ) + + with pytest.raises(RuntimeError, match="clean failed"): + apply_node_runbook.main() + + +def test_main_requires_node(mocker, capsys): + mocker.patch("sys.argv", ["apply-node-runbook", "--runbooks", '["first"]']) + mocker.patch.object(apply_node_runbook.helpers, "setup_logger") + client = mocker.patch.object(apply_node_runbook, "IronicClient") + + with pytest.raises(SystemExit) as exc: + apply_node_runbook.main() + + assert exc.value.code == 2 + assert "--node" in capsys.readouterr().err + client.assert_not_called() + + +@pytest.mark.parametrize( + ("state", "field"), + [ + (state, field) + for state in ["available", "manageable", "active"] + for field in [ + "maintenance", + "allocation_id", + "instance_uuid", + "reservation", + "target_provision_state", + ] + if state != "active" or field not in {"instance_uuid", "allocation_id"} + ], +) +def test_standalone_refuses_reserved_or_unhealthy_node(mocker, field, state): + client = _patch_client(mocker, provision_state=state) + mocker.patch.object( + ironic_node, + "refresh", + return_value=SimpleNamespace( + uuid="node-123", provision_state=state, **{field: True} + ), + ) + with pytest.raises(ValueError, match=field): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first"] + ) + client.set_node_provision_state.assert_not_called() + + +def test_sequence_brackets_once_and_stops_without_provide_on_failure(mocker): + client = _patch_client(mocker, provision_state="available") + client.wait_for_node_provision_state.side_effect = [ + None, + None, + RuntimeError("clean failed"), + ] + node = cast(Node, SimpleNamespace(uuid="node-123")) + with pytest.raises(RuntimeError, match="clean failed"): + ironic_node.run_runbooks_standalone(node, ["first", "second", "third"]) + assert [c.args[1] for c in client.set_node_provision_state.call_args_list] == [ + "manage", + "clean", + "clean", + ] + + +def test_sequence_restores_available_only_once(mocker): + client = _patch_client(mocker, provision_state="available") + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first", "second"] + ) + assert [c.args[1] for c in client.set_node_provision_state.call_args_list] == [ + "manage", + "clean", + "clean", + "provide", + ] + + +def test_empty_sequence_does_not_change_state(mocker): + refresh = mocker.patch.object(ironic_node, "refresh") + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), [] + ) + refresh.assert_not_called() + + +def test_sequence_preflights_later_runbooks_before_state_change(mocker): + client = _patch_client(mocker, provision_state="available") + client.get_runbook.side_effect = [ + SimpleNamespace(uuid="r-1", traits=["CUSTOM_DELL"]), + SimpleNamespace(uuid="r-2", traits=["CUSTOM_OTHER"]), + ] + with pytest.raises(ValueError, match="matching node trait"): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first", "second"] + ) + client.set_node_provision_state.assert_not_called() + + +def test_sequence_does_not_provide_when_manage_fails(mocker): + client = _patch_client(mocker, provision_state="available") + client.wait_for_node_provision_state.side_effect = RuntimeError("manage failed") + with pytest.raises(RuntimeError, match="manage failed"): + ironic_node.run_runbooks_standalone( + cast(Node, SimpleNamespace(uuid="node-123")), ["first", "second"] + ) + assert [c.args[1] for c in client.set_node_provision_state.call_args_list] == [ + "manage" + ] diff --git a/python/understack-workflows/tests/test_enroll_server.py b/python/understack-workflows/tests/test_enroll_server.py index f2cec7911..3f9a9d1f8 100644 --- a/python/understack-workflows/tests/test_enroll_server.py +++ b/python/understack-workflows/tests/test_enroll_server.py @@ -1,4 +1,3 @@ -import argparse import secrets from types import SimpleNamespace from typing import cast @@ -104,17 +103,6 @@ def make_bmc(mocker, fake_sushy=None): return bmc -def make_raid_hardware(): - controller = SimpleNamespace( - identity="RAID.Integrated.1-1", - drives=[SimpleNamespace(identity="Disk1"), SimpleNamespace(identity="Disk2")], - ) - system = SimpleNamespace(storage=SimpleNamespace(get_members=lambda: [controller])) - return SimpleNamespace( - get_system_collection=lambda: SimpleNamespace(get_members=lambda: [system]) - ) - - def make_ironic_client( *, node_name: str, @@ -178,263 +166,6 @@ def make_port(*, address: str, pxe_enabled: bool, bios_name: str | None): ) -def test_enrol_happy_path_uses_virtual_media_inspect_and_flips_back(mocker): - device_info = make_device_info() - fake_bmc = make_bmc(mocker, fake_sushy=make_raid_hardware()) - inventory = make_node_inventory( - connected_interface_names=["NIC.Integrated.1-1", "NIC.Integrated.1-2"], - storage_controllers=[ - { - "id": "RAID.Integrated.1-1", - "drives": [ - {"id": "Disk1", "size": 479559942144}, - {"id": "Disk2", "size": 479559942144}, - ], - } - ], - ) - # port-enroll-config hook has run during agent inspection and flagged - # pxe_enabled on LLDP-connected ports. port-bios-name (OOB) stamped - # extra.bios_name earlier. - ports = [ - make_port( - address="aa:aa:aa:aa:aa:01", - pxe_enabled=True, - bios_name="NIC.Integrated.1-1", - ), - make_port( - address="aa:aa:aa:aa:aa:02", - pxe_enabled=True, - bios_name="NIC.Integrated.1-2", - ), - make_port( - address="aa:aa:aa:aa:aa:03", - pxe_enabled=False, - bios_name="NIC.Slot.1-1", - ), - ] - fake_ironic, created_node = make_ironic_client( - node_name="Dell-ABC123", - # OOB inspect, agent inspect, OOB inspect (post-RAID). - inspect_interfaces=["idrac-redfish", "idrac-redfish", "idrac-redfish"], - inventory=inventory, - ports=ports, - ) - - mocker.patch.object(enroll_server, "bmc_for_ip_address", return_value=fake_bmc) - mocker.patch.object(enroll_server, "chassis_info", return_value=device_info) - set_bmc_password = mocker.patch.object(enroll_server, "set_bmc_password") - update_dell_drac_settings = mocker.patch.object( - enroll_server, "update_dell_drac_settings" - ) - bmc_set_hostname = mocker.patch.object(enroll_server, "bmc_set_hostname") - update_dell_bios_settings = mocker.patch.object( - enroll_server, "update_dell_bios_settings" - ) - mocker.patch( - "understack_workflows.ironic.client.get_ironic_client", - return_value=fake_ironic, - ) - - enroll_server.enroll( - ip_address="10.0.0.10", - firmware_update=False, - raid_configure=True, - old_password="old-password", - external_cmdb_id="cmdb-1", - ) - - set_bmc_password.assert_called_once_with( - ip_address="10.0.0.10", - new_password="calvin", - old_password="old-password", - ) - update_dell_drac_settings.assert_called_once_with(fake_bmc) - bmc_set_hostname.assert_called_once_with(fake_bmc, "None", "Dell-ABC123") - - # BIOS settings configured exactly once, using pxe_enabled port BIOS names. - update_dell_bios_settings.assert_any_call( - fake_bmc, - pxe_interface="NIC.Integrated.1-1", - ) - - fake_ironic.node.create.assert_called_once_with( - name="Dell-ABC123", - driver="idrac", - driver_info={ - "redfish_address": "https://10.0.0.10", - "redfish_verify_ca": False, - "redfish_username": "root", - "redfish_password": "calvin", - }, - boot_interface="http-ipxe", - deploy_interface="direct", - inspect_interface="idrac-redfish", - extra={"external_cmdb_id": "cmdb-1"}, - ) - - fake_ironic.node.set_target_raid_config.assert_called_once_with( - created_node.uuid, - { - "logical_disks": [ - { - "controller": "RAID.Integrated.1-1", - "is_root_volume": True, - "physical_disks": ["Disk1", "Disk2"], - "raid_level": "1", - "size_gb": "MAX", - } - ] - }, - ) - - fake_ironic.node.set_provision_state.assert_has_calls( - [ - call( - created_node.uuid, - "manage", - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - created_node.uuid, - "inspect", # OOB redfish inspect for bios_name / basic info - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - created_node.uuid, - "inspect", # agent inspect via virtual media - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - created_node.uuid, - "clean", - cleansteps=[ - {"interface": "raid", "step": "delete_configuration"}, - {"interface": "raid", "step": "create_configuration"}, - ], - runbook=None, - disable_ramdisk=False, - ), - call( - created_node.uuid, - "inspect", # OOB redfish inspect to refresh disks post-RAID - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - created_node.uuid, - "provide", - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - ] - ) - - expected_reset = [{"op": "remove", "path": "/inspect_interface"}] - expected_ipxe_boot = [ - {"op": "add", "path": "/boot_interface", "value": "http-ipxe"} - ] - expected_agent = [{"op": "add", "path": "/inspect_interface", "value": "agent"}] - assert fake_ironic.node.update.call_args_list == [ - call(created_node.uuid, expected_reset), # OOB inspect prep - call(created_node.uuid, expected_ipxe_boot), - call(created_node.uuid, expected_agent), - call(created_node.uuid, expected_ipxe_boot), - call(created_node.uuid, expected_reset), # Post-RAID OOB inspect prep - ] - - -def test_enrol_existing_failed_node_recovers_and_updates(mocker): - device_info = make_device_info() - existing_node = SimpleNamespace( - uuid="node-999", - provision_state="inspect failed", - driver="idrac", - inspect_interface="idrac-redfish", - ) - inventory = make_node_inventory( - connected_interface_names=["NIC.Integrated.1-1"], - ) - ports = [ - make_port( - address="aa:aa:aa:aa:aa:01", - pxe_enabled=True, - bios_name="NIC.Integrated.1-1", - ), - ] - fake_bmc = make_bmc(mocker) - fake_ironic, _ = make_ironic_client( - node_name="Dell-ABC123", - node_uuid="node-999", - existing_node=existing_node, - inspect_interfaces=["idrac-redfish", "idrac-redfish"], - inventory=inventory, - ports=ports, - ) - - mocker.patch.object(enroll_server, "bmc_for_ip_address", return_value=fake_bmc) - mocker.patch.object(enroll_server, "chassis_info", return_value=device_info) - mocker.patch.object(enroll_server, "set_bmc_password") - mocker.patch.object(enroll_server, "update_dell_drac_settings") - mocker.patch.object(enroll_server, "bmc_set_hostname") - mocker.patch.object(enroll_server, "update_dell_bios_settings") - mocker.patch( - "understack_workflows.ironic.client.get_ironic_client", - return_value=fake_ironic, - ) - - enroll_server.enroll( - ip_address="10.0.0.10", - firmware_update=False, - raid_configure=False, - old_password=None, - external_cmdb_id="cmdb-1", - ) - - fake_ironic.node.create.assert_not_called() - fake_ironic.node.set_target_raid_config.assert_not_called() - fake_ironic.node.set_provision_state.assert_has_calls( - [ - call( - existing_node.uuid, - "manage", - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - existing_node.uuid, - "inspect", # OOB inspect - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - existing_node.uuid, - "inspect", # Agent inspect via virtual media - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - call( - existing_node.uuid, - "provide", - cleansteps=None, - runbook=None, - disable_ramdisk=None, - ), - ] - ) - - def test_apply_firmware_updates_runs_traits_in_numeric_order(mocker): fake_ironic, _ = make_ironic_client( node_name="unused", @@ -600,178 +331,57 @@ def test_get_lldp_connected_interfaces_absent_from_lldp(): assert result == [] -@pytest.mark.parametrize("runbooks", [None, [], ["first", "second"]]) -def test_post_inspection_runbook_order(mocker, runbooks): +@pytest.mark.parametrize("configure_hardware", [False, True]) +def test_minimal_enrollment_only_reads_chassis_and_provides_node( + mocker, configure_hardware +): node = SimpleNamespace(uuid="node-123") bmc = make_bmc(mocker) mocker.patch.object(enroll_server, "bmc_for_ip_address", return_value=bmc) - mocker.patch.object( - enroll_server, "initialize_bmc", return_value=make_device_info() - ) - mocker.patch.object( - enroll_server.ironic_node, "create_or_update", return_value=node - ) - mocker.patch.object(enroll_server.ironic_node, "inspect_out_of_band") - mocker.patch.object( - enroll_server.ironic_node, "get_node_inventory", return_value={} - ) - mocker.patch.object( - enroll_server.ironic_node, "pxe_enabled_bios_name", return_value="NIC.1" - ) events = MagicMock() - for name in ("agent_inspection", "update_dell_bios_settings", "configure_raid"): - events.attach_mock(mocker.patch.object(enroll_server, name), name) - for name in ("run_post_inspection_runbook", "apply_firmware_updates", "transition"): + events.attach_mock( + mocker.patch.object( + enroll_server, "chassis_info", return_value=make_device_info() + ), + "chassis_info", + ) + events.attach_mock( + mocker.patch.object( + enroll_server.ironic_node, "create_or_update", return_value=node + ), + "create_or_update", + ) + for name in ( + "inspect_out_of_band", + "get_node_inventory", + "pxe_enabled_bios_name", + "inspect", + "apply_firmware_updates", + "transition", + ): events.attach_mock(mocker.patch.object(enroll_server.ironic_node, name), name) result = enroll_server.enroll( ip_address=bmc.ip_address, - firmware_update=True, - raid_configure=True, - old_password=None, - post_inspection_runbooks=runbooks, + firmware_update=configure_hardware, + raid_configure=configure_hardware, + old_password="unused", + external_cmdb_id="cmdb-1", ) - expected = [ - call.update_dell_bios_settings(bmc, pxe_interface="NIC.1"), - call.agent_inspection(node, virtual_media=False), - call.update_dell_bios_settings(bmc, pxe_interface="NIC.1"), + assert events.mock_calls == [ + call.chassis_info(bmc), + call.create_or_update( + bmc=bmc, + name="Dell-ABC123", + manufacturer="Dell", + external_cmdb_id="cmdb-1", + ), + call.transition(node, target_state="provide", expected_state="available"), ] - expected.extend( - call.run_post_inspection_runbook(node, runbook) for runbook in runbooks or [] - ) - expected.extend( - [ - call.configure_raid(node, {}), - call.apply_firmware_updates(node), - call.transition(node, target_state="provide", expected_state="available"), - ] - ) - assert events.mock_calls == expected assert result == node.uuid -def test_post_inspection_runbook_uses_uuid_and_waits(mocker): - client = mocker.patch.object(enroll_server.ironic_node, "IronicClient").return_value - client.get_runbook.return_value = SimpleNamespace( - uuid="runbook-123", traits=["CUSTOM_DELL", "CUSTOM_OTHER"], disable_ramdisk=True - ) - client.get_node_traits.return_value = ["CUSTOM_DELL"] - node = cast(Node, SimpleNamespace(uuid="node-123")) - - enroll_server.ironic_node.run_post_inspection_runbook(node, "post-inspection") - - client.get_runbook.assert_called_once_with("post-inspection") - client.get_node_traits.assert_called_once_with(node.uuid) - client.set_node_provision_state.assert_called_once_with( - node.uuid, - "clean", - clean_steps=None, - runbook="runbook-123", - disable_ramdisk=None, - ) - client.wait_for_node_provision_state.assert_called_once_with( - node.uuid, - "manageable", - timeout=enroll_server.ironic_node.NODE_STATE_TIMEOUT_SECS, - ) - - -@pytest.mark.parametrize("traits", [[], ["CUSTOM_OTHER"], None]) -def test_post_inspection_runbook_rejects_ineligible_node(mocker, traits): - client = mocker.patch.object(enroll_server.ironic_node, "IronicClient").return_value - client.get_runbook.return_value = SimpleNamespace(uuid="runbook-123", traits=traits) - client.get_node_traits.return_value = ["CUSTOM_DELL"] - node = cast(Node, SimpleNamespace(uuid="node-123")) - - with pytest.raises(ValueError, match=r"traits metadata|matching node trait"): - enroll_server.ironic_node.run_post_inspection_runbook(node, "post-inspection") - - client.set_node_provision_state.assert_not_called() - - -def test_post_inspection_runbook_failure_propagates(mocker): - client = mocker.patch.object(enroll_server.ironic_node, "IronicClient").return_value - client.get_runbook.return_value = SimpleNamespace( - uuid="runbook-123", traits=["CUSTOM_DELL"] - ) - client.get_node_traits.return_value = ["CUSTOM_DELL"] - client.wait_for_node_provision_state.side_effect = RuntimeError("clean failed") - node = cast(Node, SimpleNamespace(uuid="node-123")) - - with pytest.raises(RuntimeError, match="clean failed"): - enroll_server.ironic_node.run_post_inspection_runbook(node, "post-inspection") - - -def test_post_inspection_runbook_cli(mocker): - mocker.patch( - "sys.argv", - [ - "enroll-server", - "--ip-address", - "10.0.0.10", - "--post-inspection-runbooks", - '["first", "second"]', - ], - ) - mocker.patch.object(enroll_server.helpers, "setup_logger") - enroll = mocker.patch.object(enroll_server, "enroll") - - enroll_server.main() - - assert enroll.call_args.kwargs["post_inspection_runbooks"] == ["first", "second"] - - -@pytest.mark.parametrize( - "value", ["invalid", '"first"', "{}", "null", "[null]", "[1]", '[""]', '[" "]'] -) -def test_post_inspection_runbooks_reject_invalid_cli(value): - with pytest.raises(argparse.ArgumentTypeError, match="JSON array"): - enroll_server.parse_runbooks(value) - - -def test_post_inspection_runbooks_stop_on_failure(mocker): - node = SimpleNamespace(uuid="node-123") - mocker.patch.object(enroll_server, "bmc_for_ip_address") - mocker.patch.object( - enroll_server, "initialize_bmc", return_value=make_device_info() - ) - mocker.patch.object( - enroll_server.ironic_node, "create_or_update", return_value=node - ) - mocker.patch.object(enroll_server.ironic_node, "inspect_out_of_band") - mocker.patch.object( - enroll_server.ironic_node, "get_node_inventory", return_value={} - ) - mocker.patch.object( - enroll_server.ironic_node, "pxe_enabled_bios_name", return_value="NIC.1" - ) - mocker.patch.object(enroll_server, "agent_inspection") - mocker.patch.object(enroll_server, "update_dell_bios_settings") - runbook = mocker.patch.object( - enroll_server.ironic_node, - "run_post_inspection_runbook", - side_effect=RuntimeError("clean failed"), - ) - raid = mocker.patch.object(enroll_server, "configure_raid") - firmware = mocker.patch.object(enroll_server.ironic_node, "apply_firmware_updates") - transition = mocker.patch.object(enroll_server.ironic_node, "transition") - - with pytest.raises(RuntimeError, match="clean failed"): - enroll_server.enroll( - ip_address="10.0.0.10", - firmware_update=True, - raid_configure=True, - old_password=None, - post_inspection_runbooks=["first", "second"], - ) - - runbook.assert_called_once_with(node, "first") - raid.assert_not_called() - firmware.assert_not_called() - transition.assert_not_called() - - def test_main_emits_node_uuid_only_after_success(mocker, tmp_path): output = tmp_path / "node-uuid" mocker.patch( diff --git a/python/understack-workflows/understack_workflows/ironic_node.py b/python/understack-workflows/understack_workflows/ironic_node.py index 54baa97e3..92632e298 100644 --- a/python/understack-workflows/understack_workflows/ironic_node.py +++ b/python/understack-workflows/understack_workflows/ironic_node.py @@ -339,35 +339,6 @@ def inspect(node: Node, inspect_interface: str = "agent"): transition(node, target_state="inspect", expected_state="manageable") -def run_post_inspection_runbook(node: Node, name_or_id: str) -> None: - """Run a selected runbook while the inspected node is manageable. - - Require at least one matching runbook trait before starting cleaning. - Let the runbook determine whether cleaning needs an agent ramdisk. - """ - client = IronicClient() - runbook = client.get_runbook(name_or_id) - traits = client.get_node_traits(node.uuid) - runbook_traits = getattr(runbook, "traits", None) - if runbook_traits is None: - raise ValueError( - f"[node:{node.uuid}] Runbook {name_or_id!r} has no traits metadata; " - "Ironic API 1.112 or newer is required" - ) - if not set(traits).intersection(runbook_traits): - raise ValueError( - f"[node:{node.uuid}] Runbook {name_or_id!r} requires at least one " - f"matching node trait from {sorted(runbook_traits)}" - ) - logger.info("[node:%s] Running post-inspection runbook %s", node.uuid, name_or_id) - transition( - node, - target_state="clean", - expected_state="manageable", - runbook=runbook.uuid, - ) - - def apply_firmware_updates(node: Node) -> None: client = IronicClient() traits = client.get_node_traits(node.uuid) diff --git a/python/understack-workflows/understack_workflows/main/apply_node_runbook.py b/python/understack-workflows/understack_workflows/main/apply_node_runbook.py new file mode 100644 index 000000000..829503099 --- /dev/null +++ b/python/understack-workflows/understack_workflows/main/apply_node_runbook.py @@ -0,0 +1,57 @@ +import argparse +import json + +from understack_workflows import helpers +from understack_workflows import ironic_node +from understack_workflows.ironic.client import IronicClient + + +def main() -> None: + """Run Ironic runbooks in order against an enrolled node.""" + helpers.setup_logger() + args = argument_parser().parse_args() + node = IronicClient().get_node(args.node) + ironic_node.run_runbooks_standalone(node, args.runbooks) + + +def parse_runbooks(value: str) -> list[str]: + """Parse an ordered JSON array of nonempty runbook names or UUIDs.""" + try: + runbooks = json.loads(value) + except ValueError as exc: + raise argparse.ArgumentTypeError( + "Expected a JSON array of runbook names or UUIDs" + ) from exc + if not isinstance(runbooks, list) or any( + not isinstance(runbook, str) or not runbook.strip() for runbook in runbooks + ): + raise argparse.ArgumentTypeError( + "Expected a JSON array of nonempty runbook names or UUIDs" + ) + return runbooks + + +def argument_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="apply-node-runbook", + description="Run Ironic runbooks in order against an enrolled node.", + ) + parser.add_argument( + "--runbooks", + type=parse_runbooks, + required=True, + help=( + "JSON array of runbook names or UUIDs to run in order, e.g. " + "'[\"bmc-configure-snmpv3-user\"]'" + ), + ) + parser.add_argument( + "--node", + required=True, + help="Node name or UUID", + ) + return parser + + +if __name__ == "__main__": + main() diff --git a/python/understack-workflows/understack_workflows/main/enroll_server.py b/python/understack-workflows/understack_workflows/main/enroll_server.py index b58ad4209..b23035dc1 100644 --- a/python/understack-workflows/understack_workflows/main/enroll_server.py +++ b/python/understack-workflows/understack_workflows/main/enroll_server.py @@ -1,65 +1,25 @@ # pylint: disable=E1131,C0103 import argparse -import json import logging import os from pathlib import Path -from ironicclient.v1.node import Node - from understack_workflows import helpers from understack_workflows import ironic_node -from understack_workflows.bmc import Bmc from understack_workflows.bmc import bmc_for_ip_address -from understack_workflows.bmc_bios import update_dell_bios_settings from understack_workflows.bmc_chassis_info import ChassisInfo from understack_workflows.bmc_chassis_info import chassis_info -from understack_workflows.bmc_credentials import set_bmc_password -from understack_workflows.bmc_hostname import bmc_set_hostname -from understack_workflows.bmc_settings import update_dell_drac_settings -from understack_workflows.raid import configure_raid logger = logging.getLogger(__name__) -MAX_PXE_INTERFACES = 8 - def main() -> None: - """On-board a new baremetal node, or refresh an existing one. - - - Connect to the BMC using standard password, if that fails then use the - password supplied in --old-bmc-password option, or factory default. - - - Ensure standard BMC password is set, and other basic BMC settings. - - - From BMC, discover basic chassis info: manufacturer, model, serial. - - - Find or create this baremetal node in Ironic: - - Set the name to "{manufacturer}-{servicetag}". - - Set the driver as appropriate for the manufacturer/model. - - Set the node's external_cmdb_id, if one was provided. - - - Perform agent inspection via virtual-media boot to capture the full - hardware inventory and LLDP-confirmed switch topology. - - Using virtual-media avoids having to configure PXE during the initial - phase, however the IPA agent still requires DHCP autoconfiguration, so it - needs to be connected to the provisioning VLAN, and it needs at least one - port in Neutron to be configured for "enroll". - - - Configure the Dell HTTP-boot BIOS entries using the LLDP-confirmed PXE - interfaces, then flip the node back to its final production http-ipxe boot - mode for cleaning and provisioning (we prefer PXE-based booting over - virtual media, for performance reasons). - - - Optionally execute post-inspection runbooks in the supplied order. - - - Optionally configure RAID. + """Use minimal enrollment to test post-enrollment workflows. - - Optionally apply firmware updates. - - - Transition the node to the 'available' state (implies cleaning). + Read chassis info, create or update the Ironic node, then provide it and + return its UUID. BMC configuration, inspection, BIOS, RAID, and firmware + updates are skipped. The existing CLI arguments remain accepted for Argo. """ helpers.setup_logger() args = argument_parser().parse_args() @@ -70,7 +30,6 @@ def main() -> None: firmware_update=args.firmware_update, raid_configure=args.raid_configure, external_cmdb_id=args.external_cmdb_id, - post_inspection_runbooks=args.post_inspection_runbooks, ) if args.node_uuid_output: @@ -83,15 +42,16 @@ def enroll( raid_configure: bool, old_password: str | None, external_cmdb_id: int | str | None = None, - post_inspection_runbooks: list[str] | None = None, ) -> str: - logger.info("Starting enroll workflow for bmc_ip_address=%s", ip_address) + logger.info( + "Starting minimal test enroll workflow for bmc_ip_address=%s", ip_address + ) if external_cmdb_id: logger.info(" external_cmdb_id=%s", external_cmdb_id) bmc = bmc_for_ip_address(ip_address) - device_info = initialize_bmc(bmc, old_password) + device_info = chassis_info(bmc) node = ironic_node.create_or_update( bmc=bmc, @@ -100,105 +60,11 @@ def enroll( external_cmdb_id=external_cmdb_id, ) - # Out-of-band redfish inspection populates data including baremetal ports. - # - # Our hooks augment the ironic baremetal port with the BMC-reported - # interface name (e.g. NIC.Integrated.1-1) as well as some placeholder - # "enrol" dummy data that is required by Ironic/Neutron to perform agent - # inspection. Neutron needs to assign a port to the provisioning network - # and it bails out unless we have ports with pxe_enabled, local_link_info, - # etc. - ironic_node.inspect_out_of_band(node) - inventory = ironic_node.get_node_inventory(node) - - # Agent inspection gathers LLDP and full hardware inventory. - # - # Virtual media boot makes Neutron behave differently to normal HTTP agent - # boot - in our normal configuration it fails to set up ports on the - # provisioning network. - # - # Therefore, we only use virtual media during our "enroll" phase, when the - # port data is set up in a manner that suits the Neutron algorithm. If a - # normal PXE/HTTP port is available then we use it instead: - - pxe_interface = ironic_node.pxe_enabled_bios_name(node) - update_dell_bios_settings(bmc, pxe_interface=pxe_interface) - agent_inspection(node, virtual_media=not pxe_interface) - - pxe_interface = ironic_node.pxe_enabled_bios_name(node) - if not pxe_interface: - raise RuntimeError( - f"[node:{node.uuid}] Agent inspection produced no pxe_enabled ports " - "cannot configure HTTP boot." - ) - logger.info("[node:%s] Selected PXE interface %s", node.uuid, pxe_interface) - - update_dell_bios_settings(bmc, pxe_interface=pxe_interface) - - for runbook in post_inspection_runbooks or []: - ironic_node.run_post_inspection_runbook(node, runbook) - - if raid_configure: - configure_raid(node, inventory) - # RAID reconfiguration changes the disk layout; refresh inventory. - ironic_node.inspect_out_of_band(node) - else: - logger.info("%s RAID configuration was not requested", node.uuid) - - if firmware_update: - ironic_node.apply_firmware_updates(node) - ironic_node.transition(node, target_state="provide", expected_state="available") - logger.info("Completed enroll workflow for bmc_ip_address=%s", ip_address) - return str(node.uuid) - - -def agent_inspection(node: Node, virtual_media=False) -> None: - """Run agent inspection booted, optionally via virtual media. - - If requested, the node is temporarily flipped to a virtual-media - boot_interface so the agent ramdisk boots from a BMC-attached ISO rather - than via PXE. After initial inspection, the boot_interface is set back to - the steady-state value used for cleaning and provisioning. - - If cleaning fails we don't reset the boot_interface (as Ironic won't let us - change that attribute in the inspect_failed state). - """ - if virtual_media: - refreshed = ironic_node.refresh(node, fields=["driver"]) - boot_interface = ironic_node.virtual_media_boot_interface_for(refreshed.driver) - else: - boot_interface = ironic_node.STEADY_STATE_BOOT_INTERFACE - logger.info( - "[node:%s] Switching boot_interface to %s for agent inspection", - node.uuid, - boot_interface, - ) - ironic_node.patch(node, [f"boot_interface={boot_interface}"]) - ironic_node.inspect(node) - ironic_node.patch( - node, [f"boot_interface={ironic_node.STEADY_STATE_BOOT_INTERFACE}"] + "Completed minimal test enroll workflow for bmc_ip_address=%s", ip_address ) - - -def initialize_bmc(bmc: Bmc, old_password: str | None) -> ChassisInfo: - """Discover and configure BMC with Undercloud standard settings.""" - set_bmc_password( - ip_address=bmc.ip_address, - new_password=bmc.password, - old_password=old_password, - ) - - device_info = chassis_info(bmc) - for line in device_info.dump: - logger.info("Discovered %s", line) - - if device_info.manufacturer == "Dell": - update_dell_drac_settings(bmc) - - bmc_set_hostname(bmc, device_info.bmc_hostname, device_name(device_info)) - return device_info + return str(node.uuid) def device_name(device_info: ChassisInfo) -> str: @@ -209,23 +75,6 @@ def parse_bool(value: str) -> bool: return value.lower() == "true" -def parse_runbooks(value: str) -> list[str]: - """Parse an ordered JSON array of runbook names or UUIDs.""" - try: - runbooks = json.loads(value) - except ValueError as exc: - raise argparse.ArgumentTypeError( - "Expected a JSON array of runbook names or UUIDs" - ) from exc - if not isinstance(runbooks, list) or any( - not isinstance(runbook, str) or not runbook.strip() for runbook in runbooks - ): - raise argparse.ArgumentTypeError( - "Expected a JSON array of nonempty runbook names or UUIDs" - ) - return runbooks - - def argument_parser(): parser = argparse.ArgumentParser( prog=os.path.basename(__file__), @@ -241,15 +90,6 @@ def argument_parser(): required=False, help="Old (current) BMC password", ) - parser.add_argument( - "--post-inspection-runbooks", - type=parse_runbooks, - default=[], - help=( - "JSON array of runbook names or UUIDs to execute in order " - "after inspection and BIOS configuration" - ), - ) parser.add_argument( "--firmware-update", type=parse_bool, diff --git a/workflows/argo-events/kustomization.yaml b/workflows/argo-events/kustomization.yaml index ac59f6bf3..fe18f8fce 100644 --- a/workflows/argo-events/kustomization.yaml +++ b/workflows/argo-events/kustomization.yaml @@ -16,6 +16,7 @@ resources: - workflowtemplates/enroll-fw.yaml - workflowtemplates/enroll-netdev.yaml - workflowtemplates/enroll-server.yaml + - workflowtemplates/workflowtemplate-bmc-configure-snmpv3-user.yaml - workflowtemplates/reclean-server.yaml - workflowtemplates/openstack-oslo-event.yaml - workflowtemplates/netapp-configure-net.yaml diff --git a/workflows/argo-events/workflowtemplates/enroll-server.yaml b/workflows/argo-events/workflowtemplates/enroll-server.yaml index 675411356..8bc4c7d26 100644 --- a/workflows/argo-events/workflowtemplates/enroll-server.yaml +++ b/workflows/argo-events/workflowtemplates/enroll-server.yaml @@ -29,8 +29,6 @@ spec: arguments: parameters: - name: ip_address - - name: post_inspection_runbooks - value: "[]" - name: firmware_update value: "false" - name: raid_configure @@ -46,7 +44,8 @@ spec: # A site can configure this value with a JSON array of # WorkflowTemplate names. Each WorkflowTemplate must expose a `run` # template that accepts `node_uuid`. If the ConfigMap is absent, no - # post-enrollment templates are run. + # post-enrollment templates are run. Entries run in list order, and + # a failure prevents subsequent entries from running. - name: post_enroll_workflows valueFrom: configMapKeyRef: @@ -62,14 +61,41 @@ spec: - - name: enroll-server template: enroll-server - - name: post-enroll + template: post-enroll-sequence + when: "{{=len(fromJSON(inputs.parameters.post_enroll_workflows)) > 0}}" + arguments: + parameters: + - name: node_uuid + value: "{{steps.enroll-server.outputs.parameters.node_uuid}}" + - name: workflows + value: "{{inputs.parameters.post_enroll_workflows}}" + # Recurse over the remaining names only after the current hook succeeds. + # A withParam loop would schedule independent hooks concurrently. + - name: post-enroll-sequence + inputs: + parameters: + - name: node_uuid + - name: workflows + steps: + - - name: run-workflow templateRef: name: "{{item}}" template: run arguments: parameters: - name: node_uuid - value: "{{steps.enroll-server.outputs.parameters.node_uuid}}" - withParam: "{{inputs.parameters.post_enroll_workflows}}" + value: "{{inputs.parameters.node_uuid}}" + # Expand only the first entry; the next step handles the tail. + withParam: "{{=toJson(fromJSON(inputs.parameters.workflows)[:1])}}" + - - name: remaining-workflows + template: post-enroll-sequence + when: "{{=len(fromJSON(inputs.parameters.workflows)) > 1}}" + arguments: + parameters: + - name: node_uuid + value: "{{inputs.parameters.node_uuid}}" + - name: workflows + value: "{{=toJson(fromJSON(inputs.parameters.workflows)[1:])}}" - name: enroll-server outputs: parameters: @@ -77,7 +103,7 @@ spec: valueFrom: path: /run/enrollment/node-uuid container: - image: ghcr.io/rackerlabs/understack/ironic-nautobot-client:latest + image: ghcr.io/rackerlabs/understack/ironic-nautobot-client:pr-2608 command: - enroll-server args: @@ -85,8 +111,6 @@ spec: - /run/enrollment/node-uuid - --ip-address - "{{workflow.parameters.ip_address}}" - - --post-inspection-runbooks - - "{{workflow.parameters.post_inspection_runbooks}}" - --firmware-update - "{{workflow.parameters.firmware_update}}" - --raid-configure diff --git a/workflows/argo-events/workflowtemplates/workflowtemplate-bmc-configure-snmpv3-user.yaml b/workflows/argo-events/workflowtemplates/workflowtemplate-bmc-configure-snmpv3-user.yaml new file mode 100644 index 000000000..b47a10c02 --- /dev/null +++ b/workflows/argo-events/workflowtemplates/workflowtemplate-bmc-configure-snmpv3-user.yaml @@ -0,0 +1,52 @@ +apiVersion: argoproj.io/v1alpha1 +kind: WorkflowTemplate +metadata: + name: bmc-configure-snmpv3-user + annotations: + workflows.argoproj.io/title: Configure the SNMPv3 user on a node BMC + workflows.argoproj.io/description: | + Defined in `workflows/argo-events/workflowtemplates/workflowtemplate-bmc-configure-snmpv3-user.yaml` + + Applies the bmc-configure-snmpv3-user runbook to the specified Ironic node. + Active nodes use out-of-band servicing and return to active on success. + Available or manageable nodes use the existing manual cleaning path. +spec: + entrypoint: run + serviceAccountName: workflow + arguments: + parameters: + - name: node_uuid + templates: + - name: run + inputs: + parameters: + - name: node_uuid + synchronization: + mutexes: + - name: ironic-runbooks-{{inputs.parameters.node_uuid}} + volumes: + - name: openstack + secret: + secretName: baremetal-manage + items: + - key: clouds.yaml + path: clouds.yaml + container: + image: ghcr.io/rackerlabs/understack/ironic-nautobot-client:pr-2607 + command: + - apply-node-runbook + args: + - --node + - "{{inputs.parameters.node_uuid}}" + - --runbooks + - '["bmc-configure-snmpv3-user"]' + envFrom: + - configMapRef: + name: cluster-metadata + env: + - name: OS_CLOUD + value: understack + volumeMounts: + - name: openstack + mountPath: /etc/openstack + readOnly: true