Skip to content

Commit

Permalink
Address requests from code review (and then some)
Browse files Browse the repository at this point in the history
  • Loading branch information
azawlocki committed Aug 18, 2021
1 parent b23700e commit 318fb5f
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 49 deletions.
11 changes: 10 additions & 1 deletion goth/default-assets/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 24 additions & 26 deletions goth/runner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from goth.runner.container.compose import (
ComposeConfig,
ComposeNetworkManager,
ContainerInfo,
run_compose_network,
)
from goth.runner.container.yagna import YagnaContainerConfig
Expand All @@ -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."""

Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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)

Expand Down
30 changes: 15 additions & 15 deletions goth/runner/container/compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"""

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down
28 changes: 23 additions & 5 deletions goth/runner/container/utils.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
"""Utilities related to Docker containers."""
from pathlib import Path
from typing import Dict
from typing import Dict, List, Tuple

from docker import DockerClient

from goth.runner.container import DockerContainer
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
Expand All @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions goth/runner/probe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down

0 comments on commit 318fb5f

Please sign in to comment.