From 318fb5fa09a527165cb10f594b847daeba0b0142 Mon Sep 17 00:00:00 2001 From: azawlocki Date: Wed, 18 Aug 2021 18:25:44 +0200 Subject: [PATCH] Address requests from code review (and then some) --- goth/default-assets/docker/docker-compose.yml | 11 +++- goth/runner/__init__.py | 50 +++++++++---------- goth/runner/container/compose.py | 30 +++++------ goth/runner/container/utils.py | 28 +++++++++-- goth/runner/probe/__init__.py | 2 - 5 files changed, 72 insertions(+), 49 deletions(-) diff --git a/goth/default-assets/docker/docker-compose.yml b/goth/default-assets/docker/docker-compose.yml index 697555f4d..6999292b0 100644 --- a/goth/default-assets/docker/docker-compose.yml +++ b/goth/default-assets/docker/docker-compose.yml @@ -13,7 +13,16 @@ services: # harness on the host machine image: proxy-nginx ports: - - "16000-16100:6000-6100" + # Requests to ports 6001-6100 in proxy-nginx are forwarded + # to the MITM proxy started by the test runner, and further + # to yagna API port (usually 6000) in yagna containers: + # request to port 6001 is forwarded to (yagna API port in) + # the first yagna container, request to port 6002 -- to + # the second one, and so on. + # To make these ports available from Docker host (on some + # systems, Docker network may be unreachable from the host) + # we map them to ports 16001-16100 on the host. + - "16001-16100:6001-6100" ethereum: image: docker.pkg.github.com/golemfactory/gnt2/gnt2-docker-yagna:483c6f241edd diff --git a/goth/runner/__init__.py b/goth/runner/__init__.py index 003e1f7a2..45f393866 100644 --- a/goth/runner/__init__.py +++ b/goth/runner/__init__.py @@ -25,7 +25,6 @@ from goth.runner.container.compose import ( ComposeConfig, ComposeNetworkManager, - ContainerInfo, run_compose_network, ) from goth.runner.container.yagna import YagnaContainerConfig @@ -43,6 +42,14 @@ ProbeType = TypeVar("ProbeType", bound=Probe) +PROXY_NGINX_SERVICE_NAME = "proxy-nginx" +"""Name of the nginx proxy service in the Docker network. + +Must match the service name in the config file used by the runner's +compose network manager. +""" + + class Runner: """Manages the nodes and runs the scenario on them.""" @@ -73,14 +80,11 @@ class Runner: _compose_manager: ComposeNetworkManager """Manager for the docker-compose network portion of the test.""" - _container_info: Optional[Dict[str, ContainerInfo]] - """Info on containers started by this runner, indexed by container name.""" - _exit_stack: AsyncExitStack """A stack of `AsyncContextManager` instances to be closed on runner shutdown.""" - _nginx_container_address: Optional[str] - """The IP address of the container running nginx proxy.""" + _nginx_service_address: Optional[str] + """The IP address of the nginx service in the Docker network.""" _topology: List[YagnaContainerConfig] """A list of configuration objects for the containers to be instantiated.""" @@ -114,8 +118,7 @@ def __init__( config=compose_config, docker_client=docker.from_env(), ) - self._container_info = None - self._nginx_container_address = None + self._nginx_service_address = None self._web_server = ( WebServer(web_root_path, web_server_port) if web_root_path else None ) @@ -245,23 +248,10 @@ def host_address(self) -> str: @property def nginx_container_address(self) -> str: - """Return the IP address in the Docker network of the nginx-proxy container.""" - - if not self._nginx_container_address: - if self._container_info is None: - raise RuntimeError("Docker compose network not started") - nginx_containers = [ - info.address - for name, info in self._container_info.items() - if "nginx" in name - ] - assert len(nginx_containers) == 1, ( - "Expected to find a single nginx proxy container," - f" found {len(nginx_containers)} instead" - ) - self._nginx_container_address = nginx_containers[0] - - return self._nginx_container_address + """Return the IP address of the proxy-nginx service in the Docker network.""" + if not self._nginx_service_address: + raise RuntimeError("Docker network not started") + return self._nginx_service_address @property def web_server_port(self) -> Optional[int]: @@ -304,9 +294,17 @@ async def _enter(self) -> None: self._exit_stack.enter_context(configure_logging_for_test(self.log_dir)) logger.info(colors.yellow("Running test: %s"), self.test_name) - self._container_info = await self._exit_stack.enter_async_context( + container_info = await self._exit_stack.enter_async_context( run_compose_network(self._compose_manager, self.log_dir) ) + for info in container_info.values(): + if PROXY_NGINX_SERVICE_NAME in info.aliases: + self._nginx_service_address = info.address + break + else: + raise RuntimeError( + f"Service {PROXY_NGINX_SERVICE_NAME} not found in the Docker network" + ) self._create_probes(self.log_dir) diff --git a/goth/runner/container/compose.py b/goth/runner/container/compose.py index 708c28a9e..7824023f6 100644 --- a/goth/runner/container/compose.py +++ b/goth/runner/container/compose.py @@ -5,7 +5,7 @@ import logging import os from pathlib import Path -from typing import AsyncIterator, ClassVar, Dict, Optional +from typing import AsyncIterator, ClassVar, Dict, List, Optional from docker import DockerClient import yaml @@ -16,7 +16,7 @@ build_yagna_image, YagnaBuildEnvironment, ) -from goth.runner.container.utils import get_container_address +from goth.runner.container.utils import get_container_network_info from goth.runner.exceptions import ContainerNotFoundError from goth.runner.log import LogConfig from goth.runner.log_monitor import LogEventMonitor @@ -49,11 +49,14 @@ class ComposeConfig: @dataclass class ContainerInfo: - """Info on a Docker container started by a Runner.""" + """Info on a Docker container started by Docker compose.""" address: str """The container's IP address in the Docker network""" + aliases: List[str] + """Container aliases in the Docker network""" + image: str """The container's image name""" @@ -98,6 +101,7 @@ async def start_network( ) -> Dict[str, ContainerInfo]: """Start the compose network based on this manager's compose file. + Returns information on containers started in the compose network. This step may include (re)building the network's docker images. """ # Stop the network in case it's already running (e.g. from a previous test) @@ -183,20 +187,13 @@ def network_gateway_address(self) -> str: def _get_running_containers(self) -> Dict[str, ContainerInfo]: info = {} for container in self._docker_client.containers.list(): - address = get_container_address(self._docker_client, container.name) + address, aliases = get_container_network_info( + self._docker_client, container.name + ) image = container.image.tags[0] - info[container.name] = ContainerInfo(address, image) + info[container.name] = ContainerInfo(address, aliases, image) return info - def _log_running_containers(self): - for container in self._docker_client.containers.list(): - logger.info( - "[%-25s] IP address: %-15s image: %s", - container.name, - get_container_address(self._docker_client, container.name), - container.image.tags[0], - ) - def _start_log_monitors(self, log_dir: Path) -> None: for service_name in self._get_compose_services(): log_config = LogConfig(service_name) @@ -222,7 +219,10 @@ def _start_log_monitors(self, log_dir: Path) -> None: async def run_compose_network( compose_manager: ComposeNetworkManager, log_dir: Path, force_build: bool = False ) -> AsyncIterator[Dict[str, ContainerInfo]]: - """Implement AsyncContextManager for starting/stopping docker compose network.""" + """Implement AsyncContextManager for starting/stopping docker compose network. + + Yields information on containers started in the compose network. + """ try: logger.debug( diff --git a/goth/runner/container/utils.py b/goth/runner/container/utils.py index c040a6624..5a8d9ea2e 100755 --- a/goth/runner/container/utils.py +++ b/goth/runner/container/utils.py @@ -1,6 +1,6 @@ """Utilities related to Docker containers.""" from pathlib import Path -from typing import Dict +from typing import Dict, List, Tuple from docker import DockerClient @@ -8,12 +8,12 @@ from goth.runner.exceptions import ContainerNotFoundError -def get_container_address( +def get_container_network_info( client: DockerClient, container_name: str, network_name: str = DockerContainer.DEFAULT_NETWORK, -) -> str: - """Get the IP address of a container in a given network. +) -> Tuple[str, List[str]]: + """Get the IP address and the aliases of a container in a given network. The name of the container does not have to be exact, it may be a substring. In case of more than one container name matching the given string, the first @@ -29,7 +29,25 @@ def get_container_address( container = matching_containers[0] container_networks = container.attrs["NetworkSettings"]["Networks"] - return container_networks[network_name]["IPAddress"] + network = container_networks[network_name] + return network["IPAddress"], network["Aliases"] + + +def get_container_address( + client: DockerClient, + container_name: str, + network_name: str = DockerContainer.DEFAULT_NETWORK, +) -> str: + """Get the IP address and the aliases of a container in a given network. + + The name of the container does not have to be exact, it may be a substring. + In case of more than one container name matching the given string, the first + container is returned, as listed by the Docker daemon. + + Raises `ContainerNotFoundError` if no matching container is found. + Raises `KeyError` if the container is not connected to the specified network. + """ + return get_container_network_info(client, container_name, network_name)[0] def get_volumes_spec( diff --git a/goth/runner/probe/__init__.py b/goth/runner/probe/__init__.py index 01f097e67..befae5de0 100644 --- a/goth/runner/probe/__init__.py +++ b/goth/runner/probe/__init__.py @@ -236,8 +236,6 @@ async def _start_container(self) -> None: self.ip_address = get_container_address( self._docker_client, self.container.name ) - self._logger.info("IP address: %s", self.ip_address) - nginx_ip_address = self.runner.nginx_container_address self._logger.info( "Yagna API host:port in Docker network: "