search

netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Try NetBox Cloud free: https://netboxlabs.com/free-netbox-cloud/
http://netboxlabs.com/oss/netbox/
Apache License 2.0
16.16k stars 2.59k forks source link

Regression on scenarios in #3633 #5469

Closed steffann closed 3 years ago

steffann commented 3 years ago

Environment

Steps to Reproduce

  1. Create a circuit with two circuit terminations
  2. Create two devices, each with multiple front ports connected to a rear port (MUXes)
  3. Connect the devices' rear ports to the circuit terminations
  4. Create two devices with two interfaces each
  5. Connect the interface of the first device to the first port of the first MUX
  6. Connect the interface of the seconf device to the first port of the second MUX
  7. Check the connected endpoint of each of the interfaces
  8. Check the trace from each of the interfaces

This is basically the first scenario in #3633:

Interface <-> Front port                Front port <-> Interface
                  ^                         ^
                  |                         |
                  v                         v
               Rear port <-> Circuit <-> Rear port

Expected Behavior

In step 7 I expected to see the interface of the other device as the connected andpoint.

In step 8 I expected to see the end-to-end trace from the interface of the first device to the interface of the second device, through the first MUX, circuit and second MUX.

This is the functionality provided by NetBox 2.8 and 2.9.

Observed Behavior

In step 7 the circuit termination is shown as the connected endpoint.

In step 8 the trace ends at the circuit termination.

This is a regression from the behaviour of NetBox 2.8 and 2.9.

Impact

DanSheps commented 3 years ago

Can you dig up the FR that added this?

steffann commented 3 years ago

Yes, it's in the title: #3633

jeremystretch commented 3 years ago

Allow me to provide some context. Per the v2.10 release notes:

As part of this change, cable traces will no longer traverse circuits: A circuit termination will be considered the origin or destination of an end-to-end path.

This change was made as part of the overhaul of the cable tracing logic (#4900), which allowed us to finally resolve various bugs that have plagued NetBox since v2.5. As part of this work, it became clear that we needed to decide whether a circuit termination was to be regarded as a termination of a cable path (like an interface) or a transit node (like a pass-through port).

I made the decision that it will now be treated as a termination primarily to address the (very common) scenario wherein there is no peer circuit termination defined. This is most often the case when modeling Internet circuits, where the NetBox user doesn't particularly care (and may not even know) about the far end of the circuit. With the new approach to cable tracing using CablePath, if we were to treat circuit termination as transit nodes, it would be impossible to directly reference a circuit termination from its originating interface or vice versa. (The CablePath model tracks an origin, destination, and intermediate nodes for each path.)

The immediate "fix" for the behavior described in this issue would be to treat circuit terminations as transit nodes within a path rather than termination points. This would effectively cause them to behave identical to front/rear ports: We would lose the direct association between origin and destination if either is a circuit termination. This would break the ability to directly list the circuit connected to an interface that exists today.

An illustration might help. Consider the following topology:

 Interface A                       Interface B
      |                                 |
 Front Port A                      Front Port B
 Rear Port A                       Rear Port B       
      |                                 |
Circuit Term A <--- Circuit X --> Circuit Term B

In v2.10.0, this is conveyed using two* CablePath instances:

CablePath(
    origin=Interface A,
    destination=Circuit Term A,
    path=[Cable 1, Front Port A, Rear Port A, Cable 2]
)
CablePath(
    origin=Circuit Term B,
    destination=Interface B,
    path=[Cable 3, Rear Port B, Front Port B, Cable 4]
)

(*There are in fact four CablePath instances, two for each direction, but we can ignore that detail for the sake of this discussion.)

If we were to treat circuit terminations as transit nodes, we would instead have a single CablePath instance:

CablePath(
    origin=Interface A,
    destination=Interface B,
    path=[Cable 1, Front Port A, Rear Port A Cable 2, Circuit Term A, Circuit Term B,
          Cable 3, Rear Port B, Front Port B, Cable 4]
)

This would make it impossible to query the originating path for a circuit termination, and circuit terminations could never appear as the endpoint for a path. Thus, listing interfaces would never include a circuit termination as the far end connection (just like front/rear ports).

Where we've gotten into trouble in the past is trying to apply both treatments simultaneously, depending on whether a far-end circuit termination existed. As we've learned, this does not work (#4925 provides a fairly substantial accounting).

I'm happy to pursue potential solutions for cross-circuit cable tracing, however I do think we need to preserve the ability to model a path between interface and circuit termination that exists today. Maybe it's worth revisiting how we treat circuits versus cables, and when to use one over another. Or maybe it's possible to extend CablePath in some manner.

Finally, I'll caution that any work we do from this point forward should take into consideration possible future support for overlay networks built using circuits (MPLS, EVPN, etc.), wherein there may not be a direct A-to-Z relationship. Of course there's no firm model proposed yet; just something to keep in mind.

steffann commented 3 years ago

This has been discussed under #4812. Please follow the implementation described there. It shows the most logical endpoints for both the CT and the Interface.

jeremystretch commented 3 years ago

It has been discussed, but in my opinion never fully resolved. We've made several efforts since the v2.5 release to address various bugs in the cabling system, and each successive change has introduced further problems. This is why during the maintainers' meeting on 2020-09-29, which you attended, we agreed to bump several milestones from the then-upcoming v2.10 release in favor of prioritizing an overhaul of the cabling system. At the following meeting two weeks later, I shared my work on #4900, which introduced the new CablePath model and the behavior exhibited by the current release. The new model was included for testing as part of the first v2.10 beta release on 2020-11-17, and I received no negative feedback pertaining to it prior to the final 2.10 release on 2020-12-14.

I understand that the new model as it stands today does not fully meet your needs. That's regrettable, and serves to highlight the importance of community participation in testing and evaluation. At the same time, the new approach does solve a number of long-standing bugs, and greatly improves cable tracing performance. I'm confident that we can identify a solution to the problems you've identified, but it will need to be done by leveraging the new cabling model.

dxks commented 3 years ago

We are also using Circuits in the same case where we need to traverse the circuit transparently.

What's about having a CircuitType having a boolean-property indicating a Circuit becoming transparent like an E-Line will do and use this exception within the generation of the cablepath to not terminate on the circuit and traverse the circuit. The gui just needs to hide the trace-buttons on this kind of Circuits.

This would preserve the implementation and would enable the transparency feature on a per circuit case.

jeremystretch commented 3 years ago

What's about having a CircuitType having a boolean-property indicating a Circuit becoming transparent

Unfortunately it's half-measures like this that got us into trouble in the first place. When you make the tracing logic conditional, it becomes extremely susceptible to error and quickly falls apart. (Consider the implications of changing a circuit's type after connections had already been made.) A robust solution demands a consistent approach.

One potential solution would be to instead always treat circuit terminations as path nodes, and require that every circuit have two terminations. In the case where we don't know/care about the far end, we could use a "cloud" instance as the peer termination. The CablePath instance would then extend from the near-end interface to the cloud instance. This idea needs a lot more thought, however, and as I mentioned earlier should be pursued with potential support for overlay networks in mind.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

stale[bot] commented 3 years ago

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.