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

AharonMalkin · 2022-09-15T08:17:47Z

Description of PR

Summary:
A new test case was added to fast reboot script.
This test case mimics a fast reboot from other vendor NOS to SONiC, by flushing all DBs that are part of the boot process, so the boot will be initiated with clean DBs as if it was booting from other NOSes.

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911
202012

Approach

What is the motivation for this PR?

The current flow doesn't support booting from other vendor nos, and by that, we cannot measure and determine the performance of the fast-reboot process in that specific sub-case.
The new test case enables us to mimic the flow of booting from other nos and by verifying the performance of this kind of reboot.

How did you do it?

I added a new flag of "other_vendor_nos" which was added to the original fast_reboot test case in False mode and in the new test case in True mode, and added this flag to the AdvancedReboot class.
This flag will initiate pre-boot function, that flushes all relevant DBs. By flushing them, the fast reboot flow is initiated with clean DBs, as if the system boots from other vendor nos.

How did you verify/test it?

I ran full regression on community testbeds,
ran the test on the regular fast boot mode and on the new mode and verified the reboot process succeed.

Any platform specific information?

No

Supported testbed topology if it's a new test case?

t0

Details: A new test case was added to fast reboot script. This test case mimics a fast reboot from other vendor nos to SONiC, by flushing all DBs that are part of the boot process, so the boot will be initiated with clean DBs as if it was booting from other nos.

lgtm-com · 2022-09-15T08:50:12Z

This pull request introduces 1 alert when merging 871a82c into bdd7663 - view on LGTM.com

new alerts:

1 for Use of the return value of a procedure

Details: Added the other_vendor_nos parameter as False by default to the AdvancedReboot class, and updated the reboot test cases correspondingly.

lgtm-com · 2022-09-20T13:42:53Z

This pull request introduces 1 alert when merging 36e72ff into 9074e57 - view on LGTM.com

new alerts:

1 for Use of the return value of a procedure

lgtm-com · 2022-09-28T14:20:34Z

This pull request introduces 1 alert when merging 6aba89e851e4b0934fe5a3f165c46ce8c174b74c into d6d8d9c - view on LGTM.com

new alerts:

1 for Use of the return value of a procedure

vaibhavhd · 2022-10-03T06:16:08Z

tests/common/fixtures/advanced_reboot.py

@@ -116,7 +117,9 @@ 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:
+            if self.other_vendor_nos:
+                self.rebootLimit = 50 # Threshold set for reboot from other vendor


Why is this 50s expectation set? I don't think this is correct. Expectation for fastboot from vendor NOS is still <30s.

The flushing of the DBs is increasing the downtime in a few seconds, we would like to push it with the current downtime.

tests/platform_tests/test_advanced_reboot.py

vaibhavhd · 2022-10-03T06:19:17Z

tests/platform_tests/test_advanced_reboot.py

@@ -42,6 +43,21 @@ 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,


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

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.

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.

liat-grozovik · 2022-10-03T06:28:22Z

Imo It takes much more time when you delete the configuration Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Vaibhav Hemant Dixit ***@***.***> Sent: Monday, October 3, 2022 9:21:06 AM To: sonic-net/sonic-mgmt ***@***.***> Cc: Liat Grozovik ***@***.***>; Review requested ***@***.***> Subject: Re: [sonic-net/sonic-mgmt] Added a test case for fast reboot from other vendor NOS to SONiC (PR #6348) @vaibhavhd commented on this pull request.

________________________________ In tests/common/fixtures/advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985407752&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3I6U8OksAV6mpyLx%2FIwrYPxkdeJP5pJZWgwH%2BOpPr%2FI%3D&reserved=0>:

@@ -116,7 +117,9 @@ 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: + if self.other_vendor_nos: + self.rebootLimit = 50 # Threshold set for reboot from other vendor Why is this 50s expectation set? I don't think this is correct. Expectation for fastboot from vendor NOS is still <30s.

________________________________ In tests/platform_tests/test_advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985408285&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ymdDvMXRFHiTq2B%2FwdZMSp6%2BKqxf9v5%2FDHf9JYs7Ee8%3D&reserved=0>:

@@ -129,3 +145,17 @@ 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)) Please add a new line at the EOF

________________________________ In tests/platform_tests/test_advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985409412&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zRlPDoxu%2BF2aLIfS0YiZC%2F8V6hAlidgqDi9%2BMZTtt5Y%3D&reserved=0>:

@@ -42,6 +43,21 @@ def test_fast_reboot(request, get_advanced_reboot, verify_dut_health,

advancedReboot.runRebootTestcase() ***@***.***('get_advanced_reboot') +def test_fast_reboot_from_other_vendor(duthosts, rand_one_dut_hostname, request, get_advanced_reboot, verify_dut_health, I think it will be better to add this test in a new script. test_fastboot_vendor_to_sonic.py? — Reply to this email directly, view it on GitHub<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23pullrequestreview-1127838073&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=adJ2uEHvR0PBeDOnKZ2iGRtSp7AwCJNUKlkSUdqJyVg%3D&reserved=0>, or unsubscribe<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAKTABA32XMQALRJ6YLDGXH3WBJ3NFANCNFSM6AAAAAAQNEMAJY&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zZwjqUyuRQisBwCU3cLgdkoNXn7%2FmgZ7NLzUOAVUStw%3D&reserved=0>. You are receiving this because your review was requested.Message ID: ***@***.***>

liat-grozovik · 2022-10-11T09:17:39Z

But I agree we should not have it as 50 We should have small diff between the 2 flows Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Liat Grozovik ***@***.***> Sent: Monday, October 3, 2022 9:28:16 AM To: sonic-net/sonic-mgmt ***@***.***>; sonic-net/sonic-mgmt ***@***.***> Cc: Review requested ***@***.***> Subject: Re: [sonic-net/sonic-mgmt] Added a test case for fast reboot from other vendor NOS to SONiC (PR #6348) Imo It takes much more time when you delete the configuration Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Vaibhav Hemant Dixit ***@***.***> Sent: Monday, October 3, 2022 9:21:06 AM To: sonic-net/sonic-mgmt ***@***.***> Cc: Liat Grozovik ***@***.***>; Review requested ***@***.***> Subject: Re: [sonic-net/sonic-mgmt] Added a test case for fast reboot from other vendor NOS to SONiC (PR #6348) @vaibhavhd commented on this pull request.

________________________________ In tests/common/fixtures/advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985407752&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3I6U8OksAV6mpyLx%2FIwrYPxkdeJP5pJZWgwH%2BOpPr%2FI%3D&reserved=0>:

@@ -116,7 +117,9 @@ 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: + if self.other_vendor_nos: + self.rebootLimit = 50 # Threshold set for reboot from other vendor Why is this 50s expectation set? I don't think this is correct. Expectation for fastboot from vendor NOS is still <30s.

________________________________ In tests/platform_tests/test_advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985408285&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ymdDvMXRFHiTq2B%2FwdZMSp6%2BKqxf9v5%2FDHf9JYs7Ee8%3D&reserved=0>:

@@ -129,3 +145,17 @@ 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)) Please add a new line at the EOF

________________________________ In tests/platform_tests/test_advanced_reboot.py<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23discussion_r985409412&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zRlPDoxu%2BF2aLIfS0YiZC%2F8V6hAlidgqDi9%2BMZTtt5Y%3D&reserved=0>:

@@ -42,6 +43,21 @@ def test_fast_reboot(request, get_advanced_reboot, verify_dut_health,

advancedReboot.runRebootTestcase() ***@***.***('get_advanced_reboot') +def test_fast_reboot_from_other_vendor(duthosts, rand_one_dut_hostname, request, get_advanced_reboot, verify_dut_health, I think it will be better to add this test in a new script. test_fastboot_vendor_to_sonic.py? — Reply to this email directly, view it on GitHub<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2Fsonic-mgmt%2Fpull%2F6348%23pullrequestreview-1127838073&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=adJ2uEHvR0PBeDOnKZ2iGRtSp7AwCJNUKlkSUdqJyVg%3D&reserved=0>, or unsubscribe<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAKTABA32XMQALRJ6YLDGXH3WBJ3NFANCNFSM6AAAAAAQNEMAJY&data=05%7C01%7Cliatg%40nvidia.com%7C0bf52de362fd4a60830008daa50774c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638003748687786475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zZwjqUyuRQisBwCU3cLgdkoNXn7%2FmgZ7NLzUOAVUStw%3D&reserved=0>. You are receiving this because your review was requested.Message ID: ***@***.***>

azure-pipelines · 2022-10-20T16:31:29Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/test_advanced_reboot.py check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 ansible/roles/test/files/ptftests/advanced-reboot.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/vsts/.cache/pre-commit/repot_wd1lly/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

lgtm-com · 2022-10-20T16:58:43Z

This pull request introduces 1 alert when merging dac48be96aa0db8a54c7dcc84172266c74b9739c into 61acf83 - view on LGTM.com

new alerts:

1 for Use of the return value of a procedure

azure-pipelines · 2022-10-31T12:09:43Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/test_advanced_reboot.py check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 ansible/roles/test/files/ptftests/advanced-reboot.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/vsts/.cache/pre-commit/repo3sbowq1j/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

lgtm-com · 2022-10-31T12:30:38Z

This pull request introduces 2 alerts when merging e19dbbff603d03471bc42d6a64e8d5e79dc57ca8 into da453f9 - view on LGTM.com

new alerts:

1 for Use of the return value of a procedure
1 for Unused local variable

azure-pipelines · 2022-11-03T12:34:42Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/test_advanced_reboot.py check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 ansible/roles/test/files/ptftests/advanced-reboot.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/vsts/.cache/pre-commit/repobsbfc1ap/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

Details: A new test case was added to fast reboot script. This test case mimics a fast reboot from other vendor nos to SONiC, by flushing all DBs that are part of the boot process, so the boot will be initiated with clean DBs as if it was booting from other nos.

azure-pipelines · 2022-11-03T12:41:43Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/test_advanced_reboot.py check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 ansible/roles/test/files/ptftests/advanced-reboot.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/vsts/.cache/pre-commit/repo01riejjf/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

AharonMalkin · 2022-11-06T07:35:17Z

/azpw run

mssonicbld · 2022-11-06T07:35:18Z

/AzurePipelines run

azure-pipelines · 2022-11-06T07:35:27Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2022-11-06T07:36:12Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/test_advanced_reboot.py check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Failed - hook id: check-ast - exit code: 1 ansible/roles/test/files/ptftests/advanced-reboot.py: failed parsing with CPython 3.8.10: Traceback (most recent call last): File "/home/vsts/.cache/pre-commit/repom0msdyqq/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main ast.parse(f.read(), filename=filename) File "/usr/lib/python3.8/ast.py", line 47, in parse ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

roy-sror · 2022-11-08T08:29:58Z

@vaibhavhd - could you please merge this PR?

vaibhavhd

Added a non blocking comment.

AharonMalkin requested a review from sujinmkang as a code owner September 15, 2022 08:17

AharonMalkin requested review from vaibhavhd, liat-grozovik and roy-sror September 15, 2022 08:18

AharonMalkin changed the title ~~Aharon other vendor reboot~~ Added a test case for fast reboot from other vendor NOS to SONiC Sep 15, 2022

AharonMalkin force-pushed the aharon_other_vendor_reboot branch from 742b546 to ff5c90f Compare September 15, 2022 08:23

Update advanced_reboot.py

871a82c

roy-sror previously approved these changes Sep 19, 2022

View reviewed changes

Added a fix to advanced reboot script

36e72ff

Details: Added the other_vendor_nos parameter as False by default to the AdvancedReboot class, and updated the reboot test cases correspondingly.

AharonMalkin dismissed roy-sror’s stale review via 36e72ff September 20, 2022 13:19

AharonMalkin requested a review from roy-sror September 20, 2022 13:20

liat-grozovik mentioned this pull request Sep 20, 2022

Fast-reboot Flow Improvements HLD sonic-net/SONiC#980

Merged

roy-sror previously approved these changes Sep 21, 2022

View reviewed changes

AharonMalkin dismissed roy-sror’s stale review via 6aba89e September 28, 2022 13:55

vaibhavhd reviewed Oct 3, 2022

View reviewed changes

AharonMalkin force-pushed the aharon_other_vendor_reboot branch from 6aba89e to dac48be Compare October 20, 2022 16:30

AharonMalkin force-pushed the aharon_other_vendor_reboot branch from dac48be to e19dbbf Compare October 31, 2022 12:08

AharonMalkin force-pushed the aharon_other_vendor_reboot branch from e19dbbf to 49ceee1 Compare November 3, 2022 12:33

AharonMalkin force-pushed the aharon_other_vendor_reboot branch from 49ceee1 to 78bac90 Compare November 3, 2022 12:40

vaibhavhd approved these changes Nov 8, 2022

View reviewed changes

roy-sror approved these changes Nov 9, 2022

View reviewed changes

liat-grozovik merged commit a18318e into sonic-net:master Nov 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

AharonMalkin commented Sep 15, 2022 •

edited by liat-grozovik

Loading

lgtm-com bot commented Sep 15, 2022

lgtm-com bot commented Sep 20, 2022

lgtm-com bot commented Sep 28, 2022

vaibhavhd Oct 3, 2022

AharonMalkin Oct 23, 2022

vaibhavhd Oct 3, 2022

AharonMalkin Oct 23, 2022 •

edited

Loading

vaibhavhd Nov 8, 2022

liat-grozovik commented Oct 3, 2022 via email

liat-grozovik commented Oct 11, 2022 via email

azure-pipelines bot commented Oct 20, 2022

lgtm-com bot commented Oct 20, 2022

azure-pipelines bot commented Oct 31, 2022

lgtm-com bot commented Oct 31, 2022

azure-pipelines bot commented Nov 3, 2022

azure-pipelines bot commented Nov 3, 2022

AharonMalkin commented Nov 6, 2022

mssonicbld commented Nov 6, 2022

azure-pipelines bot commented Nov 6, 2022

azure-pipelines bot commented Nov 6, 2022

roy-sror commented Nov 8, 2022

vaibhavhd left a comment

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

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

Conversation

AharonMalkin commented Sep 15, 2022 • edited by liat-grozovik Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

lgtm-com bot commented Sep 15, 2022

lgtm-com bot commented Sep 20, 2022

lgtm-com bot commented Sep 28, 2022

vaibhavhd Oct 3, 2022

Choose a reason for hiding this comment

AharonMalkin Oct 23, 2022

Choose a reason for hiding this comment

vaibhavhd Oct 3, 2022

Choose a reason for hiding this comment

AharonMalkin Oct 23, 2022 • edited Loading

Choose a reason for hiding this comment

vaibhavhd Nov 8, 2022

Choose a reason for hiding this comment

liat-grozovik commented Oct 3, 2022 via email

liat-grozovik commented Oct 11, 2022 via email

azure-pipelines bot commented Oct 20, 2022

lgtm-com bot commented Oct 20, 2022

azure-pipelines bot commented Oct 31, 2022

lgtm-com bot commented Oct 31, 2022

azure-pipelines bot commented Nov 3, 2022

azure-pipelines bot commented Nov 3, 2022

AharonMalkin commented Nov 6, 2022

mssonicbld commented Nov 6, 2022

azure-pipelines bot commented Nov 6, 2022

azure-pipelines bot commented Nov 6, 2022

roy-sror commented Nov 8, 2022

vaibhavhd left a comment

Choose a reason for hiding this comment

AharonMalkin commented Sep 15, 2022 •

edited by liat-grozovik

Loading

AharonMalkin Oct 23, 2022 •

edited

Loading