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

Regression on scenarios in #3633 #5469

Closed
steffann opened this issue Dec 15, 2020 · 9 comments
Closed

Regression on scenarios in #3633 #5469

steffann opened this issue Dec 15, 2020 · 9 comments
Labels
pending closure Requires immediate attention to avoid being closed for inactivity

Comments

@steffann
Copy link
Contributor

steffann commented Dec 15, 2020

Environment

  • Python version: 3.7.3
  • NetBox version: 2.10.0

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

  • It hides crucial operational information from NOC engineers that are debugging issues with interfaces
  • It messes up all automation that needs to know where an interface is end-to-end connected to, for example for including path information in interface descriptions or where the configuration depends on what an interface is connected to
  • It breaks our reports that check that both interfaces at the end of a connection are correctly configured (tagged/untagged, tags etc)
@DanSheps
Copy link
Member

Can you dig up the FR that added this?

@steffann
Copy link
Contributor Author

Yes, it's in the title: #3633

@jeremystretch
Copy link
Member

jeremystretch commented Dec 16, 2020

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

dxks commented Dec 18, 2020

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
Copy link
Member

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
Copy link

stale bot commented Feb 2, 2021

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 stale bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Feb 2, 2021
@stale
Copy link

stale bot commented Feb 18, 2021

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.

@stale stale bot closed this as completed Feb 18, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity
Projects
None yet
Development

No branches or pull requests

4 participants