feat: set primary IP action#303
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds request-aware primary-IP resolution and automatic primary IP assignment using LibreNMS management IPs during IP synchronisation. It introduces utilities (same_host, resolve_set_primary_ip), flags management IP rows in the IP table view, extends SyncIPAddressesView to optionally set primary_ip4/primary_ip6 and report outcomes/errors, and adds a "Set Primary IP" UI toggle persisted via user preference. It also provides reusable HTMX platform mapping partials, an AddPlatformMappingView HTMX endpoint and route, bulk-import UI/JS refinements (including an auto-create-ipam toggle), comprehensive mapping/module-sync documentation, and tests covering the new behaviours. Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant SyncIPView
participant LibreNMSAPI
participant NetBoxDB
User->>Browser: Trigger IP sync with "Set Primary IP" enabled
Browser->>SyncIPView: POST sync request (set_primary_ip=true)
SyncIPView->>LibreNMSAPI: get_management_ip(device)
LibreNMSAPI-->>SyncIPView: management_ip
SyncIPView->>NetBoxDB: Read/create candidate IP objects
loop For each selected IP
SyncIPView->>SyncIPView: _same_host(candidate.ip, management_ip)
alt Host matches and interface present
SyncIPView->>NetBoxDB: Set primary_ip4/primary_ip6 on device/interface
NetBoxDB-->>SyncIPView: confirmation
else No match or no interface
SyncIPView-->>SyncIPView: record primary_no_interface or skip
end
end
SyncIPView->>Browser: Return sync results with primary_set / errors
Browser->>User: Display flash messages and updated table
sequenceDiagram
participant User
participant Browser
participant ImportModal
participant AddPlatformMappingView
participant LibreNMSAPI
participant NetBoxDB
User->>ImportModal: Open validation modal and start platform mapping
Browser->>NetBoxDB: dcim-api:platform-list?q=...
NetBoxDB-->>Browser: list of platforms
User->>ImportModal: Select platform -> POST platform_id + libre_device_id
ImportModal->>AddPlatformMappingView: HTMX POST
AddPlatformMappingView->>LibreNMSAPI: derive LibreNMS OS string
AddPlatformMappingView->>NetBoxDB: select_for_update PlatformMapping row
alt Mapping exists
AddPlatformMappingView->>NetBoxDB: update mapping
else Create mapping
AddPlatformMappingView->>NetBoxDB: insert mapping (handle IntegrityError)
end
AddPlatformMappingView->>NetBoxDB: clear import cache
AddPlatformMappingView-->>ImportModal: return OOB swaps (modal + row)
Browser->>User: Modal and device row refreshed
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/README.md (1)
7-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing documentation for IPAM auto-create feature.
The PR introduces automatic IPAM record creation during device/VM import (per the PR objectives and code in layers 2-6, 11-12), but this feature is not documented in the Features section. Users need to know:
- That IP addresses can be automatically created during import
- How to enable/configure the
auto_create_ipam_defaultsetting- How the per-request toggle works
- That created IPs are placed in global scope
Consider adding a subsection under "Device Import" or a new "IPAM Auto-Create" feature section.
docs/feature_list.md (1)
1-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing IPAM auto-create feature in Device Import section.
The Device Import bullets should include the new automatic IPAM creation capability introduced in this PR. Consider adding a bullet such as:
- Automatic IPAM record creation for primary IP addresses (configurable)
This feature is implemented across multiple code layers (settings, helpers, import operations, UI toggles) but is not documented for users.
docs/librenms_import/validation.md (1)
1-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing documentation for IPAM auto-create import option.
The validation/configuration page should document the
auto_create_ipamtoggle that appears on the import page (see layer 11). Users need to know that they can opt-in to automatic IP address creation during import, and what scope those IPs will be created in (global).Consider adding a section or note explaining this optional behaviour after the "Import Settings" cross-reference.
🧹 Nitpick comments (2)
docs/usage_tips/README.md (1)
1-22: ⚡ Quick winConsider documenting IPAM auto-create setting in Initial Setup.
The plugin settings now include an
auto_create_ipam_defaultboolean (layer 2) that controls whether IP addresses are automatically created during import. This should be mentioned in the "Plugin Settings" section (step 4 or a new step) so users know to configure it during initial setup.netbox_librenms_plugin/utils.py (1)
366-417: 💤 Low valueWell-structured request-aware resolution with defensive fallbacks.
The function correctly implements the three-tier cascade and handles edge cases (None request, string preferences, DB exceptions) defensively. The pattern is consistent with
resolve_naming_preferences.♻️ Optional: extract duplicated settings lookup
The settings lookup logic (lines 391–396 and 412–416) is identical. Consider extracting to a small helper:
+def _get_auto_create_ipam_from_settings() -> bool: + """Retrieve auto_create_ipam_default from LibreNMSSettings, returning False on error.""" + try: + settings = LibreNMSSettings.objects.first() + return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False + except Exception: + return False + def resolve_auto_create_ipam(request) -> bool: ... if request is None: - try: - settings = LibreNMSSettings.objects.first() - return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False - except Exception: - return False + return _get_auto_create_ipam_from_settings() ... - try: - settings = LibreNMSSettings.objects.first() - return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False - except Exception: - return False + return _get_auto_create_ipam_from_settings()This is a minor polish; the current duplication is acceptable for such a short block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d4b2f0fe-0434-4470-9d55-1f117a3188e7
⛔ Files ignored due to path filters (13)
docs/img/Netbox-librenms-plugin-device-sync-fields.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-import-page.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-module-sync-tab.pngis excluded by!**/*.pngdocs/img/carrier_auto_install_rules/list.pngis excluded by!**/*.pngdocs/img/device_type_mappings/list.pngis excluded by!**/*.pngdocs/img/inventory_ignore_rules/list.pngis excluded by!**/*.pngdocs/img/module_bay_mappings/list.pngis excluded by!**/*.pngdocs/img/module_type_mappings/add.pngis excluded by!**/*.pngdocs/img/module_type_mappings/list.pngis excluded by!**/*.pngdocs/img/normalization_rules/add.pngis excluded by!**/*.pngdocs/img/normalization_rules/list.pngis excluded by!**/*.pngdocs/img/platform_mappings/add.pngis excluded by!**/*.pngdocs/img/platform_mappings/list.pngis excluded by!**/*.png
📒 Files selected for processing (26)
docs/README.mddocs/feature_list.mddocs/librenms_import/validation.mddocs/usage_tips/README.mddocs/usage_tips/mapping_rules.mddocs/usage_tips/module_sync.mdnetbox_librenms_plugin/forms.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.py
Contributor
Author
|
hi @bonzo81 - I'm actually not sure this is right direction. Because to get it right - would need to create an interface at import time - so would need to select an interface holding primary IP - which is making it quite complicated/busy import view - so that Primary IP (or OOB) could be assigned (esp. for bulk imports) |
Contributor
Author
|
@bonzo81 I'm going to rewrite it - after thinking about it - while semi-logical place to do it on import - it doesn't really play well esp. when having to onboard in bulk - and duplicating code around IP address import which is not a good idea. |
marcinpsk
force-pushed
the
feat/ipam
branch
2 times, most recently
from
d980373 to
5fb5cfc
Compare
marcinpsk
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
netbox_librenms_plugin/views/sync/ip_addresses.py (1)
125-128: 💤 Low valueRemove the unnecessary wrapper method.
The
_same_host()static method simply delegates to the already-importedsame_host()utility. Callsame_host()directly on line 206 instead of introducing this indirection layer.♻️ Proposed simplification
Remove the static method and update the call site:
- `@staticmethod` - def _same_host(a, b): - """True if two address strings refer to the same host IP.""" - return same_host(a, b) -Then on line 206, change:
- if mgmt_ip and self._same_host(ip_data["ip_address"], mgmt_ip): + if mgmt_ip and same_host(ip_data["ip_address"], mgmt_ip):
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c1cdefe-d560-4fc8-a7fe-7ab01827de0c
📒 Files selected for processing (16)
docs/usage_tips/mapping_rules.mdnetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tables/ipaddresses.pynetbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_sync_views.pynetbox_librenms_plugin/tests/test_import_utils.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/base/ip_addresses_view.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/sync/ip_addresses.py
✅ Files skipped from review due to trivial changes (3)
- netbox_librenms_plugin/tables/ipaddresses.py
- netbox_librenms_plugin/import_utils/device_operations.py
- netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html
🚧 Files skipped from review as they are similar to previous changes (4)
- netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html
- netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
- netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
- docs/usage_tips/mapping_rules.md
marcinpsk
added a commit
to marcinpsk/netbox-librenms-plugin
that referenced
this pull request
May 31, 2026
… locking AddPlatformMappingView now rejects an ambiguous OS string up front, and inside the transaction materialises the candidate rows with select_for_update()[:2] instead of count(). count() silently drops the FOR UPDATE clause, so the rows were never actually locked before the create/update decision (race window). Addresses CodeRabbit feedback on PR bonzo81#303; the platform-mapping logic belongs to this branch / PR bonzo81#299.
Contributor
Author
|
good to go |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e9bff3d-41fb-4520-9526-5bf56fc1bb74
📒 Files selected for processing (19)
docs/usage_tips/mapping_rules.mdnetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tables/ipaddresses.pynetbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_sync_views.pynetbox_librenms_plugin/tests/test_import_utils.pynetbox_librenms_plugin/tests/test_template_comments.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/base/ip_addresses_view.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/sync/ip_addresses.py
✅ Files skipped from review due to trivial changes (2)
- netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html
- netbox_librenms_plugin/import_utils/device_operations.py
🚧 Files skipped from review as they are similar to previous changes (13)
- netbox_librenms_plugin/utils.py
- netbox_librenms_plugin/tests/test_coverage_sync_views.py
- netbox_librenms_plugin/tests/test_coverage_base_views.py
- netbox_librenms_plugin/tables/ipaddresses.py
- netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html
- netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
- docs/usage_tips/mapping_rules.md
- netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html
- netbox_librenms_plugin/views/imports/actions.py
- netbox_librenms_plugin/import_utils/vm_operations.py
- netbox_librenms_plugin/tests/test_import_utils.py
- netbox_librenms_plugin/views/base/ip_addresses_view.py
- netbox_librenms_plugin/views/sync/ip_addresses.py
Open
Owner
|
Hi @marcinpsk, I think you landed in the right place for this one. Simple toggle on IP address sync page works. I see you are still working on this, but just a couple of thoughts:
|
Merged
- Extract device type mapping form into reusable partial (_dt_mapping_form.html) - Extract platform mapping form into reusable partial (_platform_mapping_form.html) - Add AddPlatformMappingView for creating PlatformMapping from import modal - Register add_platform_mapping URL - device_validation_details.html: replace 100-line inline DT form with include; add DT mapping option in mismatch branch; add no-mapping elif with warning; add platform mapping form in platform row - librenms_sync_base.html: fix VM-without-cluster breadcrumb; add object_model_name guard on platform modal; rename _create_platform_url -> create_platform_url - librenms_import.html: wrap toggles with span+hidden-input so unchecked values submit; rename checkbox IDs to -cb suffix - librenms_import.js: rename toggle IDs to match -cb suffix; fix nested modal dismiss so inner modals (e.g. Promote-to-host picker) are not broken
… reachable
Outer {% if validation.existing_device and ... %} at line 293 was missing
its {% endif %}, breaking template compilation. The new-import branch's
platform mapping include was also guarded on sync_info, which is only set
when an existing device matches — so the form never rendered for new
imports. Drop the sync_info clause and the unreachable preselect_platform.
The upfront permission check at line 1849-1854 selects "add" or "change" based on whether a mapping exists, but the get_or_create that follows could race with a concurrent delete: a caller with only "change" permission could end up creating a new row. Mirror the established AddDeviceTypeMappingView pattern — lock the row with select_for_update, re-derive the required permission from the locked state, and surface IntegrityError on concurrent inserts.
Mirror the sibling AddDeviceTypeMappingView style for consistent HTMX toast behavior on validation errors, and reject the LibreNMS "-" placeholder OS that previously slipped past the empty-string check.
Existing devices whose LibreNMS OS has no PlatformMapping previously only got the 'Create Platform' path, nudging users toward duplicate Platform records. Mirror the device-type flow: also offer the platform-mapping form — preselected to the device's current platform when it has one, or unselected when it doesn't — so the LibreNMS OS can be mapped to an existing platform instead. Addresses CodeRabbit feedback surfaced on PR #84 (the file belongs to this branch / PR bonzo81#299).
… locking AddPlatformMappingView now rejects an ambiguous OS string up front, and inside the transaction materialises the candidate rows with select_for_update()[:2] instead of count(). count() silently drops the FOR UPDATE clause, so the rows were never actually locked before the create/update decision (race window). Addresses CodeRabbit feedback on PR bonzo81#303; the platform-mapping logic belongs to this branch / PR bonzo81#299.
…ed modal The platform cell in the import validation modal had become cluttered (inline mapping typeahead + Create Platform button + sync button all at once). Replace the inline forms/buttons with a single pencil icon that opens one modal offering BOTH options: - map the LibreNMS OS to an existing platform (typeahead), and - create a new platform. The icon also appears where a platform is already resolved (direct match or via mapping), so the mapping can be changed compactly. The combined modal reuses the existing create-platform modal plus the _platform_mapping_form partial, guarded so the device-sync page (create-only, no libre_device) is unaffected. - _platform_manage_icon.html: the compact pencil-icon trigger - create_platform_modal.html: adds a 'map to existing' section when libre_device is present; neutral alert wording - CreatePlatformFromImportView.get: passes libre_device + current_platform - device_validation_details.html: platform cell rewritten to use the icon; inline _platform_mapping_form includes + Create Platform buttons removed
… leaked as text)
Django's {# #} comment cannot span multiple lines, so the multi-line header
comment rendered literally in the platform cell. Switch to a {% comment %}
block.
Static scan of all plugin .html templates flagging any line with an unbalanced
{# / #} count (the signature of a multi-line {# #}, which Django renders as
literal text). Would have caught the _platform_manage_icon regression that
get_template()/template-compile checks missed.
- _dt_mapping_form / _platform_mapping_form: post the server_key hidden field
(inherited from the render context) so Add{DeviceType,Platform}MappingView run
the post-save re-validation + cache invalidation against the same LibreNMS
server the modal was rendered for, not the default — fixes wrong-server
context in multi-server setups.
- AddPlatformMappingView: logger.exception instead of logger.warning in the save
failure handler, to preserve the traceback and match AddDeviceTypeMappingView.
Addresses Copilot review on bonzo81#299.
- Add auto_create_ipam option to device/VM import to automatically create IP addresses in NetBox from LibreNMS data - Fix auto_create_ipam bool coercion and dirty-state wiring - Address PR #84 review comments: resolve auto_create_ipam once before VM loop instead of per-VM query; surface all HTMX-path toasts via OOB swap instead of Django messages queue - Fix UnboundLocalError caused by inner mark_safe import shadowing
… outer transaction IntegrityError caught without a nested savepoint leaves the outer transaction.atomic() marked as needing rollback, causing TransactionManagementError on any subsequent ORM query in the same block. Wrapping create() in transaction.atomic() creates a savepoint so the IntegrityError is absorbed without affecting the outer transaction. Also update docs to remove device_type normalization scope from mapping_rules.md since apply_normalization_rules() is not yet called by match_librenms_hardware_to_device_type(). Deferred to issue #90.
…k result AddPlatformMappingView now fails closed (error response) when more than one PlatformMapping row exists for the same librenms_os string, both before the permission check (fast path) and inside the transaction.atomic() block after SELECT FOR UPDATE (to catch concurrent duplicates). bulk_import_vms now includes 'created_ips' in each success entry, populated from vm._librenms_created_ips, mirroring bulk_import_devices_shared so callers receive consistent IPAM creation feedback for VMs.
…stale-lookup guards, simplify auto_create_ipam coercion - Add hidden server_key input to device-type and platform mapping forms so multi-server POSTs resolve against the correct LibreNMS API/cache namespace (AddDeviceTypeMappingView/AddPlatformMappingView read request.POST[server_key]) - Guard the typeahead .catch handlers with seq !== requestSeq so a stale failed lookup can't hide the dropdown from a newer successful one - Drop redundant string truthy-parsing of auto_create_ipam in bulk_import_vms and import_single_device; the flag is already normalised to bool by resolve_auto_create_ipam() at the request boundary
Drops the unassigned-global-/32 pre-create behaviour from device/VM import. It added busy import side-effects, was stuck in the default VRF, and never actually set primary_ip (NetBox requires the IP be interface-assigned), so it only parked records for manual wiring. Replaced (next commit) by Primary-IP auto-match on the IP-sync tab. Removed: ip_helpers.py + tests, migration 0011 + the auto_create_ipam_default model/form field, resolve_auto_create_ipam, the import-utils re-export; the pre-create blocks + created_ips plumbing in device_operations/vm_operations/ bulk_import; the import-page and settings-page toggles + their JS; and Kept: PR #84 server_key inputs + .catch stale-lookup guards, the PlatformMapping duplicate guard, and the htmx_toasts success/failed/skipped mechanism. oob-sync still references the removed helper for OOB/promote; that is reworked to interface-assigned IPs in a later commit on feat/oob-sync.
Replaces the removed import-time auto-create with an interface-assigned model. When the 'Set as Primary IP' toggle is on, SyncIPAddressesView matches the synced IP against the device/VM's LibreNMS management IP (from get_device_info) and, if that IP ends up assigned to one of the object's interfaces, sets it as primary_ip4/primary_ip6. This satisfies NetBox's interface-assignment constraint for primary_ip and creates no unassigned global records. - utils.resolve_set_primary_ip: POST/GET toggle -> user pref -> False cascade - SaveUserPrefView: allow the set_primary_ip pref - SyncIPAddressesView: get_management_ip / _same_host / _set_primary_ip helpers; process_ip_sync sets primary + reports it; display_sync_results surfaces it - IP-sync tab: 'Set as Primary IP' toggle, pre-checked from pref, persisted via save_user_pref - tests for the cascade and the process_ip_sync primary-set behaviour
…s on Flags the IP-sync row whose IP equals the device/VM's LibreNMS management IP (enrich_ip_data -> data-mgmt-ip row attribute) and auto-ticks its select box when the 'Set Primary IP' toggle is enabled, so syncing it sets the Primary IP in one action. Mirrors the parent-child row-auto-select construct (data-* attribute + JS), keyed off the LibreNMS-mapped interface. - utils.same_host: shared host-equality helper (SyncIPAddressesView._same_host now delegates to it) - BaseIPAddressTableView._flag_management_ip: marks the matching enriched entry - IPAddressTable: data-mgmt-ip row attribute - _ipaddress_sync_content.html: toggle JS now also auto-checks the mgmt row - tests for _flag_management_ip
…erface
- Wrap each IP in its own savepoint so one bad address rolls back only itself
instead of poisoning the whole batch; log the exception and include the
reason in the failure toast (previously a bare 'Failed: <ip>').
- When 'Set Primary IP' is on and the management IP has no NetBox interface,
emit a clear warning ('Sync interfaces first, then re-run') instead of
silently skipping the primary assignment.
…e pref A 403/500 from save_user_pref was silently swallowed (fetch only rejects on network errors), so the toggle looked persisted when it wasn't. Addresses CodeRabbit review on PR #84.
Contributor
Author
|
hi @bonzo81, I did open PRs against develop - but those are stacked on top of each other - because if I would open PRs against each other (to get a easier diff ) - merging first would just close all others. as it was: when #288 merges - update branch on this one would drop all doc changes. for PR #288 - I've moved base of PR #299 now and the rest to skip those commits in it so it should be a clean docs-only PR |
filter(address=...) alone can match the same address in a different VRF and then rewrite its VRF on save, hijacking an unrelated IP. Scope the lookup to the selected VRF so a new IP is created in the target VRF instead. Addresses PR #79 CodeRabbit.
Owner
|
Hi @marcinpsk , thanks that helps. I see the stacked sequence now. I'll merge them in order, including #288 first to ensure they merge as intended. |
Owner
|
Thanks @marcinpsk, merging now. |
This was referenced Jun 5, 2026
Merged
marcinpsk
pushed a commit
to marcinpsk/netbox-librenms-plugin
that referenced
this pull request
Jun 8, 2026
- Add Module Sync and Mapping Rules to mkdocs and SUMMARY navigation - Document the opt-in "Set Primary IP" toggle on IP Address Sync (bonzo81#303) - Add developer Extension Points page for the predict_module_interface_names signal and VC normalization report (bonzo81#298) - Note module-interface adoption in the Module Sync guide (bonzo81#296) - Sync root README and docs README feature sections (Module/Inventory Sync, hardware-model filter, Platform Mappings link, multi-server)
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.
Summary
Adds a Set Primary IP option to the device/VM LibreNMS IP-sync tab. When enabled, the synced IP that matches the device's LibreNMS management IP — and is assigned to one of its interfaces — is set as the object's
primary_ip4/primary_ip6.Motivation / Problem
Feature. After syncing IPs from LibreNMS, the device's Primary IP still had to be set by hand. This automates it for the management IP, reusing the interface the IP is already attached to (NetBox requires
primary_ipbe interface-assigned).Scope of Change
SyncIPAddressesViewon the IP-sync tab)How Was This Tested?
process_ip_syncsets the Primary IP only when the synced IP matches the management IP and is interface-assigned (plus skip cases); management-IP row flagging.clean()error.Risk Assessment
Backwards Compatibility
Other Notes
follows #299