Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connecting rear ports through circuit causes error when connecting devices #3288

Closed
steffann opened this issue Jun 22, 2019 · 17 comments
Closed
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@steffann
Copy link
Contributor

Environment

  • Python version: Python 3.6.8
  • NetBox version: 2.6.0

Steps to Reproduce

  1. Create a circuit
  2. Create two devices with multiple front ports mapped to a single rear port (mux
  3. Connect the two rear ports to the circuit
  4. Connect an interface to a front port of one mux
  5. Connect an interface to the corresponding front port of the other mux

At this point all is still well: both devices are connected to each other.

  1. Repeat step 4

This will fail as the first interface connection will have used the circuit termination point id in its _connected_circuittermination_id. The second interface will try to put the same termination point id in its _connected_circuittermination_id, which will fail because that is a OneToOneField.

Expected Behavior

When connecting multiple devices/interfaces through a mux they are expected to share the same circuit. Use cases are CWDM and DWDM. In Django terms: I expected _connected_circuittermination_id to be a ForeignKey, not a OneToOneField.

Observed Behavior

<class 'django.db.utils.IntegrityError'>

duplicate key value violates unique constraint "dcim_interface__connected_circuittermination_id_key"
DETAIL: Key (_connected_circuittermination_id)=(42) already exists.

Notes

This case seems similar to #3193, but is different. That issue applies to cables where the rear port of one mux isn't directly connected to the rear port of another mux, but is passed through other patch panels (implemented as 1 front port mapped to 1 rear port).

@steffann
Copy link
Contributor Author

I have tested the following changes, and have been working with them in production:

  • Change dcim.Interface._connected_circuittermination to a ForeignKey instead of OneToOne
  • Change dcim.cable.get_path_endpoints to follow circuits

That way there won't be a change to the endpoint data model, CWDM and DWDM circuits will work, and endpoints will show the correct peer endpoint.

If this is an acceptable solution I can have a pull request ready in minutes.

@steffann
Copy link
Contributor Author

A preview of what I suggest can be found here: develop...steffann:multilink-circuit

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application labels Jun 24, 2019
@jeremystretch
Copy link
Member

The root issue here is a conflict between two approaches, namely whether or not a circuit termination should be considered a path endpoint. In the scenario you describe, it makes sense to follow a connection across the circuit to its peer termination. But in other cases (e.g. attaching to an Internet transit circuit with no defined peer termination) it makes sense to stop at the circuit. I think we need to come up with some reference models and tests before we proceed any further.

@steffann
Copy link
Contributor Author

Agreed. I'll write up some possible scenarios, and we can see where the common cases are.

@steffann
Copy link
Contributor Author

steffann commented Jul 7, 2019

Agreed. I'll write up some possible scenarios, and we can see where the common cases are.

Sorry this is taking a bit more time. I had some urgent work related things I had to attend to. I haven't forgotten about this though!

@steffann
Copy link
Contributor Author

Ok, finally time to work on this :)

Let's look at the possible scenario's from the user's point of view. I am ignoring the dcim_interface._connected_interface_id and dcim_interface._connected_circuittermination_id fields for now. Those are implementation-specific cache fields, and I have the feeling that we might come up with better cache fields after evaluating the scenarios.

I see these possible scenario's. These are the "classical" ones without front/rear ports. Let's assume that the A side is connected to a device interface through a circuit-termination (CT). Obviously all scenario's apply with A and B sides reversed as well.

+----------+           +------+   +---------+
| Device A |--[cable]--| CT A |---| Circuit |---[?]
+----------+           +------+   +---------+
  1. B side has no circuit-termination
  2. B side has a circuit-termination but no device connected to it
  3. B side is connected to a device

In scenario 1 we only know that the device on the A side is connected to the circuit. The user interface should show which circuit the device interface is connected to. Display "via circuit in the user interface".

Scenario 2 expands on this a bit because we now know, through the circuit-termination, which site the other end of the circuit goes to. I suggest we show both the circuit and the remote site in the user interface. Display "to site via circuit".

Scenario 3 should show the user that the device interface is connected to which remote device interface, plus which circuit the connection uses. As we already know the remote device, I don't see much value in showing the remote site of the circuit-termination as well. Display "to device port via circuit".

Then on to some more complex scenarios where there are multiple circuits on the path:

+----------+           +-------+   +-----------+   +-------+
| Device A |--[cable]--| CT 1A |---| Circuit 1 |---| CT 1B |---\
+----------+           +-------+   +-----------+   +-------+    |
                                                                |
  /-------------------------[ cable ]--------------------------/
 |
 |    +-------+   +-----------+
  \---| CT 2A |---| Circuit 2 |---[?]
      +-------+   +-----------+
  1. 2B side has no circuit-termination
  2. 2B side has a circuit-termination but no device connected to it
  3. 2B side is connected to a device

I think these scenarios are similar to the first three. In the user interface I personally would like to see all the circuits to make it more intuitive, especially when looking a the impact of outages. I think it would also be useful to see the sites of CT 1B, CT 2A, etc. (removing duplicates when CT 1B and 2A are in the same site, as they will probably be in most cases). For example: "via circuit1 - ct1B/2A-site - circuit2" etc.

And then on to the scenarios that I opened this ticket about: CWDM and DWDM circuits and suchlike multiplexers:

+----------+           +---MUX1---+
| Device A |--[cable]--| Front 1  |
+----------+           |          |           +------+   +---------+
                       |     Rear |--[cable]--| CT A |---| Circuit |---[?]
+----------+           |          |           +------+   +---------+
| Device B |--[cable]--| Front 2  |
+----------+           +----------+
  1. B side has no circuit-termination
  2. B side has a circuit-termination but no device connected to it
  3. B side is connected to a rear port, but there is no corresponding front port
  4. B side is connected to a rear port, but nothing is connected to the front port
  5. B side is connected to a device interface through a rear+front port

I'm assuming that the names of the front ports will correspond to channel names or something similar. Is that a reasonable assumption?

For scenario 7 I think it would be useful to show the front port name and the circuit in the user interface. That is probably the information a user needs most. Display "via mux1-front - circuit" (e.g. "via CH32 - DarkFiber123").

Scenario 8 would benefit from showing the remote site name as well, similar to scenario 2. Display "to ctB-site via mux1-front - circuit" (e.g. "to Amsterdam via CH32 - DarkFiber123").

Scenario 9 seems to be an error, but maybe there are cases where the mux device has a separate channel for out-of-band management which doesn't have a corresponding front port. Display "to mux2 via mux1-front - circuit" (e.g. "to mux01.ams1 via CH32 - DarkFiber123").

In scenario 10 the user probably wants to know the front port that the connection ends up at. Display "to mux2 mux2-front via mux1-front - circuit" (e.g. "to mux01.ams1 CH32 via CH32 - DarkFiber123").

A possible optimization could be to display "to mux2 mux2-front via circuit" (e.g. "to mux01.ams1 CH32 via DarkFiber123") when the front names on both MUXes are the same, as will be the case when those names are CWDM/DWDM channel names.

Scenario 11 would of course show the connected device and interface together with the circuit. If we can assume that front port names are indeed channel names, then that information would be useful to show as well. Display "to remote-device remote-interface via mux1-front - circuit - mux2-front" (e.g. "to router1.ams1 xe-0/0/0 via CH32 - DarkFiber123 - CH32").

The same optimization with duplicate front port names would apply here too: "to remote-device remote-interface via mux1-front - circuit" (e.g. "to router1.ams1 xe-0/0/0 via CH32 - DarkFiber123").

For the CWDM/DWDM scenarios we might need to rethink the circuits_circuittermination.connected_endpoint_id field as a circuit-termination may be connected to a rear port, in which case there is no single interface that it's connected to.

And then there are the scenarios where there are multiple CWDM/DWDM MUXes on the path:

+----------+           +---MUX1---+
| Device A |--[cable]--| Front 1  |
+----------+           |          |           +-------+   +-----------+
                       |     Rear |--[cable]--| CT 1A |---| Circuit 1 |---\
+----------+           |          |           +-------+   +-----------+    |
| Device B |--[cable]--| Front 2  |                                        |
+----------+           +----------+                                        |
                                                                           |
  /-----------------------------------------------------------------------/
 |
 |                        +---MUX2---+           +---MUX3---+
 |                        |  Front 1 |--[cable]--| Front 1  |
 |    +-------+           |          |           |          |
  \---| CT 1B |--[cable]--| Rear     |           |     Rear |---\
      +-------+           |          |           |          |    |
                          |  Front 2 |--[cable]--| Front 2  |    |
                          +----------+           +----------+    |
                                                                 |
  /-------------------------[ cable ]---------------------------/
 |
 |    +-------+   +-----------+
  \---| CT 2A |---| Circuit 2 |---[?]
      +-------+   +-----------+

For these scenarios I would suggest prefixing the display text of the previous section with "via mux1-front - circuit - mux2-front" (or the shorter optimized version, with front port names omitted if they are the same as the previous front port name).

  1. 2B side has no circuit-termination
  2. 2B side has a circuit-termination but no device connected to it
  3. 2B side is connected to a rear port, but there is no corresponding front port
  4. 2B side is connected to a rear port, but nothing is connected to the front port
  5. 2B side is connected to a device interface through a rear+front port

Scenario 12 would display "via mux1-front - circuit1 - mux2-front - mux3-front - circuit2" (e.g. "via CH32 - DarkFiber123 - CH32 - CH32 - DarkFiber456"), or optimized to "via CH32 - DarkFiber123 - DarkFiber456"

Scenario 13 would display "to ct2B-site via mux1-front - circuit1 - mux2-front - mux3-front - circuit2" (e.g. "to Amsterdam via CH32 - DarkFiber123 - CH32 - CH32 - DarkFiber456"), or optimized to "to Amsterdam via CH32 - DarkFiber123 - DarkFiber456"

Scenario 14 would display "to mux4 via mux1-front - circuit1 - mux2-front - mux3-front - circuit2" (e.g. "to mux01.ams1 via CH32 - DarkFiber123 - CH32 - CH32 - DarkFiber456"), or optimized to "to mux01.ams1 via CH32 - DarkFiber123 - DarkFiber456".

In scenario 15 would display "to mux4 mux4-front via mux1-front - circuit1 - mux2-front - mux3-front - circuit2" (e.g. "to mux01.ams1 CH32 via CH32 - DarkFiber123"), or optimized to "to mux01.ams1 CH32 via DarkFiber123 - DarkFiber456".

Scenario 16 would display "to remote-device remote-interface via mux1-front - circuit1 - mux2-front - mux3-front - circuit2 - mux4-front" (e.g. "to router1.ams1 xe-0/0/0 via CH32 - DarkFiber123 - CH32 - CH32 - DarkFiber456 - CH32"), or optimized to "to router1.ams1 xe-0/0/0 via CH32 - DarkFiber123 - DarkFiber456".


I hope I covered all likely scenarios here. Once we agree that these are correct, and that the display text for each scenario is what we want it to be, then I will write a proposal in pseudo-code to capture the common elements and try to come up with a maintainable code structure.

@steffann
Copy link
Contributor Author

Oh, another thing to think about: what to do when we are showing the connected endpoint of a cable that ends up at a rear port without having gone through a front port (for example when showing what a circuit termination is connected to). I think we should show the rear port and its device as the endpoint. Going any further would make us end up at one of the endpoints of a front port, which would make no sense.

@bsteinert
Copy link

@steffann i see a scenario that would not be covered. when you connect a multiplexer to a extension port of another multiplexer (often 1300nm cwdm or 1550nm dwdm) to extend your network this could break things in documentation..

+----------+           +---MUX1---+          +---MUX2---+
| Device A |--[cable]--| Front 1  |          |          |
+----------+           |          |          |          |
                       |     Rear |--[cbl]---| Front 1  |
+----------+           |          |          |          |  +-----------+
| Device B |--[cable]--| Front 2  |          |     Rear |--| Circuit 1 |--\
+----------+           +----------+          +----------+  +-----------+   |
                                                                           |
  /-----------------------------------------------------------------------/
 |
 |            +---MUX3---+           +---MUX4---+
 |            |  Front 1 |---[cbl]---| Rear     |
 |            |          |           |          |           +-----------+
  \-----------| Rear     |           |  Front 1 |--[cable]--| Device A2 |  
              |          |           |          |           +-----------+
              |  Front 2 |           |  Front 2 |--[cable]--| Device B2 |
              +----------+           +----------+           +-----------+

--> "From Site1 to Site2 via Mux-1 1330nm - Mux-2 1300nm - DarkFiberX - Mux-3 1300nm - Mux-4 1330nm"

That get even more complicated when i add PatchPanel to NetBox...

Thank you very mutch for working on this!

@garnser
Copy link

garnser commented Sep 26, 2019

@bsteinert I've the exact same issue as you've, to make things worse some multiplexers only have front-ports which isn't supported at all since a Front port requires a rear-port.

@steffann
Copy link
Contributor Author

@bsteinert I've the exact same issue as you've, to make things worse some multiplexers only have front-ports which isn't supported at all since a Front port requires a rear-port.

Well, the term "rear-port" is used for the port that aggregates one or more front ports. It doesn't have to be physically on the rear of the device :) And a "front-port" may physically be on the back.

@steffann
Copy link
Contributor Author

A lot of work has been done in #3585. I could use some extra eyes on the changes at this point. Because of the large number of changes I would like to ask for some guinea pigs^W^Wvolunteers to thoroughly test these changes

@amtypaldos
Copy link

We have just hit a wall due the no MUX support so I would gladly do some testing for you!

@steffann
Copy link
Contributor Author

I created a demo test case where Site A and Site B are connected using a DWDM circuit with a mux at both ends. Eth1 of router at Site A is connected over CH35 to the router at Site B.

I also connected a second mux at Site A to the Upg port of the first mux. Eth2 of the router at Site A is connected to CH50 of the second mux. At site B the Upg port of the mux is conneced to another circuit going to Site C. At that site there is a mux of the same type as the second mux at Site A, and the router at Site C is connected to CH50 of that one.

Just to create a scenario with nested muxes and multiple circuits :)

The interfaces of Router A now look like this:

Schermafbeelding 2019-10-23 om 13 43 57

@steffann
Copy link
Contributor Author

This issue started with an error message, but the cause for that error message was much deeper. It is caused by the interactions between front ports, rear ports and circuits. A quick bug fix is therefore not possible (or just plain ugly).

I'll copy the use cases to a new feature request so we can discuss that properly, and by implementing that this bug will be solved as well.

@steffann
Copy link
Contributor Author

Oh, another reminder of why I think that this is not fixable without a bigger overhaul: if only circuit terminations and interfaces are considered "connected" endpoints, then a path that ends anywhere else (a rear port for example) is considered "not connected", which is very misleading. The physical port is connected to something, it's just that the path doesn't end at one of the supported endpoint types. Trying to fix that basically led to the feature request in #3633.

@steffann
Copy link
Contributor Author

steffann commented Oct 23, 2019

Unless someone has a workable short-term fix, I'd like to suggest closing this bug report as "unfixable without major overhaul".

@jeremystretch
Copy link
Member

This was resolved by the work done on #3193 in PR #4387. (There are still issues with the cabling model, however this specific bug as detailed in the "steps to reproduce" has been fixed.)

@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants