Add baremetal node adoption

[janhorstmann]

Being able to adopt nodes is a necessity when losing the ironic DB or rebuilding the metalbox during upgrades or disaster recovery.

The mechanism allows to force adoption by specifiying the --adopt option during baremetal sync. Nodes will automatically be adopted if they have their provision_state set to active` in the NetBox.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• Accessing device.custom_fields["provision_state"] without guarding for missing keys can raise KeyError; consider using .get("provision_state") (with a sensible default) wherever you compute is_adoption or check the field.
• The logic that derives is_adoption = adopt or device.custom_fields["provision_state"] == "active" effectively makes the provision_state custom field override the CLI intent; consider clarifying or refining this precedence so a user explicitly not passing --adopt is not surprised by automatic adoption based solely on NetBox state.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Accessing `device.custom_fields["provision_state"]` without guarding for missing keys can raise `KeyError`; consider using `.get("provision_state")` (with a sensible default) wherever you compute `is_adoption` or check the field.
- The logic that derives `is_adoption = adopt or device.custom_fields["provision_state"] == "active"` effectively makes the `provision_state` custom field override the CLI intent; consider clarifying or refining this precedence so a user explicitly *not* passing `--adopt` is not surprised by automatic adoption based solely on NetBox state.

## Individual Comments

### Comment 1
<location path="osism/tasks/conductor/ironic.py" line_range="416-417" />
<code_context>
         )
         openstack.baremetal_port_delete(node_port["id"])

+    # NOTE: Adopt nodes with provisioning state active in NetBox or if explicitly requested
+    is_adoption = adopt or device.custom_fields["provision_state"] == "active"
+
     node_validation = openstack.baremetal_node_validate(node["uuid"])
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against missing `provision_state` in `device.custom_fields` when computing `is_adoption`.

Directly indexing `device.custom_fields["provision_state"]` will raise if `custom_fields` is `None` or the key is missing, causing failures before any Ironic call. Use a safe access pattern instead, e.g. `device.custom_fields.get("provision_state") == "active"` (or a chained `.get` with a default) so adoption logic remains robust even if some devices lack this field or the NetBox schema changes.
</issue_to_address>

### Comment 2
<location path="osism/commands/netbox.py" line_range="34-37" />
<code_context>
             type=int,
             help="Timeout for a scheduled task that has not been executed yet",
         )
+        parser.add_argument(
+            "--adopt",
+            help="Adopt nodes rather than moving them to available",
+            action="store_true",
+        )
         parser.add_argument(
</code_context>
<issue_to_address>
**suggestion:** Clarify CLI help text to mention implicit adoption based on NetBox `provision_state`.

`_sync_ironic_device` now also adopts nodes when `device.custom_fields["provision_state"] == "active"`, even if `--adopt` is not passed. Since adoption can occur implicitly based on NetBox data, please update the help text (or related docs) to reflect this behaviour so users understand that the flag is not the only trigger for adoption.

Suggested implementation:

```python
        parser.add_argument(
            "--adopt",
            help=(
                "Explicitly adopt nodes rather than moving them to available. "
                "Note: nodes are also adopted implicitly when the NetBox "
                "custom field 'provision_state' is set to 'active'."
            ),
            action="store_true",
        )

```

If there is user-facing documentation for this command (e.g. README, CLI usage docs, or Sphinx docs), update the description of the `--adopt` flag there in the same way: clarify that adoption can occur implicitly when the NetBox `provision_state` custom field is `"active"`, even without passing `--adopt`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[janhorstmann]

Hey - I've found 2 issues, and left some high level feedback:

• Accessing device.custom_fields["provision_state"] without guarding for missing keys can raise KeyError; consider using .get("provision_state") (with a sensible default) wherever you compute is_adoption or check the field.
• The logic that derives is_adoption = adopt or device.custom_fields["provision_state"] == "active" effectively makes the provision_state custom field override the CLI intent; consider clarifying or refining this precedence so a user explicitly not passing --adopt is not surprised by automatic adoption based solely on NetBox state.

Prompt for AI Agents
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

First point is fixed, second is intentional