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

mclag enhancements as per HLD at Azure/SONIC#596 #1138

Merged
merged 74 commits into from
Jul 16, 2021

Conversation

gitsabari
Copy link
Contributor

@gitsabari gitsabari commented Sep 28, 2020

What I did
This PR contains the code changes to support click cli changes for mclag enhancements as per HLD sonic-net/SONiC#596

Why I did it
Implemented as per the MCLAG enhancements HLD: sonic-net/SONiC#596

How I verified it
Mclag specific VS tests included part of check-in.

Details:
The PR includes the click cli changes as part of MCLAG enhancements
This PR must work with submitted PR's in other sonic repositories which are listed below.
> Iccpd: sonic-net/sonic-buildimage#4819
> Swss: sonic-net/sonic-swss#1331
> swss-common: sonic-net/sonic-swss-common#405
> Mclagsyncd: sonic-net/sonic-swss#1349
> Mgmt-FW: sonic-net/sonic-mgmt-framework#59
added mclag sonic yang file sonic-net/sonic-buildimage#7622

@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2020

This pull request introduces 1 alert when merging 73f90d2 into 07a4e26 - view on LGTM.com

new alerts:

  • 1 for Unnecessary pass

@lgtm-com

This comment has been minimized.

@tapashdas
Copy link
Contributor

retest this please

@gitsabari
Copy link
Contributor Author

@gechiang , @lguohan, @rlhui
can you please help review and approve this merge request

@gitsabari
Copy link
Contributor Author

@gechiang , @lguohan, @rlhui
can you please help review and approve this merge request

@gitsabari
Copy link
Contributor Author

retest this please

1 similar comment
@gitsabari
Copy link
Contributor Author

retest this please

@gitsabari
Copy link
Contributor Author

@rathnasabapathyv , @prvattem
can you please approve this PR if there are no more questions

tests/mclag_test.py Outdated Show resolved Hide resolved
tests/mclag_test.py Outdated Show resolved Hide resolved
tests/mclag_test.py Outdated Show resolved Hide resolved
@gitsabari
Copy link
Contributor Author

the unit test only checks the cli return value, in other unit test, it also checks if the config db is correct or not. not sure why this unit test does not validate config db values.

@lguohan thanks for the comments. addressed it in latest

@gitsabari
Copy link
Contributor Author

@arlakshm have addressed all comments and all checks are passed. please help approve and merge the changes

config/mclag.py Outdated Show resolved Hide resolved
Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

lgtm, one minor comment.

@arlakshm arlakshm self-requested a review July 16, 2021 00:07
@haslersn
Copy link

Can this get merged?

@gechiang gechiang merged commit 0efd297 into sonic-net:master Jul 16, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-utilities that referenced this pull request Aug 10, 2021
* mclagsyncd enhancements as per HLD at sonic-net/SONiC#596

* addressed LGTM alert

* UT Fix unique IP configuration

* modified ip address validate function for mclag config verication

* Add soft-reboot reboot type (sonic-net#1453)

What I did
Add a new reboot named as soft-reboot which can be performed by "kexec -e"

How I did it
Replace the platform reboot with "kexec -e" for the cold reboot case.

How to verify it
Verified the reboot on DUT and check the reboot-cause

* [warm-reboot] Check if warm restart flag is set when issuing a warm-reboot (sonic-net#1460)

Check if any warm restart flag is set when issuing a warm-reboot. This check avoids starting a warm reboot while another warm restart is in progress. In the scenario where a warm reboot is issued with another warm restart in progress, the warm restart flag may be reset and part of the components have a risk of doing cold reboot.

* Added mclag config commands

* removed unwanted imports

* added mclag tests

* fixed build issue

* corrected mclag test

* corrected mclag test

* corrected mclag test case

* updated testcase for mclag

* updated mclag config

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* fixed alert

* updated mclag test cases

* updated mclag test cases

* updated mclag config

* modified mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test case

* updated mclag config to use swsscommon instead of swssdk

* updated mclag config to use swsscommon

* updated mclag config script file

* fixed mclag test cases to verify config db

* updated mclag test case with config db verify function

* fixed build issue

* updated test case

* updated mclag test case

* addressed review comments

Co-authored-by: Tapash Das <[email protected]>
Co-authored-by: Tapash Das <[email protected]>
Co-authored-by: Sujin Kang <[email protected]>
Co-authored-by: Shi Su <[email protected]>
judyjoseph pushed a commit that referenced this pull request Aug 20, 2021
* mclagsyncd enhancements as per HLD at sonic-net/SONiC#596

* addressed LGTM alert

* UT Fix unique IP configuration

* modified ip address validate function for mclag config verication

* Add soft-reboot reboot type (#1453)

What I did
Add a new reboot named as soft-reboot which can be performed by "kexec -e"

How I did it
Replace the platform reboot with "kexec -e" for the cold reboot case.

How to verify it
Verified the reboot on DUT and check the reboot-cause

* [warm-reboot] Check if warm restart flag is set when issuing a warm-reboot (#1460)

Check if any warm restart flag is set when issuing a warm-reboot. This check avoids starting a warm reboot while another warm restart is in progress. In the scenario where a warm reboot is issued with another warm restart in progress, the warm restart flag may be reset and part of the components have a risk of doing cold reboot.

* Added mclag config commands

* removed unwanted imports

* added mclag tests

* fixed build issue

* corrected mclag test

* corrected mclag test

* corrected mclag test case

* updated testcase for mclag

* updated mclag config

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* fixed alert

* updated mclag test cases

* updated mclag test cases

* updated mclag config

* modified mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test case

* updated mclag config to use swsscommon instead of swssdk

* updated mclag config to use swsscommon

* updated mclag config script file

* fixed mclag test cases to verify config db

* updated mclag test case with config db verify function

* fixed build issue

* updated test case

* updated mclag test case

* addressed review comments

Co-authored-by: Tapash Das <[email protected]>
Co-authored-by: Tapash Das <[email protected]>
Co-authored-by: Sujin Kang <[email protected]>
Co-authored-by: Shi Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.