You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 3 (#2199). osism/tasks/conductor/ironic.py is 1137 LOC, split across two sub-issues:
This issue: pure helpers and the _prepare_node_attributes builder — the things you can test without exercising the full sync orchestration.
Companion issue: the sync entry points (_sync_ironic_device, _sync_ironic_device_dry_run, sync_ironic, sync_netbox_from_ironic).
Scope
Add tests/unit/tasks/conductor/test_ironic_helpers.py covering the helpers below in osism/tasks/conductor/ironic.py.
Patch osism.tasks.conductor.ironic.deep_decrypt, ...deep_merge, ...get_vault, ...get_device_oob_ip, ...SUPPORTED_IPA_TYPES, ..._derive_as_from_hostname_yrzn, ..._get_metalbox_primary_ip4. Stub get_ironic_parameters to return a base dict.
Base merging
Base dict (no config_context, no custom-field ironic_parameters) → returned with resource_class=device.name and extra={}
device.config_context["ironic_parameters"] present → deep_decrypt + deep_merge invoked once for it
device.custom_fields["ironic_parameters"] present → deep_decrypt + deep_merge invoked once
Both present → both merged (verify call order: config_context first, then custom field)
Driver pruning
driver="ipmi" and driver_info contains keys for redfish_* → those keys popped
driver="redfish" and driver_info contains ipmi_* → those popped
For _prepare_node_attributes, the inputs are deeply nested dicts; build them inline per test rather than via fixtures to keep each scenario self-contained.
Build device with SimpleNamespace(name="server-1", custom_fields={...}, config_context={...}).
Definition of Done
tests/unit/tasks/conductor/test_ironic_helpers.py created
All listed cases covered
pytest --cov=osism.tasks.conductor.ironic for the targeted helpers ≥ 90 %
pipenv run pytest tests/unit/tasks/conductor/test_ironic_helpers.py passes locally
Background
Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 3 (#2199).
osism/tasks/conductor/ironic.pyis 1137 LOC, split across two sub-issues:_prepare_node_attributesbuilder — the things you can test without exercising the full sync orchestration._sync_ironic_device,_sync_ironic_device_dry_run,sync_ironic,sync_netbox_from_ironic).Scope
Add
tests/unit/tasks/conductor/test_ironic_helpers.pycovering the helpers below inosism/tasks/conductor/ironic.py.Test targets
_derive_as_from_hostname_yrzn(hostname)—ironic.py:39Pure function (already has a doctest example).
"stor-nw-22-60-59-6"→"4200155960"(stor → type 5, rack 59, server 60)"comp-nw-22-3-7-1"→"4200143307"(non-stor → type 4, padded rack/server)None"stor"(e.g."net") →t="4""7"→"07")_get_metalbox_primary_ip4_fallback()—ironic.py:67Patch
osism.tasks.conductor.ironic.utils.nbandosism.settings.NETBOX_FILTER_CONDUCTOR_IRONIC.yaml.YAMLErrorcaught, returnsNoneNonetagremoved androle="metalbox"added → verified via captured kwargs tonb.dcim.devices.filterprimary_ip4="10.0.0.5/24"→ returns"10.0.0.5"(prefix stripped)primary_ip4but second one does → returns the second IPprimary_ip4→ returnsNone, warning loggedfilter→ returnsNone, warning logged_get_metalbox_primary_ip4(device)—ironic.py:99Patch
get_device_oob_ip,osism.tasks.conductor.ironic.utils.nb, and_get_metalbox_primary_ip4_fallback.get_device_oob_ipreturnsNone→ returnsNone, fallback not called10.0.0.5, metalbox interface IP10.0.0.1/24→ returns"10.0.0.1"primary_ip4→ returnsNone(no fallback call — note the earlyreturn None)_render_templates(obj, template_vars)—ironic.py:161Pure recursive Jinja2 rendering. Build small dicts/lists inline.
{{→ unchangedstrwith{{triggers rendering)_render_templates(obj, vars)returnsNone, butobjis mutated)_prepare_node_attributes(device, get_ironic_parameters, skip_kernel_params=None, extra_kernel_params=None)—ironic.py:184Patch
osism.tasks.conductor.ironic.deep_decrypt,...deep_merge,...get_vault,...get_device_oob_ip,...SUPPORTED_IPA_TYPES,..._derive_as_from_hostname_yrzn,..._get_metalbox_primary_ip4. Stubget_ironic_parametersto return a base dict.Base merging
config_context, no custom-fieldironic_parameters) → returned withresource_class=device.nameandextra={}device.config_context["ironic_parameters"]present →deep_decrypt+deep_mergeinvoked once for itdevice.custom_fields["ironic_parameters"]present →deep_decrypt+deep_mergeinvoked onceDriver pruning
driver="ipmi"anddriver_infocontains keys forredfish_*→ those keys poppeddriver="redfish"anddriver_infocontainsipmi_*→ those poppedTemplate variables
node_secrets["remote_board_username"]/"remote_board_password"honored, defaults"admin"/"password"get_device_oob_ipreturns("10.0.0.5", 24)→template_vars["remote_board_address"] = "10.0.0.5"get_device_oob_ipreturnsNone→ key not presentironic_osism_propagated intotemplate_vars(stripped)Kernel append params (
osism-ipa-type=yrzn001)kapcontainsosism-ipa-type=yrzn001,frr_parameterspopulated → entries appended forosism-ipa-as,osism-ipa-ipv4,osism-ipa-ipv6frr_parametersmissing the required keys → only available ones appendedosism-ipa-metalboxresolved via_get_metalbox_primary_ip4(returns IP → appended; returnsNone→ not appended)osism-ipa-asfalls back to_derive_as_from_hostname_yrzn(device.name)whenfrr_loopback_v4not infrrosism-ipa-type→ no enrichmentskip_kernel_paramsskip_kernel_params(e.g."osism-ipa-as") → removed; other params preservedextra_kernel_paramskapwith single space separatorkapbecomes the first param (no leading space)Driver_info persistence
kapis also stored indriver_info["kernel_append_params"](verify creation ofdriver_infoif absent)extraupdatesinstance_infonon-empty → JSON-serialized intoextra["instance_info"]device.custom_fields["netplan_parameters"]truthy → JSON-serialized intoextra["netplan_parameters"]device.custom_fields["frr_parameters"]truthy → decrypted then JSON-serialized intoextra["frr_parameters"]extrastays empty (or untouched)Returns
(node_attributes, template_vars)tuple_prettify_for_display(obj)—ironic.py:335extra={"instance_info": '{"foo": "bar"}'}→ returns dict withextra["instance_info"] == {"foo": "bar"}extra→ string left untouched (caughtJSONDecodeError)extra→ returned unchanged (deep copy, not the same object)Mocking hints
mocker.patch(...)at the import site insideironic.pyso the original modules elsewhere stay intact.deep_decryptas a no-op (lambda obj, vault: None) — its real implementation is covered in Unit tests for osism/tasks/conductor/utils.py #2202._prepare_node_attributes, the inputs are deeply nested dicts; build them inline per test rather than via fixtures to keep each scenario self-contained.devicewithSimpleNamespace(name="server-1", custom_fields={...}, config_context={...}).Definition of Done
tests/unit/tasks/conductor/test_ironic_helpers.pycreatedpytest --cov=osism.tasks.conductor.ironicfor the targeted helpers ≥ 90 %pipenv run pytest tests/unit/tasks/conductor/test_ironic_helpers.pypasses locallyflake8,mypy,python-blackremain greenpython-osism-unit-testspassesDependencies
ironic.pyare tracked in a companion sub-issue.