Unit tests for osism/commands/ — OpenStack (compute, volume, manage, migrate, status, report, vault, amphora, octavia, loadbalancer)

[berendt]

Background

Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 8 (#2199). This issue covers the OpenStack-facing CLI command group under osism/commands/: compute.py (919 LOC), manage.py (1319 LOC), migrate.py (897 LOC), status.py (786 LOC), report.py (625 LOC), loadbalancer.py (425 LOC), volume.py (340 LOC), vault.py (325 LOC), amphora.py (162 LOC), octavia.py (60 LOC) — ~5,860 LOC total. All are cliff Command classes plus a number of pure helper functions (RabbitMQ queue classification, Galera status validation, marker/checksum validators, time formatting, Octavia DB helpers).

Scope

Existing coverage (do not duplicate):

• tests/unit/commands/test_compute.py — ComputeMigrationList error paths (unknown user/domain/project/server, duplicate server, changes-since > changes-before), ComputeEnable returning 1 on BadRequestException during force-up.
• tests/unit/commands/test_volume.py — VolumeList exit-code contract for unknown domain / project-domain / project.
• tests/unit/commands/test_manage_validators.py — _is_sha256 and _validate_marker fully covered (M1–M9, S1–S6).
• tests/unit/commands/test_manage_wiring.py — ImageOctavia, ImageClusterapi, ImageClusterapiGardener, ImageGardenlinux validator wiring to fetch_text (W1–W4).
• tests/unit/commands/test_status.py — Run returns 1 for an unknown resource type.
• tests/unit/commands/test_report.py — all four report commands: inventory load failure, inventory timeout, empty inventory.
• tests/unit/commands/test_vault.py — View (encrypted/plain/relative path/missing file/permission error/no path) and Decrypt (no path, argv form, exit-code propagation).

Gaps to close (this issue): migrate.py, octavia.py, amphora.py and loadbalancer.py have no tests at all; the remaining modules need the per-host/per-resource processing logic and remaining error paths covered. Prioritize pure-logic and decision-heavy functions; thin tabulate-printing loops only need representative happy paths.

New test files: tests/unit/commands/test_migrate.py, test_octavia.py, test_amphora.py, test_loadbalancer.py. Extend: test_compute.py, test_volume.py, test_manage_wiring.py (or a new test_manage_commands.py), test_status.py, test_report.py, test_vault.py.

Note: This is a very large group. Like the Tier 3 issues, it may be split into per-module sub-issues during implementation (suggested split: migrate / octavia+amphora+loadbalancer / compute+volume / status+report / manage+vault).

Test targets

osism/commands/migrate.py — Rabbitmq3to4 — migrate.py:134 (no existing tests, highest priority)

Patch osism.commands.migrate.requests (responses as MagicMock with status_code, json.return_value, raise_for_status); for take_action patch osism.utils.rabbitmq.get_rabbitmq_node_addresses and osism.utils.rabbitmq.load_rabbitmq_password (imported inside the method, patch at source).

_check_kolla_configuration() — migrate.py:184 (patch builtins.open with config text)

• File missing (FileNotFoundError) → False; OSError → False
• om_enable_rabbitmq_quorum_queues: false or "no" → False; true or absent → passes that check
• Commented # om_enable_rabbitmq_quorum_queues: false is ignored
• om_rpc_vhost missing → False; quoted ("openstack"/'openstack') and unquoted forms accepted
• om_notify_vhost missing → False
• Fully valid config → True, info logged

_prepare_vhost() — migrate.py:254

• Kolla config check fails → False, no HTTP calls
• dry_run=True → True, no requests.put
• Happy path → two PUTs (vhost body {"default_queue_type": "quorum"}, then permissions configure/write/read = ".*") → True
• HTTPError with status 409 → warning "already exists", True
• HTTPError non-409 → False; generic RequestException → False

Queue classification (pure) — _get_classic_queues() migrate.py:331, _get_quorum_queues() migrate.py:349, _match_queues_for_service() migrate.py:365

• Missing type key defaults to "classic"; "quorum" routed to the other list
• nova: compute, compute.host1, compute_fanout_x match; computex does not
• designate: reply_abc123 matches reply_[a-f0-9]+; reply_XYZ does not
• Unknown service name → empty list (SERVICE_QUEUE_PATTERNS.get(service, []))
• A queue matching several patterns appears only once (break after first match)

_get_all_queues() migrate.py:317 / _get_all_exchanges() migrate.py:523

• Success returns parsed JSON; RequestException → None
• Exchanges: vhost given → URL-encoded vhost path; default exchanges (empty name, amq.* prefix) filtered out

_close_queue_connections() — migrate.py:388

• Queue GET returns 404 → 0; no consumer_details → 0; consumers without connection_name → 0
• dry_run → counts connections, no DELETE issued
• DELETE 200/204 counted; DELETE 404 silently skipped; failing DELETE logs warning and continues with remaining connections
• Outer RequestException → 0, warning logged

_delete_queue() — migrate.py:468 and _delete_exchange() — migrate.py:554

• close_connections=True triggers _close_queue_connections first
• dry_run → True, no DELETE
• Success → True; HTTPError 404 → warning, True (idempotent); other HTTPError / RequestException → False
• Vhost and queue/exchange names are URL-encoded (/ vhost → %2F)

take_action() — migrate.py:595 (selected wiring + the pure check decision logic)

• No command → 1; get_rabbitmq_node_addresses() returns None → 1; load_rabbitmq_password() returns None → 1
• --server not in node list → 1 with "Available:" listing; matching --server selects that address; default uses first node
• check: only classic → "REQUIRED"; only quorum → "NOT required"; classic in / + quorum in /openstack (and the legacy quorum-in-/ variant) → "IN PROGRESS"; no queues → "NOT required"; all return 0
• list: --quorum lists quorum queues, default lists classic; service filter applied; vhost filter excludes queues of other vhosts; empty → 0 with "No ... queues found"
• delete: failed deletion increments failed_count → 1; all succeed → 0; dry-run summary log

osism/commands/octavia.py — octavia.py:17 / octavia.py:40 (no existing tests)

Patch osism.commands.octavia.sleep; pass a MagicMock connection.

• wait_for_amphora_boot() (octavia.py:17): first poll returns no amphorae → loop exits, sleep never called; non-empty twice then empty → exactly two sleeps; never empty → terminates after 24 iterations (120/5); asserts query uses status="BOOTING" and the loadbalancer ID
• wait_for_amphora_delete() (octavia.py:40): analogous with status="PENDING_DELETE" and 12 iterations (60/5)

osism/commands/amphora.py — AmphoraRestore amphora.py:16, AmphoraRotate amphora.py:72 (no existing tests)

Note: unlike compute/volume, this module imports the three cloud helpers directly — patch osism.tasks.openstack.setup_cloud_environment, osism.tasks.openstack.get_openstack_connection, osism.tasks.openstack.cleanup_cloud_environment individually. Also patch osism.commands.amphora.sleep and osism.commands.amphora.wait_for_amphora_boot / wait_for_amphora_delete (module-level imports).

AmphoraRestore.take_action() — amphora.py:35

• setup_cloud_environment returns success=False → 1, no connection attempted
• Without --loadbalancer → amphorae(status="ERROR"); with → loadbalancer_id included in query
• Per ERROR amphora: failover_amphora(amphora.id) then both wait helpers; cleanup_cloud_environment called even when failover raises

AmphoraRotate.take_action() — amphora.py:97

• Amphora older than 30 days (created_at 31 days ago) → failover_load_balancer called
• Younger amphora, no --force → skipped entirely
• --force rotates regardless of age
• Two amphorae of the same loadbalancer → second skipped via done list
• openstack.exceptions.ConflictException during failover → warning, loop continues, LB not added to done

osism/commands/loadbalancer.py (no existing tests)

_load_kolla_configuration() — loadbalancer.py:16 and _load_octavia_database_password() — loadbalancer.py:33

• Config: file missing → None; yaml.safe_load raising → None; valid YAML → dict
• Password: secrets file missing → None; load_yaml_file (patch osism.tasks.conductor.utils.load_yaml_file) returns None/non-dict → None; octavia_database_password preferred over database_password; fallback to database_password; neither present → None; value coerced via str(...).strip(); loader raising → None

_get_octavia_database_connection() — loadbalancer.py:67 (patch osism.commands.loadbalancer.pymysql.connect and the two loaders above at osism.commands.loadbalancer.*)

• Config None / missing kolla_internal_vip_address / password None → None
• enable_proxysql: true → user octavia_shard_0, else octavia; database octavia, DictCursor
• pymysql.Error → None, error logged; happy path returns the connection

_reset_provisioning_status() loadbalancer.py:103 / _reset_operating_status() loadbalancer.py:112

• Assert the exact UPDATE load_balancer SET ... WHERE id = '<id>'; SQL passed to cursor.execute (cursor via database.cursor.return_value.__enter__.return_value) and that commit() is called; custom status argument is interpolated (also used by LoadbalancerDelete with status="ERROR")

LoadbalancerList.take_action() — loadbalancer.py:141 (use the get_cloud_helpers patch pattern from test_volume.py)

• provisioning_status type queries the three statuses PENDING_CREATE, PENDING_UPDATE, ERROR; operating_status type queries operating_status="ERROR" only
• No results → "No loadbalancers with problematic status found" logged, no table

LoadbalancerReset.take_action() — loadbalancer.py:244 (patch osism.commands.loadbalancer.prompt, _get_octavia_database_connection, wait_for_amphora_boot, sleep)

• get_load_balancer raising → 1
• provisioning_status reset with status not in [PENDING_UPDATE, ERROR] → 1 (message points to manage loadbalancer delete)
• operating_status reset: operating_status != ERROR → 1; provisioning_status != ACTIVE → 1
• Prompt answer "no" → 0, nothing reset ("Aborted")
• DB connection None → 1
• Happy path: _reset_provisioning_status (or _reset_operating_status) called, failover triggered, wait_for_amphora_boot called, database.close() always reached
• --no-failover skips failover_load_balancer

LoadbalancerDelete.take_action() — loadbalancer.py:359

• get_load_balancer raising → 1; provisioning_status != PENDING_CREATE → 1
• Prompt "no" → 0; --yes skips prompt
• Happy path: provisioning status set to ERROR first, then delete_load_balancer(lb.id); database.close() in finally

osism/commands/compute.py (extend test_compute.py)

Reuse the _run helper pattern (patch osism.tasks.openstack.get_cloud_helpers). Patch osism.commands.compute.time.sleep and osism.commands.compute.prompt everywhere.

• ComputeEnable — compute.py:13: service not forced down → only enable_service called; forced-down service successfully forced up then enabled; setup_cloud_environment failure → 1
• ComputeDisable — compute.py:82: disable_service called with disabled_reason="MAINTENANCE"
• ComputeList — compute.py:129: host given → project filter / domain filter (via identity.get_project) / unfiltered rows; no host + --details → uptime parsed via jc.parse, unparsable or missing uptime falls back to "-" placeholders; no host without details → 4-column rows
• ComputeMigrate — compute.py:401: status classification — ACTIVE/PAUSED → live, SHUTOFF → cold, SHUTOFF + --no-cold-migration → skipped, other status → skipped; prompt "no" → no migration call; live → live_migrate_server(..., block_migration="auto", force=force); cold → migrate_server; --no-wait skips polling; wait loop: VERIFY_RESIZE → confirm_server_resize then break, confirm raising → exception re-raised; status leaving MIGRATING/RESIZE → "completed" break; empty result → "No migratable instances found"
• ComputeMigrationList — compute.py:576 (happy path gap): all filters resolve → conn.compute.migrations(**query) receives exactly the expected keys (host, instance_uuid, status, migration_type, user_id, project_id, changes_since, changes_before)
• ComputeStart — compute.py:792 / ComputeStop — compute.py:858: only SHUTOFF servers startable / only ACTIVE+PAUSED stoppable; prompt "no" skips; --yes starts/stops without prompting
• ComputeEvacuate — compute.py:276 (lower priority, sleep-heavy): prompt "no" → no API mutation; ACTIVE servers stopped first and recorded for restart; update_service_forced_down(..., forced=True) before evacuate_server; non-ACTIVE/SHUTOFF servers skipped; service disabled with reason EVACUATE at the end

osism/commands/volume.py (extend test_volume.py)

• VolumeList — volume.py:20 stuck-volume path (no domain/project): volume with created_at 3 h ago appears in result, 1 h ago does not (parametrize over the five queried statuses detaching, creating, error_deleting, deleting, error); domain path emits one row per project volume
• VolumeRepair — volume.py:224 (patch osism.commands.volume.sleep, osism.commands.volume.prompt):
  • stuck DETACHING (> STUCK_VOLUME_THRESHOLD_SECONDS) → abort_volume_detaching without prompting; fresh DETACHING untouched
  • stuck CREATING + --yes → delete_volume(id, force=True); prompt "no" → no delete
  • ERROR_DELETING handled regardless of age → prompted delete
  • stuck DELETING + confirm → reset_volume_status(status="available", attach_status=None, migration_status=None) then delete_volume(force=True) with a sleep in between

osism/commands/manage.py (extend; validators and fetch wiring already covered)

Focus on argument-assembly wiring into the Celery task signatures. Patch manage.utils.check_task_lock_and_exit, osism.tasks.openstack.image_manager / flavor_manager / project_manager / project_manager_sync, osism.tasks.ansible.run, osism.tasks.handle_task — assert on mock.si.call_args (the commands only build .si(...) signatures; no broker needed).

• ImageClusterapi — manage.py:51: no --filter → iterates all of SUPPORTED_CLUSTERAPI_K8S_IMAGES (6 fetch_text calls); --tag appends --tag <tag>; --dry-run appends --dry-run; version regex extracts 1.33.1 from ubuntu-2404-kube-v1.33.1.qcow2
• ImageGardenlinux — manage.py:289: --filter 1877.2 → builddate placeholder "unknown"; default uses SUPPORTED_GARDENLINUX_VERSIONS entry (1877.7 → 2025-11-14)
• Images — manage.py:487: --delete adds --delete --yes-i-really-know-what-i-do; no --images defaults to /etc/images; --stuck-retry 1 always appended; --hide default True
• Flavors — manage.py:588: --name/--cloud always present; --recommended and --url only when set; signature goes to flavor_manager.si
• Dnsmasq — manage.py:662: ansible.run.si("infrastructure", "dnsmasq", []); handle_task(..., format="log", timeout=300); --no-wait passes wait=False
• ProjectCreate — manage.py:691: defaults render the positive/negative flag pairs correctly (e.g. default → --assign-admin-user, --nocreate-domain); --nocreate-admin-user flips to negative form; optional quota multipliers / --internal-id / --owner / --password / --service-network-cidr only included when set; integers stringified
• ProjectSync — manage.py:1114: same pattern; defaults --noassign-admin-user --nodry-run ... --manage-privatevolumetypes --manage-privateflavors; --domain/--name only when given

osism/commands/status.py (extend test_status.py)

• display_time() — status.py:21 (pure): 0 → ""; 1 → "1 second" (singular); 90061 → "1 day, 1 hour" (granularity 2); granularity=5 shows all units; 604800 → "1 week"
• Database._load_database_password() — status.py:106: secrets missing → None; empty/non-dict → None; no database_password key → None; value stripped/str-coerced; loader raising → None (patch osism.tasks.conductor.utils.load_yaml_file, os.path.exists)
• Database._check_galera_status() — status.py:134: feed a mock connection (connection.cursor.return_value.__enter__.return_value.fetchall.side_effect = [wsrep_rows, general_rows] as (name, value) tuples):
  • Healthy fixture (Primary/ON/ON/3/Synced) → no errors
  • Each failing check appends its error: non-Primary status, wsrep_connected != ON, wsrep_ready != ON, cluster size 0, non-numeric size, state not Synced
  • Warnings: wsrep_flow_control_paused > 0.05, wsrep_local_recv_queue_avg > 0.5, wsrep_local_cert_failures > 0; non-numeric values silently ignored
  • Cursor raising → single "Failed to query Galera status" error
• Database.take_action() — status.py:311: config None → 1; missing kolla_internal_vip_address → 1; password None → 1; pymysql.connect raising pymysql.Error → 1 (script format prints FAILED: Connection error - ...); enable_proxysql selects user root_shard_0 vs root; errors → 1 / clean → 0 in both log and script formats (PASSED/FAILED printed); connection.close() in finally
• Messaging._check_rabbitmq_status() — status.py:468 (patch requests.get, imported inside the method): overview totals/rates parsed; node running: false → error; mem_alarm/disk_free_alarm → errors; non-empty partitions → CRITICAL error and partitioned_nodes; resources taken from the node matching rabbit@<target_host>; first node used when no target; per-endpoint RequestExceptions appended as errors (overview, nodes, alarms each guarded); alarms status != ok → alarm errors
• Messaging.take_action() — status.py:626: node addresses None → 1; host filter with unknown host → warning; no host matching → 1; password None → 1; any node error → FAILED/1, all clean → PASSED/0; script format prints - [node] error lines

osism/commands/report.py (extend test_report.py; per-host logic gap)

Patch osism.commands.report.subprocess.run with a side_effect list (inventory call first, then per-host SSH calls), plus resolve_host_with_fallback, get_hosts_from_inventory, ensure_known_hosts_file, get_inventory_path as in the existing tests.

• Memory — report.py:18: happy path sums Memory (GB) over hosts; UUID command failing → "n/a" but row kept; memory SSH failure → host in failed_hosts, no row; TimeoutExpired and non-numeric stdout (ValueError) → failed_hosts
• Lldp — report.py:156: single-interface dict response normalized to list (the isinstance(interfaces, dict) branch); multi-interface list handled; missing chassis/port keys → "n/a" defaults; invalid JSON → failed_hosts
• Bgp — report.py:308: --afi filter is case-insensitive (ipv4Unicast matches --afi ipv4unicast... assert via the lowered set); peers flattened with .get defaults; Established summary counts only state == "Established" rows
• Status — report.py:489: fact file parsed via configparser (section bootstrap, status/timestamp fallbacks); SSH returncode != 0 → row ["host", "False", "n/a"] (not a failed host); --status True filter drops non-matching rows; configparser.Error → failed_hosts

osism/commands/vault.py (extend test_vault.py; View/Decrypt covered)

utils.redis is a lazy attribute — patch osism.commands.vault.utils.redis. The keyfile is a class attribute: point vault.SetPassword.keyfile / vault.Check.keyfile at a tmp_path file via monkeypatch.setattr.

• SetPassword.take_action() — vault.py:24: existing keyfile reused (no new key written); missing keyfile → Fernet.generate_key() result written to the file; piped stdin (sys.stdin.isatty() → False, patch sys.stdin) reads and strips the password without prompting; tty path uses prompt(..., is_password=True); redis.set("ansible_vault_password", ...) stores a token that round-trips through Fernet(key).decrypt
• UnsetPassword.take_action() — vault.py:56: redis.delete("ansible_vault_password") called
• Check._find_secrets_file() — vault.py:156: first existing path from SECRETS_SEARCH_PATHS wins (patch os.path.isfile); none exist → glob fallback (patch glob.glob); nothing found → None
• Check.take_action() — vault.py:171: step-by-step failure chain, each returning 1 and short-circuiting (script format markers in parentheses): keyfile missing (keyfile_missing), invalid Fernet key (invalid_keyfile), password not in Redis (password_not_set), InvalidToken on decrypt (decryption_failed), whitespace-only password (password_empty); wrong vault password against an encrypted file → wrong_password, rc 1; happy path with a real Fernet key in tmp_path + a VaultLib-encrypted test file → rc 0, PASSED printed in script format; no secrets file found → warning but rc 0; relative --path resolved against /opt/configuration; non-encrypted test file → decryption test skipped with warning, rc 0

Mocking hints

• Cloud helper pattern (compute, volume, loadbalancer): reuse the established _run helper — patch("osism.tasks.openstack.get_cloud_helpers", return_value=(setup, getconn, cleanup)) with setup.return_value = ("pw", [], None, True). Exception: amphora.py imports setup_cloud_environment / get_openstack_connection / cleanup_cloud_environment directly inside take_action, so patch those three names on osism.tasks.openstack individually.
• Always patch sleeps or tests will hang for minutes: osism.commands.compute.time.sleep, osism.commands.volume.sleep, osism.commands.octavia.sleep, osism.commands.amphora.sleep, osism.commands.loadbalancer.sleep (ComputeEvacuate alone sleeps 30 s).
• Prompts: patch osism.commands.<module>.prompt and return "yes"/"no" to drive both branches.
• Celery tasks: the manage commands only build .si(...) signatures and pass the result to handle_task — patch the task object (osism.tasks.openstack.image_manager etc.) and osism.tasks.handle_task, then assert on task.si.call_args. No broker needed. test_manage_wiring.py has the working pattern, including patch.object(manage.utils, "check_task_lock_and_exit").
• HTTP errors in migrate.py: requests.exceptions.HTTPError needs a response attribute with status_code for the 404/409 branches: HTTPError(response=MagicMock(status_code=409)), attached via response.raise_for_status.side_effect.
• PyMySQL cursors (status, loadbalancer): the code uses with connection.cursor() as cursor: — configure connection.cursor.return_value.__enter__.return_value. _check_galera_status issues two fetchall() calls; use side_effect=[wsrep_rows, general_rows] with (name, value) tuples.
• Stuck-volume timestamps: created_at is parsed with dateutil and localized via pytz.utc — pass naive ISO strings, e.g. (datetime.now(timezone.utc) - timedelta(hours=3)).strftime("%Y-%m-%dT%H:%M:%S.%f").
• Log assertions: the loguru_logs fixture (tests/conftest.py:86) is available, as used in test_vault.py.
• status.Run / Celery inspect: only worth a happy-path test if cheap — patch celery.Celery and feed inspect().stats() / ping(); the unknown-type branch is already covered.

Definition of Done

• New test files created: tests/unit/commands/test_migrate.py, test_octavia.py, test_amphora.py, test_loadbalancer.py
• Existing test files extended: test_compute.py, test_volume.py, test_manage_wiring.py (or new test_manage_commands.py), test_status.py, test_report.py, test_vault.py
• All listed cases covered (no duplication of existing tests)
• pytest --cov shows ≥ 95 % for osism.commands.octavia, ≥ 90 % for osism.commands.migrate, and ≥ 80 % for each remaining module in the group
• pipenv run pytest tests/unit/commands/ 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.
• May be split into per-module sub-issues during implementation (see note in Scope).