From d2d00f3169083353b3d1325716b29337aecbe107 Mon Sep 17 00:00:00 2001 From: Arvindsrinivasan Lakshminarasimhan Date: Fri, 18 Sep 2020 13:21:39 -0700 Subject: [PATCH 1/2] Multi asic parameterization Signed-off-by: Arvindsrinivasan Lakshminarasimhan --- ansible/library/config_facts.py | 13 ++++---- tests/bgp/test_bgp_fact.py | 21 +++++++------ tests/common/devices.py | 7 ++++- tests/common/helpers/constants.py | 6 ++++ tests/conftest.py | 50 +++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 tests/common/helpers/constants.py diff --git a/ansible/library/config_facts.py b/ansible/library/config_facts.py index 39d21faf32..30b6f51ab3 100644 --- a/ansible/library/config_facts.py +++ b/ansible/library/config_facts.py @@ -83,9 +83,11 @@ def create_maps(config): } -def get_running_config(module): - - rt, out, err = module.run_command("sonic-cfggen -d --print-data") +def get_running_config(module, namespace): + cmd = "sonic-cfggen -d --print-data" + if namespace: + cmd += " -n {}".format(namespace) + rt, out, err = module.run_command(cmd) if rt != 0: module.fail_json(msg="Failed to dump running config! {}".format(err)) json_info = json.loads(out) @@ -111,6 +113,7 @@ def main(): host=dict(required=True), source=dict(required=True, choices=["running", "persistent"]), filename=dict(), + namespace=dict(default=None), ), supports_check_mode=True ) @@ -118,7 +121,7 @@ def main(): m_args = module.params try: config = {} - + namespace = m_args['namespace'] if m_args["source"] == "persistent": if 'filename' in m_args and m_args['filename'] is not None: cfg_file_path = "%s" % m_args['filename'] @@ -127,7 +130,7 @@ def main(): with open(cfg_file_path, "r") as f: config = json.load(f) elif m_args["source"] == "running": - config = get_running_config(module) + config = get_running_config(module, namespace) results = get_facts(config) module.exit_json(ansible_facts=results) except Exception as e: diff --git a/tests/bgp/test_bgp_fact.py b/tests/bgp/test_bgp_fact.py index 2803457b5e..d8bb544e81 100644 --- a/tests/bgp/test_bgp_fact.py +++ b/tests/bgp/test_bgp_fact.py @@ -5,20 +5,23 @@ pytest.mark.device_type('vs') ] -def test_bgp_facts(duthost): +def test_bgp_facts(duthosts, dut_index, asic_index): """compare the bgp facts between observed states and target state""" - bgp_facts = duthost.bgp_facts()['ansible_facts'] - mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] + duthost = duthosts[dut_index] + bgp_facts =duthost.bgp_facts(instance_id=asic_index)['ansible_facts'] + namespace = duthost.get_namespace_from_asic_id(asic_index) + config_facts = duthost.config_facts(host=duthost.hostname, source="running",namespace=namespace)['ansible_facts'] for k, v in bgp_facts['bgp_neighbors'].items(): # Verify bgp sessions are established assert v['state'] == 'established' # Verify locat ASNs in bgp sessions - assert v['local AS'] == mg_facts['minigraph_bgp_asn'] + assert v['local AS'] == int(config_facts['DEVICE_METADATA']['localhost']['bgp_asn'].decode("utf-8")) + + for k, v in config_facts['BGP_NEIGHBOR'].items(): + # Compare the bgp neighbors name with config db bgp neigbhors name + assert v['name'] == bgp_facts['bgp_neighbors'][k]['description'] + # Compare the bgp neighbors ASN with config db + assert int(v['asn'].decode("utf-8")) == bgp_facts['bgp_neighbors'][k]['remote AS'] - for v in mg_facts['minigraph_bgp']: - # Compare the bgp neighbors name with minigraph bgp neigbhors name - assert v['name'] == bgp_facts['bgp_neighbors'][v['addr'].lower()]['description'] - # Compare the bgp neighbors ASN with minigraph - assert v['asn'] == bgp_facts['bgp_neighbors'][v['addr'].lower()]['remote AS'] diff --git a/tests/common/devices.py b/tests/common/devices.py index 92d53df4e3..2e482b3090 100644 --- a/tests/common/devices.py +++ b/tests/common/devices.py @@ -21,7 +21,7 @@ from errors import RunAnsibleModuleFail from errors import UnsupportedAnsibleModule - +from tests.common.helpers.constants import DEFAULT_ASIC_ID, DEFAULT_NAMESPACE, NAMESPACE_PREFIX # HACK: This is a hack for issue https://github.com/Azure/sonic-mgmt/issues/1941 and issue # https://github.com/ansible/pytest-ansible/issues/47 @@ -1016,6 +1016,11 @@ def show_and_parse(self, show_cmd, **kwargs): """ output = self.shell(show_cmd, **kwargs)["stdout_lines"] return self._parse_show(output) + + def get_namespace_from_asic_id(self, asic_id): + if asic_id is DEFAULT_ASIC_ID: + return DEFAULT_NAMESPACE + return "{}{}".format(NAMESPACE_PREFIX, asic_id) class EosHost(AnsibleHostBase): diff --git a/tests/common/helpers/constants.py b/tests/common/helpers/constants.py new file mode 100644 index 0000000000..e5e7b94ef3 --- /dev/null +++ b/tests/common/helpers/constants.py @@ -0,0 +1,6 @@ + +DEFAULT_ASIC_ID = None +DEFAULT_NAMESPACE = None +NAMESPACE_PREFIX = 'asic' +ASIC_PARAM_TYPE_ALL = 'num_asics' +ASIC_PARAM_TYPE_FRONTEND = 'frontend_asics' \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index d456f4331f..a5d65076b0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ from tests.common.fixtures.conn_graph_facts import conn_graph_facts from tests.common.devices import SonicHost, Localhost from tests.common.devices import PTFHost, EosHost, FanoutHost +from tests.common.helpers.constants import ASIC_PARAM_TYPE_ALL, ASIC_PARAM_TYPE_FRONTEND, DEFAULT_ASIC_ID logger = logging.getLogger(__name__) @@ -455,3 +456,52 @@ def disable_container_autorestart(duthost, request): for name, state in container_autorestart_states.items(): if state == "enabled": duthost.command(cmd_enable.format(name)) + +def generate_param_asic_index(request, dut_indices, param_type): + logging.info("generating {} asic indicies for DUT [{}] in ".format(param_type, dut_indices)) + inv_file = request.config.getoption("ansible_inventory") + tbname = request.config.getoption("--testbed") + tbfile = request.config.getoption("--testbed_file") + if tbname is None or tbfile is None: + raise ValueError("testbed and testbed_file are required!") + + with open(inv_file, 'r') as fh: + inv = yaml.safe_load(fh) + + hosts = inv['all']['children']['sonic']['hosts'] + tbinfo = TestbedInfo(tbfile) + + #if the params are not present treat the device as a single asic device + asic_index_params = [DEFAULT_ASIC_ID] + + for dut_id in dut_indices: + dut = tbinfo.testbed_topo[tbname]["duts"][dut_id] + 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: + asic_index_params = inv_data[ASIC_PARAM_TYPE_ALL] + logging.info("dut_index {} dut name {} asics params = {}".format( + dut_id, dut, asic_index_params)) + return asic_index_params + +def generate_params_dut_index(request): + tbname = request.config.getoption("--testbed") + tbfile = request.config.getoption("--testbed_file") + if tbname is None or tbfile is None: + raise ValueError("testbed and testbed_file are required!") + tbinfo = TestbedInfo(tbfile) + num_duts = len(tbinfo.testbed_topo[tbname]["duts"]) + logging.info("Num of duts in testbed topology {}".format(num_duts)) + return range(num_duts) + +def pytest_generate_tests(metafunc): + # The topology always has atleast 1 dut + dut_indices = [0] + if "dut_index" in metafunc.fixturenames: + dut_indices = generate_params_dut_index(metafunc) + metafunc.parametrize("dut_index",dut_indices) + if "asic_index" in metafunc.fixturenames: + metafunc.parametrize("asic_index",generate_param_asic_index(metafunc, dut_indices, ASIC_PARAM_TYPE_ALL)) + if "frontend_asic_index" in metafunc.fixturenames: + metafunc.parametrize("frontend_asic_index",generate_param_asic_index(metafunc, dut_indices, ASIC_PARAM_TYPE_FRONTEND)) From af682b746e9a991bd53e08945129280ada2f4d46 Mon Sep 17 00:00:00 2001 From: Arvindsrinivasan Lakshminarasimhan Date: Sun, 25 Oct 2020 23:03:22 -0700 Subject: [PATCH 2/2] update as per review comments Signed-off-by: Arvindsrinivasan Lakshminarasimhan --- tests/conftest.py | 60 +++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 41d5f43d58..f6bca78c09 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,8 @@ import yaml import jinja2 import ipaddr as ipaddress +from ansible.parsing.dataloader import DataLoader +from ansible.inventory.manager import InventoryManager from collections import defaultdict from datetime import datetime @@ -431,43 +433,28 @@ def tag_test_report(request, pytestconfig, tbinfo, duthost, record_testsuite_pro record_testsuite_property("hwsku", duthost.facts["hwsku"]) record_testsuite_property("os_version", duthost.os_version) -@pytest.fixture(scope="module", autouse=True) -def disable_container_autorestart(duthost, request): - skip = False - for m in request.node.iter_markers(): - if m.name == "enable_container_autorestart": - skip = True - break - if skip: - yield - return - container_autorestart_states = duthost.get_container_autorestart_states() - # Disable autorestart for all containers - logging.info("Disable container autorestart") - cmd_disable = "config feature autorestart {} disabled" - for name, state in container_autorestart_states.items(): - if state == "enabled": - duthost.command(cmd_disable.format(name)) - yield - # Recover autorestart states - logging.info("Recover container autorestart") - cmd_enable = "config feature autorestart {} enabled" - for name, state in container_autorestart_states.items(): - if state == "enabled": - duthost.command(cmd_enable.format(name)) +def get_host_data(request, dut): + ''' + This function parses multple inventory files and returns the dut information present in the inventory + ''' + inv_data = None + inv_files = [inv_file.strip() for inv_file in request.config.getoption("ansible_inventory").split(",")] + for inv_file in inv_files: + inv_mgr = InventoryManager(loader=DataLoader(), sources=inv_file) + if dut in inv_mgr.hosts: + return inv_mgr.get_host(dut).get_vars() + + return inv_data def generate_param_asic_index(request, dut_indices, param_type): logging.info("generating {} asic indicies for DUT [{}] in ".format(param_type, dut_indices)) - inv_file = request.config.getoption("ansible_inventory") + tbname = request.config.getoption("--testbed") tbfile = request.config.getoption("--testbed_file") if tbname is None or tbfile is None: raise ValueError("testbed and testbed_file are required!") - with open(inv_file, 'r') as fh: - inv = yaml.safe_load(fh) - - hosts = inv['all']['children']['sonic']['hosts'] + tbinfo = TestbedInfo(tbfile) #if the params are not present treat the device as a single asic device @@ -475,13 +462,14 @@ def generate_param_asic_index(request, dut_indices, param_type): for dut_id in dut_indices: dut = tbinfo.testbed_topo[tbname]["duts"][dut_id] - 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: - asic_index_params = inv_data[ASIC_PARAM_TYPE_ALL] - logging.info("dut_index {} dut name {} asics params = {}".format( - dut_id, dut, asic_index_params)) + inv_data = get_host_data(request, dut) + if inv_data is not None: + 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])) + elif param_type == ASIC_PARAM_TYPE_FRONTEND and ASIC_PARAM_TYPE_FRONTEND in inv_data: + asic_index_params = inv_data[ASIC_PARAM_TYPE_FRONTEND] + logging.info("dut_index {} dut name {} asics params = {}".format( + dut_id, dut, asic_index_params)) return asic_index_params def generate_params_dut_index(request):