Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions osism/commands/baremetal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,12 @@ def get_parser(self, prog_name):
type=str,
help="Clean given baremetal node when in provision state available",
)
parser.add_argument(
"--metadata-only",
default=False,
help="Only erase metadata on disks",
action="store_true",
)
parser.add_argument(
"--all",
default=False,
Expand All @@ -1201,6 +1207,7 @@ def take_action(self, parsed_args):
cloud = parsed_args.cloud
all_nodes = parsed_args.all
name = parsed_args.name
metadata_only = parsed_args.metadata_only
yes_i_really_really_mean_it = parsed_args.yes_i_really_really_mean_it

if not all_nodes and not name:
Expand All @@ -1213,7 +1220,10 @@ def take_action(self, parsed_args):
)
return 1

clean_steps = [{"interface": "deploy", "step": "erase_devices"}]
if metadata_only:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fully hold to "Only erase metadata". Further down, baremetal.py:1256-1259 unconditionally prepends {"interface": "raid", "step": "delete_configuration"} for any node whose raid_interface is not no-raid, so osism baremetal clean --metadata-only NODE on a hardware-RAID node emits [raid delete_configuration, deploy erase_devices_metadata] and tears down the logical disks in addition to wiping metadata. Ironic runs manual clean steps in the order supplied, so the RAID teardown happens first.

Since the commit message frames this as a workaround for hardware where secure erase errors out, that's exactly the node class where an operator would reach for the light-touch path and be surprised.

Two ways out: gate the prepend on not metadata_only, or keep the behavior and reword the help to say what actually happens (e.g. "Erase only disk metadata instead of a full device erase. RAID configuration is still deleted on nodes with a RAID interface."). Which one is your call, but the current pairing of contract and behavior shouldn't ship as-is.

clean_steps = [{"interface": "deploy", "step": "erase_devices_metadata"}]
else:
clean_steps = [{"interface": "deploy", "step": "erase_devices"}]

from osism.tasks.openstack import get_cloud_helpers

Expand Down Expand Up @@ -1243,7 +1253,10 @@ def take_action(self, parsed_args):
continue

# NOTE: If the node has an agent raid interface, include step to delete the raid configuration
if node.get("raid_interface", "no-raid") != "no-raid":
if (
not metadata_only
and node.get("raid_interface", "no-raid") != "no-raid"
):
clean_steps = [
{"interface": "raid", "step": "delete_configuration"}
] + clean_steps
Expand Down
25 changes: 25 additions & 0 deletions tests/unit/commands/test_baremetal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,7 @@ def test_burnin_unsupported_state_warns(loguru_logs):
# --- BaremetalClean ---

ERASE_DEVICES_STEP = {"interface": "deploy", "step": "erase_devices"}
ERASE_METADATA_STEP = {"interface": "deploy", "step": "erase_devices_metadata"}
RAID_DELETE_STEP = {"interface": "raid", "step": "delete_configuration"}


Expand Down Expand Up @@ -1358,6 +1359,30 @@ def test_clean_raid_interface_prepends_delete_configuration():
)


def test_clean_metadata_only_uses_erase_devices_metadata():
node = FakeNode(provision_state="manageable")
conn = MagicMock()
conn.baremetal.find_node.return_value = node

_run_baremetal_clean(["node1", "--metadata-only"], conn)

conn.baremetal.set_node_provision_state.assert_called_once_with(
node.id, "clean", clean_steps=[ERASE_METADATA_STEP]
)


def test_clean_metadata_only_skips_delete_configuration_on_raid_node():
node = FakeNode(provision_state="manageable", raid_interface="agent")
conn = MagicMock()
conn.baremetal.find_node.return_value = node

_run_baremetal_clean(["node1", "--metadata-only"], conn)

conn.baremetal.set_node_provision_state.assert_called_once_with(
node.id, "clean", clean_steps=[ERASE_METADATA_STEP]
)


def test_clean_available_node_moved_to_manageable_first(loguru_logs):
available = FakeNode(provision_state="available")
manageable = FakeNode(provision_state="manageable")
Expand Down