Skip to content
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

[GCU] [MA] Adding support in existing tests - Common changes #15182

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion tests/common/gu_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ def apply_patch(duthost, json_data, dest_file):
json_data: Source json patch to apply
dest_file: Destination file on duthost
"""
duthost.copy(content=json.dumps(json_data, indent=4), dest=dest_file)
patch_content = json.dumps(json_data, indent=4)
duthost.copy(content=patch_content, dest=dest_file)
logger.debug("Patch Content: {}".format(patch_content))
okaravasi marked this conversation as resolved.
Show resolved Hide resolved

cmds = 'config apply-patch {}'.format(dest_file)

Expand Down Expand Up @@ -512,6 +514,56 @@ def expect_acl_rule_removed(duthost, rulename, setup):
pytest_assert(removed, "'{}' showed a rule, this following rule should have been removed".format(cmds))


def save_backup_test_config(duthost, file_postfix="bkp"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

save_backup_test_config

We do have checkpoint and rollback cli.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was already available as part of setup fixtures in the GCU test suites.
The main change introduced by this PR is moving the code into a separate function to avoid code duplication, as it is used by multiple setup module fixtures. Additionally, support has been added to save backup files for different namespaces on multi-ASIC platforms.
Most GCU setup module fixtures create a checkpoint and save backup configuration JSON files before the test (this block of code), then roll back to the checkpoint afterward. If the rollback fails, these backup files are used to reload the configdb.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the code could be existing in old implementation, but my point is that we should leverage the checkpoint and rollback feature.

"""Save test env before a test case starts.
Back up the existing config_db.json file(s).
Args:
duthost: Device Under Test (DUT)
file_postfix: Postfix string to be used for the backup files.
Returns:
None.
"""
CONFIG_DB = "/etc/sonic/config_db.json"
CONFIG_DB_BACKUP = "/etc/sonic/config_db.json.{}".format(file_postfix)

logger.info("Backup {} to {} on {}".format(
CONFIG_DB, CONFIG_DB_BACKUP, duthost.hostname))
duthost.shell("cp {} {}".format(CONFIG_DB, CONFIG_DB_BACKUP))
if duthost.is_multi_asic:
for n in range(len(duthost.asics)):
asic_config_db = "/etc/sonic/config_db{}.json".format(n)
asic_config_db_backup = "/etc/sonic/config_db{}.json.{}".format(n, file_postfix)
logger.info("Backup {} to {} on {}".format(
asic_config_db, asic_config_db_backup, duthost.hostname))
duthost.shell("cp {} {}".format(asic_config_db, asic_config_db_backup))


def restore_backup_test_config(duthost, file_postfix="bkp", config_reload=True):
"""Restore test env after a test case finishes.
Args:
duthost: Device Under Test (DUT)
file_postfix: Postfix string to be used for restoring the saved backup files.
Returns:
None.
"""
CONFIG_DB = "/etc/sonic/config_db.json"
CONFIG_DB_BACKUP = "/etc/sonic/config_db.json.{}".format(file_postfix)

logger.info("Restore {} with {} on {}".format(
CONFIG_DB, CONFIG_DB_BACKUP, duthost.hostname))
duthost.shell("mv {} {}".format(CONFIG_DB_BACKUP, CONFIG_DB))
if duthost.is_multi_asic:
for n in range(len(duthost.asics)):
asic_config_db = "/etc/sonic/config_db{}.json".format(n)
asic_config_db_backup = "/etc/sonic/config_db{}.json.{}".format(n, file_postfix)
logger.info("Restore {} with {} on {}".format(
asic_config_db, asic_config_db_backup, duthost.hostname))
duthost.shell("mv {} {}".format(asic_config_db_backup, asic_config_db))

if config_reload:
config_reload(duthost)


def get_bgp_speaker_runningconfig(duthost):
""" Get bgp speaker config that contains src_address and ip_range

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -801,9 +801,9 @@ fib/test_fib.py::test_ipinip_hash:
#######################################
generic_config_updater:
skip:
reason: 'generic_config_updater is not a supported feature for T2'
reason: 'generic_config_updater is not a supported feature for T2 platform on older releases than 202205.'
conditions:
- "'t2' in topo_name"
- "('t2' in topo_name) and (release in ['201811', '201911', '202012', '202106', '202111'])"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no 202211, 202305, 202311. We will start support from 202405.


generic_config_updater/test_bgp_prefix.py::test_bgp_prefix_tc1_suite[empty]:
skip:
Expand Down
25 changes: 24 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,29 @@ def rand_one_dut_lossless_prio(request):
return lossless_prio_list[0]


@pytest.fixture(scope="module")
def rand_asic_namespace(request, duthosts, rand_one_dut_hostname):
"""
Return the randomly selected asic namespace in case of multi-asic duthost.
"""

if "rand_one_dut_front_end_hostname" in request.fixturenames:
dut_hostname = request.getfixturevalue("rand_one_dut_front_end_hostname")
else:
dut_hostname = rand_one_dut_hostname

duthost = duthosts[dut_hostname]

asic_namespace = None
asic_index = None
if duthost.is_multi_asic:
namespace_list = duthost.get_asic_namespace_list()
asic_namespace = random.choice(namespace_list)
asic_index = duthost.get_asic_id_from_namespace(asic_namespace)

return asic_namespace, asic_index
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two functions are identical.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, they could potentially use different duthost types. rand_asic_namespace can use any type of duthost, while rand_front_end_asic_namespace requires only front-end duthosts. This separation was necessary because some features run only on front-end duthosts in a multi-ASIC T2 platform and are not applicable to supervisor cards. Test suites are adapted accordingly based on their requirements to select the appropriate fixture, depending on when the feature is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, although these two functions execute differently, the code is indeed duplicated. I will work on a solution to keep a single common function that adapts based on the availability of the two duthost fixtures.



@pytest.fixture(scope="module", autouse=True)
def reset_critical_services_list(duthosts):
"""
Expand Down Expand Up @@ -2204,7 +2227,7 @@ def dut_test_params_qos(duthosts, tbinfo, ptfhost, get_src_dst_asic_and_duts, lo
yield rtn_dict


@ pytest.fixture(scope='class')
@pytest.fixture(scope='class')
def dut_test_params(duthosts, enum_rand_one_per_hwsku_frontend_hostname, tbinfo,
ptf_portmap_file, lower_tor_host, creds): # noqa F811
"""
Expand Down
55 changes: 32 additions & 23 deletions tests/generic_config_updater/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from tests.common.utilities import skip_release
from tests.common.config_reload import config_reload
from tests.common.gu_utils import apply_patch
from tests.common.gu_utils import apply_patch, restore_backup_test_config, save_backup_test_config
from tests.common.gu_utils import generate_tmpfile, delete_tmpfile

CONFIG_DB = "/etc/sonic/config_db.json"
Expand All @@ -12,48 +12,61 @@
logger = logging.getLogger(__name__)


@pytest.fixture(scope="module")
def selected_dut_hostname(request, rand_one_dut_hostname):
"""Fixture that returns either `rand_one_dut_hostname` or `rand_one_dut_front_end_hostname`
depending on availability."""
if "rand_one_dut_front_end_hostname" in request.fixturenames:
logger.info("Running on front end duthost")
return request.getfixturevalue("rand_one_dut_front_end_hostname")
else:
logger.info("Running on any type of duthost")
return rand_one_dut_hostname


# Module Fixture
@pytest.fixture(scope="module")
def cfg_facts(duthosts, rand_one_dut_hostname):
def cfg_facts(duthosts, selected_dut_hostname, rand_asic_namespace):
"""
Config facts for selected DUT
Args:
duthosts: list of DUTs.
rand_one_dut_hostname: Hostname of a random chosen dut
selected_dut_hostname: Hostname of a random chosen dut
"""
duthost = duthosts[rand_one_dut_hostname]
return duthost.config_facts(host=duthost.hostname, source="persistent")['ansible_facts']
duthost = duthosts[selected_dut_hostname]
asic_namespace, asic_id = rand_asic_namespace
return duthost.config_facts(host=duthost.hostname, source="persistent", namespace=asic_namespace)['ansible_facts']


@pytest.fixture(scope="module", autouse=True)
def check_image_version(duthosts, rand_one_dut_hostname):
def check_image_version(duthosts, selected_dut_hostname):
"""Skips this test if the SONiC image installed on DUT is older than 202111

Args:
duthosts: list of DUTs.
rand_one_dut_hostname: Hostname of a random chosen dut
selected_dut_hostname: Hostname of a random chosen dut

Returns:
None.
"""
duthost = duthosts[rand_one_dut_hostname]
duthost = duthosts[selected_dut_hostname]
skip_release(duthost, ["201811", "201911", "202012", "202106", "202111"])


@pytest.fixture(scope="module", autouse=True)
def reset_and_restore_test_environment(duthosts, rand_one_dut_hostname):
def reset_and_restore_test_environment(duthosts, selected_dut_hostname):
"""Reset and restore test env if initial Config cannot pass Yang

Back up the existing config_db.json file and restore it once the test ends.

Args:
duthosts: list of DUTs.
rand_one_dut_hostname: Hostname of a random chosen dut
selected_dut_hostname: Hostname of a random chosen dut

Returns:
None.
"""
duthost = duthosts[rand_one_dut_hostname]
duthost = duthosts[selected_dut_hostname]
json_patch = []
tmpfile = generate_tmpfile(duthost)

Expand All @@ -62,9 +75,7 @@ def reset_and_restore_test_environment(duthosts, rand_one_dut_hostname):
finally:
delete_tmpfile(duthost, tmpfile)

logger.info("Backup {} to {} on {}".format(
CONFIG_DB, CONFIG_DB_BACKUP, duthost.hostname))
duthost.shell("cp {} {}".format(CONFIG_DB, CONFIG_DB_BACKUP))
save_backup_test_config(duthost, file_postfix="before_gcu_test")

if output['rc'] or "Patch applied successfully" not in output['stdout']:
logger.info("Running config failed SONiC Yang validation. Reload minigraph. config: {}"
Expand All @@ -73,27 +84,25 @@ def reset_and_restore_test_environment(duthosts, rand_one_dut_hostname):

yield

logger.info("Restore {} with {} on {}".format(
CONFIG_DB, CONFIG_DB_BACKUP, duthost.hostname))
duthost.shell("mv {} {}".format(CONFIG_DB_BACKUP, CONFIG_DB))
restore_backup_test_config(duthost, file_postfix="before_gcu_test", config_reload=False)

if output['rc'] or "Patch applied successfully" not in output['stdout']:
logger.info("Restore Config after GCU test.")
config_reload(duthost)


@pytest.fixture(scope="module", autouse=True)
def verify_configdb_with_empty_input(duthosts, rand_one_dut_hostname):
def verify_configdb_with_empty_input(duthosts, selected_dut_hostname):
"""Fail immediately if empty input test failure

Args:
duthosts: list of DUTs.
rand_one_dut_hostname: Hostname of a random chosen dut
selected_dut_hostname: Hostname of a random chosen dut

Returns:
None.
"""
duthost = duthosts[rand_one_dut_hostname]
duthost = duthosts[selected_dut_hostname]
json_patch = []
tmpfile = generate_tmpfile(duthost)

Expand All @@ -119,19 +128,19 @@ def skip_when_buffer_is_dynamic_model(duthost):

# Function Fixture
@pytest.fixture(autouse=True)
def ignore_expected_loganalyzer_exceptions(duthosts, rand_one_dut_hostname, loganalyzer):
def ignore_expected_loganalyzer_exceptions(duthosts, selected_dut_hostname, loganalyzer):
"""
Ignore expected yang validation failure during test execution

GCU will try several sortings of JsonPatch until the sorting passes yang validation

Args:
duthosts: list of DUTs.
rand_one_dut_hostname: Hostname of a random chosen dut
selected_dut_hostname: Hostname of a random chosen dut
loganalyzer: Loganalyzer utility fixture
"""
# When loganalyzer is disabled, the object could be None
duthost = duthosts[rand_one_dut_hostname]
duthost = duthosts[selected_dut_hostname]
if loganalyzer:
ignoreRegex = [
".*ERR sonic_yang.*",
Expand Down
Loading