From c5ad69193b1c24ef9ad92544427c986babc109d2 Mon Sep 17 00:00:00 2001 From: isabel Date: Fri, 3 Dec 2021 22:26:25 -0800 Subject: [PATCH 1/7] Add pfcwd interval config tests --- .../test_pfcwd_interval.py | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 tests/generic_config_updater/test_pfcwd_interval.py diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py new file mode 100644 index 00000000000..1a3e2d13b94 --- /dev/null +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -0,0 +1,135 @@ +import logging +import json +import pytest + +from tests.common.helpers.assertions import pytest_assert +from tests.common.utilities import wait_until +from tests.common.config_reload import config_reload +from tests.generic_config_updater.gu_utils import apply_patch, expect_op_success, expect_res_success, expect_op_failure +from tests.generic_config_updater.gu_utils import generate_tmpfile, delete_tmpfile + +pytestmark = [ + pytest.mark.sanity_check(skip_sanity=True), + pytest.mark.asic('mellanox') +] + +logger = logging.getLogger(__name__) + + +@pytest.fixture(scope="module") +def ensure_dut_readiness(duthost): + """ + Setup/teardown fixture for incremental qos config update tst + + Args: + duthost: DUT host object + """ + config_tmpfile = generate_tmpfile(duthost) + logger.info("config_tmpfile {}".format(config_tmpfile)) + logger.info("Backing up config_db.json") + duthost.shell("sudo cp /etc/sonic/config_db.json {}".format(config_tmpfile)) + + yield + + logger.info("Restoring config_db.json") + duthost.shell("sudo cp {} /etc/sonic/config_db.json".format(config_tmpfile)) + delete_tmpfile(duthost, config_tmpfile) + config_reload(duthost) + + logger.info("TEARDOWN COMPLETED") + + +def prepare_pfcwd_interval_config(duthost, value): + """ + Prepares config db by setting pfcwd poll interval to specified value. If value is empty string, delete the current entry. + + Args: + duthost: DUT host object + value: poll interval value to be set + """ + + logger.info("Setting configdb entry pfcwd poll interval to value: {}".format(value)) + + if value: + cmd = "sudo pfcwd interval {}".format(value) + else: + cmd = "sonic-db-cli CONFIG_DB del \PFC_WD\GLOBAL\POLL_INTERVAL" + + +def get_detection_restoration_times(duthost): + """ + Returns detection_time, restoration_time for an interface. Poll_interval must be greater than both in order to be valid + + Args: + duthost: DUT host object + """ + + pfcwd_config = duthost.shell("show pfcwd config") + pytest_assert(not pfcwd_config['rc'], "Unable to read pfcwd config") + + for line in pfcwd_config['stdout_lines']: + if line.startswith('Ethernet'): + interface = line.split()[0] #Since line starts with Ethernet, we can safely use 0 index + + cmd = "sonic-db-cli CONFIG_DB hget \"PFC_WD|{}\" \"detection_time\" ".format(interface) + output = duthost.shell(cmd, module_ignore_errors=True) + pytest_assert(not output['rc'], "Unable to read detection time") + detection_time = output["stdout"] + + cmd = "sonic-db-cli CONFIG_DB hget \"PFC_WD|{}\" \"restoration_time\" ".format(interface) + output = duthost.shell(cmd, module_ignore_errors=True) + pytest_assert(not output['rc'], "Unable to read restoration time") + restoration_time = output["stdout"] + + return int(detection_time), int(restoration_time) + + pytest_assert(True, "Failed to read detection_time and/or restoration time") + + +def get_new_interval(duthost, is_valid): + """ + Returns new interval value for pfcwd poll interval, based on the operation being performed + + Args: + duthost: DUT host object + is_valid: if is_valid is true, return a valid new interval. Config update should succeed. If is_valid is false, return an invalid new interval. Config update should fail. + """ + + detection_time, restoration_time = get_detection_restoration_times(duthost) + if is_valid: + return max(detection_time, restoration_time) + 10 + else: + return min(detection_time, restoration_time) - 10 + + +@pytest.mark.parametrize("operation", ["add", "replace", "remove"]) +@pytest.mark.parametrize("field_pre_status", ["existing", "nonexistent"]) +@pytest.mark.parametrize("is_valid_config_update", [True, False]) +def test_pfcwd_interval_config_updates(duthost, ensure_dut_readiness, operation, field_pre_status, is_valid_config_update): + new_interval = get_new_interval(duthost, is_valid_config_update) + + operation_to_new_value_map = {"add": "".format(new_interval), "replace": "".format(new_interval), "remove": ""} + detection_time, restoration_time = get_detection_restoration_times(duthost) + pre_status = max(detection_time, restoration_time) + field_pre_status_to_value_map = {"existing": "".format(pre_status), "nonexistent": ""} + + prepare_pfcwd_interval_config(duthost, field_pre_status_to_value_map[field_pre_status]) + + tmpfile = generate_tmpfile(duthost) + logger.info("tmpfile {} created for json patch of pfcwd poll interval and operation: {}".format(tmpfile, operation)) + value = operation_to_new_value_map[operation] + logger.info("value to be added to json patch: {}".format(value)) + + json_patch = [ + { + "op": "{}".format(operation), + "path": "/PFC_WD/GLOBAL/POLL_INTERVAL", + "value": "{}".format(value) + }] + + output = apply_patch(duthost, json_data=json_patch, dest_file=tmpfile) + + if is_valid_config_update: + expect_op_success(duthost, output) + else: + expect_op_failure(output) From 6294384cf799d1f48ea09b9622f05370d85e44da Mon Sep 17 00:00:00 2001 From: isabel Date: Sun, 5 Dec 2021 15:34:54 -0800 Subject: [PATCH 2/7] typo --- tests/generic_config_updater/test_pfcwd_interval.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index 1a3e2d13b94..711423b6c7d 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -108,10 +108,10 @@ def get_new_interval(duthost, is_valid): def test_pfcwd_interval_config_updates(duthost, ensure_dut_readiness, operation, field_pre_status, is_valid_config_update): new_interval = get_new_interval(duthost, is_valid_config_update) - operation_to_new_value_map = {"add": "".format(new_interval), "replace": "".format(new_interval), "remove": ""} + operation_to_new_value_map = {"add": "{}".format(new_interval), "replace": "{}".format(new_interval), "remove": ""} detection_time, restoration_time = get_detection_restoration_times(duthost) pre_status = max(detection_time, restoration_time) - field_pre_status_to_value_map = {"existing": "".format(pre_status), "nonexistent": ""} + field_pre_status_to_value_map = {"existing": "{}".format(pre_status), "nonexistent": ""} prepare_pfcwd_interval_config(duthost, field_pre_status_to_value_map[field_pre_status]) From 5d7790039ca907693a9cd1268392951a0b979970 Mon Sep 17 00:00:00 2001 From: isabelmsft <67024108+isabelmsft@users.noreply.github.com> Date: Tue, 7 Dec 2021 10:52:03 -0600 Subject: [PATCH 3/7] Update test_pfcwd_interval.py --- tests/generic_config_updater/test_pfcwd_interval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index 711423b6c7d..926d7a75b24 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -19,7 +19,7 @@ @pytest.fixture(scope="module") def ensure_dut_readiness(duthost): """ - Setup/teardown fixture for incremental qos config update tst + Setup/teardown fixture for pfcwd interval config update tst Args: duthost: DUT host object From 278800e3e72e0354ece14b240126643859b596f4 Mon Sep 17 00:00:00 2001 From: isabel Date: Wed, 26 Jan 2022 14:23:05 -0800 Subject: [PATCH 4/7] Add post check --- .../test_pfcwd_interval.py | 46 +++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index 926d7a75b24..bd9d0ded34c 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -7,14 +7,17 @@ from tests.common.config_reload import config_reload from tests.generic_config_updater.gu_utils import apply_patch, expect_op_success, expect_res_success, expect_op_failure from tests.generic_config_updater.gu_utils import generate_tmpfile, delete_tmpfile +from tests.generic_config_updater.gu_utils import create_checkpoint, delete_checkpoint, rollback_or_reload pytestmark = [ - pytest.mark.sanity_check(skip_sanity=True), pytest.mark.asic('mellanox') ] logger = logging.getLogger(__name__) +READ_FLEXCOUNTER_DB_TIMEOUT = 480 +READ_FLEXCOUNTER_DB_INTERVAL = 20 + @pytest.fixture(scope="module") def ensure_dut_readiness(duthost): @@ -24,19 +27,33 @@ def ensure_dut_readiness(duthost): Args: duthost: DUT host object """ - config_tmpfile = generate_tmpfile(duthost) - logger.info("config_tmpfile {}".format(config_tmpfile)) - logger.info("Backing up config_db.json") - duthost.shell("sudo cp /etc/sonic/config_db.json {}".format(config_tmpfile)) + create_checkpoint(duthost) yield - - logger.info("Restoring config_db.json") - duthost.shell("sudo cp {} /etc/sonic/config_db.json".format(config_tmpfile)) - delete_tmpfile(duthost, config_tmpfile) - config_reload(duthost) - logger.info("TEARDOWN COMPLETED") + try: + logger.info("Rolled back to original checkpoint") + rollback_or_reload(duthost) + finally: + delete_checkpoint(duthost) + + +def ensure_application_of_updated_config(duthost, value): + """ + Ensures application of the JSON patch config update by verifying field value presence in FLEX COUNTER DB + + Args: + duthost: DUT host object + value: expected value of POLL_INTERVAL + """ + def _confirm_value_in_flex_counter_db(): + poll_interval = duthost.shell('redis-cli -n 5 hget FLEX_COUNTER_GROUP_TABLE:PFC_WD POLL_INTERVAL')["stdout"] + return value == poll_interval + + pytest_assert( + wait_until(READ_FLEXCOUNTER_DB_TIMEOUT, READ_FLEXCOUNTER_DB_INTERVAL, 0, _confirm_value_in_flex_counter_db), + "FLEX COUNTER DB does not properly reflect newly POLL_INTERVAL expected value: {}".format(value) + ) def prepare_pfcwd_interval_config(duthost, value): @@ -97,12 +114,12 @@ def get_new_interval(duthost, is_valid): detection_time, restoration_time = get_detection_restoration_times(duthost) if is_valid: - return max(detection_time, restoration_time) + 10 + return max(detection_time, restoration_time) - 10 else: - return min(detection_time, restoration_time) - 10 + return min(detection_time, restoration_time) + 10 -@pytest.mark.parametrize("operation", ["add", "replace", "remove"]) +@pytest.mark.parametrize("operation", ["add", "replace"]) @pytest.mark.parametrize("field_pre_status", ["existing", "nonexistent"]) @pytest.mark.parametrize("is_valid_config_update", [True, False]) def test_pfcwd_interval_config_updates(duthost, ensure_dut_readiness, operation, field_pre_status, is_valid_config_update): @@ -131,5 +148,6 @@ def test_pfcwd_interval_config_updates(duthost, ensure_dut_readiness, operation, if is_valid_config_update: expect_op_success(duthost, output) + ensure_application_of_updated_config(duthost, value) else: expect_op_failure(output) From 62a0ee0f85877893dfb206531b25cad42e74d540 Mon Sep 17 00:00:00 2001 From: isabel Date: Wed, 26 Jan 2022 14:24:38 -0800 Subject: [PATCH 5/7] fix format --- tests/generic_config_updater/test_pfcwd_interval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index bd9d0ded34c..91f1a4a0eef 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -86,7 +86,7 @@ def get_detection_restoration_times(duthost): for line in pfcwd_config['stdout_lines']: if line.startswith('Ethernet'): - interface = line.split()[0] #Since line starts with Ethernet, we can safely use 0 index + interface = line.split()[0] # Since line starts with Ethernet, we can safely use 0 index cmd = "sonic-db-cli CONFIG_DB hget \"PFC_WD|{}\" \"detection_time\" ".format(interface) output = duthost.shell(cmd, module_ignore_errors=True) From c64ac2ce97775dabd0d18e832531850d4545d875 Mon Sep 17 00:00:00 2001 From: isabel Date: Tue, 1 Feb 2022 09:25:36 -0800 Subject: [PATCH 6/7] address PR comments --- tests/generic_config_updater/test_pfcwd_interval.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index 91f1a4a0eef..14d09fcf9c9 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -4,7 +4,6 @@ from tests.common.helpers.assertions import pytest_assert from tests.common.utilities import wait_until -from tests.common.config_reload import config_reload from tests.generic_config_updater.gu_utils import apply_patch, expect_op_success, expect_res_success, expect_op_failure from tests.generic_config_updater.gu_utils import generate_tmpfile, delete_tmpfile from tests.generic_config_updater.gu_utils import create_checkpoint, delete_checkpoint, rollback_or_reload @@ -47,7 +46,7 @@ def ensure_application_of_updated_config(duthost, value): value: expected value of POLL_INTERVAL """ def _confirm_value_in_flex_counter_db(): - poll_interval = duthost.shell('redis-cli -n 5 hget FLEX_COUNTER_GROUP_TABLE:PFC_WD POLL_INTERVAL')["stdout"] + poll_interval = duthost.shell('sonic-db-cli PFC_WD_DB hget FLEX_COUNTER_GROUP_TABLE:PFC_WD POLL_INTERVAL')["stdout"] return value == poll_interval pytest_assert( @@ -68,9 +67,11 @@ def prepare_pfcwd_interval_config(duthost, value): logger.info("Setting configdb entry pfcwd poll interval to value: {}".format(value)) if value: - cmd = "sudo pfcwd interval {}".format(value) + cmd = "pfcwd interval {}".format(value) else: cmd = "sonic-db-cli CONFIG_DB del \PFC_WD\GLOBAL\POLL_INTERVAL" + + duthost.shell(cmd) def get_detection_restoration_times(duthost): From bcd3e72309c12648b68114ba3452750463e09a00 Mon Sep 17 00:00:00 2001 From: isabel Date: Tue, 1 Feb 2022 10:17:14 -0800 Subject: [PATCH 7/7] Add try catch --- .../generic_config_updater/test_pfcwd_interval.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/generic_config_updater/test_pfcwd_interval.py b/tests/generic_config_updater/test_pfcwd_interval.py index 14d09fcf9c9..2b3d15fcd08 100644 --- a/tests/generic_config_updater/test_pfcwd_interval.py +++ b/tests/generic_config_updater/test_pfcwd_interval.py @@ -145,10 +145,13 @@ def test_pfcwd_interval_config_updates(duthost, ensure_dut_readiness, operation, "value": "{}".format(value) }] - output = apply_patch(duthost, json_data=json_patch, dest_file=tmpfile) + try: + output = apply_patch(duthost, json_data=json_patch, dest_file=tmpfile) - if is_valid_config_update: - expect_op_success(duthost, output) - ensure_application_of_updated_config(duthost, value) - else: - expect_op_failure(output) + if is_valid_config_update: + expect_op_success(duthost, output) + ensure_application_of_updated_config(duthost, value) + else: + expect_op_failure(output) + finally: + delete_tmpfile(duthost, tmpfile)