From 7f2368788d9f698ae634734277cbdd06de314d2c Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Wed, 15 Apr 2020 21:00:55 +0000 Subject: [PATCH 1/8] [link flap] add link flap pytest Signed-off-by: Ying Xie --- tests/common/platform/device_utils.py | 25 +++++++++ tests/platform/test_link_flap.py | 80 +++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 tests/common/platform/device_utils.py create mode 100644 tests/platform/test_link_flap.py diff --git a/tests/common/platform/device_utils.py b/tests/common/platform/device_utils.py new file mode 100644 index 0000000000..4e1e4f4117 --- /dev/null +++ b/tests/common/platform/device_utils.py @@ -0,0 +1,25 @@ +""" +Helper script for fanout switch operations +""" + +import logging + +def fanout_switch_port_lookup(fanout_switches, dut_port): + """ + look up the fanout switch instance and the fanout switch port + connecting to the dut_port + + Args: + fanout_switches (list FanoutHost): list of fanout switch + instaances. + dut_port (str): port name on the DUT + + Returns: + None, None if fanout switch instance and port is not found + FanoutHost, Portname(str) if found + """ + for _, fanout in fanout_switches.items(): + if dut_port in fanout.host_to_fanout_port_map.keys(): + return fanout, fanout.host_to_fanout_port_map[dut_port] + + return None, None diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py new file mode 100644 index 0000000000..bfbb0ee8a9 --- /dev/null +++ b/tests/platform/test_link_flap.py @@ -0,0 +1,80 @@ +import logging + +import pytest + +from common.platform.device_utils import fanout_switch_port_lookup +from common.utilities import wait_until + +class TestLinkFlap: + def __init__(self): + self.ports_shutdown_by_test = set() + + + def __get_dut_if_facts(self, dut): + interface_facts = dut.interface_facts() + ansible_facts = interface_facts['ansible_facts'] + if_facts = ansible_facts['ansible_interface_facts'] + + return if_facts + + + def __check_if_status(self, dut, dut_port, exp_state): + ifstate = self.__get_dut_if_facts(dut)[dut_port] + return ifstate['active'] == exp_state + + + def __toggle_one_port(self, dut, dut_port, fanout, fanout_port): + logging.info("Testing port toggle on {}".format(dut_port)) + + ifstate = self.__get_dut_if_facts(dut)[dut_port] + assert ifstate['active'], "dut port {} is down".format(dut_port) + + logging.debug("Shutting down fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) + self.ports_shutdown_by_test.add((fanout, fanout_port)) + fanout.shutdown(fanout_port) + wait_until(30, 0.2, self.__check_if_status, dut, dut_port, False) + ifstate = self.__get_dut_if_facts(dut)[dut_port] + logging.debug("Interface fact : {}".format(ifstate)) + assert not ifstate['active'], "dut port {} didn't go down as expected".format(dut_port) + + logging.debug("Bring up fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) + fanout.no_shutdown(fanout_port) + wait_until(30, 0.2, self.__check_if_status, dut, dut_port, True) + ifstate = self.__get_dut_if_facts(dut)[dut_port] + logging.debug("Interface fact : {}".format(ifstate)) + assert ifstate['active'], "dut port {} didn't come up as expected".format(dut_port) + self.ports_shutdown_by_test.discard((fanout, fanout_port)) + + + def __build_test_candidates(self, dut, fanouthosts): + if_facts = self.__get_dut_if_facts(dut) + candidates = [] + for dut_port in if_facts.keys(): + fanout, fanout_port = fanout_switch_port_lookup(fanouthosts, dut_port) + + if not fanout or not fanout_port: + logging.info("Skipping port {} that is not found in connection graph".format(dut_port)) + else: + candidates.append((dut_port, fanout, fanout_port)) + + return candidates + + + def run_link_flap_test(self, dut, fanouthosts): + candidates = self.__build_test_candidates(dut, fanouthosts) + + try: + for dut_port, fanout, fanout_port in candidates: + self.__toggle_one_port(dut, dut_port, fanout, fanout_port) + finally: + logging.info("Restoring fanout switch ports that were shut down by test") + for fanout, fanout_port in self.ports_shutdown_by_test: + logging.debug("Restoring fanout switch {} port {} shut down by test".format(fanout.hostname, fanout_port)) + fanout.no_shutdown(fanout_port) + + +@pytest.mark.topology_agnostic +@pytest.mark.platform_physical +def test_link_flap(duthost, fanouthosts): + tlf = TestLinkFlap() + tlf.run_link_flap_test(duthost, fanouthosts) From 28566d944cf7fc787075c201c65215e33e40a034 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Wed, 15 Apr 2020 21:38:09 +0000 Subject: [PATCH 2/8] address LGTM alert and wording --- tests/common/platform/device_utils.py | 2 -- tests/platform/test_link_flap.py | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/common/platform/device_utils.py b/tests/common/platform/device_utils.py index 4e1e4f4117..8186c3c1af 100644 --- a/tests/common/platform/device_utils.py +++ b/tests/common/platform/device_utils.py @@ -2,8 +2,6 @@ Helper script for fanout switch operations """ -import logging - def fanout_switch_port_lookup(fanout_switches, dut_port): """ look up the fanout switch instance and the fanout switch port diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index bfbb0ee8a9..c213787106 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -23,8 +23,8 @@ def __check_if_status(self, dut, dut_port, exp_state): return ifstate['active'] == exp_state - def __toggle_one_port(self, dut, dut_port, fanout, fanout_port): - logging.info("Testing port toggle on {}".format(dut_port)) + def __toggle_one_link(self, dut, dut_port, fanout, fanout_port): + logging.info("Testing link flap on {}".format(dut_port)) ifstate = self.__get_dut_if_facts(dut)[dut_port] assert ifstate['active'], "dut port {} is down".format(dut_port) @@ -65,7 +65,7 @@ def run_link_flap_test(self, dut, fanouthosts): try: for dut_port, fanout, fanout_port in candidates: - self.__toggle_one_port(dut, dut_port, fanout, fanout_port) + self.__toggle_one_link(dut, dut_port, fanout, fanout_port) finally: logging.info("Restoring fanout switch ports that were shut down by test") for fanout, fanout_port in self.ports_shutdown_by_test: From e204aade2c6c7c78d957308e407609632dfbf9c5 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 00:20:07 +0000 Subject: [PATCH 3/8] Use show interface status to obtain operational status of interfaces. Filter out operational down interfaces from test list. --- ansible/library/show_interface.py | 2 +- tests/platform/test_link_flap.py | 53 ++++++++++++++++--------------- tests/pytest.ini | 2 ++ 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/ansible/library/show_interface.py b/ansible/library/show_interface.py index e56b9852a6..4ca76d456a 100644 --- a/ansible/library/show_interface.py +++ b/ansible/library/show_interface.py @@ -93,7 +93,7 @@ def collect_interface_status(self): rc, self.out, err = self.module.run_command(command, executable='/bin/bash', use_unsafe_shell=True) for line in self.out.split("\n"): line = line.strip() - if regex_int.match(line): + if regex_int.match(line) and interface == regex_int.match(line).group(1): self.int_status[interface]['name'] = regex_int.match(line).group(1) self.int_status[interface]['speed'] = regex_int.match(line).group(2) self.int_status[interface]['alias'] = regex_int.match(line).group(4) diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index c213787106..b6d68ab83d 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -6,54 +6,57 @@ from common.utilities import wait_until class TestLinkFlap: - def __init__(self): - self.ports_shutdown_by_test = set() - - - def __get_dut_if_facts(self, dut): - interface_facts = dut.interface_facts() - ansible_facts = interface_facts['ansible_facts'] - if_facts = ansible_facts['ansible_interface_facts'] + def __get_dut_if_status(self, dut, ifname=None): + if not ifname: + status = dut.show_interface(command='status')['ansible_facts']['int_status'] + else: + status = dut.show_interface(command='status', interfaces=[ifname])['ansible_facts']['int_status'] - return if_facts + return status def __check_if_status(self, dut, dut_port, exp_state): - ifstate = self.__get_dut_if_facts(dut)[dut_port] - return ifstate['active'] == exp_state + status = self.__get_dut_if_status(dut, dut_port)[dut_port] + return status['oper_state'] == exp_state def __toggle_one_link(self, dut, dut_port, fanout, fanout_port): logging.info("Testing link flap on {}".format(dut_port)) - ifstate = self.__get_dut_if_facts(dut)[dut_port] - assert ifstate['active'], "dut port {} is down".format(dut_port) + status = self.__get_dut_if_status(dut, dut_port)[dut_port] + logging.debug("Dut port status {}".format(status)) + assert status['oper_state'] == 'up', "Skipping dut port {}: link operational down".format(status) - logging.debug("Shutting down fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) + logging.info("Shutting down fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) self.ports_shutdown_by_test.add((fanout, fanout_port)) fanout.shutdown(fanout_port) - wait_until(30, 0.2, self.__check_if_status, dut, dut_port, False) - ifstate = self.__get_dut_if_facts(dut)[dut_port] - logging.debug("Interface fact : {}".format(ifstate)) - assert not ifstate['active'], "dut port {} didn't go down as expected".format(dut_port) + wait_until(30, 1, self.__check_if_status, dut, dut_port, 'down') + status = self.__get_dut_if_status(dut, dut_port)[dut_port] + logging.debug("Interface fact : {}".format(status)) + assert status['oper_state'] == 'down', "dut port {} didn't go down as expected".format(dut_port) - logging.debug("Bring up fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) + logging.info("Bring up fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) fanout.no_shutdown(fanout_port) - wait_until(30, 0.2, self.__check_if_status, dut, dut_port, True) - ifstate = self.__get_dut_if_facts(dut)[dut_port] - logging.debug("Interface fact : {}".format(ifstate)) - assert ifstate['active'], "dut port {} didn't come up as expected".format(dut_port) + wait_until(30, 1, self.__check_if_status, dut, dut_port, 'up') + status = self.__get_dut_if_status(dut, dut_port)[dut_port] + logging.debug("Interface fact : {}".format(status)) + assert status['oper_state'] == 'up', "dut port {} didn't come up as expected".format(dut_port) self.ports_shutdown_by_test.discard((fanout, fanout_port)) def __build_test_candidates(self, dut, fanouthosts): - if_facts = self.__get_dut_if_facts(dut) + self.ports_shutdown_by_test = set() + + status = self.__get_dut_if_status(dut) candidates = [] - for dut_port in if_facts.keys(): + + for dut_port in status.keys(): fanout, fanout_port = fanout_switch_port_lookup(fanouthosts, dut_port) if not fanout or not fanout_port: logging.info("Skipping port {} that is not found in connection graph".format(dut_port)) + elif status[dut_port]['admin_state'] == 'down': + logging.info("Skipping port {} that is admin down".format(dut_port)) else: candidates.append((dut_port, fanout, fanout_port)) diff --git a/tests/pytest.ini b/tests/pytest.ini index a3f8ab4d42..e9aa859365 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -7,3 +7,5 @@ markers: disable_loganalyzer: make to disable automatic loganalyzer broadcom: test specific to Broadcom platform sanity_check: override the default sanity check settings + topology_agnostic: test that can run on any topology (t0, t1, ptf, etc) + platform_physical: test can only be executed on physical testbed. From cf42fb7259896d3841f9ea0569fcbc8e11b70006 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 01:50:09 +0000 Subject: [PATCH 4/8] move initialization code to main function and rename marker --- tests/platform/test_link_flap.py | 6 +++--- tests/pytest.ini | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index b6d68ab83d..c59104bd58 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -45,8 +45,6 @@ def __toggle_one_link(self, dut, dut_port, fanout, fanout_port): def __build_test_candidates(self, dut, fanouthosts): - self.ports_shutdown_by_test = set() - status = self.__get_dut_if_status(dut) candidates = [] @@ -64,6 +62,8 @@ def __build_test_candidates(self, dut, fanouthosts): def run_link_flap_test(self, dut, fanouthosts): + self.ports_shutdown_by_test = set() + candidates = self.__build_test_candidates(dut, fanouthosts) try: @@ -76,7 +76,7 @@ def run_link_flap_test(self, dut, fanouthosts): fanout.no_shutdown(fanout_port) -@pytest.mark.topology_agnostic +@pytest.mark.topology_any @pytest.mark.platform_physical def test_link_flap(duthost, fanouthosts): tlf = TestLinkFlap() diff --git a/tests/pytest.ini b/tests/pytest.ini index e9aa859365..3cd3ace8d5 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -7,5 +7,5 @@ markers: disable_loganalyzer: make to disable automatic loganalyzer broadcom: test specific to Broadcom platform sanity_check: override the default sanity check settings - topology_agnostic: test that can run on any topology (t0, t1, ptf, etc) + topology_any: test that can run on any topology (t0, t1, ptf, etc) platform_physical: test can only be executed on physical testbed. From 646ad74022c7f48504cbd6510f698afed6f11ec7 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 15:33:33 +0000 Subject: [PATCH 5/8] fix typo --- tests/common/platform/device_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/platform/device_utils.py b/tests/common/platform/device_utils.py index 8186c3c1af..d858192ab3 100644 --- a/tests/common/platform/device_utils.py +++ b/tests/common/platform/device_utils.py @@ -9,7 +9,7 @@ def fanout_switch_port_lookup(fanout_switches, dut_port): Args: fanout_switches (list FanoutHost): list of fanout switch - instaances. + instances. dut_port (str): port name on the DUT Returns: From 61ea23b4e9c825567041c5c29b1f5e8858c66b47 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 17:19:46 +0000 Subject: [PATCH 6/8] rename markers, skip when no candidate --- tests/common/platform/device_utils.py | 2 +- tests/platform/test_link_flap.py | 6 ++++-- tests/pytest.ini | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/common/platform/device_utils.py b/tests/common/platform/device_utils.py index d858192ab3..e4978d3ea1 100644 --- a/tests/common/platform/device_utils.py +++ b/tests/common/platform/device_utils.py @@ -17,7 +17,7 @@ def fanout_switch_port_lookup(fanout_switches, dut_port): FanoutHost, Portname(str) if found """ for _, fanout in fanout_switches.items(): - if dut_port in fanout.host_to_fanout_port_map.keys(): + if dut_port in fanout.host_to_fanout_port_map: return fanout, fanout.host_to_fanout_port_map[dut_port] return None, None diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index c59104bd58..427af4b2a1 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -65,6 +65,8 @@ def run_link_flap_test(self, dut, fanouthosts): self.ports_shutdown_by_test = set() candidates = self.__build_test_candidates(dut, fanouthosts) + if not candidates: + pytest.skip("Didn't find any port that is admin up and present in the connection graph") try: for dut_port, fanout, fanout_port in candidates: @@ -76,8 +78,8 @@ def run_link_flap_test(self, dut, fanouthosts): fanout.no_shutdown(fanout_port) -@pytest.mark.topology_any -@pytest.mark.platform_physical +@pytest.mark.topology('any') +@pytest.mark.platform('physicaa') def test_link_flap(duthost, fanouthosts): tlf = TestLinkFlap() tlf.run_link_flap_test(duthost, fanouthosts) diff --git a/tests/pytest.ini b/tests/pytest.ini index 3cd3ace8d5..07a5e9240e 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -7,5 +7,5 @@ markers: disable_loganalyzer: make to disable automatic loganalyzer broadcom: test specific to Broadcom platform sanity_check: override the default sanity check settings - topology_any: test that can run on any topology (t0, t1, ptf, etc) - platform_physical: test can only be executed on physical testbed. + topology: specify which topology testcase can be executed on: (t0, t1, ptf, etc) + platform: specify which platform testcase can be executed on: (physical, virtual, broadcom, mellanox, etc) From 1d2127d9a436f674323f3f1d3adb2a0e54433c26 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 18:52:52 +0000 Subject: [PATCH 7/8] fix typo --- tests/platform/test_link_flap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index 427af4b2a1..5e31066996 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -79,7 +79,7 @@ def run_link_flap_test(self, dut, fanouthosts): @pytest.mark.topology('any') -@pytest.mark.platform('physicaa') +@pytest.mark.platform('physical') def test_link_flap(duthost, fanouthosts): tlf = TestLinkFlap() tlf.run_link_flap_test(duthost, fanouthosts) From 83f5f53f2abe43dbe3735eb8d63adf66ea2205a1 Mon Sep 17 00:00:00 2001 From: Ying Xie Date: Thu, 16 Apr 2020 20:01:05 +0000 Subject: [PATCH 8/8] condense code --- tests/platform/test_link_flap.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/platform/test_link_flap.py b/tests/platform/test_link_flap.py index 5e31066996..3604f038a8 100644 --- a/tests/platform/test_link_flap.py +++ b/tests/platform/test_link_flap.py @@ -15,32 +15,28 @@ def __get_dut_if_status(self, dut, ifname=None): return status - def __check_if_status(self, dut, dut_port, exp_state): + def __check_if_status(self, dut, dut_port, exp_state, verbose=False): status = self.__get_dut_if_status(dut, dut_port)[dut_port] + if verbose: + logging.debug("Interface status : {}".format(status)) return status['oper_state'] == exp_state def __toggle_one_link(self, dut, dut_port, fanout, fanout_port): logging.info("Testing link flap on {}".format(dut_port)) - status = self.__get_dut_if_status(dut, dut_port)[dut_port] - logging.debug("Dut port status {}".format(status)) - assert status['oper_state'] == 'up', "Skipping dut port {}: link operational down".format(status) + assert self.__check_if_status(dut, dut_port, 'up', verbose=True), "Fail: dut port {}: link operational down".format(status) logging.info("Shutting down fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) self.ports_shutdown_by_test.add((fanout, fanout_port)) fanout.shutdown(fanout_port) wait_until(30, 1, self.__check_if_status, dut, dut_port, 'down') - status = self.__get_dut_if_status(dut, dut_port)[dut_port] - logging.debug("Interface fact : {}".format(status)) - assert status['oper_state'] == 'down', "dut port {} didn't go down as expected".format(dut_port) + assert self.__check_if_status(dut, dut_port, 'down', verbose=True), "dut port {} didn't go down as expected".format(dut_port) logging.info("Bring up fanout switch {} port {} connecting to {}".format(fanout.hostname, fanout_port, dut_port)) fanout.no_shutdown(fanout_port) wait_until(30, 1, self.__check_if_status, dut, dut_port, 'up') - status = self.__get_dut_if_status(dut, dut_port)[dut_port] - logging.debug("Interface fact : {}".format(status)) - assert status['oper_state'] == 'up', "dut port {} didn't come up as expected".format(dut_port) + assert self.__check_if_status(dut, dut_port, 'up', verbose=True), "dut port {} didn't go down as expected".format(dut_port) self.ports_shutdown_by_test.discard((fanout, fanout_port))