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

Explicitly disconnect yagna containers before stopping Docker network #540

Merged
merged 3 commits into from
Aug 27, 2021
Merged
Changes from 1 commit
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
49 changes: 46 additions & 3 deletions goth/runner/container/compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import AsyncIterator, ClassVar, Dict, List, Optional

from docker import DockerClient
from docker.models.networks import Network
import yaml

from goth.runner.container import DockerContainer
Expand Down Expand Up @@ -145,12 +146,52 @@ async def _wait_for_containers(self) -> None:
await monitor.wait_for_entry(pattern, timeout=CONTAINER_READY_TIMEOUT)
logger.info("Compose network ready")

async def stop_network(self):
"""Stop the running compose network, removing its containers."""
def _disconnect_containers(self, excluded_containers: List[str]) -> None:
"""Disconnect containers from the default Docker compose network.

This corresponds to executing the command
>>> docker network disconnect -f DEFAULT_NETWORK CONTAINER_NAME
for each container.

All containers except those with names in `excluded_containers` are
disconnected.
"""
networks = self._docker_client.networks.list(
names=[DockerContainer.DEFAULT_NETWORK]
)
if networks:
compose_network: Network = networks[0]
compose_network.reload()
connected_containers = [
c["Name"] for c in compose_network.attrs["Containers"].values()
]
yagna_containers = [
name for name in connected_containers if name not in excluded_containers
]
for container in yagna_containers:
logger.info(
"Disconnecting container %s from network %s...",
container,
DockerContainer.DEFAULT_NETWORK,
)
compose_network.disconnect(container, force=True)

async def stop_network(self, compose_containers: Optional[List[str]] = None):
"""Stop the running compose network, removing its containers.

Before the network is stopped, all yagna containers need to be disconnected,
due to https://github.com/golemfactory/goth/issues/539.
Copy link
Contributor

Choose a reason for hiding this comment

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

This links to the issue which this PR is going to resolve! I believe the underlying problem in Docker is described here: moby/moby#23302

To avoid explicitly disconnecting some containers -- for example, the ones
started by `docker-compose` itself that will be disconnected by
`docker-compose down` -- pass their names in `compose_containers`.
"""

for name, monitor in self._log_monitors.items():
logger.debug("stopping log monitor. name=%s", name)
await monitor.stop()

self._disconnect_containers(compose_containers or [])

await run_command(
["docker-compose", "-f", str(self.config.file_path), "down", "-t", "0"]
)
Expand Down Expand Up @@ -224,12 +265,14 @@ async def run_compose_network(
Yields information on containers started in the compose network.
"""

compose_containers = []
try:
logger.debug(
"Starting compose network. log_dir=%s, force_build=%s", log_dir, force_build
)
containers = await compose_manager.start_network(log_dir, force_build)
compose_containers = list(containers.keys())
yield containers
finally:
logger.debug("Stopping compose network")
await compose_manager.stop_network()
await compose_manager.stop_network(compose_containers)