starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Clean up code for showing CircuitTermination on interfaces #3826

New Issue
Closed
opened 2025-12-29 18:31:23 +01:00 by adam · 9 comments
adam commented 2025-12-29 18:31:23 +01:00
Owner
Copy Link

Originally created by @steffann on GitHub (Jul 1, 2020).

Originally assigned to: @steffann on GitHub.

This template code relies on trace() not traversing circuits:
43d610405f/netbox/templates/dcim/inc/interface.html (L94-L123)

Since e143158f12 this is no longer the case, which makes the template code partially obsolete. This needs to be cleaned up.

Example 1

Consider the following topology:

+--------+     +------+  +-----------+  +------+     +--------+
| Intf X +-----+ CT A +--+ Circuit C +--+ CT Z +-----+ Intf Y |
+--------+  1  +------+  +-----------+  +------+  2  +--------+

Old implementation

Before e143158f12 the signal handler to update connected_endpoints would call trace(follow_circuits=False). This would cause the connected_endpoint for Interface X to be CircuitTermination A (and vice versa), and the connected_endpoint for Interface Y to be CircuitTermination Z (and vice versa).

When the template is showing the connected endpoint it will try to call get_peer_termination on it. Because the connected endpoint is a CircuitTermination this may succeed (depending on whether both CircuitTerminations exist for the Circuit, in this example it exists) and if it does the CircuitTermination on the other end of the Circuit will be used. If that CircuitTermination has a connected endpoint then that connected endpoint will be shown as the connected endpoint of the interface, and the circuit information will be displayed as "via …".

So in the example above, for Interface X, the old code would have CircuitTermination A stored as the connected endpoint in the database, but it would display Interface Y via Circuit C in the user interface.

Current implementation

Since e143158f12 the follow_circuits parameter to trace() no longer exists, and the implementation always follows circuits. So now the connected_endpoint of Interface X will be Interface Y (and vice versa). The code that displays the connected endpoint of the peer of a circuit termination if the original connected endpoint is a circuit termination is therefore obsolete (and considering the length of that sentence probably rightfully so).

Example 2

Consider the following topology:

+--------+     +------+  +-----------+  +------+
| Intf X +-----+ CT A +--+ Circuit C +--+ CT Z |
+--------+  1  +------+  +-----------+  +------+

Old implementation

The old implementation would stop at CircuitTermination A and store that as the connected endpoint. The user interface would then find the other CircuitTermination through get_peer_termination and display that.

Current implementation

Because circuits are now always traversed trace will now return these steps in the path:

CableTermination Cable CableTermination
Interface X 1 CircuitTermination A
CircuitTermination Z None None

Because the last CableTermination is empty the connected_endpoint will be empty and the user interface will show "Not connected"

Example 3

Consider the following topology:

+--------+     +------+  +-----------+
| Intf X +-----+ CT A +--+ Circuit C |
+--------+  1  +------+  +-----------+

Both the old and the new implementation handle this in the same way. What makes this a special case is that this is the only remaining situation where the connected_endpoint of CircuitTermination A is updated. When CircuitTermination Z is later added and connected to an interface the connected_endpoint of Interface X will be updated to reflect that but the connected_endpoint of CircuitTermination A will still be Interface X.

This behaviour is inconsistent because it only occurs when there is only one CircuitTermination connected to the Circuit. When both CircuitTerminations are created before connecting the cable the connected_endpoint of CableTermination A will always remain empty.

Originally created by @steffann on GitHub (Jul 1, 2020). Originally assigned to: @steffann on GitHub. This template code relies on `trace()` not traversing circuits: https://github.com/netbox-community/netbox/blob/43d610405f625f2044ebdab0c21df01b75dc856d/netbox/templates/dcim/inc/interface.html#L94-L123 Since https://github.com/netbox-community/netbox/commit/e143158f121c7d147bec820af227388deca6dfb5 this is no longer the case, which makes the template code partially obsolete. This needs to be cleaned up. # Example 1 Consider the following topology: ``` +--------+ +------+ +-----------+ +------+ +--------+ | Intf X +-----+ CT A +--+ Circuit C +--+ CT Z +-----+ Intf Y | +--------+ 1 +------+ +-----------+ +------+ 2 +--------+ ``` ## Old implementation Before https://github.com/netbox-community/netbox/commit/e143158f121c7d147bec820af227388deca6dfb5 the signal handler to update `connected_endpoint`s would call `trace(follow_circuits=False)`. This would cause the `connected_endpoint` for Interface X to be CircuitTermination A (and vice versa), and the `connected_endpoint` for Interface Y to be CircuitTermination Z (and vice versa). When the template is showing the connected endpoint it will try to call `get_peer_termination` on it. Because the connected endpoint is a CircuitTermination this may succeed (depending on whether both CircuitTerminations exist for the Circuit, in this example it exists) and if it does the CircuitTermination on the other end of the Circuit will be used. If that CircuitTermination has a connected endpoint then that connected endpoint will be shown as the connected endpoint of the interface, and the circuit information will be displayed as "via …". So in the example above, for Interface X, the old code would have CircuitTermination A stored as the connected endpoint in the database, but it would display Interface Y via Circuit C in the user interface. ## Current implementation Since https://github.com/netbox-community/netbox/commit/e143158f121c7d147bec820af227388deca6dfb5 the `follow_circuits` parameter to `trace()` no longer exists, and the implementation always follows circuits. So now the `connected_endpoint` of Interface X will be Interface Y (and vice versa). The code that displays the connected endpoint of the peer of a circuit termination if the original connected endpoint is a circuit termination is therefore obsolete (and considering the length of that sentence probably rightfully so). # Example 2 Consider the following topology: ``` +--------+ +------+ +-----------+ +------+ | Intf X +-----+ CT A +--+ Circuit C +--+ CT Z | +--------+ 1 +------+ +-----------+ +------+ ``` ## Old implementation The old implementation would stop at CircuitTermination A and store that as the connected endpoint. The user interface would then find the other CircuitTermination through `get_peer_termination` and display that. ## Current implementation Because circuits are now always traversed `trace` will now return these steps in the path: | CableTermination | Cable | CableTermination | |----------------------|-------|----------------------| | Interface X | 1 | CircuitTermination A | | CircuitTermination Z | None | None | Because the last CableTermination is empty the `connected_endpoint` will be empty and the user interface will show "Not connected" # Example 3 Consider the following topology: ``` +--------+ +------+ +-----------+ | Intf X +-----+ CT A +--+ Circuit C | +--------+ 1 +------+ +-----------+ ``` Both the old and the new implementation handle this in the same way. What makes this a special case is that this is the only remaining situation where the `connected_endpoint` of CircuitTermination A is updated. When CircuitTermination Z is later added and connected to an interface the `connected_endpoint` of Interface X will be updated to reflect that but the `connected_endpoint` of CircuitTermination A will still be Interface X. This behaviour is inconsistent because it only occurs when there is only one CircuitTermination connected to the Circuit. When both CircuitTerminations are created before connecting the cable the `connected_endpoint` of CableTermination A will always remain empty.
adam added the status: acceptedtype: housekeeping labels 2025-12-29 18:31:23 +01:00
adam closed this issue 2025-12-29 18:31:23 +01:00
adam commented 2025-12-29 18:31:23 +01:00
Author
Owner
Copy Link

@steffann commented on GitHub (Jul 1, 2020):

I propose the following clean-ups:

Example 1

There is no good way to consistently determine the connected_endpoint based on the connected_endpoint of a CircuitTermination. Going back to the old implementation where we store the CircuitTermination instead of the Interface will break whenever there is a one-to-many RearPort connected to the CircuitTermination. I therefore propose to keep the current implementation and remove the template code for displaying "via Circuit C".

Example 2

In this scenario I think it makes the most sense to store CircuitTermination Z as the connected_endpoint for Interface X (and vice versa). The template can then display the remote site again with the "via Circuit C", as it used to be in the old implementation but with cleaner template code.

Example 3

Here a choice has to be made about what to put in the connected_endpoint of CircuitTerminations. I propose to make it consistent that a CircuitTermination consistently has an Interface as its connected endpoint, but not vice-versa. So when taking the topology from example 1 we would end up with:

CableTermination Connected endpoint
Interface X Interface Y
Interface Y Interface X
CircuitTermination A Interface X
CircuitTermination Z Interface Y

I think this would make the most sense to the user.

@steffann commented on GitHub (Jul 1, 2020): I propose the following clean-ups: # Example 1 There is no good way to consistently determine the `connected_endpoint` based on the `connected_endpoint` of a CircuitTermination. Going back to the old implementation where we store the CircuitTermination instead of the Interface will break whenever there is a one-to-many RearPort connected to the CircuitTermination. I therefore propose to keep the current implementation and remove the template code for displaying "via Circuit C". # Example 2 In this scenario I think it makes the most sense to store CircuitTermination Z as the `connected_endpoint` for Interface X (and vice versa). The template can then display the remote site again with the "via Circuit C", as it used to be in the old implementation but with cleaner template code. # Example 3 Here a choice has to be made about what to put in the `connected_endpoint` of CircuitTerminations. I propose to make it consistent that a CircuitTermination consistently has an Interface as its connected endpoint, but *not* vice-versa. So when taking the topology from example 1 we would end up with: | CableTermination | Connected endpoint | |----------------------|--------------------| | Interface X | Interface Y | | Interface Y | Interface X | | CircuitTermination A | Interface X | | CircuitTermination Z | Interface Y | I think this would make the most sense to the user.
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@tyler-8 commented on GitHub (Jul 6, 2020):

I don't think there should be inconsistency between the way the 'via Circuit ID' is displayed as presented in these 3 examples. I understand the challenges with Example 1, however that behavior is going to cause confusion with users. In a way, the more complete your circuit connection model, the less information the GUI gives. That seems counter-intuitive.

Personally, I find the information that an interface is connected to a circuit more valuable than the distant-end interface (that may or may not exist) on the device detail page. If I need the distant-end information, then I could click the cable trace button to get that.

This would also cause conflict with https://github.com/netbox-community/netbox/issues/4619

[Edit] I realize you also mentioned in the above issue that you were working on it. Wasn't sure if this issue was a separate workstream altogether or not.

@tyler-8 commented on GitHub (Jul 6, 2020): I don't think there should be inconsistency between the way the 'via Circuit ID' is displayed as presented in these 3 examples. I understand the challenges with Example 1, however that behavior is going to cause confusion with users. In a way, the more complete your circuit connection model, the less information the GUI gives. That seems counter-intuitive. Personally, I find the information that an interface is connected to a circuit more valuable than the distant-end interface (that may or may not exist) on the device detail page. If I need the distant-end information, then I could click the cable trace button to get that. This would also cause conflict with https://github.com/netbox-community/netbox/issues/4619 [Edit] I realize you also mentioned in the above issue that you were working on it. Wasn't sure if this issue was a separate workstream altogether or not.
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@steffann commented on GitHub (Jul 13, 2020):

Personally, I find the information that an interface is connected to a circuit more valuable than the distant-end interface (that may or may not exist) on the device detail page. If I need the distant-end information, then I could click the cable trace button to get that.

This would be a major change to NetBox and goes beyond this cleanup issue. It would require a different implementation of tracing connections and determining the endpoint.

@steffann commented on GitHub (Jul 13, 2020): > Personally, I find the information that an interface is connected to a circuit more valuable than the distant-end interface (that may or may not exist) on the device detail page. If I need the distant-end information, then I could click the cable trace button to get that. This would be a major change to NetBox and goes beyond this cleanup issue. It would require a different implementation of tracing connections and determining the endpoint.
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@tyler-8 commented on GitHub (Jul 13, 2020):

This would be a major change to NetBox and goes beyond this cleanup issue. It would require a different implementation of tracing connections and determining the endpoint.

I'm not suggesting anything different than the way NetBox has displayed interfaces connected to circuits since the beginning of the project being open-sourced v1.8.0. Up until recently, NetBox has always shown 'Via CircuitXYZ' for an interface connected to a circuit, regardless of where the distant-end connection's modeling (circuit term, interface, etc) ended.

I was stating my preference of which information I found more helpful (from the historical behavior of NetBox) when looking at a device detail page vs the cable trace page.

Adding...

I understand this is a complex data model and also appreciate that you're working to make it better. I wanted to voice my concern about the change to displaying via inconsistently so we can hopefully continue the conversation in further refinements. Thanks for what you do!

@tyler-8 commented on GitHub (Jul 13, 2020): > This would be a major change to NetBox and goes beyond this cleanup issue. It would require a different implementation of tracing connections and determining the endpoint. I'm not suggesting anything different than the way NetBox has displayed interfaces connected to circuits since ~the beginning of the project being open-sourced~ [v1.8.0](https://github.com/netbox-community/netbox/releases/tag/v1.8.0). Up until recently, NetBox has always shown 'Via CircuitXYZ' for an interface connected to a circuit, regardless of where the distant-end connection's modeling (circuit term, interface, etc) ended. I was stating my preference of which information I found more helpful (from the historical behavior of NetBox) when looking at a device detail page vs the cable trace page. Adding... I understand this is a complex data model and also appreciate that you're working to make it better. I wanted to voice my concern about the change to displaying `via` inconsistently so we can hopefully continue the conversation in further refinements. Thanks for what you do!
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@steffann commented on GitHub (Jul 13, 2020):

What has changed is that cable traces ended at the circuit termination, and the assumption was made that every circuit would lead to one interface. However that assumption is not always true, for example when using multiplexed connections or multiple circuits after each other. The code was fixed to deal with that bad assumption, but the consequence (for now) is that the circuit isn't shown anymore.

I have ideas for improving this, but they require a major overhaul of the way we trace paths and store the useful data about them. It would definitely be nice, but I don't think it's feasible for now. There are too many other issues that need to be handled for the next releases.

@steffann commented on GitHub (Jul 13, 2020): What has changed is that cable traces ended at the circuit termination, and the assumption was made that every circuit would lead to one interface. However that assumption is not always true, for example when using multiplexed connections or multiple circuits after each other. The code was fixed to deal with that bad assumption, but the consequence (for now) is that the circuit isn't shown anymore. I have ideas for improving this, but they require a major overhaul of the way we trace paths and store the useful data about them. It would definitely be nice, but I don't think it's feasible for now. There are too many other issues that need to be handled for the next releases.
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@wols commented on GitHub (Jul 14, 2020):

Please check a dependency with #4851

@wols commented on GitHub (Jul 14, 2020): Please check a dependency with #4851
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Jul 24, 2020):

Related to #4900

@jeremystretch commented on GitHub (Jul 24, 2020): Related to #4900
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@XioNoX commented on GitHub (Aug 3, 2020):

Thanks for working on that and clearly documenting the different behaviors. I think we're hitting 2 issues related to "Example 1". Please let me know if that's not the right place, or if I should provide more details.

Issue 1:
In our infrastructure we have paths in the following form:
Intf X +-----+ CT A +--+ Circuit C +--+ CT Z +-----+ Intf Y
But some show up the (old?) way (as "via XXX" in the UI, and "connected_endpoint_type": "circuits.circuittermination" through the API.
And others as they were directly connected (no "via XXX" and "connected_endpoint_type": "dcim.interface" through the API).
So there are inconsistencies between paths that should be similar. Maybe it only happen for newly created paths?

Issue 2:
Our automation relies on having "connected_endpoint_type": "circuits.circuittermination" when an interface is connected to a circuit but still continues all the way to a remote interface.
If I understand correctly the "regression" happened to accommodate for more complex use-cases, although I was wondering if it would be possible to keep the same behavior for the current/simple use-cases.

@XioNoX commented on GitHub (Aug 3, 2020): Thanks for working on that and clearly documenting the different behaviors. I think we're hitting 2 issues related to "Example 1". Please let me know if that's not the right place, or if I should provide more details. Issue 1: In our infrastructure we have paths in the following form: `Intf X +-----+ CT A +--+ Circuit C +--+ CT Z +-----+ Intf Y` But some show up the (old?) way (as "via XXX" in the UI, and `"connected_endpoint_type": "circuits.circuittermination"` through the API. And others as they were directly connected (no "via XXX" and `"connected_endpoint_type": "dcim.interface"` through the API). So there are inconsistencies between paths that should be similar. Maybe it only happen for newly created paths? Issue 2: Our automation relies on having `"connected_endpoint_type": "circuits.circuittermination"` when an interface is connected to a circuit but still continues all the way to a remote interface. If I understand correctly the "regression" happened to accommodate for more complex use-cases, although I was wondering if it would be possible to keep the same behavior for the current/simple use-cases.
adam commented 2025-12-29 18:31:24 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Nov 17, 2020):

This has been addressed under #4900.

@jeremystretch commented on GitHub (Nov 17, 2020): This has been addressed under #4900.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label status: accepted type: housekeeping
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#3826
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?