[test plan] 4-byte AS Translation #8973
Conversation
Catch up on current master
Merge current master
Azarack
requested review from
jcaiMR,
StormLiangMS,
wangxin and
yxieca
as code owners
|
@wenyiz2021 can you review this PR? |
|
@kishorekunal01 review |
| -c "address-family ipv4 unicast" \ | ||
| -c "network {}" \ | ||
| -c "neighbor {} soft-reconfiguration inbound" \ | ||
| -c "neighbor {} route-map FROM_BGP_PEER_V4 in" \ |
There was a problem hiding this comment.
route-map define is missing in the file, what exactly the route-map contain, similar other route map in the files as well.
There was a problem hiding this comment.
@kishorekunal01 This is a pre-configured route map that is part of the T2 BGP configuration. I did add a comment specifying this at the start of the command block.
|
|
||
| def test_4_byte_asn_translation(setup): | ||
| # copy existing BGP config to a new 4-byte ASN. Use existing route-maps for consistancy. | ||
| cmd = 'vtysh{} \ |
There was a problem hiding this comment.
there is a space needed after vtysh : cli_options = "-n " + duthost.get_namespace_from_asic_id(asic_index)
There was a problem hiding this comment.
Yes, sorry I missed this.
| setup['peer_group_v6'], setup['peer_group_v6'], setup['neigh_ip_v6'], setup['neigh2_ip_v6']) | ||
| logger.debug(setup['duthost'].shell(cmd, module_ignore_errors=True)) | ||
|
|
||
| cmd = 'vtysh \ |
There was a problem hiding this comment.
can you add the cli_options = "-n " + duthost.get_namespace_from_asic_id(asic_index) here also? T2 neighbor could be multi-asic
There was a problem hiding this comment.
Yes, I added cli options for the neighbor.
| logger.debug("DUT BGP Config: {}".format(setup['duthost'].shell("show run bgp")['stdout'])) | ||
| logger.debug("Neighbor BGP Config: {}".format(setup['neighhost'].shell("show run bgp")['stdout'])) | ||
|
|
||
| time.sleep(bgp_sleep) |
There was a problem hiding this comment.
could you elaborate on what the previous cmd is doing?
the sleep time could be different among T2s
There was a problem hiding this comment.
The previous command configures a 4-byte ASN on the neighbor device.
I'm not sure what you mean by the sleep time being different as this is a variable set in the script.
There was a problem hiding this comment.
I mean how did you decide bgp_sleep?
There was a problem hiding this comment.
It is the standard amount of time we wait for our tests to allow BGP to converge. Then after running the test case if there is an issue it is extended.
There was a problem hiding this comment.
LGTM if this sleep time of 120 works well on T2
|
@wenyiz2021 Hello, can you please look at the updates I have made as per your comments? |
|
LGTM @kishorekunal01 can you check if your question is resolved |
Yes, it is resolved |
|
@kishorekunal01 @wenyiz2021 Does someone else need to approve this for it to be merged? |
|
@wenyiz2021 Can you merge this or does someone else need to approve? |
|
|
||
| # Test name | ||
|
|
||
| BGP Session Flaps |
There was a problem hiding this comment.
looks like the test name and overview is not aligned with test description. Can you check?
There was a problem hiding this comment.
@prsunny You are correct that this was a mistake. I have updated it to the proper name.
|
|
||
| ## Overview | ||
|
|
||
| The goal of this test is to verify that the CPU and memory do not spike during cycles of BGP sessions flapping. |
There was a problem hiding this comment.
Is this really meant to be a test for session flap and memory check?
There was a problem hiding this comment.
@prsunny You are correct that this was a mistake. I have updated it to the proper overview.
|
@BYGX-wcr Please review this PR. |
|
@wangxin , would you please merge? |
Test plan and automation to verify 4-byte to 2-byte ASN translation feature functions as expected. * adding test and test plan * change to use random frontend dut * adding clarification for route-map and updating tor and namespace * adding neighbor cli options * Updating name and overview
Test plan and automation to verify 4-byte to 2-byte ASN translation feature functions as expected. * adding test and test plan * change to use random frontend dut * adding clarification for route-map and updating tor and namespace * adding neighbor cli options * Updating name and overview
Description of PR
Test plan and automation to verify 4-byte to 2-byte ASN translation feature functions as expected.
Summary:
Fixes # (issue)
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?
vsonic neighbors
Supported testbed topology if it's a new test case?
T2 topology
Documentation