Unit tests for osism/tasks/openstack.py

[berendt]

Background

Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 5 (#2199). osism/tasks/openstack.py (931 LOC) is the OpenStack Celery worker module: thin task wrappers around the OpenStack SDK (image/network/baremetal CRUD), baremetal info getters used by CLI and API (incl. NetBox enrichment and secret masking), cloud credential/environment helpers (secrets.yml password loading, clouds.yaml injection into /tmp, secure.yml fallback), and manager tasks (image_manager, flavor_manager, project_manager*).

Scope

Add tests/unit/tasks/test_openstack.py. There is no existing test file for this module — existing command tests (tests/unit/commands/test_server.py, test_volume.py, test_compute.py, test_baremetal.py, test_manage_wiring.py) only patch osism.tasks.openstack.get_cloud_helpers / image_manager as collaborators, so nothing below duplicates existing coverage.

The module is large (~32 functions). Prioritize the pure-logic / high-value functions listed first (credential handling, environment setup, baremetal getters, secret masking); the thin SDK task wrappers are covered by one parametrized test block. Like the Tier 3 issues, this issue may be split further during implementation, e.g. (a) cloud credential/environment helpers, (b) baremetal/NetBox getters incl. masking, (c) Celery task wrappers + manager tasks.

Test targets

get_cloud_password(cloud) — openstack.py:417

Patch:

• osism.tasks.openstack.os.path.exists
• osism.tasks.openstack.load_yaml_file (imported at module top from osism.tasks.conductor.utils)

Cases:

• cloud=None / "" → returns None, warning "No cloud parameter provided"
• Secrets file missing (os.path.exists → False) → None, warning
• load_yaml_file returns None → None, warning "Empty or invalid secrets file"
• load_yaml_file returns a non-dict (list) → None
• Hyphen normalization: cloud="admin-system" → looks up key os_password_admin_system
• Key-format guard: cloud="admin system" → password_key is not an identifier → error logged, None
• Key not present in dict → None (debug log only, no warning)
• Value " hunter2 " → returns "hunter2" (stripped)
• Value 42 (int) → coerced via str(...) → "42"
• Value "" / whitespace-only → None, warning "empty after conversion"
• load_yaml_file raises → None, error logged

setup_cloud_environment(cloud) — openstack.py:487

Patch:

• osism.tasks.openstack.get_cloud_password
• osism.tasks.openstack.os.path.exists (side-effect map over the hardcoded paths), os.getcwd, os.chdir
• osism.tasks.openstack.shutil.copy2
• builtins.open (mock_open) + osism.tasks.openstack.yaml.safe_load / yaml.dump for the password-injection branch

Cases:

• cloud falsy → (None, [], original_cwd, False), warning; get_cloud_password not called
• Password branch (password found):
  • /etc/openstack/clouds.yaml exists, cloud present in config → password injected into clouds["<cloud>"]["auth"]["password"], combined config written to /tmp/clouds.yaml, empty /tmp/secure.yml written, os.chdir("/tmp"), returns (password, ["/tmp/clouds.yaml", "/tmp/secure.yml"], original_cwd, True)
  • Only /etc/openstack/clouds.yml exists → destination is /tmp/clouds.yml
  • Cloud profile present but without auth key → auth dict created before injection
  • Cloud profile missing from clouds_config → (None, [], original_cwd, False), warning
  • Neither clouds.yaml nor clouds.yml → returns (password, [], original_cwd, True) (password still usable for direct SDK), no chdir
  • open/yaml raises → (None, ..., False), error logged
• Fallback branch (no password):
  • /etc/openstack/secure.yml exists → clouds.yaml + secure.yml copied to /tmp, chdir to /tmp, returns (None, [copied files], original_cwd, True)
  • Only /etc/openstack/secure.yaml exists → destination /tmp/secure.yaml
  • Neither secure file → (None, [], original_cwd, False), error message names os_password_<cloud_normalized>
  • shutil.copy2 raises → (None, ..., False), error logged

cleanup_cloud_environment(temp_files_to_cleanup, original_cwd) — openstack.py:609

Patch osism.tasks.openstack.os.chdir, os.path.exists, os.remove.

• Restores cwd and removes each existing temp file
• os.chdir raises → warning, file removal still happens
• File no longer exists → os.remove not called for it
• os.remove raises for one file → warning, remaining files still processed

get_openstack_connection(cloud, password=None) — openstack.py:633

This is the module's own two-argument helper (raises SystemExit) — not to be confused with osism.utils.get_openstack_connection() used by the Celery tasks. openstack / keystoneauth1 are imported inside the function: patch openstack.connect at source; openstacksdk and keystoneauth1 are direct dependencies, so raise the real exception classes. Also patch osism.tasks.openstack.os.path.exists for the secure_yml_exists check.

• password given, connect OK → openstack.connect(cloud=cloud, auth={"password": password}), conn.current_project touched, conn returned
• password given, Unauthorized, secure.yml exists → second openstack.connect(cloud=cloud) (no auth) succeeds → conn returned (use side_effect=[Unauthorized(), mock_conn])
• password given, Unauthorized, no secure.yml → SystemExit raised (pytest.raises(SystemExit)), error names os_password_<cloud_normalized>
• password given, MissingRequiredOptions → SystemExit
• password given, SDKException → SystemExit
• No password, connect OK → conn returned
• No password, MissingRequiredOptions whose message contains "password" → SystemExit, specific "No password configured" error
• No password, MissingRequiredOptions without "password" → SystemExit, generic "Missing configuration" error
• No password, Unauthorized → SystemExit
• No password, SDKException → SystemExit
• connect succeeds but current_project access raises (PropertyMock side effect) → exercises test_connection failure path

Hint: MissingRequiredOptions(options) builds its message from option objects — construct with e.g. types.SimpleNamespace(name="password").

run_openstack_command_with_cloud(...) — openstack.py:723

Patch osism.tasks.openstack.setup_cloud_environment, cleanup_cloud_environment, run_command.

• run_command called with (request_id, command, {}, *arguments) plus publish / locking / auto_release_time / ignore_env=True; return value propagated
• cleanup_cloud_environment called with the temp files and cwd from setup even when run_command raises (finally)
• Setup returning success=False does not short-circuit — run_command still runs (assert current behavior)

get_baremetal_nodes() — openstack.py:78

Patch osism.tasks.openstack.utils.get_openstack_connection (the no-arg helper on osism.utils).

• Two fake nodes (types.SimpleNamespace — code uses getattr) → list of dicts with all expected keys; conn.baremetal.nodes called with details=True
• uuid missing/None → falls back to id
• traits=None → []
• driver_info=None → redfish_address is None (via or {}); driver_info={"redfish_address": "https://..."} → extracted
• Empty node generator → []
• device_role / primary_ip4 / primary_ip6 are always None placeholders

get_baremetal_node_netbox_info(node_name) — openstack.py:131

Patch osism.tasks.openstack.utils.nb (lazy attribute on osism.utils, see hints) and osism.tasks.openstack.settings.NETBOX_URL.

• utils.nb is None → default dict (all None)
• node_name empty → default dict
• Device found via nb.dcim.devices.get(name=...): role.name extracted; primary_ip4="10.0.0.1/24" → "10.0.0.1" (split on /); same for primary_ip6; netbox_url = NETBOX_URL.rstrip('/') + /dcim/devices/<id>/
• get returns None, filter(cf_inventory_hostname=...) returns devices → first match used
• filter returns empty → defaults returned
• device.role is None or has no name attribute → device_role stays None
• NETBOX_URL unset → netbox_url stays None
• NetBox lookup raises → defaults returned, debug log

get_baremetal_nodes_netbox_info(node_names) — openstack.py:170

• utils.nb falsy → {}; node_names empty → {}
• Two names → dict keyed by name; patch osism.tasks.openstack.get_baremetal_node_netbox_info and assert one call per name

get_baremetal_node_ports(node_uuid) — openstack.py:190

• conn.baremetal.ports called with details=True, node_uuid=...; returned objects mapped to dicts, uuid falls back to id; empty generator → []

get_baremetal_node_parameters(node_uuid) — openstack.py:214 (high value: secret masking)

Patch osism.tasks.openstack.utils.get_openstack_connection, osism.tasks.openstack.utils.nb; the conductor helpers are imported inside the function, so patch at source: osism.tasks.conductor.utils.get_vault, osism.tasks.conductor.utils.deep_decrypt (the real mask_secrets can be exercised, or patched likewise).

• extra empty / None → {"kernel_append_params": None, "netplan_parameters": None, "frr_parameters": None}
• extra["instance_info"] valid JSON with kernel_append_params → returned
• instance_info invalid JSON or non-string (TypeError) → ignored, result None
• netplan_parameters / frr_parameters valid JSON → parsed dicts; invalid JSON → None
• utils.nb is None or node has no name → no masking, raw kernel_append_params returned
• Device found, custom_fields["secrets"] is None → treated as {}
• Secrets with password/secret in key → string values (stripped) replaced by *** in kernel_append_params
• ironic_osism_aa key → param-name collected before deep_decrypt; osism-aa=value masked to osism-aa=*** by regex even when deep_decrypt drops the key (failed vault decryption)
• Non-string secret values are not collected
• Collected secret_values passed into mask_secrets(...) for frr_parameters and netplan_parameters with mask="***"
• Device lookup via cf_inventory_hostname fallback
• NetBox/vault path raises → debug log, parameters returned unmasked

Thin Celery task wrappers (parametrized block)

image_get (openstack.py:25), network_get (:32), baremetal_node_create (:39), baremetal_node_delete (:49), baremetal_node_update (:56), baremetal_node_show (:65), baremetal_node_list (:72), baremetal_node_validate (:337), baremetal_node_wait_for_nodes_provision_state (:347), baremetal_node_set_boot_device (:357), baremetal_node_set_provision_state (:363), baremetal_node_set_power_state (:370), baremetal_port_list (:381), baremetal_port_create (:390), baremetal_port_delete (:399), baremetal_node_set_target_raid_config (:406).

Patch osism.tasks.openstack.utils.get_openstack_connection; call each task object directly (synchronous, no broker). Mostly one happy-path assertion each (correct SDK method + args, return value propagated) via pytest.mark.parametrize; plus these specific branches:

• baremetal_node_create: attributes=None → create_node(name=node_name); attributes given → name injected into them
• baremetal_node_wait_for_nodes_provision_state: non-empty result list → first element; empty list → None
• baremetal_node_set_power_state: wait=True → wait_for_node_power_state(node, state, timeout=timeout); wait=False → get_node(node)
• baremetal_node_list / baremetal_port_list: generator materialized via list()
• baremetal_node_set_target_raid_config: get_node then conn.baremetal.put("/nodes/<uuid>/states/raid", microversion="1.12", json=...), returns (response.ok, response.content)
• get_cloud_helpers() (openstack.py:483): returns the 3-tuple (setup_cloud_environment, get_openstack_connection, cleanup_cloud_environment)

image_manager(...) — openstack.py:765

Patch osism.tasks.openstack.utils.check_task_lock_and_exit, setup_cloud_environment, cleanup_cloud_environment, run_command, and (for the no-configs case) run_openstack_command_with_cloud.

• check_task_lock_and_exit called first in both branches
• No configs → delegates to run_openstack_command_with_cloud with the manager binary path and pass-through kwargs
• configs=["yaml-a", "yaml-b"] → each config written to a temp file inside a TemporaryDirectory; --images=foo args filtered out; --images foo pair removed; --images <temp_dir> appended; run_command called with ignore_env=True
• cleanup_cloud_environment called even when run_command raises (finally)
• Edge (document current behavior): a trailing --images without value raises IndexError (only ValueError is caught)

flavor_manager (openstack.py:831), project_manager (:855), project_manager_sync (:895)

• flavor_manager: patch run_openstack_command_with_cloud → called with /usr/local/bin/openstack-flavor-manager, cloud and kwargs; lock check invoked
• project_manager / project_manager_sync (parametrize over create.py / manage.py): patch setup_cloud_environment, cleanup_cloud_environment, run_command, os.chdir → script path prepended to arguments, os.chdir("/openstack-project-manager") called, cleanup in finally

Mocking hints

• Two different get_openstack_connections: the Celery tasks/getters use the no-arg osism.utils.get_openstack_connection() — patch osism.tasks.openstack.utils.get_openstack_connection. The module-level get_openstack_connection(cloud, password) at openstack.py:633 is a separate function under test. Don't mix them up.
• All tasks are bound (bind=True); invoke the task object directly (e.g. openstack.image_get("cirros")) — direct calls execute the body synchronously without a broker; self.request.id is None, which is fine for the run_command mocks.
• utils.nb is a lazy module attribute resolved via __getattr__ on osism.utils — patch with mocker.patch("osism.tasks.openstack.utils.nb", mock_nb) (same pattern as in Unit tests for osism/utils/rabbitmq.py #2232 for utils.redis).
• load_yaml_file is imported at module top → patch osism.tasks.openstack.load_yaml_file. deep_decrypt / get_vault / mask_secrets are imported inside get_baremetal_node_parameters → patch at osism.tasks.conductor.utils.*. The root tests/conftest.py already stubs ansible.*, so importing conductor utils is safe in the unit-test env.
• run_command is imported at module top from osism.tasks → patch osism.tasks.openstack.run_command.
• Use types.SimpleNamespace for fake SDK Resource objects — the getters use getattr(node, ..., default) throughout, so plain namespaces model missing attributes correctly (unlike MagicMock, which auto-creates attributes).
• setup_cloud_environment / cleanup_cloud_environment use hardcoded paths (/etc/openstack/..., /tmp/...) and os.chdir — always patch os.path.exists (side-effect map keyed by path), os.chdir, os.getcwd, shutil.copy2, and builtins.open so tests never touch the real filesystem or working directory.
• openstacksdk and keystoneauth1 are pinned runtime dependencies — raise the real keystoneauth1.exceptions.http.Unauthorized, keystoneauth1.exceptions.auth_plugins.MissingRequiredOptions, and openstack.exceptions.SDKException classes via side_effect instead of inventing fakes.

Definition of Done

• tests/unit/tasks/test_openstack.py created
• All listed cases covered (split into multiple sub-issues/PRs if too large — the priority order above applies)
• pytest --cov=osism.tasks.openstack shows ≥ 90 %
• pipenv run pytest tests/unit/tasks/test_openstack.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.