-
Notifications
You must be signed in to change notification settings - Fork 2
Save the full solution path of a request connection to DB #476
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
Save the full solution path of a request connection to DB #476
Conversation
Pull Request Test Coverage Report for Build 17332100181Details
💛 - Coveralls |
|
@congwang09 @italovalcy I finished the implementation and unittest. Somehow my end-to-end tests in a Fabric node produces many failures in other unrelated tests. I'll try again later in a clean environment. If you happen to run end-to-end tests in your environment, please give this PR a try and let me know your results. Thx |
Merge branch 'main' into 456-l2vpn-status-fails-to-change-after-an-intra-domain-link-is-down
Hi @YufengXin yes, the e2e tests failed when I try this branch. |
@congwang09 Thx. Better than my previous test using the 'e2e_test_use_cases', where the even the 'test_01_topology.py' failed. Which branch of sdx-end-to-end-tests did you use, the main or e2e_test_use_cases? I'll go ever the 'F' (Failed?) and 'E' (Error?) tests one by one. |
|
My test results on the main branch of end-to-end test: tests/test_01_topology.py ..... [ 5%] =========================== short test summary info ============================ |
|
Hi @YufengXin after the PCE update, all the tests have passed. |
|
Thanks, @congwang09 is ready for review/merge, which will resolve multiple issues related to port and link (inter- and intra-) failure handling and status reporting. |
italovalcy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @YufengXin please fix the regression errors introduced by this PR as showed below:
============================= test session starts ==============================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0+repack
rootdir: /sdx-end-to-end-tests
plugins: unordered-0.7.0
collected 92 items
tests/test_01_topology.py ..... [ 5%]
tests/test_05_l2vpn.py ........ [ 14%]
tests/test_06_l2vpn_return_codes.py .................................. [ 51%]
tests/test_07_l2vpn_return_codes.py ....................... [ 76%]
tests/test_08_l2vpn_return_codes.py ...... [ 82%]
tests/test_20_use_case_topology.py xx.xFxxF.xEEE [ 96%]
tests/test_99_topology_big_changes.py ... [100%]
==================================== ERRORS ====================================
_ ERROR at setup of TestE2ETopologyUseCases.test_081_link_missing_with_alternate_path _
cls = <class 'test_20_use_case_topology.TestE2ETopologyUseCases'>
@classmethod
def setup_method(cls):
"""Reset network configuration before each test."""
api_url = SDX_CONTROLLER + '/l2vpn/1.0'
response = requests.get(api_url)
assert response.status_code == 200, response.text
response_json = response.json()
for l2vpn in response_json:
response = requests.delete(api_url+f'/{l2vpn}')
> assert response.status_code == 200, response.text
E AssertionError: "Failed, reason: Unknown connection request (ID: 63fd714b-147d-454c-b8b0-f9335868af64)"
E
E assert 500 == 200
E + where 500 = <Response [500]>.status_code
tests/test_20_use_case_topology.py:63: AssertionError
_____ ERROR at setup of TestE2ETopologyUseCases.test_100_vlan_range_change _____
cls = <class 'test_20_use_case_topology.TestE2ETopologyUseCases'>
@classmethod
def setup_method(cls):
"""Reset network configuration before each test."""
api_url = SDX_CONTROLLER + '/l2vpn/1.0'
response = requests.get(api_url)
assert response.status_code == 200, response.text
response_json = response.json()
for l2vpn in response_json:
response = requests.delete(api_url+f'/{l2vpn}')
> assert response.status_code == 200, response.text
E AssertionError: "Failed, reason: Unknown connection request (ID: 63fd714b-147d-454c-b8b0-f9335868af64)"
E
E assert 500 == 200
E + where 500 = <Response [500]>.status_code
tests/test_20_use_case_topology.py:63: AssertionError
_ ERROR at setup of TestE2ETopologyUseCases.test_110_service_no_longer_supported _
cls = <class 'test_20_use_case_topology.TestE2ETopologyUseCases'>
@classmethod
def setup_method(cls):
"""Reset network configuration before each test."""
api_url = SDX_CONTROLLER + '/l2vpn/1.0'
response = requests.get(api_url)
assert response.status_code == 200, response.text
response_json = response.json()
for l2vpn in response_json:
response = requests.delete(api_url+f'/{l2vpn}')
> assert response.status_code == 200, response.text
E AssertionError: "Failed, reason: Unknown connection request (ID: 63fd714b-147d-454c-b8b0-f9335868af64)"
E
E assert 500 == 200
E + where 500 = <Response [500]>.status_code
tests/test_20_use_case_topology.py:63: AssertionError
=================================== FAILURES ===================================
________________ TestE2ETopologyUseCases.test_030_uni_port_down ________________
self = <test_20_use_case_topology.TestE2ETopologyUseCases object at 0x7fe8603f95d0>
def test_030_uni_port_down(self):
"""
Use case 3: OXPO sends a topology update with a Port Down and that port is an UNI for some L2VPN.
"""
l2vpn_data = self.create_new_l2vpn(vlan='300')
l2vpn_id = l2vpn_data['id']
# Get UNI ports
port = 'urn:sdx:port:tenet.ac.za:Tenet01:50'
response = requests.get(API_URL_TOPO)
data = response.json()
ports = {port["id"] for node in data["nodes"] for port in node["ports"] if port['nni'] == ''}
assert port in ports
Tenet01 = self.net.net.get('Tenet01')
Tenet01.intf('Tenet01-eth50').ifconfig('down')
time.sleep(15)
data = requests.get(API_URL).json()
> assert data[l2vpn_id]["status"] == "down"
E AssertionError: assert 'up' == 'down'
E - down
E + up
tests/test_20_use_case_topology.py:324: AssertionError
_________________ TestE2ETopologyUseCases.test_060_port_up_uni _________________
self = <test_20_use_case_topology.TestE2ETopologyUseCases object at 0x7fe8603fb390>
def test_060_port_up_uni(self):
"""
Use Case 6: OXPO sends a topology update with a Port UP and that port is UNI for some L2VPNs.
Expected behavior:
SDX Controller: update the statuses involved. Use Case 3 is explicit saying the configs should not be removed in case of a Port Down
which means the data plane config is already there.
"""
l2vpn_data = self.create_new_l2vpn(vlan='600')
l2vpn_id = l2vpn_data['id']
# Simulate UNI port going down
ampath_node = self.net.net.get('Ampath1')
ampath_node.intf('Ampath1-eth50').ifconfig('down')
time.sleep(15)
# Verify L2VPN status is down
response = requests.get(API_URL)
assert response.status_code == 200, response.text
l2vpn_response = response.json()
assert l2vpn_id in l2vpn_response
> assert l2vpn_response.get(l2vpn_id).get("status") == "down", str(l2vpn_response)
E AssertionError: {'91d7037c-3f2b-438b-9c64-140e6cfb2ec6': {'archived_date': 0, 'current_path': [{'port_id': 'urn:sdx:port:ampath.net:Ampath1:50', 'vlan': '600'}, {'port_id': 'urn:sdx:port:ampath.net:Ampath1:40', 'vlan': '1'}, {'port_id': 'urn:sdx:port:sax.net:Sax01:40', 'vlan': '1'}, {'port_id': 'urn:sdx:port:sax.net:Sax01:41', 'vlan': '1'}, {'port_id': 'urn:sdx:port:tenet.ac.za:Tenet01:41', 'vlan': '1'}, {'port_id': 'urn:sdx:port:tenet.ac.za:Tenet01:50', 'vlan': '600'}], 'description': None, 'endpoints': [{'port_id': 'urn:sdx:port:ampath.net:Ampath1:50', 'vlan': '600'}, {'port_id': 'urn:sdx:port:tenet.ac.za:Tenet01:50', 'vlan': '600'}], 'name': 'Test L2VPN', 'oxp_response': {'ampath.net': [200, {'circuit_id': 'f86e6cea00c043', 'deployed': True}], 'sax.net': [200, {'circuit_id': '4b981bd301804d', 'deployed': True}], 'tenet.ac.za': [200, {'circuit_id': '7eebdbc670b14c', 'deployed': True}]}, 'service_id': '91d7037c-3f2b-438b-9c64-140e6cfb2ec6', 'status': 'up'}}
E assert 'up' == 'down'
E - down
E + up
tests/test_20_use_case_topology.py:469: AssertionError
------------------------------- start/stop times -------------------------------
tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_030_uni_port_down: 2025-08-06,19:48:22.789116 - 2025-08-06,19:48:46.040081
tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_060_port_up_uni: 2025-08-06,19:50:03.276855 - 2025-08-06,19:50:26.508738
=========================== short test summary info ============================
FAILED tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_030_uni_port_down
FAILED tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_060_port_up_uni
ERROR tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_081_link_missing_with_alternate_path
ERROR tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_100_vlan_range_change
ERROR tests/test_20_use_case_topology.py::TestE2ETopologyUseCases::test_110_service_no_longer_supported
======== 2 failed, 81 passed, 6 xfailed, 3 errors in 1565.49s (0:26:05) ========
|
@italovalcy Thanks for the review. Were you using the main branch or another branch of end-to-end-test? LMK so I can fix the tests quickly. @italovalcy I see @gretelliz recently made new merges in end-to-end-test after my test in this PR, particularly in the test_20_use_case_topology.py that caused these new errors. @gretelliz could you pls confirm that these new test cases were passed with the main branch of sdx-controller, or not? |
Merge remote-tracking branch 'origin/main' into 456-l2vpn-status-fails-to-change-after-an-intra-domain-link-is-down
5355791 to
45e43a4
Compare
|
@italovalcy @gretelliz I decided to complete this PR by fixing the last bit on returning connections status. While this PR was originally for processing the "intra-domain" link failures, the changes are mostly about streamlining the workflow in identifying failed elements and process. The accompanying PR in PCE is here: Following is the end-to-end-test (main branch) results: Executing cd ~/awsdx/sdx-end-to-end-tests && ./wait-mininet-ready.sh tests/test_01_topology.py ..... [ 5%] ------------------------------- start/stop times ------------------------------- |
|
@YufengXin thanks for providing the update on this PR! End-to-end tests seems fine, however using your branch seems to raise exception when restarting the controller, an issue that was already fixed on datamodel -> pce -> sdx-controller. Can you please rebase your branch with latest changes from main on those repos? This PR here for instance is showing conflicts on pyproject.toml and also connection_handler.py Once you rebase your changes and update with main branches, please let me know and I will redo the tests. Here is the error I'm facing after restart the controller (once again: this issue was already fixed on datamodel, we just need you to rebase your code with main branch): |
|
@italovalcy Thanks a lot for the test. I merged main along the chain: only a minor conflict in that port in DB function (add a few logs), which I merged the one in main. |
|
Hi Yufeng,
Tested again with your updates and I'm now getting errors on the end-to-end tests: Full output attached. |
|
@italovalcy Thx, looks like my change to that function is necessary. BTW, could you just confirm that this doesn't cause the controller restart issue anymore? I'll rerun/fix the end-to-end tests. |
…DB entry format need to pair with the other two functions:handle_uni_ports_down_to_up() and handle_uni_ports_up_to_down()
|
@italovalcy I reversed the merge, "reverse the change to connection_handler.py._process_port(), since the DB entry format needs to match that in the other two pairing functions:handle_uni_ports_down_to_up() and handle_uni_ports_up_to_down()" ============================= test session starts ============================== tests/test_01_topology.py ..... [ 5%] ------------------------------- start/stop times ------------------------------- |
|
I've executed a new review on the end-to-end tests and results are going well so far: |
|
@YufengXin I believe this PR will also fix #485 |
Resolves: #485