-
Notifications
You must be signed in to change notification settings - Fork 554
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
vlanmgr changes related to EVPN VxLan warmboot #1460
Conversation
@heidinet2007 to review |
@@ -282,6 +302,7 @@ void VlanMgr::doVlanTask(Consumer &consumer) | |||
if (isVlanStateOk(key) && m_vlans.find(key) == m_vlans.end()) | |||
{ | |||
m_vlans.insert(key); | |||
m_vlanReplay.erase(kfvKey(t)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kfvKey(t) [](start = 35, length = 9)
erase #Closedit
directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qiluo-msft sorry, I didn't understand, here I am erasing the key from the set m_vlanReplay, what do you mean by erase 'it' directly?
@@ -292,6 +313,7 @@ void VlanMgr::doVlanTask(Consumer &consumer) | |||
{ | |||
addHostVlan(vlan_id); | |||
} | |||
m_vlanReplay.erase(kfvKey(t)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kfvKey(t) [](start = 31, length = 9)
The same #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's look good to me. Not the domain expert, please check with others for review comments.
retest this please |
retest this please |
WarmStart::isWarmStart()) | ||
{ | ||
replayDone = true; | ||
WarmStart::setWarmStartState("vlanmgrd", WarmStart::RECONCILED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change from REPLAYED?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prsunny test_VlanMgrdWarmRestart was failing as it expected the state to be RECONCILED. I understand now that as per HLD it is supposed to be REPLAYED. I will discuss with team members and check on how to handle this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prsunny I will change the existing pytest script to match the EVPN HLD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prsunny I will change the existing pytest to expect REPLAYED instead of RECONCILED to match the EVPN HLD
Change back state to REPLAYED
tests/test_warm_reboot.py
Outdated
@@ -437,7 +437,8 @@ def test_VlanMgrdWarmRestart(self, dvs, testlog): | |||
(status, fvs) = tbl.get("Vlan20:11.0.0.11") | |||
assert status == True | |||
|
|||
swss_app_check_RestoreCount_single(state_db, restore_count, "vlanmgrd") | |||
swss_check_RestoreCount(dvs, state_db, restore_count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qiluo-msft , could you review this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are more dependencies on the Reconciled state in the vs tests, I will check and fix.
retest this please |
…eboot (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.
* 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]>
This PR is for vlanmgr changes related to EVPN VxLan warmboot.
Please refer to the HLD of EVPN VxLan for details.
sonic-net/SONiC#437