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

Added a test case for fast reboot from other vendor NOS to SONiC #6348

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ansible/roles/test/files/ptftests/advanced-reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1327,8 +1327,13 @@ def reboot_dut(self):
self.sender_thr.start()

self.log("Rebooting remote side")
if self.reboot_type != 'service-warm-restart':
if self.reboot_type != 'service-warm-restart' and self.test_params['other_vendor_flag'] is False:
stdout, stderr, return_code = self.dut_connection.execCommand("sudo " + self.reboot_type, timeout=30)

elif self.test_params['other_vendor_flag'] is True:
ignore_db_integrity_check = " -d"
stdout, stderr, return_code = self.dut_connection.execCommand("sudo " + self.reboot_type + ignore_db_integrity_check, timeout=30)

else:
self.restart_service()
return
Expand Down
13 changes: 9 additions & 4 deletions tests/common/fixtures/advanced_reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ def __init__(self, request, duthost, ptfhost, localhost, tbinfo, creds, **kwargs
self.moduleIgnoreErrors = kwargs["allow_fail"] if "allow_fail" in kwargs else False
self.allowMacJump = kwargs["allow_mac_jumping"] if "allow_mac_jumping" in kwargs else False
self.advanceboot_loganalyzer = kwargs["advanceboot_loganalyzer"] if "advanceboot_loganalyzer" in kwargs else None
self.__dict__.update(kwargs)
self.other_vendor_nos = kwargs['other_vendor_nos'] if 'other_vendor_nos' in kwargs else False
self.__dict__.update(kwargs)
self.__extractTestParam()
self.rebootData = {}
self.hostMaxLen = 0
Expand Down Expand Up @@ -116,7 +117,7 @@ def __extractTestParam(self):
if self.rebootLimit is None:
if self.kvmTest:
self.rebootLimit = 200 # Default reboot limit for kvm
elif 'warm-reboot' in self.rebootType:
elif 'warm-reboot' in self.rebootType:
self.rebootLimit = 0
else:
self.rebootLimit = 30 # Default reboot limit for physical devices
Expand Down Expand Up @@ -605,6 +606,7 @@ def __runPtfRunner(self, rebootOper=None):
"dut_hostname" : self.rebootData['dut_hostname'],
"reboot_limit_in_seconds" : self.rebootLimit,
"reboot_type" : self.rebootType,
"other_vendor_flag" : self.other_vendor_nos,
"portchannel_ports_file" : self.rebootData['portchannel_interfaces_file'],
"vlan_ports_file" : self.rebootData['vlan_interfaces_file'],
"ports_file" : self.rebootData['ports_file'],
Expand Down Expand Up @@ -644,8 +646,11 @@ def __runPtfRunner(self, rebootOper=None):

self.__updateAndRestartArpResponder(rebootOper)


logger.info('Run advanced-reboot ReloadTest on the PTF host. TestCase: {}, sub-case: {}'.format(\
if rebootOper is None and self.other_vendor_nos is True:
logger.info('Run advanced-reboot ReloadTest on the PTF host. TestCase: {}, sub-case:'
' Reboot from other vendor nos'.format(self.request.node.name))
else:
logger.info('Run advanced-reboot ReloadTest on the PTF host. TestCase: {}, sub-case: {}'.format(\
self.request.node.name, str(rebootOper)))
result = ptf_runner(
self.ptfhost,
Expand Down
34 changes: 33 additions & 1 deletion tests/platform_tests/test_advanced_reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
pytest.mark.disable_loganalyzer,
pytest.mark.topology('t0')
]
logger = logging.getLogger()

def pytest_generate_tests(metafunc):
input_sad_cases = metafunc.config.getoption("sad_case_list")
Expand All @@ -32,7 +33,7 @@ def pytest_generate_tests(metafunc):
def test_fast_reboot(request, get_advanced_reboot, verify_dut_health,
advanceboot_loganalyzer, capture_interface_counters):
'''
Fast reboot test case is run using advacned reboot test fixture
Fast reboot test case is run using advanced reboot test fixture

@param request: Spytest commandline argument
@param get_advanced_reboot: advanced reboot test fixture
Expand All @@ -42,6 +43,22 @@ def test_fast_reboot(request, get_advanced_reboot, verify_dut_health,
advancedReboot.runRebootTestcase()


@pytest.mark.usefixtures('get_advanced_reboot')
def test_fast_reboot_from_other_vendor(duthosts, rand_one_dut_hostname, request, get_advanced_reboot, verify_dut_health,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be better to add this test in a new script. test_fastboot_vendor_to_sonic.py?

Copy link
Contributor Author

@AharonMalkin AharonMalkin Oct 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script test_advanced_reboot initiates various types of reboot, such as warm reboot, or fast reboot, with different parameters such as test_warm_reboot_mac_jump or test_cancelled_fast_reboot.
The new other vendor reboot test case is another parameter of advanced reboot and isn't different from other reboot types. In addition, all reboot types are using the infrastructure and fixtures of the advanced reboot script. Creating a new script will require duplication of fixtures and infrastructure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I am missing something, creating a new test script will not require duplication of fixtures or infra. Any new test script will only have to import the fixtures, just how this script is doing.
None of the fixtures used by tests/platform_tests/test_advanced_reboot.py are actually defined in this file. Same will be the case for test_fastboot_vendor_to_sonic.py.

I am thinking that in future we will add more cases to non-SONiC to SONiC upgrade/reboot path. With tweaking parameters and also by doing a real (not just mock) upgrade. This deems a new category/script as it grows.

advanceboot_loganalyzer, capture_interface_counters):
'''
Fast reboot test from other vendor case is run using advanced reboot test fixture

@param request: Spytest commandline argument
@param get_advanced_reboot: advanced reboot test fixture
'''
duthost = duthosts[rand_one_dut_hostname]
advancedReboot = get_advanced_reboot(rebootType='fast-reboot', other_vendor_nos = True,\
advanceboot_loganalyzer=advanceboot_loganalyzer)
# Before rebooting, we will flush all unnecessary databases, to mimic reboot from other vendor.
flush_dbs(duthost)
advancedReboot.runRebootTestcase()

@pytest.mark.device_type('vs')
def test_warm_reboot(request, get_advanced_reboot, verify_dut_health,
advanceboot_loganalyzer, capture_interface_counters):
Expand Down Expand Up @@ -129,3 +146,18 @@ def test_cancelled_warm_reboot(request, add_fail_step_to_reboot, verify_dut_heal
add_fail_step_to_reboot('warm-reboot')
advancedReboot = get_advanced_reboot(rebootType='warm-reboot', allow_fail=True)
advancedReboot.runRebootTestcase()

def flush_dbs(duthost):
"""
This Function will flush all unnecessary databases, to mimic reboot from other vendor
"""
logger.info('Flushing databases from switch')
db_dic = { 0: 'Application DB',
1: 'ASIC DB',
2: 'Counters DB',
5: 'Flex Counters DB',
6: 'State DB'
}
for db in db_dic.keys():
duthost.shell('redis-cli -n {} flushdb'.format(db))