Show virtual ips in interfaces table #18075
Open
Conversation
This file contains 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
jeremystretch
requested changes
Dec 9, 2024
| @@ -557,6 +557,12 @@ class BaseInterfaceTable(NetBoxTable): | |||
| orderable=False, | |||
| verbose_name=_('IP Addresses') | |||
| ) | |||
| virtual_ips = tables.TemplateColumn( | |||
| accessor=Accessor('fhrp_group_assignments'), | |||
There was a problem hiding this comment.
Suggested change
| accessor=Accessor('fhrp_group_assignments'), | |
| accessor=Accessor('fhrp_group_assignments__ip_addresses'), |
We should reference the IP assignments directly to ensure they are prefetched when the table column is displayed.
| @@ -557,6 +557,12 @@ class BaseInterfaceTable(NetBoxTable): | |||
| orderable=False, | |||
| verbose_name=_('IP Addresses') | |||
| ) | |||
| virtual_ips = tables.TemplateColumn( | |||
There was a problem hiding this comment.
This can be a ManyToManyColumn with linkify_item=True IMO. We should avoid custom template code where possible.
| @@ -557,6 +557,12 @@ class BaseInterfaceTable(NetBoxTable): | |||
| orderable=False, | |||
| verbose_name=_('IP Addresses') | |||
| ) | |||
| virtual_ips = tables.TemplateColumn( | |||
There was a problem hiding this comment.
I suggest renaming this to fhrp_group_ips or similar, as "virtual" IPs may not be quite clear to the user.
| @@ -675,7 +681,7 @@ class Meta(DeviceComponentTable.Meta): | |||
| 'mgmt_only', 'mtu', 'mode', 'mac_address', 'wwn', 'rf_role', 'rf_channel', 'rf_channel_frequency', | |||
| 'rf_channel_width', 'tx_power', 'description', 'mark_connected', 'cable', 'cable_color', 'wireless_link', | |||
| 'wireless_lans', 'link_peer', 'connection', 'tags', 'vdcs', 'vrf', 'l2vpn', 'tunnel', 'ip_addresses', | |||
| 'fhrp_groups', 'untagged_vlan', 'tagged_vlans', 'actions', | |||
| 'virtual_ips', 'fhrp_groups', 'untagged_vlan', 'tagged_vlans', 'actions', | |||
There was a problem hiding this comment.
This also needs to be added to VirtualMachineVMInterfaceTable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes: #18070
Adds a column named
Virtual IPsto the interfaces table.The new column behaves almost exactly like the existing column for directly assigned IP addresses, except that it doesn't limit the number of virtual IPs displayed directly on the table. Since the virtual IPs could come from multiple FHRP Groups, I don't see an easy way to link to a filtered table of IPs that are virtual IPs of the interface.
If that is a problem, I can limit the number that are displayed and add an elipsis (or something to indicate that the list is cropped), or maybe someone with real django experience can figure it out.