Unit tests for osism/tasks/conductor/ironic.py — sync orchestrators

[berendt]

Background

Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 3 (#2199). Companion to the ironic helpers issue: covers the four sync orchestrators in osism/tasks/conductor/ironic.py. These are large state-machine functions that drive transitions between Ironic provision states based on NetBox device data. Tests focus on the state transitions and branch coverage, not on exact push_task_output strings.

Scope

Add tests/unit/tasks/conductor/test_ironic_sync.py covering the four sync entry points in osism/tasks/conductor/ironic.py.

Test targets

_sync_ironic_device(request_id, device, node_attributes, ports_attributes, adopt, force) — ironic.py:354

Patch osism.tasks.conductor.ironic.osism_utils and osism.tasks.conductor.ironic.openstack (the module-level imports). Stub deep_compare so it can populate the node_updates dict on demand.

Node creation path (baremetal_node_show returns None)

• Creates node via baremetal_node_create with automated_clean=False
• target_raid_config popped from node_attributes before create; if present, baremetal_node_set_target_raid_config invoked with (uuid, target_raid_config)
• set_target_raid_config returns (False, "error") → raises Exception containing "target_raid_config"

Node update path (baremetal_node_show returns existing node)

• deep_compare populates node_updates={...} → baremetal_node_update called with full node_attributes
• Driver password key popped from node_updates["driver_info"] before evaluating change (verify password key is computed via driver_params[node_attributes["driver"]]["password"])
• All driver_info updates were just the password → driver_info removed entirely from node_updates; if no other updates remain and force=False → baremetal_node_update not called
• force=True and no updates → still calls baremetal_node_update
• target_raid_config in node_updates → baremetal_node_set_target_raid_config called

Port reconciliation

• node_ports initially [port1, port2]; ports_attributes has port1's MAC → port2 deleted via baremetal_port_delete
• New MAC in ports_attributes not in node_ports → baremetal_port_create called
• MAC comparison is case-insensitive (.upper())

State transitions (validation + provisioning)

• node_validation["management"].result=False → push warning, function returns
• Management OK, provision_state="enroll" → transitioned to manage then waited for manageable
• Management OK, provision_state="clean failed" → transitioned to manage
• After moving to manageable and power_state != "power off" and not adoption → baremetal_node_set_power_state(uuid, "power off", wait=True, timeout=300)
• Validation boot.result=False and provision_state="available" → demoted to manage
• Validation OK, adopt path: provision_state="manageable" and is_adoption=True → set_provision_state("adopt") + wait for "active"
• Adopt path with provision_state="available" → first transitions to manage ("Prepare adoption...")
• Validation OK, normal path: provision_state="manageable" → set boot device, transition to provide, wait for "available"
• automated_clean=True after available transition → updated back to True
• set_boot_device raises → caught, logged, transition continues
• is_adoption derived from adopt or device.custom_fields["provision_state"] == "active"

_sync_ironic_device_dry_run(request_id, device, node_attributes, ports_attributes, adopt, force, template_vars) — ironic.py:574

Patch osism_utils.push_task_output and capture all messages.

Secret masking

• template_vars contains a key with password / secret / ironic_osism_* and a string value → that value collected into secret_values, then passed to mask_secrets (verify the call args)
• Non-string values not collected
• Unrelated keys not collected

Create branch (baremetal_node_show returns None)

• Push contains "Would CREATE baremetal node" and "Would CREATE port with MAC" for each port
• adopt=True → adopt message pushed; otherwise available message
• device.custom_fields["provision_state"]=="active" and adopt=False → still adopt message

Update branch (baremetal_node_show returns existing node)

• node_updates populated → push "Would UPDATE baremetal node"
• No updates and force=False → push "no update needed"
• Existing port matched → not in delete list; missing port → "Would CREATE port"; extra port → "Would DELETE port"
• Always pushes the current provision_state

sync_ironic(request_id, get_ironic_parameters, node_name=None, adopt=False, force=False, dry_run=False, skip_kernel_params=None, extra_kernel_params=None) — ironic.py:685

Patch osism_utils, openstack, netbox (the osism.tasks.netbox module), _prepare_node_attributes, _sync_ironic_device, _sync_ironic_device_dry_run, osism_utils.create_redlock.

• dry_run=True → prefix "[DRY RUN] " used in messages, lock not acquired, calls _sync_ironic_device_dry_run
• dry_run=False, lock acquired → calls _sync_ironic_device, lock released even if it raises (verify release() in finally)
• Lock not acquired → message pushed, sync function not called for that device
• NetBox API not reachable (osism_utils.nb.status() raises) → error pushed, finish_task_output(rc=1), returns
• Ironic API not reachable (baremetal.nodes(limit=1) raises) → error pushed, finish_task_output(rc=1), returns
• node_name set, no matching device in NetBox → "Node ... not found in NetBox" pushed, returns rc=1
• Stale Ironic node (not in NetBox, eligible state and power_state) → deleted (dry_run=False) or "Would delete..." pushed (dry_run=True)
• provision_state="clean failed" stale node → moved to manageable first, then ports + node deleted
• Stale node not eligible (e.g. active) → "Cannot remove..." pushed
• skip_kernel_params / extra_kernel_params propagated into _prepare_node_attributes
• ports_attributes built from netbox.get_interfaces_by_device filtered by enabled and not mgmt_only and mac_address

sync_netbox_from_ironic(request_id, node_name=None, netbox_filter=None) — ironic.py:881

Patch osism_utils.nb, osism_utils.secondary_nb_list, osism_utils.get_openstack_connection, _matches_netbox_filter, openstack.baremetal_node_list, and netbox.set_provision_state / set_power_state / set_maintenance.

Filter behavior (netbox_filter set)

• Primary matches and is reachable → included; set_* called with netbox_filter=...
• Primary not reachable but secondary matches and is reachable → continues with secondary
• No NetBox instance matches filter → error pushed, rc=1, returns
• All filtered instances unreachable → error pushed, rc=1, returns

No filter

• Primary unreachable → error pushed, returns rc=1
• Primary reachable, all secondaries reachable → reachable_secondaries = full list
• Primary reachable, one secondary unreachable → that secondary skipped
• No secondaries configured → message includes "to NetBox" (no "(including secondaries)")

Node sync

• node_name set, no matching node in Ironic → "not found in Ironic" pushed, rc=1
• Each node calls set_provision_state, set_power_state, set_maintenance with secondary_nb_list=reachable_secondaries
• Any set_* returning False → device added to failed_devices; final warning lists them
• All succeed → no warning
• Ironic API unreachable → error pushed, rc=1

Mocking hints

• These functions are heavy on push_task_output calls. Tests should accept any reasonable string content — assert on substrings (e.g. assert any("Would CREATE" in call.args[1] for call in push_task_output.mock_calls)).
• Build node dicts inline as plain dicts since Ironic returns dict-like objects (node["uuid"], node["provision_state"], etc.).
• baremetal_node_validate returns an object with .result and .reason attributes — use SimpleNamespace.
• Lock fixture: mocker.patch("osism.tasks.conductor.ironic.osism_utils.create_redlock", return_value=MagicMock(acquire=MagicMock(return_value=True), release=MagicMock())).
• For port reconciliation, node_ports is mutated in-place during the loop — keep mock data simple (one or two ports per case).

Definition of Done

• tests/unit/tasks/conductor/test_ironic_sync.py created
• All listed cases covered
• pytest --cov=osism.tasks.conductor.ironic for the targeted functions ≥ 80 % (some defensive except branches and message-only push paths may stay lower)
• pipenv run pytest tests/unit/tasks/conductor/test_ironic_sync.py passes locally
• flake8, mypy, python-black remain green
• Zuul job python-osism-unit-tests passes

Dependencies

• Part of Meta: Unit test coverage for osism/ #2199 (meta). Blocked by Introduce foundation for unit tests (pytest + Zuul CI integration) #2192 / PR Introduce foundation for unit tests (pytest + Zuul CI) #2193. Pure helpers in ironic.py are tracked in a companion sub-issue.