Skip to content

Fix skip_fhrp_group_ips unbinding all interface IPs and py<3.12 import (continues #476) - #538

Merged
semx merged 25 commits into
developmentfrom
fix/445-fhrp-ip-binding
Sep 7, 2026
Merged

semx merged 25 commits into
developmentfrom
fix/445-fhrp-ip-binding

Conversation

@semx

@semx semx commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Builds on #476 by @Noah418 / @afoster (their commits are included here) and drives the v1.9.0 milestone item to a working state. Two fixes on top of their work, plus the first unit tests in the repo.

1. Import broken on Python < 3.12

The FHRP log line used an f-string with nested double quotes:

f"Ip address {grab(ip, "data.address")} ..."

That syntax is only valid from Python 3.12 (PEP 701). The project supports Python 3.6+, so on 3.6–3.11 the module fails to import with SyntaxError: f-string: unmatched '(' — exactly what @ionoci reported. Reproduced on 3.11; fixed with single quotes inside; verified the module imports again on 3.11.

2. skip_fhrp_group_ips unbound every interface IP

@ionoci also reported that enabling skip_fhrp_group_ips left all VM interface IPs unbound. The FHRP check ran inside the address-matching loop before the candidate address was compared to the IP being processed, so any FHRP-group IP anywhere in the inventory raised skip_this_ip, and the if skip_this_ip: continue downstream dropped the IP being handled. The effect depended on inventory order and ignored the VRF.

The fix moves the check to a dedicated pass before the matching loop that looks for an FHRP-group assignment on this exact address in the same VRF. An unrelated FHRP IP can no longer suppress a regular IP, the outcome no longer depends on inventory order, and the #445 behaviour (not rebinding a real FHRP-group IP) is preserved.

Tests

First unit tests in the repository (tests/test_fhrp_ip_binding.py), running against the real add_update_interface path via the in-memory NetBoxInventory — no live NetBox or vCenter needed:

  • regular IP stays bound when an unrelated FHRP IP is present, in both inventory orders (parametrized);
  • a matching FHRP-group IP is not reassigned (the Sync overwrites FHRPGroup assignement #445 feature);
  • FHRP match is scoped to the same VRF;
  • control case with the flag disabled.

Verified empirically: the regression and VRF tests fail on the original loop logic and all pass with the fix, on Python 3.11 and 3.13.

Closes #445.

Noah418 and others added 23 commits August 25, 2025 12:17
…assignment' into 445-sync-overwriting-FHRP-Group-assignment
…ured to prevent a duplicate ip address from being created, causing errors.
…nment

Option to prevent FHRP group assignment from being overwritten (#445)
The FHRP-group check ran inside the address-matching loop, before the
candidate address was compared to the IP being processed, so any FHRP
group IP in the inventory raised skip_this_ip and the IP being handled
was left unbound (issue #476). It also depended on inventory order and
ignored the VRF. Move the check to a dedicated pass before the matching
loop that scans for an FHRP-group assignment on this exact address in
the same VRF, so unrelated FHRP IPs no longer suppress a regular IP and
the outcome is order-independent, while the #445 behaviour (not
rebinding a real FHRP-group IP) is preserved.

Also fix an f-string with nested double quotes that is a SyntaxError
before Python 3.12 and broke import on the supported Python 3.6+.

Adds the first unit tests in the repo: an in-memory reproduction of the
unbinding regression in both inventory orders, the VRF scoping, the
#445 feature itself, and a control case. Verified red on the original
logic and green with the fix on Python 3.11 and 3.13.

Co-authored-by: Noah <noah@sol1.com.au>
Co-authored-by: Andrew Foster <andy@sol1.com.au>
@semx
semx merged commit f961860 into development Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants