From d04060dcb1891343d595b2184c81eaeb5c2dbdef Mon Sep 17 00:00:00 2001 From: Danny Allen Date: Thu, 30 Apr 2020 11:30:32 -0700 Subject: [PATCH 1/2] [swap_syncd] Fix failures in swap syncd Signed-off-by: Danny Allen --- tests/common/system_utils/docker.py | 119 ++++++++++++++++++++-------- 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/tests/common/system_utils/docker.py b/tests/common/system_utils/docker.py index 3b30f250bb..0a1843e052 100644 --- a/tests/common/system_utils/docker.py +++ b/tests/common/system_utils/docker.py @@ -28,36 +28,50 @@ class DockerRegistryInfo(_DockerRegistryInfo): """ pass -def parse_registry_file(registry_file): +def load_docker_registry_info(dut): """ - parse_registry_file parses the provided file to produce a DockerRegistryInfo. + Attempts to load Docker registry information. - See `SONIC_DOCKER_REGISTRY` for the expected format of this file. + This method will first search for the registry in the `secret_vars` section + of the Ansible inventory. If it's not found, then it will load the registry from + the `SONIC_DOCKER_REGISTRY` file. Args: - registry_file (str): The name of the file holding the registry information. + dut (SonicHost): The target device. Raises: - IOError: If the file cannot be opened for any reason. - ValueError: If the provided file is missing any required fields. + IOError: If the registry file cannot be read. + ValueError: If the registry information is missing from both the + Ansible inventory and the registry file. Returns: - DockerRegistryInfo: The registry info from the registry file. + DockerRegistryInfo: The registry information that was loaded. """ - try: - with open(registry_file) as contents: - registry_vars = yaml.safe_load(contents) - except IOError as err: - _LOGGER.error("Failed to parse registry file \"%s\" (%s)", registry_file, err) - raise + # FIXME: In Ansible we're able to load the facts regardless of where they're + # stored. We should figure out how to do this in pytest so the registry + # location isn't hard-coded. + registry_vars = dut.host.options['variable_manager'] \ + ._hostvars.get(dut.hostname, {}) \ + .get("secret_vars", {}) \ + .get("docker_registry") + + if not registry_vars: + _LOGGER.warning("Registry info not found in inventory, falling back to registry file") + + try: + with open(SONIC_DOCKER_REGISTRY) as contents: + registry_vars = yaml.safe_load(contents) + except IOError as err: + _LOGGER.error("Failed to parse registry file (%s)", err) + raise host = registry_vars.get("docker_registry_host") username = registry_vars.get("docker_registry_username") password = registry_vars.get("docker_registry_password") if not host or not username or not password: - error_message = "Registry file \"{}\" is missing login or hostname".format(registry_file) + error_message = "Missing registry hostname or login" _LOGGER.error(error_message) raise ValueError(error_message) @@ -65,18 +79,18 @@ def parse_registry_file(registry_file): def delete_container(dut, container_name): """ - delete_container attempts to delete the specified container from the DUT. + Attempts to delete the specified container from the DUT. Args: dut (SonicHost): The target device. container_name (str): The name of the container to delete. """ - dut.docker_container(name=container_name, state="absent") + dut.command("docker rm {}".format(container_name)) def download_image(dut, registry, image_name, image_version="latest"): """ - download_image attempts to download the specified image from the registry. + Attempts to download the specified image from the registry. Args: dut (SonicHost): The target device. @@ -85,15 +99,12 @@ def download_image(dut, registry, image_name, image_version="latest"): image_version (str): The version of the image to download. """ - dut.docker_login(registry_url=registry.host, - username=registry.username, - password=registry.password) - dut.docker_image(source="pull", - name="{}/{}:{}".format(registry.host, image_name, image_version)) + dut.command("docker login {} -u {} -p {}".format(registry.host, registry.username, registry.password)) + dut.command("docker pull {}/{}:{}".format(registry.host, image_name, image_version)) def tag_image(dut, tag, image_name, image_version="latest"): """ - tag_image applies the specified tag to a Docker image on the DUT. + Applies the specified tag to a Docker image on the DUT. Args: dut (SonicHost): The target device. @@ -102,19 +113,16 @@ def tag_image(dut, tag, image_name, image_version="latest"): image_version (str): The version of the image to tag. """ - dut.docker_image(source="local", - name="{}:{}".format(image_name, image_version), - tag=tag) + dut.command("docker tag {}:{} {}".format(image_name, image_version, tag)) -def swap_syncd(dut, registry_file=SONIC_DOCKER_REGISTRY): +def swap_syncd(dut): """ - swap_syncd replaces the default syncd container on the DUT with an RPC version of it. + Replaces the running syncd container with the RPC version of it. - This command will download a new Docker image to the DUT and restart the swss service. + This will download a new Docker image to the DUT and restart the swss service. Args: dut (SonicHost): The target device. - registry_file (str): The registry file describing where to download the RPC image. """ if is_broadcom_device(dut): @@ -129,7 +137,7 @@ def swap_syncd(dut, registry_file=SONIC_DOCKER_REGISTRY): docker_syncd_name = "docker-syncd-{}".format(vendor_id) docker_rpc_image = docker_syncd_name + "-rpc" - dut.command("systemctl stop swss") + dut.command("sudo systemctl stop swss") delete_container(dut, "syncd") # Set sysctl RCVBUF parameter for tests @@ -139,14 +147,55 @@ def swap_syncd(dut, registry_file=SONIC_DOCKER_REGISTRY): output = dut.command("sonic-cfggen -y /etc/sonic/sonic_version.yml -v build_version") sonic_version = output["stdout_lines"][0].strip() - registry = parse_registry_file(registry_file) + registry = load_docker_registry_info(dut) download_image(dut, registry, docker_rpc_image, sonic_version) tag_image(dut, - "{}/{}".format(registry.host, docker_syncd_name), - docker_rpc_image, + "{}:latest".format(docker_syncd_name), + "{}/{}".format(registry.host, docker_rpc_image), sonic_version) - dut.command("systemctl start swss") + dut.command("sudo systemctl reset-failed swss") + dut.command("sudo systemctl start swss") + + _LOGGER.info("swss has been restarted, waiting 60 seconds to initialize...") + time.sleep(60) + +def restore_default_syncd(dut): + """ + Replaces the running syncd with the default syncd that comes with the image. + + This will restart the swss service. + + Args: + dut (SonicHost): The target device. + """ + + if is_broadcom_device(dut): + vendor_id = "brcm" + elif is_mellanox_device(dut): + vendor_id = "mlnx" + else: + error_message = "\"{}\" is not currently supported".format(dut.get_asic_type()) + _LOGGER.error(error_message) + raise ValueError(error_message) + + docker_syncd_name = "docker-syncd-{}".format(vendor_id) + + dut.command("sudo systemctl stop swss") + delete_container(dut, "syncd") + + # TODO: Getting the base image version should be a common utility + output = dut.command("sonic-cfggen -y /etc/sonic/sonic_version.yml -v build_version") + sonic_version = output["stdout_lines"][0].strip() + + tag_image(dut, + "{}:latest".format(docker_syncd_name), + docker_syncd_name, + sonic_version) + + dut.command("sudo systemctl reset-failed swss") + dut.command("sudo systemctl start swss") + _LOGGER.info("swss has been restarted, waiting 60 seconds to initialize...") time.sleep(60) From cdf42ce514cd7d8da8498e5cec9ede9a6998eb2b Mon Sep 17 00:00:00 2001 From: Danny Allen Date: Thu, 30 Apr 2020 16:36:40 -0700 Subject: [PATCH 2/2] Remove redundant sudo --- tests/common/system_utils/docker.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/common/system_utils/docker.py b/tests/common/system_utils/docker.py index 0a1843e052..aecc2c3ba4 100644 --- a/tests/common/system_utils/docker.py +++ b/tests/common/system_utils/docker.py @@ -137,7 +137,7 @@ def swap_syncd(dut): docker_syncd_name = "docker-syncd-{}".format(vendor_id) docker_rpc_image = docker_syncd_name + "-rpc" - dut.command("sudo systemctl stop swss") + dut.command("systemctl stop swss") delete_container(dut, "syncd") # Set sysctl RCVBUF parameter for tests @@ -155,8 +155,8 @@ def swap_syncd(dut): "{}/{}".format(registry.host, docker_rpc_image), sonic_version) - dut.command("sudo systemctl reset-failed swss") - dut.command("sudo systemctl start swss") + dut.command("systemctl reset-failed swss") + dut.command("systemctl start swss") _LOGGER.info("swss has been restarted, waiting 60 seconds to initialize...") time.sleep(60) @@ -182,7 +182,7 @@ def restore_default_syncd(dut): docker_syncd_name = "docker-syncd-{}".format(vendor_id) - dut.command("sudo systemctl stop swss") + dut.command("systemctl stop swss") delete_container(dut, "syncd") # TODO: Getting the base image version should be a common utility @@ -194,8 +194,8 @@ def restore_default_syncd(dut): docker_syncd_name, sonic_version) - dut.command("sudo systemctl reset-failed swss") - dut.command("sudo systemctl start swss") + dut.command("systemctl reset-failed swss") + dut.command("systemctl start swss") _LOGGER.info("swss has been restarted, waiting 60 seconds to initialize...") time.sleep(60)