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

bug/usability UX: Links status and status_reason of a Path aren't always coherent #554

Open
viniarck opened this issue Oct 30, 2024 · 0 comments
Labels
bug Something isn't working tech_debt Tech debt

Comments

@viniarck
Copy link
Member

Issue:

  • Links status and status_reason of a Path aren't always coherent.

So, when looking into a dict of a current_path or any other *_path, if you're trying to confirm if the link are enabled you can't trust it. Each Link is a different ref since mef_eline has been historically structured that way, and ONLY each intf endpoint of link share the same reference as topology (which owns those links and consequently have the latest and correct status)

How this issue can be observed?

  • If you start kytosd without -E (auto enable discovered entities), any Link that mef_eline will instantiate when creating an EVC will have the status and status_reason disabled:
"current_path": [{
...
        "status": "DISABLED",
        "status_reason": [
          "disabled"
        ]

}]
...
  • If a link on topology gets disabled, mef_eline will correctly react to kytos/topology.link_down and converge and update an EVC status if needed and the Path status will also be correct, but the Link status might not be.
Impacts

The only impact is confusion it can cause when debugging or troubleshooting a path if you're trying to understand if a link of path in an output of an EVC data is enabled or not. Other than that, convergence-wise its using Path.status correctly which uses the correct topology link ref by deriving it from the endpoints refs.

How to fix

It can be thought as two-fold issues:

Path status

  • It would be helpful to also expose the path status on as_dict() since that's always coherent, but since each path is listed as a list of Links that would break the shape of the data, which might break a client or how it's parsed. So, maybe to avoid breaking there we could consider exposing somewhere else in the EVC, like current_path_status: str.

Link status

  • Quick way while building on top of tech debt way would try to keep status and status_reason in sync based on the events.
  • The cleanest and easier to maintain would be to always use the same links ref as topology which ideally should be in the core cross-ref/tech-debt: move self.links to kytos core topology#203, but this has another major refactoring where s_vlan and its metadata is assuming that each Link instance is unique, so this approach would also require changing how s_vlans are being stored, and it's a major refactoring, so won't be prioritized for now. But something to keep in mind.

Exposing and dealing only with Path status would be worth in the short/medium term. Link status let's keep in mind for future architecture stuff.

@viniarck viniarck added bug Something isn't working tech_debt Tech debt labels Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tech_debt Tech debt
Projects
None yet
Development

No branches or pull requests

1 participant