Skip to content

Add EOS free-space pre-flight check (NAPPS-1091)#370

Open
jtdub wants to merge 3 commits intonetworktocode:developfrom
jtdub:u/jtdub-napps-1091
Open

Add EOS free-space pre-flight check (NAPPS-1091)#370
jtdub wants to merge 3 commits intonetworktocode:developfrom
jtdub:u/jtdub-napps-1091

Conversation

@jtdub
Copy link
Copy Markdown
Contributor

@jtdub jtdub commented Apr 20, 2026

Summary

  • Adds a pre-flight free-space check to EOSDevice.file_copy and EOSDevice.remote_file_copy so transfers fail fast when flash lacks room for the image.
  • Establishes a cross-driver pattern on BaseDevice (_get_free_space seam + concrete _check_free_space helper) so companion work for IOS, NXOS, JUNOS, and ASA only has to implement the per-platform probe.
  • Makes FileCopyModel.file_size a required field with an optional file_size_unit (bytes / megabytes / gigabytes), mirroring how callers already declare the checksum. file_size_bytes is computed once in __post_init__.

Implementation notes

  • NotEnoughFreeSpaceError gained optional required / available / file_system kwargs; the existing positional (hostname, min_space) signature is preserved so F5Device._check_free_space keeps working untouched.
  • EOS _get_free_space parses the trailing (N bytes free) token from dir output; the file_system argument is accepted for API parity but unused because EOS only exposes a single practical filesystem.
  • remote_file_copy (HTTP / HTTPS / SCP / SFTP / FTP / TFTP) compares src.file_size_bytes against _get_free_space() after the scheme/URL validation and enable-session setup but before the copy command is built or sent — so an oversized transfer never reaches the device's copy machinery.
  • file_copy compares os.path.getsize(src) against _get_free_space() after file_copy_remote_exists() reports the file is missing.

Breaking change

  • FileCopyModel(...) now requires file_size. Every in-repo call site (tests, integration, docs) has been updated; external consumers will see a TypeError until they add the kwarg.

Not in this PR

  • IOS / NXOS / JUNOS / ASA implementations are tracked separately; this PR only wires EOS in and lays the base-class seam.

Test plan

  • poetry run pytest tests/unit -q — 651 passed (up from 639; 12 new)
  • poetry run ruff check pyntc tests
  • poetry run ruff format --check pyntc tests
  • poetry run pylint pyntc — 10.00/10
  • poetry run towncrier build --draft renders the changes/370.* fragments cleanly
  • Manual integration run against a lab cEOS node (tests/integration/test_eos_device.py) — set EOS_HOST/EOS_USER/EOS_PASS + per-protocol URL vars + FILE_CHECKSUM/FILE_SIZE/FILE_SIZE_UNIT; the new free-space tests include an oversized-transfer case that expects NotEnoughFreeSpaceError.

🤖 Generated with Claude Code

jtdub and others added 2 commits April 20, 2026 10:41
EOS file_copy and remote_file_copy now verify the target filesystem has
enough free space before starting a transfer. BaseDevice gains an
overridable _get_free_space hook and a concrete _check_free_space helper
so IOS, NXOS, JUNOS, and ASA tickets can wire in platform-specific
probes without further base-class changes.

FileCopyModel gains a required file_size field with an optional
file_size_unit (bytes/megabytes/gigabytes) that is converted internally
to file_size_bytes for the comparison, mirroring how callers already
supply the checksum.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jtdub jtdub marked this pull request as ready for review April 20, 2026 16:17
@mattmiller87
Copy link
Copy Markdown
Contributor

I am of the opinion that the file size is a required piece of information to add this feature. It is similar to file checksum as a required field. To do the copy, you need to know certain things about the file.

To leave the file size optional, likely adds more complexity in the code to process and determine file size.

@jtdub jtdub marked this pull request as draft April 20, 2026 17:38
- BaseDevice._get_free_space(file_system=None) — make the parameter
  optional so drivers like EOS (single filesystem) don't have to lie
  about the signature. Drops the `del file_system` workaround in EOS.
- Pull the mocked-pyeapi setUp/tearDown out of TestRemoteFileCopy and
  TestFreeSpaceCheck into a shared _EOSDeviceMockedTestCase base.
- Move the per-protocol FileCopyModel builder and the any_file_copy_model
  fixture out of the EOS and ASA integration test modules into
  tests/integration/_helpers.py + conftest.py so both drivers share
  one source of truth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jtdub jtdub marked this pull request as ready for review April 20, 2026 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants