Skip to content

Add unit tests for osism.utils.ssh#2350

Open
berendt wants to merge 1 commit into
mainfrom
implement/issue-2231-ssh-unit-tests
Open

Add unit tests for osism.utils.ssh#2350
berendt wants to merge 1 commit into
mainfrom
implement/issue-2231-ssh-unit-tests

Conversation

@berendt

@berendt berendt commented Jun 12, 2026

Copy link
Copy Markdown
Member

Summary

Implements #2231. Adds tests/unit/utils/test_ssh.py, a single cohesive
test module covering every function in osism/utils/ssh.py (SSH
known_hosts maintenance for reset/undeploy flows). No production code is
changed.

Part of the Tier 4 testing effort (#2199); follow-up to #2192 / #2193.

Change set (single commit)

Add unit tests for osism.utils.ssh — adds 44 test functions grouped
by the function under test:

Target (ssh.py) Cases
ensure_known_hosts_file dir+file exist (no writes), missing dir → os.makedirs(..., mode=0o755, exist_ok=True), missing file → open(path,"a") + os.chmod(path,0o644), PermissionError/OSError/catch-all branches, custom path propagation
get_host_identifiers unique DNS IP, DNS returns hostname (no dup), socket.gaierror, utils.nb is None, NetBox primary_ip4 with prefix stripped, device None, NetBox lookup raises (no propagation), same IP from DNS+NetBox de-duplicated, list always starts with hostname
remove_known_hosts_entries empty/whitespace hostname, missing file → True, empty identifiers → False, lookup raises, ssh-keygen success (updated / identifier-in-stderr / neither), non-zero exit continues, TimeoutExpired/CalledProcessError/generic → success=False, multi-host one-timeout, empty-identifier skip, success/empty final logs
backup_known_hosts empty/missing/unreadable path, missing/unwritable backup dir, happy path (shutil.copy2 + path regex), post-copy exists/getsize verification, PermissionError/OSError/catch-all copy failures
cleanup_ssh_known_hosts_for_node backup succeeds (debug log), backup None (cleanup still runs), create_backup=False, remove returns False, remove raises (caught) — always uses KNOWN_HOSTS_PATH

Notes for the reviewer

  • Tests use pytest-mock (mocker) and the shared loguru_logs fixture
    from tests/conftest.py, matching the existing tests/unit/utils style.
  • socket, shutil and datetime are imported inside the functions
    under test. The issue's hint to patch osism.utils.ssh.datetime.datetime
    cannot resolve (no module-level datetime attribute), so the timestamped
    backup path is asserted with a regular expression instead — the
    alternative the issue explicitly allows. For the same reason
    socket.gethostbyname and shutil.copy2 are patched on their real
    modules; utils.nb is patched via osism.utils.ssh.utils.nb
    (create=True), mirroring tests/unit/tasks/conductor/test_netbox.py.

Verification

  • black --check, flake8 (repo config): clean on the new file.
  • mypy: no type errors in the test logic (local run reports only
    third-party import-not-found notes, which resolve inside the CI venv).
  • Per-line trace of ssh.py shows every executable line is exercised,
    comfortably above the ≥95 % target.
  • pytest/coverage left to the Zuul python-osism-unit-tests job.

Closes #2231.

Assisted-by: Claude:anthropic-opus-4-8

Add tests/unit/utils/test_ssh.py covering every function in
osism/utils/ssh.py:

- ensure_known_hosts_file: existing dir/file, directory and
  file creation, PermissionError/OSError/catch-all branches,
  and a custom path argument
- get_host_identifiers: DNS resolution, gaierror, NetBox
  fallback with primary_ip4 prefix stripping, missing and
  raising NetBox lookups, and de-duplication of DNS/NetBox IPs
- remove_known_hosts_entries: empty hostname, missing file,
  empty and raising identifier lookups, ssh-keygen
  success/no-op/non-zero exit, TimeoutExpired,
  CalledProcessError and generic errors, mixed multi-host
  results, and empty-identifier skipping
- backup_known_hosts: empty/missing/unreadable path, missing
  and unwritable backup directory, happy path, post-copy
  verification, and PermissionError/OSError/catch-all copy
  failures
- cleanup_ssh_known_hosts_for_node: backup success and None,
  the create_backup toggle, and failure/exception propagation

Tests use pytest-mock and the shared loguru_logs fixture,
mirroring the conventions in tests/unit/utils. Because socket,
shutil and datetime are imported inside the functions under
test, socket.gethostbyname and shutil.copy2 are patched on
their real modules and the timestamped backup path is asserted
via a regular expression.

Part of #2199. Implements #2231.

Assisted-by: Claude:anthropic-opus-4-8
Signed-off-by: Christian Berendt <berendt@osism.tech>
@berendt berendt force-pushed the implement/issue-2231-ssh-unit-tests branch from 6b7deaa to 1de3e94 Compare June 12, 2026 11:20
@berendt berendt marked this pull request as ready for review June 12, 2026 11:27

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

Unit tests for osism/utils/ssh.py

2 participants