feat(sandbox): Add support for mock device creation + testing for local Kind cluster#35
Open
jojung1 wants to merge 28 commits into
Open
feat(sandbox): Add support for mock device creation + testing for local Kind cluster#35jojung1 wants to merge 28 commits into
jojung1 wants to merge 28 commits into
Conversation
jojung1
requested review from
dmathur0,
polarweasel,
ryanheffernan,
spidercensus,
susanhooks and
zsblevins
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a Kind-based mock-device sandbox, DHCP socket-type support, mock DHCP/ZTP/wiring workflows, fixture generation, and updated local development documentation. ChangesMock Device Sandbox Workflow
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/configs/local-dgxc-sandbox.yaml`:
- Line 8: The header comment in local-dgxc-sandbox.yaml incorrectly states "Used
by: make sandbox-up"; update that header to reference the correct make target or
usage (or remove the misleading line). Specifically, change the header so it no
longer claims it's used by the Makefile target sandbox-up (which actually points
to deploy/configs/local-superpod-sandbox.yaml in the Makefile), and optionally
add the correct usage note if this file is referenced by a different target or
script; check references to local-dgxc-sandbox.yaml and
local-superpod-sandbox.yaml in the Makefile to confirm the right target name to
put in the header.
In `@development/mock_devices/Dockerfile`:
- Line 26: The Dockerfile currently sets both files to 644 making the private
key world-readable; change the permission so /app/tls.key is owner-readable (and
writable) only (i.e., 600) while keeping /app/tls.crt as 644, by replacing the
existing chmod line that references /app/tls.key and /app/tls.crt with two
permission changes: one for the key (owner-only) and one for the cert
(public-readable).
- Around line 37-57: The Dockerfile runtime stage runs as root because there is
no USER directive; change it to create and use an unprivileged user/group (e.g.,
add a non-root UID/GID and chown runtime files) and set USER to that account so
ENTRYPOINT ["python", "-m", "mock_device.cli"] and CMD ["serve"] run
unprivileged; ensure you chown /app, /app/.venv, /app/mock_device, /app/fixtures
and copied TLS files to the same UID/GID so ownership matches at runtime.
In `@development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json`:
- Around line 2-3: The fallback inventory fixture currently has an empty
"serial" value for the entry with "model": "MSN2201", which breaks
serial-validation in ZTP paths; update the "serial" key to a deterministic
non-empty mock value (e.g., a consistent test serial like "MSN2201-MOCK-0001" or
similar) so downstream serial-required flows can validate against it, and ensure
only this JSON entry (the object containing "model": "MSN2201") is modified.
In `@development/mock_devices/manifests/mock-devices.yaml`:
- Around line 37-61: The mock device containers (container name "device-api")
lack a securityContext and run with default privileges and writable rootfs;
update each Deployment's pod/container spec for every device-api container to
add a securityContext that enforces non-root execution and minimal privileges:
set runAsNonRoot: true (and a non-root runAsUser if present in your
environment), set allowPrivilegeEscalation: false, drop all capabilities
(capabilities.drop: ["ALL"]), set readOnlyRootFilesystem: true, and add a
seccompProfile (e.g., type: RuntimeDefault) if supported; apply the same changes
to every device-api container instance referenced in the manifest.
In `@development/mock_devices/manifests/mock-dhcp-discover.yaml`:
- Around line 31-50: Add a restrictive securityContext to the dhcp-client
container: under the container named "dhcp-client" add a securityContext that
enforces non-root (runAsNonRoot: true and set runAsUser to a non-zero UID such
as 1000), disables privilege escalation (allowPrivilegeEscalation: false), drops
all Linux capabilities (capabilities: drop: ["ALL"]), and makes the root
filesystem read-only (readOnlyRootFilesystem: true); ensure these fields are
added to the container spec so the dhcp job runs as a non-root, non-privileged,
read-only process.
In `@development/mock_devices/manifests/mock-dhcp-validate.yaml`:
- Around line 24-42: The dhcp-validator container runs with default privileges;
update the pod spec for container name "dhcp-validator" to set a securityContext
that enforces runAsNonRoot: true, disable privilege escalation via
allowPrivilegeEscalation: false, set readOnlyRootFilesystem: true, and drop
unnecessary capabilities (e.g., add capabilityDrops: ["ALL"] or drop NET_RAW
etc. as appropriate); ensure any required file writes use an explicit writable
emptyDir volume mounted to a non-root writable path and set runAsUser/runAsGroup
if needed to match that volume.
In `@development/mock_devices/manifests/mock-wire-devices.yaml`:
- Around line 22-39: The wire container is a token-bearing workload (env
NAUTOBOT_TOKEN) and must get a restrictive securityContext; update the container
definition for the container named "wire" (image mock-device:local, args include
"wire") to enforce hardening: set runAsNonRoot: true and runAsUser to a non-root
UID (e.g. 1000), set readOnlyRootFilesystem: true, set allowPrivilegeEscalation:
false, drop all Linux capabilities (capabilities.drop: ["ALL"]), and add a
seccompProfile (runtime/default) and/or disable privilege escalation at pod
level if present. Apply these changes to the container's securityContext (or
pod.spec.securityContext) so the NAUTOBOT_TOKEN-bearing container runs with
least privilege.
In `@development/mock_devices/manifests/mock-ztp-validate.yaml`:
- Around line 38-57: The ztp-validator container currently runs with default
privileges; update the container spec for the container named "ztp-validator" to
enforce non-root and least-privilege settings by adding a securityContext: set
runAsNonRoot: true and runAsUser to a non-root UID (e.g. 1000), set
allowPrivilegeEscalation: false, drop all Linux capabilities (capabilities: {
drop: ["ALL"] }), and enable a readOnlyRootFilesystem: true (optionally set
runAsGroup and fsGroup if needed for writable volumes). Ensure these fields are
added under the "containers" -> name: ztp-validator block so the job runs
non-root, with no privilege escalation, no extra capabilities, and a read-only
root filesystem.
In `@development/mock_devices/mock_device/cli.py`:
- Line 150: The handlers dhcp, validate, and ztp_validate call os._exit(0 if
result.success else 1) which forcibly terminates the process; change these to
raise SystemExit(0 if result.success else 1) or call sys.exit(0 if
result.success else 1) so normal teardown runs. Locate the os._exit usage in the
functions named dhcp, validate, and ztp_validate in cli.py and replace it with a
SystemExit raise or sys.exit call, ensuring any imports (sys) are added if
needed.
In `@development/mock_devices/mock_device/device_api/eapi.py`:
- Around line 287-289: Loop over the incoming commands and validate each item
before calling .get(): if an item is a str use it as cmd_str, if it is a dict
extract cmd with cmd.get("cmd",""), otherwise treat it as an invalid param and
return a JSON-RPC error with code -32602 ("Invalid params") instead of calling
_dispatch_command; modify the block that currently iterates over commands and
calls _dispatch_command(cmd_str) so it performs isinstance checks on cmd and
short-circuits with the JSON-RPC -32602 response for non-str/non-dict items.
In `@development/mock_devices/README.md`:
- Around line 120-139: Add explicit language identifiers ("text") to the two
fenced code blocks in development/mock_devices/README.md that currently lack
them: the block starting with "fixtures/" (contains the nvue/eapi/devices tree)
and the block listing "make mock-topology ... make mock-workflow-*" (contains
make targets). Edit the opening triple-backtick lines for those blocks to be
```text so markdownlint MD040 is satisfied; do not alter the content inside the
blocks (look for the literal lines beginning with "fixtures/" and "make
mock-topology" to locate the exact blocks).
In `@Makefile`:
- Around line 964-987: The port-forward block launches a background process via
"kubectl port-forward" and assigns its PID to NB_PF but does not guarantee
cleanup if a command fails or the Make target is interrupted; add a trap to
always kill the NB_PF PID (and any other port-forward PIDs like T_PF) on
EXIT/ERR/INT to prevent stale port-forwards. Modify the section that starts the
background port-forward (the line using "kubectl port-forward -n $(NAMESPACE)
svc/nv-config-manager-nautobot 18080:80 &>/dev/null & \ NB_PF=$$!;") to register
a trap immediately after capturing $$! (e.g., trap 'kill $$NB_PF 2>/dev/null ||
true' EXIT INT TERM) and remove/reset the trap after the explicit kill is
performed later; apply the same pattern where T_PF is used in the other similar
blocks.
- Around line 932-939: The mock-devices-down target omits deleting the ZTP
validation job; update the mock-devices-down Makefile recipe to also remove the
mock-ztp-validate manifest by adding a kubectl delete -f
development/mock_devices/manifests/mock-ztp-validate.yaml --ignore-not-found
command so the job created by mock-ztp-validate is cleaned up; ensure this line
is added alongside the other kubectl delete invocations in the mock-devices-down
target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 148ef1b5-4e78-4169-a40d-e3567940fbe4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (97)
Makefiledeploy/configs/local-dgxc-sandbox.yamldeploy/configs/local-superpod-sandbox.yamldeploy/helm/templates/dhcp.yamldeploy/helm/values-local-medium.yamldeploy/helm/values-local-small.yamldeploy/helm/values-local.yamldeploy/helm/values.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_hostname.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_version.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/system.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/interface.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/system.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/system.jsondevelopment/mock_devices/manifests/dhcp-dev-service.yamldevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/__init__.pydevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/config.pydevelopment/mock_devices/mock_device/device_api/__init__.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/device_api/nvue.pydevelopment/mock_devices/mock_device/device_api/server.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.pydevelopment/mock_devices/pyproject.tomldevelopment/mock_devices/requirements.txtdevelopment/mock_topology/context/dgx_cloud/locations.yamldevelopment/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.jsondevelopment/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.jsondevelopment/mock_topology/context/superpod/locations.yamldevelopment/mock_topology/jobs/designs/config_contexts.yaml.j2pyproject.tomlsrc/nv_config_manager/dhcp/kea_dhcp_confgen.py
jojung1
force-pushed
the
port/network-device-mocking-mr183
branch
from
634b919 to
4f05f2b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.json`:
- Around line 2-3: The VLAN fixtures use names like "Vlan1001" but
_nvue_iface_type in development/mock_devices/mock_device/fixture_generator.py
checks for "vlan" case-sensitively so VLANs are mis-typed as "swp"; update
_nvue_iface_type(name) to normalize the interface name (e.g., lowercase it) or
perform a case-insensitive prefix check (e.g., startswith("vlan")) so names like
"Vlan*" return "svi", then regenerate/verify the fixtures such as
development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.json to
ensure Vlan1001/Vlan1002 have type "svi".
In `@development/mock_devices/README.md`:
- Line 195: Update the table cell text for "Config Deploy (diff)" to fix the
double-modal phrasing by replacing "Requires rendered config in Config Store"
with a clearer option such as "Requires a rendered config in Config Store" or
"Requires config to be rendered in Config Store" so the README reads naturally;
locate the table row labeled "Config Deploy (diff)" and edit that cell
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8f1f1475-edc9-4e3d-a740-ef32622572d4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (97)
Makefiledeploy/configs/local-dgxc-sandbox.yamldeploy/configs/local-superpod-sandbox.yamldeploy/helm/templates/dhcp.yamldeploy/helm/values-local-medium.yamldeploy/helm/values-local-small.yamldeploy/helm/values-local.yamldeploy/helm/values.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_hostname.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_version.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/system.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/interface.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/system.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/system.jsondevelopment/mock_devices/manifests/dhcp-dev-service.yamldevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/__init__.pydevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/config.pydevelopment/mock_devices/mock_device/device_api/__init__.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/device_api/nvue.pydevelopment/mock_devices/mock_device/device_api/server.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.pydevelopment/mock_devices/pyproject.tomldevelopment/mock_devices/requirements.txtdevelopment/mock_topology/context/dgx_cloud/locations.yamldevelopment/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.jsondevelopment/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.jsondevelopment/mock_topology/context/superpod/locations.yamldevelopment/mock_topology/jobs/designs/config_contexts.yaml.j2pyproject.tomlsrc/nv_config_manager/dhcp/kea_dhcp_confgen.py
✅ Files skipped from review due to trivial changes (47)
- development/mock_devices/mock_device/init.py
- development/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.json
- development/mock_devices/fixtures/nvue/5.11.0/system.json
- development/mock_devices/fixtures/nvue/5.11.0/platform.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.json
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/nvue/5.14.0/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.json
- development/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.14.0/system.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_hostname.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.13.1/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.json
- development/mock_devices/mock_device/device_api/init.py
- development/mock_devices/fixtures/nvue/5.11.0/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.json
- development/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.json
- development/mock_devices/fixtures/nvue/5.13.1/interface.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/nvue/5.14.0/interface.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.json
- development/mock_devices/fixtures/nvue/5.14.0/platform.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/nvue/5.13.1/system.json
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.json
🚧 Files skipped from review as they are similar to previous changes (25)
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_version.json
- development/mock_devices/fixtures/nvue/5.11.0/interface.json
- deploy/helm/values-local-small.yaml
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.json
- development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.json
- deploy/helm/values.yaml
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.json
- deploy/helm/values-local-medium.yaml
- development/mock_devices/manifests/dhcp-dev-service.yaml
- deploy/helm/values-local.yaml
- deploy/configs/local-dgxc-sandbox.yaml
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.json
- deploy/helm/templates/dhcp.yaml
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.json
- development/mock_devices/mock_device/config.py
- deploy/configs/local-superpod-sandbox.yaml
- development/mock_devices/mock_device/device_api/fixtures.py
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.json
- development/mock_devices/mock_device/cli.py
- development/mock_devices/mock_device/device_api/eapi.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/configs/local-dgxc-sandbox.yaml`:
- Line 8: The header comment in deploy/configs/local-dgxc-sandbox.yaml
incorrectly states "Used by: make sandbox-up (dgx_cloud topology)"; update that
header to reflect the actual use (either remove the “Used by” line entirely or
change it to the correct target) — specifically replace the incorrect reference
with the real INSTALL_CONFIG used by make sandbox-up, e.g., "Used by: make
sandbox-up (deploy/configs/local-superpod-sandbox.yaml)" or delete the line if
this file is not referenced anywhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f6a5aed6-b61e-4914-ac35-4230be63abb1
📒 Files selected for processing (17)
Makefiledeploy/configs/local-dgxc-sandbox.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.py
✅ Files skipped from review due to trivial changes (1)
- development/mock_devices/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json
- development/mock_devices/Dockerfile
- development/mock_devices/manifests/mock-devices.yaml
- development/mock_devices/mock_device/cli.py
- development/mock_devices/mock_device/device_api/eapi.py
- Makefile
Collaborator
Author
|
/ok to test 92bdb21 |
zsblevins
reviewed
Jun 8, 2026
zsblevins
reviewed
Jun 8, 2026
zsblevins
reviewed
Jun 8, 2026
zsblevins
reviewed
Jun 8, 2026
zsblevins
reviewed
Jun 8, 2026
Collaborator
Author
|
/ok to test c42861f |
zsblevins
reviewed
Jun 8, 2026
zsblevins
reviewed
Jun 8, 2026
jojung1
force-pushed
the
port/network-device-mocking-mr183
branch
from
e6a83ae to
95b232b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 1063-1067: The Makefile targets mock-workflow-backup and
mock-workflow-cable-validate currently capture curl output into RESP but never
fail the recipe on HTTP errors; update the curl invocation to also capture the
HTTP status code and exit non‑zero for 4xx/5xx responses. Concretely, change the
curl call (the RESP assignment) to write the response body and HTTP status
separately (e.g., use curl -s -k -w "%{http_code}" to get the code or use
--fail/--fail-with-body where supported), set a RESP and RESP_CODE (or
equivalent), print the body, and if RESP_CODE >= 400 then echo an error and exit
1; otherwise print the success message. Target the RESP assignment and the
echo/exit flow in the mock-workflow-backup and mock-workflow-cable-validate
recipes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a1ca07be-6138-4343-8479-ab2ee72bf32a
📒 Files selected for processing (2)
Makefiledocs/getting-started/local-development-quick-start.mdx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.json (1)
9-10: 💤 Low valueConfirm empty eth0 IP address is intentional.
The
eth0interface has an emptyaddressobject, meaning no IP addresses are assigned. This is unusual for a management interface, especially since other devices in the fixture set (e.g.,core1-cg1-cp1-tan1-dc01) have IPs assigned to eth0. Please confirm this is correct forspine2-cp1-smn1-dc01.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.json` around lines 9 - 10, The eth0 interface for device spine2-cp1-smn1-dc01 currently has an empty ip.address object; confirm whether this is intentional for a management interface and either populate ip.address with the correct management IP (matching the format used by other fixtures) or add a clear comment/flag indicating it should be empty, or remove the eth0 ip block if eth0 is not meant to exist—locate the eth0 entry by the "eth0" interface name and the "ip.address" property in the JSON and update accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 1042-1045: The Makefile currently masks failures by appending "||
true" to kubectl wait, so change the kubectl wait invocations for the
mock-dhcp-validate job (and the analogous lines for mock-ztp-validate,
mock-wire-devices and downstream sandbox-up) to propagate failure instead of
swallowing it: remove the "|| true" fallback (or capture the kubectl wait exit
status and call "exit 1" on non-zero) so the make target fails when the Job
times out or fails; keep the subsequent kubectl logs call for diagnostics but
ensure the make recipe exits non-zero when kubectl wait does not succeed. Ensure
you update the three other blocks (lines referencing job/mock-ztp-validate,
job/mock-wire-devices and the sandbox-up flow) to use the same failure-handling
pattern.
- Around line 985-991: The Makefile targets (e.g., mock-devices-up) load images
into $(KIND_CLUSTER_NAME) but call kubectl without pinning context; update these
targets to use the existing $(KUBECTL_KIND) wrapper (or add --context
$(KIND_CLUSTER_NAME)) for all kubectl apply/delete/wait/logs/log calls so they
run against the Kind cluster; specifically replace instances of "kubectl" with
"$(KUBECTL_KIND)" in mock-devices-up and the other similar targets referenced
(the ranges around lines 1023-1045, 1048-1070, 1079-1101, 1104-1112) to ensure
commands target the Kind kubecontext.
---
Nitpick comments:
In
`@development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.json`:
- Around line 9-10: The eth0 interface for device spine2-cp1-smn1-dc01 currently
has an empty ip.address object; confirm whether this is intentional for a
management interface and either populate ip.address with the correct management
IP (matching the format used by other fixtures) or add a clear comment/flag
indicating it should be empty, or remove the eth0 ip block if eth0 is not meant
to exist—locate the eth0 entry by the "eth0" interface name and the "ip.address"
property in the JSON and update accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a359c378-8e4e-4e68-894d-6bfdc8d85a2f
📒 Files selected for processing (94)
Makefiledeploy/configs/local-superpod-sandbox.yamldeploy/helm/templates/dhcp.yamldeploy/helm/values-local-medium.yamldeploy/helm/values-local-small.yamldeploy/helm/values-local.yamldeploy/helm/values.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_hostname.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_version.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/system.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/interface.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/system.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/system.jsondevelopment/mock_devices/manifests/dhcp-dev-service.yamldevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/__init__.pydevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/config.pydevelopment/mock_devices/mock_device/device_api/__init__.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/device_api/nvue.pydevelopment/mock_devices/mock_device/device_api/server.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.pydevelopment/mock_devices/pyproject.tomldevelopment/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.jsondevelopment/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.jsondevelopment/mock_topology/context/superpod/locations.yamldevelopment/mock_topology/jobs/designs/config_contexts.yaml.j2docs/getting-started/local-development-quick-start.mdxsrc/nv_config_manager/dhcp/kea_dhcp_confgen.py
💤 Files with no reviewable changes (10)
- development/mock_devices/mock_device/device_api/init.py
- development/mock_devices/mock_device/init.py
- development/mock_devices/manifests/mock-ztp-validate.yaml
- development/mock_devices/manifests/mock-dhcp-validate.yaml
- development/mock_devices/manifests/mock-dhcp-discover.yaml
- development/mock_devices/manifests/mock-wire-devices.yaml
- development/mock_devices/mock_device/config.py
- development/mock_devices/mock_device/device_api/fixtures.py
- development/mock_devices/mock_device/cli.py
- development/mock_devices/mock_device/device_api/eapi.py
✅ Files skipped from review due to trivial changes (52)
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/system.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_hostname.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.11.0/interface.json
- development/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.json
- development/mock_devices/fixtures/nvue/5.13.1/platform.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/nvue/5.14.0/interface.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.json
- development/mock_devices/fixtures/nvue/5.14.0/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.14.0/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.json
- development/mock_devices/fixtures/nvue/5.14.0/system.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json
- development/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.13.1/system.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.13.1/interface.json
- development/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_version.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/platform_inventory.json
- development/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.json
- deploy/helm/values-local-small.yaml
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.json
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.json
🚧 Files skipped from review as they are similar to previous changes (14)
- development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.json
- deploy/helm/templates/dhcp.yaml
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.json
- deploy/helm/values-local-medium.yaml
- deploy/helm/values.yaml
- deploy/configs/local-superpod-sandbox.yaml
- development/mock_devices/manifests/dhcp-dev-service.yaml
- development/mock_devices/Dockerfile
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.json
- development/mock_devices/README.md
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Makefile (2)
1144-1153:⚠️ Potential issue | 🟠 MajorFail workflow targets on curl transport errors
mock-workflow-backupandmock-workflow-cable-validateonly fail when the parsedRESP_CODEis>= 400; DNS/TLS/connectivity failures letcurlexit non-zero while%{http_code}typically becomes000, so the recipes can still print the success banner even though the POST never reached the workflow API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 1144 - 1153, The recipes (mock-workflow-backup and mock-workflow-cable-validate) only check RESP_CODE >= 400 and thus succeed on curl transport failures where http_code becomes 000; update the Makefile block around RESP_AND_CODE/RESP/RESP_CODE to also detect failures by either enabling curl's fail behavior (e.g., add -fS) or by capturing curl's exit status and failing when curl returned non-zero or when RESP_CODE == "000", so the recipe exits non-zero and prints an error instead of the success banner.
1029-1037:⚠️ Potential issue | 🔴 CriticalFail fast on Nautobot lookup/auth/parse errors instead of treating them as “MAC missing”.
Inmock-dhcp-validate/mock-dhcp-discover/mock-ztp-validate,MOCK_*_MACis derived fromcurl ... | python3 ... 2>/dev/null, and then emptyMOCK_*_MACtriggers the “No eth0 MAC in Nautobot — falling back to client-id/serial matching” path. Without explicit handling for failed token retrieval, non-2xx responses, or JSON parse errors, broken Nautobot connectivity/auth looks identical to “no eth0 MAC stored”, masking dependency failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 1029 - 1037, The Nautobot lookup currently masks failures by silencing errors and treating any empty MOCK_*_MAC as "missing MAC"; update the Makefile logic (the NB_TOKEN assignment and MOCK_DHCP_MAC computation used by mock-dhcp-validate / mock-dhcp-discover / mock-ztp-validate) to fail fast: verify NB_TOKEN is non-empty after the kubectl secret read and exit non-zero with a clear error if it is missing, run curl with failure/error reporting (do not redirect stderr to /dev/null and use curl -f or check its exit code and HTTP status), and ensure the JSON parse step detects and propagates parse errors (check python/json exit status) instead of suppressing them—on any of these failures print an explicit error about Nautobot connectivity/auth/parse and exit non-zero rather than falling back to client-id/serial matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 14-15: The kind-up target currently hardcodes
"kind-$(KIND_CLUSTER_NAME)" instead of using the declared KIND_CONTEXT (and the
KUBECTL_KIND helper), so update the kind-up target to reference $(KIND_CONTEXT)
(or use the KUBECTL_KIND variable) wherever the hardcoded context string
"kind-$(KIND_CLUSTER_NAME)" appears (e.g., kubectl --context and context-setting
commands) so overrides of KIND_CONTEXT are honored; ensure all occurrences in
the kind-up recipe (and any related targets around it) are replaced.
---
Outside diff comments:
In `@Makefile`:
- Around line 1144-1153: The recipes (mock-workflow-backup and
mock-workflow-cable-validate) only check RESP_CODE >= 400 and thus succeed on
curl transport failures where http_code becomes 000; update the Makefile block
around RESP_AND_CODE/RESP/RESP_CODE to also detect failures by either enabling
curl's fail behavior (e.g., add -fS) or by capturing curl's exit status and
failing when curl returned non-zero or when RESP_CODE == "000", so the recipe
exits non-zero and prints an error instead of the success banner.
- Around line 1029-1037: The Nautobot lookup currently masks failures by
silencing errors and treating any empty MOCK_*_MAC as "missing MAC"; update the
Makefile logic (the NB_TOKEN assignment and MOCK_DHCP_MAC computation used by
mock-dhcp-validate / mock-dhcp-discover / mock-ztp-validate) to fail fast:
verify NB_TOKEN is non-empty after the kubectl secret read and exit non-zero
with a clear error if it is missing, run curl with failure/error reporting (do
not redirect stderr to /dev/null and use curl -f or check its exit code and HTTP
status), and ensure the JSON parse step detects and propagates parse errors
(check python/json exit status) instead of suppressing them—on any of these
failures print an explicit error about Nautobot connectivity/auth/parse and exit
non-zero rather than falling back to client-id/serial matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 85c9abc9-a12c-4f62-8c34-bbbba50c19c2
📒 Files selected for processing (1)
Makefile
Collaborator
Author
|
/ok to test c18f79c |
jojung1
force-pushed
the
port/network-device-mocking-mr183
branch
from
c18f79c to
690d612
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
997-1004:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
mock-devices-downstill misses ZTP validation job cleanup.
mock-ztp-validatecreates the Job at line 1137, but its manifest is not deleted during teardown. This leaves stale job resources across runs.Suggested fix
mock-devices-down: `@echo` "🗑️ Removing mock devices..." $(KUBECTL_KIND) delete -f development/mock_devices/manifests/mock-devices.yaml --ignore-not-found $(KUBECTL_KIND) delete -f development/mock_devices/manifests/mock-dhcp-discover.yaml --ignore-not-found $(KUBECTL_KIND) delete -f development/mock_devices/manifests/mock-dhcp-validate.yaml --ignore-not-found + $(KUBECTL_KIND) delete -f development/mock_devices/manifests/mock-ztp-validate.yaml --ignore-not-found $(KUBECTL_KIND) delete -f development/mock_devices/manifests/mock-wire-devices.yaml --ignore-not-found $(KUBECTL_KIND) delete -f development/mock_devices/manifests/dhcp-dev-service.yaml --ignore-not-found `@echo` "✅ Mock devices removed."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 997 - 1004, The mock-devices-down target fails to delete the ZTP validation Job created by the mock-ztp-validate target; update the mock-devices-down recipe (target name "mock-devices-down") to also delete the ZTP validation manifest used by mock-ztp-validate (refer to the job identifier "mock-ztp-validate" and its manifest file under development/mock_devices/manifests) by adding a kubectl delete -f for that manifest (with --ignore-not-found) so the Job is cleaned up during teardown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@Makefile`:
- Around line 997-1004: The mock-devices-down target fails to delete the ZTP
validation Job created by the mock-ztp-validate target; update the
mock-devices-down recipe (target name "mock-devices-down") to also delete the
ZTP validation manifest used by mock-ztp-validate (refer to the job identifier
"mock-ztp-validate" and its manifest file under
development/mock_devices/manifests) by adding a kubectl delete -f for that
manifest (with --ignore-not-found) so the Job is cleaned up during teardown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 782e3147-79be-4a66-ae33-53252e1affa6
📒 Files selected for processing (94)
Makefiledeploy/configs/local-superpod-sandbox.yamldeploy/helm/templates/dhcp.yamldeploy/helm/values-local-medium.yamldeploy/helm/values-local-small.yamldeploy/helm/values-local.yamldeploy/helm/values.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.jsondevelopment/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_hostname.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_version.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/system.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/interface.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/system.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/system.jsondevelopment/mock_devices/manifests/dhcp-dev-service.yamldevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/__init__.pydevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/config.pydevelopment/mock_devices/mock_device/device_api/__init__.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/device_api/nvue.pydevelopment/mock_devices/mock_device/device_api/server.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.pydevelopment/mock_devices/pyproject.tomldevelopment/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.jsondevelopment/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.jsondevelopment/mock_topology/context/superpod/locations.yamldevelopment/mock_topology/jobs/designs/config_contexts.yaml.j2docs/getting-started/local-development-quick-start.mdxsrc/nv_config_manager/dhcp/kea_dhcp_confgen.py
✅ Files skipped from review due to trivial changes (55)
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_hostname.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.json
- development/mock_devices/fixtures/nvue/5.13.1/system.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.json
- development/mock_devices/mock_device/init.py
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.11.0/system.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/platform.json
- development/mock_devices/fixtures/nvue/5.13.1/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.14.0/interface.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.json
- deploy/helm/values-local-medium.yaml
- development/mock_devices/mock_device/device_api/init.py
- development/mock_devices/fixtures/nvue/5.14.0/platform.json
- development/mock_devices/fixtures/nvue/5.13.1/interface.json
- development/mock_devices/fixtures/nvue/5.11.0/platform_inventory.json
- development/mock_devices/fixtures/devices/core1-cg1-cp1-tan1-dc01/interface.json
- development/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.json
- deploy/helm/values-local-small.yaml
- development/mock_devices/fixtures/nvue/5.14.0/platform_inventory.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/devices/bbr1-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.json
- development/mock_devices/fixtures/devices/bbr2-cp1-tan1-dc01/show_lldp_neighbors_detail.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.json
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/nvue/5.11.0/interface.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.json
- development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/platform.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.json
- development/mock_devices/fixtures/devices/leaf2-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/spine1-cp1-smn1-dc01/interface.json
- development/mock_devices/fixtures/devices/leaf1-cno1-cp1-tan1-dc01/interface.json
- development/mock_devices/README.md
- development/mock_devices/fixtures/nvue/5.14.0/system.json
🚧 Files skipped from review as they are similar to previous changes (19)
- development/mock_devices/fixtures/eapi/4.29.5M/show_version.json
- deploy/helm/values-local.yaml
- development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json
- development/mock_devices/fixtures/devices/leaf1-cp1-smn1-dc01/bridge_domain_mac_table.json
- development/mock_devices/fixtures/devices/spine2-cp1-smn1-dc01/interface.json
- development/mock_devices/manifests/dhcp-dev-service.yaml
- deploy/configs/local-superpod-sandbox.yaml
- deploy/helm/templates/dhcp.yaml
- deploy/helm/values.yaml
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.json
- development/mock_devices/Dockerfile
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.json
- development/mock_devices/mock_device/cli.py
- development/mock_devices/mock_device/config.py
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.json
- development/mock_devices/mock_device/device_api/eapi.py
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
…'t be used Signed-off-by: Joseph Jung <jojung@nvidia.com>
…ormation around mock device sandbox Signed-off-by: Joseph Jung <jojung@nvidia.com>
…ward instructions Signed-off-by: Joseph Jung <jojung@nvidia.com>
…ad of port-forwards Signed-off-by: Joseph Jung <jojung@nvidia.com>
…rgets Signed-off-by: Joseph Jung <jojung@nvidia.com>
…ng and kubectl context setting Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
Signed-off-by: Joseph Jung <jojung@nvidia.com>
…n unintentionally when generating fixtures Signed-off-by: Joseph Jung <jojung@nvidia.com>
…tures being clobbered Signed-off-by: Joseph Jung <jojung@nvidia.com>
jojung1
force-pushed
the
port/network-device-mocking-mr183
branch
from
a0c511c to
41f214f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
development/mock_devices/manifests/mock-wire-devices.yaml (1)
10-26: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHardcoded namespace, including inside
--nautobot-url.Same issue flagged on
mock-devices.yaml:metadata.namespace: nv-config-manager(line 11) and the in-cluster Nautobot FQDNnv-config-manager-nautobot.nv-config-manager.svc(line 26) both hardcode the namespace instead of${NAMESPACE}. IfNAMESPACEis overridden, this Job is created in the wrong namespace and/or points at the wrong Nautobot service, and Makefile'smock-devices-down/other-n $(NAMESPACE)operations will disagree with where the Job actually lives.Template both occurrences with
${NAMESPACE}and route creation throughenvsubst, matching the pattern already used for the DHCP/ZTP job manifests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@development/mock_devices/manifests/mock-wire-devices.yaml` around lines 10 - 26, The mock-wire-devices Job manifest hardcodes the namespace in both the metadata namespace and the Nautobot service FQDN, which breaks deployments when NAMESPACE is overridden. Update the manifest to use the same namespace placeholder pattern as the other Job manifests, and ensure the rendered YAML goes through envsubst so the Job is created in ${NAMESPACE} and the --nautobot-url points to the matching in-cluster service for that namespace.development/mock_devices/manifests/mock-devices.yaml (1)
19-23: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHardcoded
namespace: nv-config-managerbreaksNAMESPACEoverrides.Every Deployment and Service in this file hardcodes
namespace: nv-config-manager, while the Makefile'smock-devices-statusandmock-devices-downtargets operate against$(NAMESPACE)(defaultnv-config-manager, but overridable). If a developer runsmake mock-devices-up NAMESPACE=<other>, resources land innv-config-managerwhilemock-devices-status/other-n $(NAMESPACE)lookups target a different namespace, silently reporting "no mock devices found."A very similar concern (
mock-*.yaml hardcodes namespace: nv-config-managervs. Makefile's$(NAMESPACE)) was raised previously and addressed for the DHCP/ZTP job manifests via${NAMESPACE}+envsubst, but this Deployment/Service manifest andmock-wire-devices.yamlwere left un-templated.Template
metadata.namespace: ${NAMESPACE}here and pipe throughenvsubstin the Makefile'smock-devices-up/mock-devices-downtargets, consistent with the DHCP/ZTP job manifests.Also applies to: 73-76, 128-131, 183-186, 238-241, 293-296, 348-351, 403-406, 458-461, 473-476, 488-491, 503-506, 518-521, 533-536, 548-551, 563-566
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@development/mock_devices/manifests/mock-devices.yaml` around lines 19 - 23, The mock device manifests hardcode metadata.namespace to nv-config-manager, which bypasses the Makefile’s overridable NAMESPACE setting. Update the Deployment/Service manifests in mock-devices.yaml to use ${NAMESPACE} instead of a fixed namespace, and ensure the mock-devices-up and mock-devices-down targets pipe the manifests through envsubst so the applied namespace matches $(NAMESPACE). Use the existing mock device manifest structure and the symbols mock-devices-up, mock-devices-down, and metadata.namespace to locate the change.
🧹 Nitpick comments (4)
development/mock_devices/manifests/mock-dhcp-discover.yaml (1)
17-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo
ttlSecondsAfterFinishedfor this one-shot Job.Unlike
mock-wire-devices.yaml(ttlSecondsAfterFinished: 300), this Job (andmock-dhcp-validate.yaml) has no TTL, so completed/failed Jobs and their Pods persist until explicitly deleted. The Makefile does delete the Job before re-running, but repeated ad-hockubectl apply/manual testing outside the Makefile flow will accumulate stale Job/Pod objects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@development/mock_devices/manifests/mock-dhcp-discover.yaml` around lines 17 - 25, The one-shot Job defined by mock-dhcp-discover (and similarly mock-dhcp-validate) is missing ttlSecondsAfterFinished, so completed or failed Job/Pod objects will linger. Add a TTL value consistent with mock-wire-devices.yaml to the Job spec in these manifests so Kubernetes cleans up finished resources automatically, and keep the existing backoffLimit behavior unchanged.Makefile (2)
1068-1273: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSignificant duplicated shell logic across five mock targets.
mock-dhcp-validate,mock-dhcp-discover,mock-ztp-validate,mock-workflow-backup, andmock-workflow-cable-validateeach re-implement the same ~15-20 line pattern: readNB_TOKENfrom thenautobot-tokensecret, validate non-empty, curl Nautobot with-w "\n%{http_code}", split body/code, fail on000/>=400, then parse JSON withpython3. checkmake also flags each of these targets for excessive recipe length (37/37/37/29/29 lines vs. the 5-line guideline).The prior review history shows several of these targets received the same fix (HTTP status checks, token validation) applied piecemeal at different times — a symptom of this duplication causing drift risk. Consider extracting the shared logic into a small script (e.g.,
development/mock_devices/scripts/nautobot_lookup.sh <query-path> <device-var>) invoked from each target, or adefine/endefcanned recipe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 1068 - 1273, The five mock targets repeat the same Nautobot token fetch, HTTP status handling, and JSON parsing logic, which is causing long recipes and drift risk. Extract that shared lookup flow from mock-dhcp-validate, mock-dhcp-discover, mock-ztp-validate, mock-workflow-backup, and mock-workflow-cable-validate into a single reusable helper (for example a shell script or a canned Makefile recipe), then have each target call it and keep only the target-specific apply/post steps.Source: Linters/SAST tools
1077-1084: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExternal
curlcalls lack a timeout.None of the Nautobot/workflow
curlinvocations set--max-time/--connect-timeout. If the target host hangs, themaketarget blocks indefinitely instead of failing fast.Suggested pattern (apply to each curl invocation)
- NB_RESP=$$(curl -sk -w "\n%{http_code}" -H "Authorization: Token $$NB_TOKEN" \ + NB_RESP=$$(curl -sk --max-time 10 -w "\n%{http_code}" -H "Authorization: Token $$NB_TOKEN" \ "https://nautobot.$(HOSTNAME)/api/dcim/interfaces/?device=$(MOCK_DHCP_DEVICE)&name=eth0&limit=1"); \Also applies to: 1117-1124, 1163-1170, 1217-1218, 1231-1233, 1250-1251, 1264-1266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 1077 - 1084, The Nautobot/workflow curl calls in the Makefile can hang indefinitely because they do not set any timeout options. Update each curl invocation in this target family to include both a connection timeout and an overall max time so failures return promptly; make sure the fix is applied consistently to the interface lookup block and the other referenced curl call sites in the same Makefile section.development/mock_devices/manifests/mock-devices.yaml (1)
15-575: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffEight near-identical Deployment/Service blocks.
Each of the 8 device blocks (and matching Services) differs only in name, serial, and a couple of labels. Since this is a raw manifest (no Kustomize/Helm templating), any future change to resources, security context, or ports must be repeated 8 times — as evidenced by the prior review requiring the same securityContext patch applied "also applies to" 7 other blocks.
Consider generating this file from a small template (Kustomize
components/patchesStrategicMerge, or extending the existingfixture_generator.py) driven by the device list, to keep future edits single-sourced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@development/mock_devices/manifests/mock-devices.yaml` around lines 15 - 575, The manifest is still hand-duplicating eight near-identical Deployment/Service pairs, which makes future updates error-prone. Refactor the mock-device manifest generation so the repeated blocks for mock-sp-tor-01 through mock-sp-pleaf-01 come from a single template or generator input (for example via fixture_generator.py or Kustomize), with only per-device values like MOCK_DEVICE_NAME, MOCK_DEVICE_SERIAL, mock-device-role, and service selectors varying. Keep shared fields like resources, securityContext, ports, and container image defined once in the generator/template so all device blocks stay single-sourced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 1029-1037: The mock-devices-up target uses the static
mock-device:local tag, so reapplying the manifests does not force Kubernetes to
restart existing pods after a rebuild. Update the deployment flow in the
Makefile target and the mock-devices manifest(s) so that each rebuild results in
a rollout, either by changing the pod template with a fresh image reference or
by explicitly triggering a rollout restart after kind load docker-image. Use the
mock-devices-up target and the mock-devices.yaml deployment definition as the
main places to apply the fix.
---
Duplicate comments:
In `@development/mock_devices/manifests/mock-devices.yaml`:
- Around line 19-23: The mock device manifests hardcode metadata.namespace to
nv-config-manager, which bypasses the Makefile’s overridable NAMESPACE setting.
Update the Deployment/Service manifests in mock-devices.yaml to use ${NAMESPACE}
instead of a fixed namespace, and ensure the mock-devices-up and
mock-devices-down targets pipe the manifests through envsubst so the applied
namespace matches $(NAMESPACE). Use the existing mock device manifest structure
and the symbols mock-devices-up, mock-devices-down, and metadata.namespace to
locate the change.
In `@development/mock_devices/manifests/mock-wire-devices.yaml`:
- Around line 10-26: The mock-wire-devices Job manifest hardcodes the namespace
in both the metadata namespace and the Nautobot service FQDN, which breaks
deployments when NAMESPACE is overridden. Update the manifest to use the same
namespace placeholder pattern as the other Job manifests, and ensure the
rendered YAML goes through envsubst so the Job is created in ${NAMESPACE} and
the --nautobot-url points to the matching in-cluster service for that namespace.
---
Nitpick comments:
In `@development/mock_devices/manifests/mock-devices.yaml`:
- Around line 15-575: The manifest is still hand-duplicating eight
near-identical Deployment/Service pairs, which makes future updates error-prone.
Refactor the mock-device manifest generation so the repeated blocks for
mock-sp-tor-01 through mock-sp-pleaf-01 come from a single template or generator
input (for example via fixture_generator.py or Kustomize), with only per-device
values like MOCK_DEVICE_NAME, MOCK_DEVICE_SERIAL, mock-device-role, and service
selectors varying. Keep shared fields like resources, securityContext, ports,
and container image defined once in the generator/template so all device blocks
stay single-sourced.
In `@development/mock_devices/manifests/mock-dhcp-discover.yaml`:
- Around line 17-25: The one-shot Job defined by mock-dhcp-discover (and
similarly mock-dhcp-validate) is missing ttlSecondsAfterFinished, so completed
or failed Job/Pod objects will linger. Add a TTL value consistent with
mock-wire-devices.yaml to the Job spec in these manifests so Kubernetes cleans
up finished resources automatically, and keep the existing backoffLimit behavior
unchanged.
In `@Makefile`:
- Around line 1068-1273: The five mock targets repeat the same Nautobot token
fetch, HTTP status handling, and JSON parsing logic, which is causing long
recipes and drift risk. Extract that shared lookup flow from mock-dhcp-validate,
mock-dhcp-discover, mock-ztp-validate, mock-workflow-backup, and
mock-workflow-cable-validate into a single reusable helper (for example a shell
script or a canned Makefile recipe), then have each target call it and keep only
the target-specific apply/post steps.
- Around line 1077-1084: The Nautobot/workflow curl calls in the Makefile can
hang indefinitely because they do not set any timeout options. Update each curl
invocation in this target family to include both a connection timeout and an
overall max time so failures return promptly; make sure the fix is applied
consistently to the interface lookup block and the other referenced curl call
sites in the same Makefile section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aa915489-3e77-4a32-afb6-c1bdadcc88c9
📒 Files selected for processing (78)
Makefiledeploy/configs/local-superpod-sandbox.yamldeploy/helm/templates/dhcp.yamldeploy/helm/values-local-medium.yamldeploy/helm/values-local-small.yamldeploy/helm/values-local.yamldeploy/helm/values.yamldevelopment/mock_devices/Dockerfiledevelopment/mock_devices/README.mddevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.jsondevelopment/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.jsondevelopment/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_hostname.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.jsondevelopment/mock_devices/fixtures/eapi/4.29.5M/show_version.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.11.0/system.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/interface.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.13.1/system.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/interface.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/platform_inventory.jsondevelopment/mock_devices/fixtures/nvue/5.14.0/system.jsondevelopment/mock_devices/manifests/dhcp-dev-service.yamldevelopment/mock_devices/manifests/mock-devices.yamldevelopment/mock_devices/manifests/mock-dhcp-discover.yamldevelopment/mock_devices/manifests/mock-dhcp-validate.yamldevelopment/mock_devices/manifests/mock-wire-devices.yamldevelopment/mock_devices/manifests/mock-ztp-validate.yamldevelopment/mock_devices/mock_device/__init__.pydevelopment/mock_devices/mock_device/cli.pydevelopment/mock_devices/mock_device/config.pydevelopment/mock_devices/mock_device/device_api/__init__.pydevelopment/mock_devices/mock_device/device_api/eapi.pydevelopment/mock_devices/mock_device/device_api/fixtures.pydevelopment/mock_devices/mock_device/device_api/nvue.pydevelopment/mock_devices/mock_device/device_api/server.pydevelopment/mock_devices/mock_device/dhcp_client.pydevelopment/mock_devices/mock_device/fixture_generator.pydevelopment/mock_devices/mock_device/wire_nautobot.pydevelopment/mock_devices/mock_device/ztp_client.pydevelopment/mock_devices/pyproject.tomldevelopment/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.jsondevelopment/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.jsondevelopment/mock_topology/context/superpod/locations.yamldevelopment/mock_topology/jobs/designs/config_contexts.yaml.j2docs/getting-started/local-development-quick-start.mdxsrc/nv_config_manager/dhcp/kea_dhcp_confgen.py
✅ Files skipped from review due to trivial changes (46)
- development/mock_devices/fixtures/eapi/4.29.5M/show_hostname.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mac_address_table.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_mpls_interface.json
- development/mock_devices/fixtures/nvue/5.11.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.13.1/system.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/platform.json
- development/mock_topology/context/superpod/devices/b09-u23-p01-ufm-02.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.11.0/system.json
- development/mock_devices/fixtures/nvue/5.14.0/system.json
- development/mock_devices/fixtures/nvue/5.13.1/interface.json
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform_inventory.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_lldp_neighbors_detail.json
- development/mock_devices/mock_device/init.py
- development/mock_devices/fixtures/nvue/5.14.0/platform.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_interfaces_status.json
- development/mock_devices/fixtures/nvue/5.13.1/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform_inventory.json
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/platform_inventory.json
- deploy/helm/values.yaml
- development/mock_devices/fixtures/nvue/5.14.0/bridge_domain_mac_table.json
- development/mock_devices/fixtures/nvue/5.11.0/platform.json
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/platform_inventory.json
- development/mock_devices/fixtures/nvue/5.13.1/platform.json
- development/mock_devices/fixtures/nvue/5.14.0/interface.json
- development/mock_devices/fixtures/nvue/5.14.0/platform_inventory.json
- deploy/helm/values-local.yaml
- development/mock_devices/fixtures/nvue/5.11.0/interface.json
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/platform.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/platform.json
- development/mock_devices/fixtures/devices/a09-u44-p01-pleaf-01/interface.json
- development/mock_devices/fixtures/eapi/4.29.5M/show_version.json
- deploy/helm/values-local-medium.yaml
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/platform.json
- development/mock_devices/mock_device/device_api/init.py
- development/mock_devices/README.md
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/platform.json
- development/mock_devices/fixtures/devices/a09-u36-p01-spine-01/interface.json
- development/mock_devices/fixtures/devices/a08-u28-p01-oobspine-01/interface.json
- development/mock_devices/fixtures/devices/a08-u44-p01-mleaf-01/interface.json
🚧 Files skipped from review as they are similar to previous changes (20)
- development/mock_topology/context/superpod/devices/a09-u23-p01-ufm-01.json
- development/mock_topology/jobs/designs/config_contexts.yaml.j2
- development/mock_devices/manifests/dhcp-dev-service.yaml
- development/mock_devices/fixtures/nvue/5.11.0/platform_inventory.json
- deploy/helm/values-local-small.yaml
- src/nv_config_manager/dhcp/kea_dhcp_confgen.py
- deploy/helm/templates/dhcp.yaml
- development/mock_topology/context/superpod/locations.yaml
- development/mock_devices/fixtures/devices/a09-u28-p01-bleaf-01/interface.json
- development/mock_devices/mock_device/device_api/fixtures.py
- development/mock_devices/mock_device/device_api/server.py
- development/mock_devices/Dockerfile
- development/mock_devices/pyproject.toml
- development/mock_devices/fixtures/devices/a04-u44-p01-tor-01/interface.json
- deploy/configs/local-superpod-sandbox.yaml
- development/mock_devices/fixtures/devices/a09-u32-p01-sleaf-01/interface.json
- development/mock_devices/mock_device/wire_nautobot.py
- development/mock_devices/mock_device/device_api/nvue.py
- development/mock_devices/fixtures/devices/a08-u32-p01-cleaf-01/interface.json
- development/mock_devices/mock_device/device_api/eapi.py
Signed-off-by: Joseph Jung <jojung@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Mock Device Sandbox for Local DHCP/ZTP Development
Mock Device Server (
development/mock_devices/)interface.json,platform_inventory.json) so each mock device returns its correct serial number and interface data via the APIgenerate-fixtures) that reads device topology JSONs and produces version-specific and per-device fixture files automaticallywireCLI /make mock-wire-devices) that resolves each mock device's Kubernetes ClusterIP and updates Nautobot'sprimary_ip4, enabling Temporal workflows to reach mock devicesDHCP / ZTP Validation Tooling
validateCLI /make mock-dhcp-validate) that queries the Kea API through the DHCP service and checks for a matching reservation byhw-addressorclient-id, with no raw sockets requireddhcpCLI /make mock-dhcp-discover) that sends real DHCP DISCOVER/REQUEST UDP packets via a relay agent (giaddr), simulating a real switch booting for the first timeztp-validateCLI /make mock-ztp-validate) that walks the full ZTP provisioning chain: DHCP reservation → boot script → serial validation → config retrievalhw-address(matching production switch behavior) and falls back toclient-id/serial for devices without a MACSuperpod Topology — DHCP/ZTP Support
superpod/locations.yaml:TEST-SITE Sandbox DHCP Subnet— registers the Kind pod network (10.244.0.0/16) as a Kea static subnetSuperpod-dhcp-options— setsreservations-global: trueoneth0interfaces and injects ZTPboot-file-name/cumulus-provision-urloptions into reservationsKubernetes Manifests
mock-wire-devices,mock-dhcp-validate,mock-dhcp-discover, andmock-ztp-validate, all parameterized viaenvsubstMakefile Targets
mock-devices— deploys all mock device pods into the Kind clustermock-wire-devices— wires mock device ClusterIPs into Nautobotmock-dhcp-validate/mock-dhcp-discover/mock-ztp-validate— end-to-end sandbox validation targets with configurable device/serial overridesmock-status— shows pod and service state for all mock devicessandbox-upandkind-upupdated to use thesuperpodtopology and deploy mock devices as part of the standard local dev setupHelm / Deploy Config
local-superpod-sandbox.yamldeploy config and updated local Helm values to enable the DHCP dev service and wire thesuperpodtopologyBreakdown of changes in this PR
development/mock_devices/fixtures/development/mock_devices/mock_devices/Though the PR looks daunting at first, most of the changes (almost 75%) are just from generated fixtures for mock_devices. Other
mock_devices/manifests/cli code makes up an additional 22.6%. Changes outside of the new mock_devices/ dir make up 2.7% of the entire PR (including boilerplate from local-superpod-sandbox.yaml). Meaningful files to look at outside ofdevelopment/should beMakefile(260 lines), topology YAML context additions (~62 lines), and thekea_dhcp_confgen.py(5 lines).Validation
The kind integration test is manual due to taking ~30 min to complete. When the PR is ready for review,
run Actions -> Kind Integration -> Run workflow against the copy-pr-bot generated
pull-request/<PR_NUMBER>branch. Use the defaulttest_pathfor the full suite,or narrow it only while debugging.
Passing Kind Integration run:
Checklist
CONTRIBUTING.md.docs screenshots, or Helm/rendered outputs.
Summary by CodeRabbit
Release Notes
New Features
sandbox-upplus new sandbox orchestration targets and improved Kind-context routing for related commands.Configuration
dhcpSocketTypesupport, with"udp"enabled for local small/medium/local values.Documentation