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

Make TEManager API easier to use #122

Open
sajith opened this issue Jun 28, 2023 · 1 comment
Open

Make TEManager API easier to use #122

sajith opened this issue Jun 28, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@sajith
Copy link
Member

sajith commented Jun 28, 2023

It is hard to reason about TEManager because of the amount of state it keeps around, and because of the ways to add/update topologies to it. For example, we instantiate and use TEManager like so:

manager = TEManager(topology1, request)
manager.add_topology(topology2)
manager.add_topology(topology3)

...
manager.update_topology(updated_topology1)
...

graph = temanager.generate_graph_te()
traffic_matrix = temanager.generate_connection_te()

...

It would be nicer to use TEManager like so:

manager = TEManager()

for topology in [topology1, topology2, topology3]
    manager.add_topology(topology)

...
# Remove/hide update_topology().  Instead, add_topology() will do the update if a topology already exists.
manager.add_topology(updated_topology1)
...

graph = temanager.generate_graph_te()
traffic_matrix = temanager.generate_connection_te(request)

...

This is related to this TODO item:

# TODO: a nicer thing to do would be to keep less state around.

@YufengXin
Copy link
Collaborator

When testing SDX Controller, a new issue surfaced that was not caught in unit tests. I have not managed to capture all the details. This issue is a reminder to myself to investigate this later.

When SDX Controller starts up, it attempts to restore at least some of its previous state by loading the combined topology from the database. When we add further topologies to this, and then try to place a connection request, PCE fails to find a path.

It was the original static topologies that were re-added, and it was the sample static connection request that we tried again. It sounds like we could add a new test: keep adding the same topologies over and over again, and make sure that nothing breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants