From 270bd75f0d76a406ae680975756f72d5dae13f81 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 19 Nov 2024 10:32:49 -0800 Subject: [PATCH 1/3] Fix orchagent crash when setting up mock dualtor environment for t0 When running dualtor tests on t0 topo, the DUT has to enter a mocked dualtor state. Part of this setup is adding a tunnel table to CONFIG_DB, which involves a Broadcom SAI attribute that is only supported when `sai_tunnel_support=1` is set in `syncd:/etc/sai.d/config.bcm` - this attribute is not set until `apply_peer_switch_table_to_dut()` is run. Changing an unsupported Broadcom SAI attribute will cause orchagent to crash. Fix this issue by first running the setup function `apply_peer_switch_table_to_dut()` that will set `sai_tunnel_support=1`, before adding the tunnel table with `apply_tunnel_table_to_dut()`. --- 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 b883d28e6c4..cafb8456ce1 100644 --- a/tests/common/dualtor/dual_tor_mock.py +++ b/tests/common/dualtor/dual_tor_mock.py @@ -458,8 +458,8 @@ def apply_mock_dual_tor_tables(request, tbinfo): ''' if is_t0_mocked_dualtor(tbinfo): request.getfixturevalue("apply_mux_cable_table_to_dut") - request.getfixturevalue("apply_tunnel_table_to_dut") request.getfixturevalue("apply_peer_switch_table_to_dut") + request.getfixturevalue("apply_tunnel_table_to_dut") logger.info("Done applying database tables for dual ToR mock") From b14bf83b758bb8e3f361880f5b3854c675862222 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 19 Nov 2024 10:44:31 -0800 Subject: [PATCH 2/3] Fix dualtor tests overwriting /etc/sonic/config_db.json When running dualtor tests on a t0 topology, the test will overwrite `/etc/sonic/config_db.json` during the test, causing `config_reload()` at the end of the test to not restore the pre-test state of CONFIG_DB. Fix by adding a fixture to backup `/etc/sonic/config_db.json` before the test, then restore and `config reload -y` it after the test. --- tests/common/dualtor/dual_tor_mock.py | 21 +++++++++++++++++++ tests/dualtor/test_orch_stress.py | 3 +++ .../test_orchagent_active_tor_downstream.py | 3 ++- tests/dualtor/test_orchagent_mac_move.py | 3 ++- .../test_orchagent_standby_tor_downstream.py | 3 ++- .../test_standby_tor_upstream_mux_toggle.py | 4 ++-- 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/tests/common/dualtor/dual_tor_mock.py b/tests/common/dualtor/dual_tor_mock.py index cafb8456ce1..ddb7194520c 100644 --- a/tests/common/dualtor/dual_tor_mock.py +++ b/tests/common/dualtor/dual_tor_mock.py @@ -32,6 +32,7 @@ 'del_dual_tor_state_from_orchagent', 'is_t0_mocked_dualtor', 'is_mocked_dualtor', + 'restore_original_config_db', 'set_mux_state' ] @@ -483,3 +484,23 @@ def cleanup_mocked_configs(duthost, tbinfo): if is_t0_mocked_dualtor(tbinfo): logger.info("Load minigraph to reset the DUT %s", duthost.hostname) config_reload(duthost, config_source="minigraph", safe_reload=True) + + +@pytest.fixture(scope="module") +def restore_original_config_db(duthost, tbinfo): + ''' + Make a config_db.json backup at the start, and restore it at the end. + This fixture should only be executed once as the first priority. + ''' + tmp_config_db = '' + if is_t0_mocked_dualtor(tbinfo): + tmp_config_db = duthost.shell("mktemp")['stdout'] + logger.info(f"Make config_db.json backup to {tmp_config_db}") + duthost.shell(f"sudo cp /etc/sonic/config_db.json {tmp_config_db}") + + yield + + if is_t0_mocked_dualtor(tbinfo): + logger.info("Restore config_db.json from backup") + duthost.shell(f"sudo mv {tmp_config_db} /etc/sonic/config_db.json") + config_reload(duthost, safe_reload=True) diff --git a/tests/dualtor/test_orch_stress.py b/tests/dualtor/test_orch_stress.py index edfd008cbd0..d68ca6e5fa3 100644 --- a/tests/dualtor/test_orch_stress.py +++ b/tests/dualtor/test_orch_stress.py @@ -147,6 +147,7 @@ def config_crm_polling_interval(rand_selected_dut): def test_change_mux_state( + restore_original_config_db, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -217,6 +218,7 @@ def add_neighbors(dut, neighbors, interface): def test_flap_neighbor_entry_active( + restore_original_config_db, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -251,6 +253,7 @@ def test_flap_neighbor_entry_active( def test_flap_neighbor_entry_standby( + restore_original_config_db, 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 ddb6854a34a..7d2a3cbd7eb 100644 --- a/tests/dualtor/test_orchagent_active_tor_downstream.py +++ b/tests/dualtor/test_orchagent_active_tor_downstream.py @@ -27,7 +27,8 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('apply_mock_dual_tor_tables', + pytest.mark.usefixtures('restore_original_config_db', + 'apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'apply_active_state_to_orchagent', 'run_garp_service', diff --git a/tests/dualtor/test_orchagent_mac_move.py b/tests/dualtor/test_orchagent_mac_move.py index 93c18c4d14b..5e76d7e2101 100644 --- a/tests/dualtor/test_orchagent_mac_move.py +++ b/tests/dualtor/test_orchagent_mac_move.py @@ -19,7 +19,8 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('apply_mock_dual_tor_tables', + pytest.mark.usefixtures('restore_original_config_db', + 'apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'run_garp_service', 'run_icmp_responder') diff --git a/tests/dualtor/test_orchagent_standby_tor_downstream.py b/tests/dualtor/test_orchagent_standby_tor_downstream.py index b59e6f4cc1b..1341b3b60d5 100644 --- a/tests/dualtor/test_orchagent_standby_tor_downstream.py +++ b/tests/dualtor/test_orchagent_standby_tor_downstream.py @@ -29,7 +29,8 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('apply_mock_dual_tor_tables', + pytest.mark.usefixtures('restore_original_config_db', + 'apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'apply_standby_state_to_orchagent', 'run_garp_service', diff --git a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py index 99ff89f88b6..992de83e21c 100644 --- a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py +++ b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py @@ -16,8 +16,8 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', - 'run_garp_service', 'run_icmp_responder') + pytest.mark.usefixtures('restore_original_config_db', 'apply_mock_dual_tor_tables', + 'apply_mock_dual_tor_kernel_configs', 'run_garp_service', 'run_icmp_responder') ] PAUSE_TIME = 10 From a2c9c896f98d7392293e209433c940abef29783b Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 5 Dec 2024 11:56:25 -0800 Subject: [PATCH 3/3] Change to reuse old config restore and use running_golden_config source --- tests/common/dualtor/dual_tor_mock.py | 23 +------------------ tests/dualtor/test_orch_stress.py | 3 --- .../test_orchagent_active_tor_downstream.py | 3 +-- tests/dualtor/test_orchagent_mac_move.py | 3 +-- .../test_orchagent_standby_tor_downstream.py | 3 +-- .../test_standby_tor_upstream_mux_toggle.py | 4 ++-- 6 files changed, 6 insertions(+), 33 deletions(-) diff --git a/tests/common/dualtor/dual_tor_mock.py b/tests/common/dualtor/dual_tor_mock.py index ddb7194520c..02d70e4f059 100644 --- a/tests/common/dualtor/dual_tor_mock.py +++ b/tests/common/dualtor/dual_tor_mock.py @@ -32,7 +32,6 @@ 'del_dual_tor_state_from_orchagent', 'is_t0_mocked_dualtor', 'is_mocked_dualtor', - 'restore_original_config_db', 'set_mux_state' ] @@ -483,24 +482,4 @@ def cleanup_mocked_configs(duthost, tbinfo): if is_t0_mocked_dualtor(tbinfo): logger.info("Load minigraph to reset the DUT %s", duthost.hostname) - config_reload(duthost, config_source="minigraph", safe_reload=True) - - -@pytest.fixture(scope="module") -def restore_original_config_db(duthost, tbinfo): - ''' - Make a config_db.json backup at the start, and restore it at the end. - This fixture should only be executed once as the first priority. - ''' - tmp_config_db = '' - if is_t0_mocked_dualtor(tbinfo): - tmp_config_db = duthost.shell("mktemp")['stdout'] - logger.info(f"Make config_db.json backup to {tmp_config_db}") - duthost.shell(f"sudo cp /etc/sonic/config_db.json {tmp_config_db}") - - yield - - if is_t0_mocked_dualtor(tbinfo): - logger.info("Restore config_db.json from backup") - duthost.shell(f"sudo mv {tmp_config_db} /etc/sonic/config_db.json") - config_reload(duthost, safe_reload=True) + config_reload(duthost, config_source="running_golden_config", safe_reload=True) diff --git a/tests/dualtor/test_orch_stress.py b/tests/dualtor/test_orch_stress.py index d68ca6e5fa3..edfd008cbd0 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( - restore_original_config_db, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -218,7 +217,6 @@ def add_neighbors(dut, neighbors, interface): def test_flap_neighbor_entry_active( - restore_original_config_db, apply_mock_dual_tor_tables, apply_mock_dual_tor_kernel_configs, rand_selected_dut, @@ -253,7 +251,6 @@ def test_flap_neighbor_entry_active( def test_flap_neighbor_entry_standby( - restore_original_config_db, 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 7d2a3cbd7eb..ddb6854a34a 100644 --- a/tests/dualtor/test_orchagent_active_tor_downstream.py +++ b/tests/dualtor/test_orchagent_active_tor_downstream.py @@ -27,8 +27,7 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('restore_original_config_db', - 'apply_mock_dual_tor_tables', + pytest.mark.usefixtures('apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'apply_active_state_to_orchagent', 'run_garp_service', diff --git a/tests/dualtor/test_orchagent_mac_move.py b/tests/dualtor/test_orchagent_mac_move.py index 5e76d7e2101..93c18c4d14b 100644 --- a/tests/dualtor/test_orchagent_mac_move.py +++ b/tests/dualtor/test_orchagent_mac_move.py @@ -19,8 +19,7 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('restore_original_config_db', - 'apply_mock_dual_tor_tables', + pytest.mark.usefixtures('apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'run_garp_service', 'run_icmp_responder') diff --git a/tests/dualtor/test_orchagent_standby_tor_downstream.py b/tests/dualtor/test_orchagent_standby_tor_downstream.py index 1341b3b60d5..b59e6f4cc1b 100644 --- a/tests/dualtor/test_orchagent_standby_tor_downstream.py +++ b/tests/dualtor/test_orchagent_standby_tor_downstream.py @@ -29,8 +29,7 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('restore_original_config_db', - 'apply_mock_dual_tor_tables', + pytest.mark.usefixtures('apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', 'apply_standby_state_to_orchagent', 'run_garp_service', diff --git a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py index 992de83e21c..99ff89f88b6 100644 --- a/tests/dualtor/test_standby_tor_upstream_mux_toggle.py +++ b/tests/dualtor/test_standby_tor_upstream_mux_toggle.py @@ -16,8 +16,8 @@ pytestmark = [ pytest.mark.topology('t0'), - pytest.mark.usefixtures('restore_original_config_db', 'apply_mock_dual_tor_tables', - 'apply_mock_dual_tor_kernel_configs', 'run_garp_service', 'run_icmp_responder') + pytest.mark.usefixtures('apply_mock_dual_tor_tables', 'apply_mock_dual_tor_kernel_configs', + 'run_garp_service', 'run_icmp_responder') ] PAUSE_TIME = 10