From a417a2118bc0763078c87d1f12da1b58daae3203 Mon Sep 17 00:00:00 2001 From: jianquanye Date: Thu, 14 Mar 2024 22:33:33 +0800 Subject: [PATCH 1/2] Revert "[image_download] Add ipv6 only mgmt image download test (#11936)" This reverts commit f5cdac4d57a2beaf6f48896f5fab56bcb6a22a15. --- .../tests_mark_conditions.yaml | 6 ----- tests/ip/test_mgmt_ipv6_only.py | 22 +------------------ 2 files changed, 1 insertion(+), 27 deletions(-) diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml index aae46373cdf..11f1d880238 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml @@ -757,12 +757,6 @@ ip/test_ip_packet.py::TestIPPacket::test_forward_ip_packet_with_0xffff_chksum_to conditions: - "asic_type in ['mellanox'] or asic_subtype in ['broadcom-dnx']" -ip/test_mgmt_ipv6_only.py::test_image_download_ipv6_only: - skip: - reason: "Skipping mgmt ipv6 test for mgmt topo" - conditions: - - "topo_type in ['m0', 'mx']" - ####################################### ##### ipfwd ##### ####################################### diff --git a/tests/ip/test_mgmt_ipv6_only.py b/tests/ip/test_mgmt_ipv6_only.py index a80ae631c49..03301361a9e 100644 --- a/tests/ip/test_mgmt_ipv6_only.py +++ b/tests/ip/test_mgmt_ipv6_only.py @@ -1,7 +1,7 @@ import pytest from tests.common.fixtures.duthost_utils import convert_and_restore_config_db_to_ipv6_only # noqa F401 -from tests.common.helpers.assertions import pytest_assert, pytest_require +from tests.common.helpers.assertions import pytest_assert pytestmark = [ pytest.mark.topology('any'), @@ -58,23 +58,3 @@ def test_show_features_ipv6_only(duthosts, enum_dut_hostname, convert_and_restor .format(cmd_key), module_ignore_errors=False)['stdout'] pytest_assert(redis_value.lower() == cmd_value.lower(), "'{}' is '{}' which does not match with config_db".format(cmd_key, cmd_value)) - - -def test_image_download_ipv6_only(creds, duthosts, enum_dut_hostname, - convert_and_restore_config_db_to_ipv6_only): # noqa F811 - """ - Test image download in mgmt ipv6 only scenario - """ - duthost = duthosts[enum_dut_hostname] - image_url = creds.get("test_image_url", {}).get("ipv6", "") - pytest_require(len(image_url) != 0, "Cannot get image url") - cfg_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] - mgmt_interfaces = cfg_facts.get("MGMT_INTERFACE", {}).keys() - for mgmt_interface in mgmt_interfaces: - output = duthost.shell("curl --fail --interface {} {}".format(mgmt_interface, image_url), - module_ignore_errors=True) - if output["rc"] == 0: - break - else: - pytest.fail("Failed to download image from image_url {} via any of {}" - .format(image_url, list(mgmt_interfaces))) From 2e0fc3c9caa0bc9d788a249177908c351ce0fbc0 Mon Sep 17 00:00:00 2001 From: jianquanye Date: Thu, 14 Mar 2024 22:33:54 +0800 Subject: [PATCH 2/2] Revert "[IPv6 only]Add a fixture to convert the DUT to IPv6 only, and enhance the connection plugin to retry with IPv6 addr if IPv4 addr is unavailable (#11957)" This reverts commit afd7fcebc0a0219c84c81e905fad0d23ee1bc346. --- .../plugins/connection/multi_passwd_ssh.py | 44 +----- tests/common/devices/duthosts.py | 13 +- tests/common/fixtures/duthost_utils.py | 134 +----------------- tests/ip/test_mgmt_ipv6_only.py | 60 -------- 4 files changed, 4 insertions(+), 247 deletions(-) delete mode 100644 tests/ip/test_mgmt_ipv6_only.py diff --git a/ansible/plugins/connection/multi_passwd_ssh.py b/ansible/plugins/connection/multi_passwd_ssh.py index c892cc780c3..f14fd7d3eef 100644 --- a/ansible/plugins/connection/multi_passwd_ssh.py +++ b/ansible/plugins/connection/multi_passwd_ssh.py @@ -1,12 +1,10 @@ import imp import os -import logging from functools import wraps -from ansible.errors import AnsibleAuthenticationFailure, AnsibleConnectionFailure +from ansible.errors import AnsibleAuthenticationFailure from ansible.plugins import connection -logger = logging.getLogger(__name__) # HACK: workaround to import the SSH connection plugin _ssh_mod = os.path.join(os.path.dirname(connection.__file__), "ssh.py") @@ -28,16 +26,8 @@ vars: - name: ansible_altpasswords - name: ansible_ssh_altpasswords - hostv6: - description: IPv6 address - vars: - - name: ansible_hostv6 """.lstrip("\n") -# A sample error message that host unreachable: -# 'Failed to connect to the host via ssh: ssh: connect to host 192.168.0.2 port 22: Connection timed out' -CONNECTION_TIMEOUT_ERR_FLAG = "Connection timed out" - def _password_retry(func): """ @@ -47,38 +37,6 @@ def _password_retry(func): """ @wraps(func) def wrapped(self, *args, **kwargs): - - # If the host have an IPv6 address, try connect IPv4 address first, - # If IPv4 host unavailable, fall back to use IPv6 address - try: - hostv6 = self.get_option("hostv6") - except KeyError: - hostv6 = None - - if hostv6: - try: - return func(self, *args, **kwargs) - except AnsibleConnectionFailure as e: - logger.info("First connection failed: {}".format(str(e))) - if CONNECTION_TIMEOUT_ERR_FLAG in e.message: - self._play_context.remote_addr = hostv6 - # args sample: - # ( [b'sshpass', b'-d18', b'ssh', b'-o', b'ControlMaster=auto', b'-o', b'ControlPersist=120s', b'-o', b'UserKnownHostsFile=/dev/null', b'-o', b'StrictHostKeyChecking=no', b'-o', b'StrictHostKeyChecking=no', b'-o', b'User="admin"', b'-o', b'ConnectTimeout=60', b'-o', b'ControlPath="/home/user/.ansible/cp/376bdcc730"', 'fc00:1234:5678:abcd::2', b'/bin/sh -c \'echo PLATFORM; uname; echo FOUND; command -v \'"\'"\'python3.10\'"\'"\'; command -v \'"\'"\'python3.9\'"\'"\'; command -v \'"\'"\'python3.8\'"\'"\'; command -v \'"\'"\'python3.7\'"\'"\'; command -v \'"\'"\'python3.6\'"\'"\'; command -v \'"\'"\'python3.5\'"\'"\'; command -v \'"\'"\'/usr/bin/python3\'"\'"\'; command -v \'"\'"\'/usr/libexec/platform-python\'"\'"\'; command -v \'"\'"\'python2.7\'"\'"\'; command -v \'"\'"\'/usr/bin/python\'"\'"\'; command -v \'"\'"\'python\'"\'"\'; echo ENDFOUND && sleep 0\''], None) # noqa: E501 - # args[0] are the parameters of ssh connection - ssh_args = args[0] - # Change the IPv4 host in the ssh_args to IPv6 - for idx in range(len(ssh_args)): - if type(ssh_args[idx]) == bytes and ssh_args[idx].decode() == self.host: - ssh_args[idx] = hostv6 - self.host = hostv6 - self.set_option("host", hostv6) - except BaseException as e: - # Only catch the connection error, won't block the multi-password functionality - logger.info("First connection failed: {}".format(str(e))) - - # Reset the sshpass_pipe for the new connections to be created - self.sshpass_pipe = os.pipe() - password = self.get_option("password") or self._play_context.password conn_passwords = [password] altpassword = self.get_option("altpassword") diff --git a/tests/common/devices/duthosts.py b/tests/common/devices/duthosts.py index 8474a424c12..1fe548e5fa8 100644 --- a/tests/common/devices/duthosts.py +++ b/tests/common/devices/duthosts.py @@ -54,15 +54,9 @@ def __init__(self, ansible_adhoc, tbinfo, duts): DUTs in the testbed should be used """ - self.ansible_adhoc = ansible_adhoc - self.tbinfo = tbinfo - self.duts = duts - self.__initialize_nodes() - - def __initialize_nodes(self): # TODO: Initialize the nodes in parallel using multi-threads? - self.nodes = self._Nodes([MultiAsicSonicHost(self.ansible_adhoc, hostname, self, self.tbinfo['topo']['type']) - for hostname in self.tbinfo["duts"] if hostname in self.duts]) + self.nodes = self._Nodes([MultiAsicSonicHost(ansible_adhoc, hostname, self, tbinfo['topo']['type']) + for hostname in tbinfo["duts"] if hostname in duts]) self.supervisor_nodes = self._Nodes([node for node in self.nodes if node.is_supervisor_node()]) self.frontend_nodes = self._Nodes([node for node in self.nodes if node.is_frontend_node()]) @@ -131,6 +125,3 @@ def config_facts(self, *module_args, **complex_args): complex_args['host'] = node.hostname result[node.hostname] = node.config_facts(*module_args, **complex_args)['ansible_facts'] return result - - def reset(self): - self.__initialize_nodes() diff --git a/tests/common/fixtures/duthost_utils.py b/tests/common/fixtures/duthost_utils.py index 51e8a4a08c4..fb4007d6441 100644 --- a/tests/common/fixtures/duthost_utils.py +++ b/tests/common/fixtures/duthost_utils.py @@ -1,5 +1,3 @@ -from typing import Dict, List - import pytest import logging import itertools @@ -7,12 +5,10 @@ import ipaddress import time import json - -from tests.common import config_reload from tests.common.helpers.assertions import pytest_assert from tests.common.utilities import wait_until from jinja2 import Template -from netaddr import valid_ipv4, valid_ipv6 +from netaddr import valid_ipv4 logger = logging.getLogger(__name__) @@ -602,131 +598,3 @@ def check_bgp_router_id(duthost, mgFacts): return False except Exception as e: logger.error("Error loading BGP routerID - {}".format(e)) - - -@pytest.fixture(scope="module") -def convert_and_restore_config_db_to_ipv6_only(duthosts): - """Back up the existing config_db.json file and restore it once the test ends. - - Some cases will update the running config during the test and save the config - to be recovered after reboot. In such a case we need to backup config_db.json before - the test starts and then restore it after the test ends. - """ - config_db_file = "/etc/sonic/config_db.json" - config_db_bak_file = "/etc/sonic/config_db.json.before_ipv6_only" - - # Sample MGMT_INTERFACE: - # "MGMT_INTERFACE": { - # "eth0|192.168.0.2/24": { - # "forced_mgmt_routes": [ - # "192.168.1.1/24" - # ], - # "gwaddr": "192.168.0.1" - # }, - # "eth0|fc00:1234:5678:abcd::2/64": { - # "gwaddr": "fc00:1234:5678:abcd::1", - # "forced_mgmt_routes": [ - # "fc00:1234:5678:abc1::1/64" - # ] - # } - # } - - # duthost_name: config_db_modified - config_db_modified: Dict[str, bool] = {duthost.hostname: False - for duthost in duthosts.nodes} - # duthost_name: [ip_addr] - ipv4_address: Dict[str, List] = {duthost.hostname: [] - for duthost in duthosts.nodes} - ipv6_address: Dict[str, List] = {duthost.hostname: [] - for duthost in duthosts.nodes} - # Check IPv6 mgmt-ip is set, otherwise the DUT will lose control after v4 mgmt-ip is removed - for duthost in duthosts.nodes: - mgmt_interface = json.loads(duthost.shell(f"jq '.MGMT_INTERFACE' {config_db_file}", - module_ignore_errors=True)["stdout"]) - # Use list() to make a copy of mgmt_interface.keys() to avoid - # "RuntimeError: dictionary changed size during iteration" error - for key in list(mgmt_interface): - ip_addr = key.split("|")[1] - ip_addr_without_mask = ip_addr.split('/')[0] - if ip_addr: - is_ipv6 = valid_ipv6(ip_addr_without_mask) - if is_ipv6: - logger.info(f"Host[{duthost.hostname}] IPv6[{ip_addr}]") - ipv6_address[duthost.hostname].append(ip_addr_without_mask) - pytest_assert(len(ipv6_address[duthost.hostname]) > 0, - f"{duthost.hostname} doesn't have IPv6 Management IP address") - - # Remove IPv4 mgmt-ip - for duthost in duthosts.nodes: - logger.info(f"Backup {config_db_file} to {config_db_bak_file} on {duthost.hostname}") - duthost.shell(f"cp {config_db_file} {config_db_bak_file}") - mgmt_interface = json.loads(duthost.shell(f"jq '.MGMT_INTERFACE' {config_db_file}", - module_ignore_errors=True)["stdout"]) - - # Use list() to make a copy of mgmt_interface.keys() to avoid - # "RuntimeError: dictionary changed size during iteration" error - for key in list(mgmt_interface): - ip_addr = key.split("|")[1] - ip_addr_without_mask = ip_addr.split('/')[0] - if ip_addr: - is_ipv4 = valid_ipv4(ip_addr_without_mask) - if is_ipv4: - ipv4_address[duthost.hostname].append(ip_addr_without_mask) - logger.info(f"Removing host[{duthost.hostname}] IPv4[{ip_addr}]") - duthost.shell(f"""jq 'del(."MGMT_INTERFACE"."{key}")' {config_db_file} > temp.json""" - f"""&& mv temp.json {config_db_file}""", module_ignore_errors=True) - config_db_modified[duthost.hostname] = True - config_reload(duthost, wait=120) - duthosts.reset() - - # Verify mgmt-interface status - mgmt_intf_name = "eth0" - for duthost in duthosts.nodes: - logger.info(f"Checking host[{duthost.hostname}] mgmt interface[{mgmt_intf_name}]") - mgmt_intf_ifconfig = duthost.shell(f"ifconfig {mgmt_intf_name}", module_ignore_errors=True)["stdout"] - assert_addr_in_ifconfig(addr_set=ipv4_address, hostname=duthost.hostname, - expect_exists=False, ifconfig_output=mgmt_intf_ifconfig) - assert_addr_in_ifconfig(addr_set=ipv6_address, hostname=duthost.hostname, - expect_exists=True, ifconfig_output=mgmt_intf_ifconfig) - - yield - - # Recover IPv4 mgmt-ip - for duthost in duthosts.nodes: - if config_db_modified[duthost.hostname]: - logger.info(f"Restore {config_db_file} with {config_db_bak_file} on {duthost.hostname}") - duthost.shell(f"mv {config_db_bak_file} {config_db_file}") - config_reload(duthost, safe_reload=True) - duthosts.reset() - - # Verify mgmt-interface status - for duthost in duthosts.nodes: - logger.info(f"Checking host[{duthost.hostname}] mgmt interface[{mgmt_intf_name}]") - mgmt_intf_ifconfig = duthost.shell(f"ifconfig {mgmt_intf_name}", module_ignore_errors=True)["stdout"] - assert_addr_in_ifconfig(addr_set=ipv4_address, hostname=duthost.hostname, - expect_exists=True, ifconfig_output=mgmt_intf_ifconfig) - assert_addr_in_ifconfig(addr_set=ipv6_address, hostname=duthost.hostname, - expect_exists=True, ifconfig_output=mgmt_intf_ifconfig) - - -def assert_addr_in_ifconfig(addr_set: Dict[str, List], hostname: str, expect_exists: bool, ifconfig_output: str): - """ - Assert the address status in the ifconfig output, - if status not as expected, assert as failure - - @param addr_set: addr_set, key is dut hostname, value is the list of ip addresses - @param hostname: hostname - @param expect_exists: Expectation of the ip, - True means expect all ip addresses in addr_set appears in the output of ifconfig - False means expect no ip addresses in addr_set appears in the output of ifconfig - @param ifconfig_output: output of 'ifconfig' - """ - for addr in addr_set[hostname]: - if expect_exists: - pytest_assert(addr in ifconfig_output, - f"{addr} not appeared in {hostname} mgmt interface") - logger.info(f"{addr} exists in the output of ifconfig") - else: - pytest_assert(addr not in ifconfig_output, - f"{hostname} mgmt interface still with addr {addr}") - logger.info(f"{addr} not exists in the output of ifconfig which is expected") diff --git a/tests/ip/test_mgmt_ipv6_only.py b/tests/ip/test_mgmt_ipv6_only.py deleted file mode 100644 index 03301361a9e..00000000000 --- a/tests/ip/test_mgmt_ipv6_only.py +++ /dev/null @@ -1,60 +0,0 @@ -import pytest - -from tests.common.fixtures.duthost_utils import convert_and_restore_config_db_to_ipv6_only # noqa F401 -from tests.common.helpers.assertions import pytest_assert - -pytestmark = [ - pytest.mark.topology('any'), - pytest.mark.device_type('vs') -] - - -def test_bgp_facts_ipv6_only(duthosts, enum_frontend_dut_hostname, enum_asic_index, - convert_and_restore_config_db_to_ipv6_only): # noqa F811 - """compare the bgp facts between observed states and target state""" - - duthost = duthosts[enum_frontend_dut_hostname] - - bgp_facts = duthost.bgp_facts(instance_id=enum_asic_index)['ansible_facts'] - namespace = duthost.get_namespace_from_asic_id(enum_asic_index) - config_facts = duthost.config_facts(host=duthost.hostname, source="running", namespace=namespace)['ansible_facts'] - sonic_db_cmd = "sonic-db-cli {}".format("-n " + namespace if namespace else "") - for k, v in list(bgp_facts['bgp_neighbors'].items()): - # Verify bgp sessions are established - assert v['state'] == 'established' - # Verify local ASNs in bgp sessions - assert v['local AS'] == int(config_facts['DEVICE_METADATA']['localhost']['bgp_asn'].encode().decode("utf-8")) - # Check bgpmon functionality by validate STATE DB contains this neighbor as well - state_fact = duthost.shell('{} STATE_DB HGET "NEIGH_STATE_TABLE|{}" "state"' - .format(sonic_db_cmd, k), module_ignore_errors=False)['stdout_lines'] - peer_type = duthost.shell('{} STATE_DB HGET "NEIGH_STATE_TABLE|{}" "peerType"' - .format(sonic_db_cmd, k), - module_ignore_errors=False)['stdout_lines'] - assert state_fact[0] == "Established" - assert peer_type[0] == "i-BGP" if v['remote AS'] == v['local AS'] else "e-BGP" - - # In multi-asic, would have 'BGP_INTERNAL_NEIGHBORS' and possibly no 'BGP_NEIGHBOR' (ebgp) neighbors. - nbrs_in_cfg_facts = {} - nbrs_in_cfg_facts.update(config_facts.get('BGP_NEIGHBOR', {})) - nbrs_in_cfg_facts.update(config_facts.get('BGP_INTERNAL_NEIGHBOR', {})) - # In VoQ Chassis, we would have BGP_VOQ_CHASSIS_NEIGHBOR as well. - nbrs_in_cfg_facts.update(config_facts.get('BGP_VOQ_CHASSIS_NEIGHBOR', {})) - for k, v in list(nbrs_in_cfg_facts.items()): - # Compare the bgp neighbors name with config db bgp neighbors name - assert v['name'] == bgp_facts['bgp_neighbors'][k]['description'] - # Compare the bgp neighbors ASN with config db - assert int(v['asn'].encode().decode("utf-8")) == bgp_facts['bgp_neighbors'][k]['remote AS'] - - -def test_show_features_ipv6_only(duthosts, enum_dut_hostname, convert_and_restore_config_db_to_ipv6_only): # noqa F811 - """Verify show features command output against CONFIG_DB - """ - - duthost = duthosts[enum_dut_hostname] - features_dict, succeeded = duthost.get_feature_status() - pytest_assert(succeeded, "failed to obtain feature status") - for cmd_key, cmd_value in list(features_dict.items()): - redis_value = duthost.shell('/usr/bin/redis-cli -n 4 --raw hget "FEATURE|{}" "state"' - .format(cmd_key), module_ignore_errors=False)['stdout'] - pytest_assert(redis_value.lower() == cmd_value.lower(), - "'{}' is '{}' which does not match with config_db".format(cmd_key, cmd_value))