Skip to content

Commit 1fa9b46

Browse files
authored
Fix T2 upgrade path logic (#21222)
What is the motivation for this PR? Logic error in upgrade_path test for T2 where DUTs were upgrading/downgrading seperately (see details in issue) How did you do it? Make whole chassis boot into base image at once at start of test, then upgrade in order. How did you verify/test it? Ran regression test on T1 and T2
1 parent c395fdd commit 1fa9b46

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

tests/upgrade_path/test_upgrade_path.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from tests.common.helpers.upgrade_helpers import install_sonic, upgrade_test_helper
44
from tests.common.helpers.upgrade_helpers import restore_image # noqa: F401
55
from tests.common.helpers.multi_thread_utils import SafeThreadPoolExecutor
6-
from tests.upgrade_path.utilities import cleanup_prev_images, boot_into_base_image
6+
from tests.upgrade_path.utilities import cleanup_prev_images, boot_into_base_image, boot_into_base_image_t2
77
from tests.common.fixtures.advanced_reboot import get_advanced_reboot # noqa: F401
88
from tests.common.fixtures.consistency_checker.consistency_checker import consistency_checker_provider # noqa: F401
99
from tests.common.platform.device_utils import verify_dut_health, verify_testbed_health # noqa: F401
@@ -49,9 +49,10 @@ def upgrade_path_lists(request):
4949
def setup_upgrade_test(duthost, localhost, from_image, to_image, tbinfo,
5050
upgrade_type, modify_reboot_script=None, allow_fail=False):
5151
logger.info("Test upgrade path from {} to {}".format(from_image, to_image))
52-
cleanup_prev_images(duthost)
53-
# Install base image
54-
boot_into_base_image(duthost, localhost, from_image, tbinfo)
52+
if tbinfo['topo']['type'] != 't2': # We do this all at once seperately for T2
53+
cleanup_prev_images(duthost)
54+
# Install base image
55+
boot_into_base_image(duthost, localhost, from_image, tbinfo)
5556

5657
# Install target image
5758
logger.info("Upgrading to {}".format(to_image))
@@ -113,6 +114,10 @@ def test_upgrade_path_t2(localhost, duthosts, ptfhost, upgrade_path_lists,
113114
upgrade_type = REBOOT_TYPE_COLD
114115
logger.info("Test upgrade path from {} to {}".format(from_image, to_image))
115116

117+
for duthost in duthosts:
118+
cleanup_prev_images(duthost)
119+
boot_into_base_image_t2(duthosts, localhost, from_image, tbinfo)
120+
116121
def upgrade_path_preboot_setup(dut):
117122
setup_upgrade_test(dut, localhost, from_image, to_image, tbinfo, upgrade_type)
118123

tests/upgrade_path/utilities.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,38 @@
11
import logging
22
import re
33
from tests.common.errors import RunAnsibleModuleFail
4+
from tests.common.helpers.multi_thread_utils import SafeThreadPoolExecutor
45
from tests.common.helpers.upgrade_helpers import install_sonic, reboot, check_sonic_version
56

67
logger = logging.getLogger(__name__)
78

89

910
def boot_into_base_image(duthost, localhost, base_image, tbinfo):
11+
target_version = _install_base_image(duthost, base_image, tbinfo)
12+
# Perform a cold reboot
13+
logger.info("Cold reboot the DUT to make the base image as current")
14+
# for 6100 devices, sometimes cold downgrade will not work, use soft-reboot here
15+
reboot_type = 'soft' if "s6100" in duthost.facts["platform"] else 'cold'
16+
reboot(duthost, localhost, reboot_type=reboot_type, safe_reboot=True)
17+
check_sonic_version(duthost, target_version)
18+
19+
20+
def boot_into_base_image_t2(duthosts, localhost, base_image, tbinfo):
21+
target_vers = {}
22+
with SafeThreadPoolExecutor(max_workers=8) as executor:
23+
for duthost in duthosts:
24+
future = executor.submit(_install_base_image, duthost, base_image, tbinfo)
25+
target_vers[duthost] = future.get() # Should all be the same, but following best practice
26+
27+
# Rebooting the supervisor host will reboot all T2 DUTs
28+
suphost = duthosts.supervisor_nodes[0]
29+
reboot(suphost, localhost, reboot_type='cold', safe_reboot=True)
30+
31+
for duthost in duthosts:
32+
check_sonic_version(duthost, target_vers[duthost])
33+
34+
35+
def _install_base_image(duthost, base_image, tbinfo):
1036
logger.info("Installing {}".format(base_image))
1137
try:
1238
target_version = install_sonic(duthost, base_image, tbinfo)
@@ -26,12 +52,8 @@ def boot_into_base_image(duthost, localhost, base_image, tbinfo):
2652
if duthost.shell("ls /host/old_config/minigraph.xml", module_ignore_errors=True)['rc'] == 0:
2753
duthost.shell("rm -f /host/old_config/config_db.json")
2854
duthost.shell("rm -f /host/old_config/golden_config_db.json")
29-
# Perform a cold reboot
30-
logger.info("Cold reboot the DUT to make the base image as current")
31-
# for 6100 devices, sometimes cold downgrade will not work, use soft-reboot here
32-
reboot_type = 'soft' if "s6100" in duthost.facts["platform"] else 'cold'
33-
reboot(duthost, localhost, reboot_type=reboot_type, safe_reboot=True)
34-
check_sonic_version(duthost, target_version)
55+
56+
return target_version
3557

3658

3759
def cleanup_prev_images(duthost):

0 commit comments

Comments
 (0)