[multi-dut] - sanity checks for multi-duts #2478
Conversation
|
|
dipalipatel25
force-pushed
the
sanity_check
branch
from
78c205f
to
bb72674
Compare
|
@wangxin @yxieca - I changed the code in init.py to use pytest_assert. But, are failing vsimage tests. We are getting We are not sure how to resolve this - would you be able to please help out. |
Because it is in a pytest plugin, pytest is trying to interpret |
dipalipatel25
force-pushed
the
sanity_check
branch
from
bb72674
to
0693fe1
Compare
|
@wangxin Thanks for the suggestion. That worked. |
checks.py:
- do_checks modified to iterate over the all the duts and return
a dictionary with key as dut.hostname and value being the list of
results for sanity for the specified items (like processes, interfaces, etc.)
- check_bgp:
- check bgp_facts for all the asics on a DUT and return the 'down_neighbors' per asic.
If there are down_neighbors, then we return a dict with key being the bgp instance
and value being the list of down_neighbors on that bgp instance.
Eg.
For single asic DUT with down_neighbor of 100.0.0.1:
bgp: {
down_neighbors : [ 100.0.0.1 ]
}
For multi-asic DUT with down_neighbor 100.0.0.1 on bgp0, no down_neighbors on bgp1 and down_neighbor 200.0.0.1 on bgp2
bgp0: {
down_neighbors : [ 100.0.0.1 ]
},
bgp2: {
down_neighbors : [ 200.0.0.1 ]
}
- no change to check_services, check_processes, check_interfaces, check_dbmemory except for
- changing the log message to have the dut's hostname
- check_interfaces and check_dbmemory do not support multi-asic yet, only multi-dut.
- print_logs - iterate through all the duts in duthosts.
sanity_check fixture modified to use duthosts instead of duthost:
- call print_logs and do_checks with duthosts
- iterate throught the sanity results of each DUT in duthosts
for sanity_check / recover.
- use pytest_assert - use variable for asic check in check_bgp
fixed logging in checks.py
dipalipatel25
force-pushed
the
sanity_check
branch
from
0693fe1
to
d7fa920
Compare
| bgp_facts = dut.bgp_facts(asic_index='all') | ||
| for asic_index, a_asic_facts in enumerate(bgp_facts): | ||
| a_asic_result = False | ||
| a_asic_neighbors = a_asic_facts['ansible_facts']['bgp_neighbors'] |
There was a problem hiding this comment.
Can we add this to the function dut.get_bgp_neighbors(). It might be useful in other tests as well.
There was a problem hiding this comment.
get_bgp_neighbors() is in SonicHost. In our case, dut is an instance of MultiAsicSonicHost. SonicHost doesn't have a reference to MultiAsicSonicHost. And we probably don't want methods like this in MultiAsicSonicHost - as then we would have to replicate all get_* methods in SonicHost.
| else: | ||
| a_asic_result = False | ||
| if dut.facts['num_asic'] == 1: | ||
| if 'bgp' in check_result: |
There was a problem hiding this comment.
If we have 2 devices, dut1 and dut2. dut1 has some down_neighbors, dut2 has no down_neighbors.
the down_neighbors will be removed from check_results here. Is this correct ?
There was a problem hiding this comment.
No. The final results is a dictionary with key being each DUT's hostname, and value a list of all sanity checks on that DUT. So, for your scenario, check_results would be something like:
{
"node1": [
....
{
"check_item": "bgp",
"failed": true,
"bgp": {
"down_neighbors": [
"2064:101::1",
"100.0.0.1",
"101.0.0.1",
"2064:100::1"
]
}
},
.....
"node2": [
.....
{
"check_item": "bgp",
"failed": false,
},
.....
]
},
|
@yxieca - can you please review and approve this PR. |
…r multi-duts
conftest.py
- collect_techsupport fixture is modified to a function called
collect_techsupport_on_dut that is called by fixtures collect_techsupport
and collect_techsupport_all_duts.
- collect_techsupport fixture is added which uses the enum_dut_hostname
to parameterize per dut.The dump is collected per dut basis.
This fixture is used for collecting dump for testcases that uses this approach.
- collect_techsupport_all_duts fixture is added that uses duthosts approach.All
duts are looped and the dump is collected for all failed duts.
This fixture is used for collecting dump for testcases that uses the duthosts.
- The dump timespan is changed to 2 hours.
reset_critical_services
- Enhanced reset_critical_services_list fixture for multi-duts support.
|
retest vsimage please |
Description of PR
Summary:
Fixes # (issue)
Type of change
Approach
What is the motivation for this PR?
Need to support sanity_check against multi-dut
How did you do it?
checks.py:
a dictionary with key as dut.hostname and value being the list of
results for sanity for the specified items (like processes, interfaces, etc.)
If there are down_neighbors, then we return a dict with key being the bgp instance
and value being the list of down_neighbors on that bgp instance.
sanity_check fixture modified to use duthosts instead of duthost:
for sanity_check / recover.
How did you verify/test it?
Tested sanity_check fixture against
Sample output of sanity check against single-asic multi-dut with boards dut4 and dut3:
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation