From 48c18cbcc9bac81306d17b8ff6ca0a82a7ae0614 Mon Sep 17 00:00:00 2001 From: xwjiang2021 Date: Tue, 8 Nov 2022 03:36:57 +0000 Subject: [PATCH 1/5] Use conditional mark to skip testcase instead of required mocked dualtor What is the motivation for this PR? Currently, we use required_mocked_dualtor to skip testcases in real dualtor under `tests/dualtor/`, but it's not efficient enough, and will cause show mux status error after toggle. How did you do it? Use conditional mark to skip testcase instead of required mocked dualtor How did you verify/test it? Run tests Any platform specific information? Supported testbed topology if it's a new test case? --- tests/common/dualtor/dual_tor_mock.py | 7 --- .../tests_mark_conditions.yaml | 52 +++++++++++++++++++ tests/dualtor/test_orch_stress.py | 3 -- .../test_orchagent_active_tor_downstream.py | 6 +-- tests/dualtor/test_orchagent_mac_move.py | 1 - .../test_orchagent_standby_tor_downstream.py | 15 +++--- .../test_standby_tor_upstream_mux_toggle.py | 4 +- 7 files changed, 62 insertions(+), 26 deletions(-) diff --git a/tests/common/dualtor/dual_tor_mock.py b/tests/common/dualtor/dual_tor_mock.py index c34f0a8c49c..1c27098e891 100644 --- a/tests/common/dualtor/dual_tor_mock.py +++ b/tests/common/dualtor/dual_tor_mock.py @@ -12,7 +12,6 @@ from tests.common.platform.processes_utils import wait_critical_processes __all__ = [ - 'require_mocked_dualtor', 'apply_active_state_to_orchagent', 'apply_dual_tor_neigh_entries', 'apply_dual_tor_peer_switch_route', @@ -139,12 +138,6 @@ def is_mocked_dualtor(tbinfo): return 'dualtor' not in tbinfo['topo']['name'] -@pytest.fixture -def require_mocked_dualtor(tbinfo): - pytest_require(is_t0_mocked_dualtor(tbinfo), "This testcase is designed for " - "single tor testbed with mock dualtor config. Skip this testcase on real dualtor testbed") - - def set_mux_state(dut, tbinfo, state, itfs, toggle_all_simulator_ports): if is_mocked_dualtor(tbinfo): set_dual_tor_state_to_orchagent(dut, state, itfs) diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml index 0935bedd388..0f36f4f6dc7 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml @@ -209,12 +209,64 @@ drop_packets: ####################################### ##### dualtor ##### ####################################### +dualtor/test_orch_stress.py: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_active_tor_downstream.py: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_mac_move.py: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_t1_link_recovered: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_bgp_recovered: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + +dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_remove_neighbor_downstream_standby: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + dualtor/test_orchagent_standby_tor_downstream.py::test_downstream_standby_mux_toggle_active: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" xfail: reason: "Image issue on Boradcom platforms, but not consistently failing" conditions: - "asic_type in ['broadcom']" +dualtor/test_standby_tor_upstream_mux_toggle.py: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" + ####################################### ##### dut_console ##### ####################################### diff --git a/tests/dualtor/test_orch_stress.py b/tests/dualtor/test_orch_stress.py index 164bb1a19c9..4e9eea75cd5 100644 --- a/tests/dualtor/test_orch_stress.py +++ b/tests/dualtor/test_orch_stress.py @@ -147,7 +147,6 @@ def config_crm_polling_interval(rand_selected_dut): def test_change_mux_state( - require_mocked_dualtor, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -215,7 +214,6 @@ def add_neighbors(dut, neighbors, interface): def test_flap_neighbor_entry_active( - require_mocked_dualtor, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -249,7 +247,6 @@ def test_flap_neighbor_entry_active( def test_flap_neighbor_entry_standby( - require_mocked_dualtor, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, diff --git a/tests/dualtor/test_orchagent_active_tor_downstream.py b/tests/dualtor/test_orchagent_active_tor_downstream.py index 9ca9576b8b4..26a52f6d506 100644 --- a/tests/dualtor/test_orchagent_active_tor_downstream.py +++ b/tests/dualtor/test_orchagent_active_tor_downstream.py @@ -65,8 +65,7 @@ def neighbor_reachable(duthost, neighbor_ip): def test_active_tor_remove_neighbor_downstream_active( conn_graph_facts, ptfadapter, ptfhost, testbed_setup, - rand_selected_dut, tbinfo, - require_mocked_dualtor, set_crm_polling_interval, + rand_selected_dut, tbinfo, set_crm_polling_interval, tunnel_traffic_monitor, vmhost ): """ @@ -139,8 +138,7 @@ def remove_neighbor(ptfhost, duthost, server_ip, ip_version, neighbor_details): def test_downstream_ecmp_nexthops( ptfadapter, rand_selected_dut, tbinfo, - require_mocked_dualtor, toggle_all_simulator_ports, - tor_mux_intfs, ip_version + toggle_all_simulator_ports, tor_mux_intfs, ip_version ): nexthops_count = 4 set_mux_state(rand_selected_dut, tbinfo, 'active', tor_mux_intfs, toggle_all_simulator_ports) diff --git a/tests/dualtor/test_orchagent_mac_move.py b/tests/dualtor/test_orchagent_mac_move.py index 1fb6cf8ac5e..692f69616d5 100644 --- a/tests/dualtor/test_orchagent_mac_move.py +++ b/tests/dualtor/test_orchagent_mac_move.py @@ -81,7 +81,6 @@ def enable_garp(duthost): def test_mac_move( - require_mocked_dualtor, announce_new_neighbor, apply_active_state_to_orchagent, conn_graph_facts, ptfadapter, ptfhost, rand_selected_dut, set_crm_polling_interval, diff --git a/tests/dualtor/test_orchagent_standby_tor_downstream.py b/tests/dualtor/test_orchagent_standby_tor_downstream.py index 06fc1e93483..d5fa5203485 100644 --- a/tests/dualtor/test_orchagent_standby_tor_downstream.py +++ b/tests/dualtor/test_orchagent_standby_tor_downstream.py @@ -128,7 +128,7 @@ def shutdown_one_bgp_session(rand_selected_dut): startup_bgp_session(rand_selected_dut, bgp_shutdown) -def test_standby_tor_downstream(rand_selected_dut, require_mocked_dualtor, get_testbed_params): +def test_standby_tor_downstream(rand_selected_dut, get_testbed_params): """ Verify tunnel traffic to active ToR is distributed equally across nexthops, and no traffic is forwarded to server from standby ToR @@ -138,8 +138,7 @@ def test_standby_tor_downstream(rand_selected_dut, require_mocked_dualtor, get_t def test_standby_tor_downstream_t1_link_recovered( - rand_selected_dut, require_mocked_dualtor, - verify_crm_nexthop_counter_not_increased, tbinfo, get_testbed_params + rand_selected_dut, verify_crm_nexthop_counter_not_increased, tbinfo, get_testbed_params ): """ Verify traffic is distributed evenly after t1 link is recovered; @@ -167,7 +166,7 @@ def test_standby_tor_downstream_t1_link_recovered( def test_standby_tor_downstream_bgp_recovered( - rand_selected_dut, require_mocked_dualtor, verify_crm_nexthop_counter_not_increased, + rand_selected_dut, verify_crm_nexthop_counter_not_increased, get_testbed_params, tbinfo ): """ @@ -253,8 +252,8 @@ def test_standby_tor_downstream_loopback_route_readded( def test_standby_tor_remove_neighbor_downstream_standby( conn_graph_facts, ptfadapter, ptfhost, rand_selected_dut, rand_unselected_dut, tbinfo, - require_mocked_dualtor, set_crm_polling_interval, - tunnel_traffic_monitor, vmhost, get_testbed_params, # noqa: F811 + set_crm_polling_interval, tunnel_traffic_monitor, # noqa: F811 + vmhost, get_testbed_params, ip_version ): """ @@ -310,8 +309,8 @@ def stop_neighbor_advertiser(ptfhost, ip_version): def test_downstream_standby_mux_toggle_active( conn_graph_facts, ptfadapter, ptfhost, rand_selected_dut, rand_unselected_dut, tbinfo, - require_mocked_dualtor, tunnel_traffic_monitor, # noqa: F811 - vmhost, toggle_all_simulator_ports, tor_mux_intfs, # noqa: F811 + tunnel_traffic_monitor, vmhost, # noqa: F811 + toggle_all_simulator_ports, tor_mux_intfs, # noqa: F811 ip_version, get_testbed_params ): # set rand_selected_dut as standby and rand_unselected_dut to active tor diff --git a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py index 10b0f67c9b6..ae94c62dab3 100644 --- a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py +++ b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py @@ -31,7 +31,7 @@ def test_cleanup(rand_selected_dut): def test_standby_tor_upstream_mux_toggle( rand_selected_dut, tbinfo, ptfadapter, rand_selected_interface, - require_mocked_dualtor, toggle_all_simulator_ports, set_crm_polling_interval): + toggle_all_simulator_ports, set_crm_polling_interval): itfs, ip = rand_selected_interface PKT_NUM = 100 # Step 1. Set mux state to standby and verify traffic is dropped by ACL rule and drop counters incremented @@ -77,5 +77,3 @@ def test_standby_tor_upstream_mux_toggle( crm_facts1 = rand_selected_dut.get_crm_facts() unmatched_crm_facts = compare_crm_facts(crm_facts0, crm_facts1) pt_assert(len(unmatched_crm_facts)==0, 'Unmatched CRM facts: {}'.format(json.dumps(unmatched_crm_facts, indent=4))) - - From 43d882e60972480f52af28a2244e143ea9ddb375 Mon Sep 17 00:00:00 2001 From: xwjiang2021 Date: Tue, 8 Nov 2022 05:57:24 +0000 Subject: [PATCH 2/5] Fix LGTM alert --- tests/common/dualtor/dual_tor_mock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/dualtor/dual_tor_mock.py b/tests/common/dualtor/dual_tor_mock.py index 1c27098e891..463482991a9 100644 --- a/tests/common/dualtor/dual_tor_mock.py +++ b/tests/common/dualtor/dual_tor_mock.py @@ -8,7 +8,7 @@ ip_address, IPv4Address from tests.common import config_reload from tests.common.dualtor.dual_tor_utils import tor_mux_intfs -from tests.common.helpers.assertions import pytest_require, pytest_assert +from tests.common.helpers.assertions import pytest_assert from tests.common.platform.processes_utils import wait_critical_processes __all__ = [ From db95ec9de35d73e0f1d2b346f25fdbb4f6b97e62 Mon Sep 17 00:00:00 2001 From: xwjiang2021 Date: Tue, 8 Nov 2022 06:54:03 +0000 Subject: [PATCH 3/5] Remove pt_require to let cases run on t0 --- tests/dualtor/test_orchagent_standby_tor_downstream.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/dualtor/test_orchagent_standby_tor_downstream.py b/tests/dualtor/test_orchagent_standby_tor_downstream.py index d5fa5203485..866c01f811b 100644 --- a/tests/dualtor/test_orchagent_standby_tor_downstream.py +++ b/tests/dualtor/test_orchagent_standby_tor_downstream.py @@ -19,7 +19,6 @@ from tests.common.fixtures.ptfhost_utils import change_mac_addresses # noqa: F401 from tests.common.fixtures.ptfhost_utils import run_garp_service # noqa: F401 from tests.common.fixtures.ptfhost_utils import run_icmp_responder # noqa: F401 # lgtm[py/unused-import] -from tests.common.helpers.assertions import pytest_require as pt_require from tests.common.helpers.assertions import pytest_assert as pt_assert from tests.common.dualtor.tunnel_traffic_utils import tunnel_traffic_monitor # noqa: F401 from tests.common.dualtor.server_traffic_utils import ServerTrafficMonitor @@ -174,8 +173,6 @@ def test_standby_tor_downstream_bgp_recovered( Verify traffic is distributed evenly after BGP session is recovered; Verify CRM that no new nexthop created """ - # require real dualtor, because for mocked testbed, the route to standby is mocked. - pt_require('dualtor' in tbinfo['topo']['name'], "Only run on dualtor testbed") PAUSE_TIME = 30 down_bgp = shutdown_random_one_bgp_session(rand_selected_dut) @@ -224,7 +221,6 @@ def test_standby_tor_downstream_loopback_route_readded( """ Verify traffic is equally distributed via loopback route """ - pt_require('dualtor' in tbinfo['topo']['name'], "Only run on dualtor testbed") params = get_testbed_params() active_tor_loopback0 = params['active_tor_ip'] From cb5ecee425ed69b9bf0232ba2429cdebdeee233f Mon Sep 17 00:00:00 2001 From: xwjiang2021 Date: Tue, 8 Nov 2022 09:23:04 +0000 Subject: [PATCH 4/5] test_standby_tor_downstream_loopback_route_readded should only run on t0 --- .../plugins/conditional_mark/tests_mark_conditions.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml index 0f36f4f6dc7..44b35998527 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml @@ -245,6 +245,12 @@ dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_bg conditions: - "(topo_type not in ['t0']) or ('dualtor' in topo_name)" +dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_loopback_route_readded: + skip: + reason: "This testcase is designed for single tor testbed with mock dualtor config." + conditions: + - "(topo_type not in ['t0'])" + dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_remove_neighbor_downstream_standby: skip: reason: "This testcase is designed for single tor testbed with mock dualtor config." From a039248ea8b1439c873108dae4b06ac0e06234bf Mon Sep 17 00:00:00 2001 From: xwjiang2021 Date: Wed, 9 Nov 2022 14:27:21 +0000 Subject: [PATCH 5/5] Improve skip reason --- .../common/plugins/conditional_mark/tests_mark_conditions.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml index 44b35998527..7488db0486b 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml @@ -247,7 +247,7 @@ dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_bg dualtor/test_orchagent_standby_tor_downstream.py::test_standby_tor_downstream_loopback_route_readded: skip: - reason: "This testcase is designed for single tor testbed with mock dualtor config." + reason: "This testcase is designed for single tor testbed with mock dualtor config and dualtor." conditions: - "(topo_type not in ['t0'])"