-
Notifications
You must be signed in to change notification settings - Fork 738
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
[Multi asic]:parameterize asic_index and dut_index #2245
Conversation
Signed-off-by: Arvindsrinivasan Lakshminarasimhan <arlakshm@microsoft.com>
retest this please |
conftest.py, function generate_param_asic_index:
We have different box types, and have grouped our inventory based on the box types. For example:
If we want to add the 'num_asic' data to box1, it won't work. One possible solution is to define a function that takes care of this situation as below.
generate_param_asic_index then calls this function with a check if it is None.
|
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 with minor comments.
This approach is good for tests that just need to check something for each ASIC of each DUT. For more complicated scenarios, the more flexible DutHosts approach may fit better. Since the dut_index
and asic_index
fixtures are optional, I think both approaches can coexist.
The only problem is that test_bgp_facts.py
may be updated by other people to use other approach.
tests/conftest.py
Outdated
if param_type == ASIC_PARAM_TYPE_ALL and ASIC_PARAM_TYPE_ALL in inv_data: | ||
asic_index_params = range(int(inv_data[ASIC_PARAM_TYPE_ALL])) | ||
if param_type == ASIC_PARAM_TYPE_FRONTEND and ASIC_PARAM_TYPE_ALL in inv_data: | ||
asic_index_params = inv_data[ASIC_PARAM_TYPE_ALL] |
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 would be more robust to use ansible.inventory.manager.InventoryManager to load inventory file and get host variables. Example: https://github.com/Azure/sonic-mgmt/blob/master/ansible/library/testbed_vm_info.py#L91
Using yaml.safe_load assumes that the inventory file must be yaml format. But anyway it is OK to use yaml.safe_load for now.
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.
updated to use the inventory manager.
tests/conftest.py
Outdated
inv_data = hosts[dut] | ||
if param_type == ASIC_PARAM_TYPE_ALL and ASIC_PARAM_TYPE_ALL in inv_data: | ||
asic_index_params = range(int(inv_data[ASIC_PARAM_TYPE_ALL])) | ||
if param_type == ASIC_PARAM_TYPE_FRONTEND and ASIC_PARAM_TYPE_ALL in inv_data: |
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.
Use elif
here?
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.
fixed in the latest commit
@arlakshm Could you fix the conflicts? |
Signed-off-by: Arvindsrinivasan Lakshminarasimhan <arlakshm@microsoft.com>
Signed-off-by: Arvindsrinivasan Lakshminarasimhan <arlakshm@microsoft.com>
@sanmalho-git |
retest this please |
Tested out your changes, and they work and look good. |
Signed-off-by: Arvindsrinivasan Lakshminarasimhan arlakshm@microsoft.com
Description of PR
Summary:
Fixes # (issue)
Type of change
Approach
What is the motivation for this PR?
This is PR is add support for testing multi asic platform.
In case of SONiC on multi asic platforms, each asic is bound to a namespace. The dataplane and control plane containers are replicated per namespace.
The tests which are testing functionality within a namespace, can be repeated on all namespaces by passing asic_index as parameter to the test. This way the testing logic remains. The test will be repeated N times, where the N is the num_asics.
Recently sonic-mgmt introduced the support for multiple duts in the topology. We are proposing to pass dut_index as parameter to the testcase. So, the test will be repeated on each dut in the topology.
If asic_index and dut_index is used in the test, the test will be repeated on each asic of each dut in the topology.
How did you do it?
To pass the asic_index or asic_frontend_index or dut_index as parameters to the test, the py test hook pytest_generate_tests is used. If the fixtures asic_index or asic_frontend_index are present then params are generated based on the _num_asics _ or frontend_asics values present for the DUT in the inventory file
If these fields are not present then the DUT is treated as single asic device and None will be passed as asic_index or asic_frontend_index
Example of multi asic DUT info in inventory
How did you verify/test it?
Logs from running on single ASIC DUT
Logs on multi asic DUT
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation