From 6e19fbf3c82d9e53b13a7f09b22a4e4bc193c614 Mon Sep 17 00:00:00 2001 From: Hari Vishwanath Date: Sat, 26 Oct 2024 18:15:37 +0530 Subject: [PATCH 1/3] feat : Add support for depends_on functionality --- core/testcontainers/core/container.py | 85 ++++++++++++++++++++++- core/tests/test_container_dependencies.py | 76 ++++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 core/tests/test_container_dependencies.py diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index 90967e953..14f61d800 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -1,9 +1,11 @@ import contextlib +import time from socket import socket from typing import TYPE_CHECKING, Optional, Union import docker.errors from docker import version +from docker.errors import NotFound from docker.types import EndpointConfig from typing_extensions import Self, assert_never @@ -44,6 +46,7 @@ def __init__( self.env = {} self.ports = {} self.volumes = {} + self._dependencies = [] self.image = image self._docker = DockerClient(**(docker_client_kw or {})) self._container = None @@ -83,14 +86,63 @@ def maybe_emulate_amd64(self) -> Self: return self.with_kwargs(platform="linux/amd64") return self + def depends_on(self, dependencies: Union["DockerContainer", list["DockerContainer"]]) -> "DockerContainer": + """ + Specify dependencies for this container. + + Args: + dependencies (Union[DockerContainer, list[DockerContainer]]): One or multiple Docker container instances + this container depends on. + + Returns: + DockerContainer: The current instance, for chaining. + """ + if isinstance(dependencies, DockerContainer): + self._dependencies.append(dependencies) + elif isinstance(dependencies, list): + self._dependencies.extend(dependencies) + else: + raise TypeError("dependencies must be a DockerContainer or list of DockerContainer instances") + return self + + def _start_dependencies(self) -> bool: + """ + Start all dependencies recursively, ensuring each dependency's dependencies are also resolved. + If a dependency fails to start, stop all previously started dependencies and raise the exception. + """ + started_dependencies = [] + for dependency in self._dependencies: + if not dependency._container: + try: + dependency._start_dependencies() + dependency.start() + started_dependencies.append(dependency) + + if not dependency.wait_until_running(): + raise ContainerStartException(f"Dependency {dependency.image} did not reach 'running' state.") + + except Exception as e: + # Clean up all dependencies started before the failure + for dep in started_dependencies: + try: + logger.debug("Stopping previously started dependency container: %s", dep.image) + dep.stop() + logger.debug("Dependency container %s stopped successfully", dep.image) + except Exception as stop_error: + logger.error("Failed to stop dependency container %s: %s", dep.image, str(stop_error)) + # Raise the exception after cleaning up + raise e + return True + def start(self) -> Self: if not c.ryuk_disabled and self.image != c.ryuk_image: logger.debug("Creating Ryuk container") Reaper.get_instance() - logger.info("Pulling image %s", self.image) docker_client = self.get_docker_client() self._configure() + self._start_dependencies() + network_kwargs = ( { "network": self._network.name, @@ -102,6 +154,7 @@ def start(self) -> Self: else {} ) + logger.info("Pulling image %s", self.image) self._container = docker_client.run( self.image, command=self._command, @@ -119,7 +172,11 @@ def start(self) -> Self: def stop(self, force=True, delete_volume=True) -> None: if self._container: - self._container.remove(force=force, v=delete_volume) + try: + self._container.remove(force=force, v=delete_volume) + except NotFound: + logger.warning("Container not found when attempting to stop.") + self._container = None self.get_docker_client().client.close() def __enter__(self) -> Self: @@ -128,6 +185,30 @@ def __enter__(self) -> Self: def __exit__(self, exc_type, exc_val, exc_tb) -> None: self.stop() + def wait_until_running(self, timeout: int = 30) -> bool: + """ + Wait until the container is in the 'running' state, up to a specified timeout. + + Args: + timeout (int): Maximum time to wait in seconds. + + Returns: + bool: True if the container is running, False if the timeout is reached. + """ + start_time = time.time() + while time.time() - start_time < timeout: + self.get_wrapped_container().reload() + if self._container and self._container.status == "running": + logger.info(f"Container {self.image} reached 'running' state.") + return True + elif self._container: + logger.debug(f"Container {self.image} state: {self._container.status}") + else: + logger.debug(f"Container {self.image} is not initialized yet.") + time.sleep(0.5) + logger.error(f"Container {self.image} did not reach 'running' state within {timeout} seconds.") + return False + def get_container_host_ip(self) -> str: connection_mode: ConnectionMode connection_mode = self.get_docker_client().get_connection_mode() diff --git a/core/tests/test_container_dependencies.py b/core/tests/test_container_dependencies.py new file mode 100644 index 000000000..eb062c114 --- /dev/null +++ b/core/tests/test_container_dependencies.py @@ -0,0 +1,76 @@ +import pytest +from docker.errors import APIError, ImageNotFound +from testcontainers.core.container import DockerContainer + + +def test_single_dependency_starts() -> None: + container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + dependency_container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container.depends_on(dependency_container) + + container.start() + + assert dependency_container.wait_until_running(), "Dependency did not reach running state" + assert container.wait_until_running(), "Container did not reach running state" + + container.stop() + dependency_container.stop() + + +def test_multiple_dependencies_start() -> None: + container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + dependency1 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + dependency2 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container.depends_on([dependency1, dependency2]) + + dependency1.start() + dependency2.start() + assert dependency1.wait_until_running(), "Dependency 1 did not reach running state" + assert dependency2.wait_until_running(), "Dependency 2 did not reach running state" + + container.start() + assert container.wait_until_running(), "Container did not reach running state" + + container.stop() + dependency1.stop() + dependency2.stop() + + +def test_dependency_failure() -> None: + container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + failing_dependency = DockerContainer("nonexistent-image") + container.depends_on(failing_dependency) + + with pytest.raises((APIError, ImageNotFound)): + container.start() + + assert container._container is None + + +def test_all_dependencies_fail() -> None: + container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + failing_dependency1 = DockerContainer("nonexistent-image1") + failing_dependency2 = DockerContainer("nonexistent-image2") + container.depends_on([failing_dependency1, failing_dependency2]) + + with pytest.raises((APIError, ImageNotFound)): + container.start() + + assert container._container is None + assert failing_dependency1._container is None + assert failing_dependency2._container is None + + +def test_dependency_cleanup_on_partial_failure() -> None: + container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + dependency1 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + failing_dependency = DockerContainer("nonexistent-image3") + + container.depends_on([dependency1, failing_dependency]) + + with pytest.raises(Exception): + container.start() + + assert dependency1._container is None, "dependency1 was not cleaned up properly" + assert failing_dependency._container is None, "failing_dependency was not cleaned up properly" + assert container._container is None, "container was not cleaned up properly" From 08485c9994d871759f591e1582f53edef2f965fa Mon Sep 17 00:00:00 2001 From: Hari Vishwanath Date: Tue, 19 Nov 2024 00:21:24 +0530 Subject: [PATCH 2/3] Fix: pass started dependencies across recursive functions --- core/testcontainers/core/container.py | 72 +++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 11 deletions(-) diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index 14f61d800..f25540083 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -103,35 +103,60 @@ def depends_on(self, dependencies: Union["DockerContainer", list["DockerContaine self._dependencies.extend(dependencies) else: raise TypeError("dependencies must be a DockerContainer or list of DockerContainer instances") + + # Check for any circular dependencies before starting + self.check_for_circular_dependencies() + return self - def _start_dependencies(self) -> bool: + def _start_dependencies(self, started_dependencies=None) -> bool: """ Start all dependencies recursively, ensuring each dependency's dependencies are also resolved. If a dependency fails to start, stop all previously started dependencies and raise the exception. """ - started_dependencies = [] + if started_dependencies is None: + started_dependencies = [] + for dependency in self._dependencies: if not dependency._container: try: - dependency._start_dependencies() + container_name = dependency._name if dependency._name else dependency.image + logger.info(f"Starting dependency container: {container_name}") + + # Start sub-dependencies recursively + dependency._start_dependencies(started_dependencies) + + # Start the actual dependency dependency.start() started_dependencies.append(dependency) - if not dependency.wait_until_running(): - raise ContainerStartException(f"Dependency {dependency.image} did not reach 'running' state.") + logger.info( + f"Dependency container started: {container_name}, " + f"ID: {dependency._container.short_id}, Name: {dependency._container.name}" + ) + + if not dependency.wait_until_running(timeout=15): + raise ContainerStartException(f"Dependency {container_name} did not reach 'running' state.") except Exception as e: - # Clean up all dependencies started before the failure + logger.error(f"Failed to start dependency {container_name}: {e}") + logger.info("Cleaning up previously started dependencies...") + + # Clean up all previously started dependencies for dep in started_dependencies: + dep_name = dep._name if dep._name else dep.image try: - logger.debug("Stopping previously started dependency container: %s", dep.image) dep.stop() - logger.debug("Dependency container %s stopped successfully", dep.image) + logger.info( + f"Successfully stopped dependency container: {dep_name}, " + f"ID: {dep._container.short_id}" + ) except Exception as stop_error: - logger.error("Failed to stop dependency container %s: %s", dep.image, str(stop_error)) - # Raise the exception after cleaning up + logger.error(f"Error stopping dependency container {dep_name}: {stop_error}") + + # Re-raise the original exception after cleanup raise e + return True def start(self) -> Self: @@ -142,7 +167,6 @@ def start(self) -> Self: self._configure() self._start_dependencies() - network_kwargs = ( { "network": self._network.name, @@ -257,6 +281,32 @@ def exec(self, command: Union[str, list[str]]) -> tuple[int, bytes]: raise ContainerStartException("Container should be started before executing a command") return self._container.exec_run(command) + def check_for_circular_dependencies(self) -> None: + """ + Check for circular dependencies before starting containers. + + Raises: + ContainerStartException: If a circular dependency is detected. + """ + visited = set() + current_path = set() + + def dfs(container: "DockerContainer"): + if container in current_path: + raise ContainerStartException(f"Circular dependency detected for container: {container.image}") + if container in visited: + return + + current_path.add(container) + visited.add(container) + + for dependency in container._dependencies: + dfs(dependency) + + current_path.remove(container) + + dfs(self) + def _configure(self) -> None: # placeholder if subclasses want to define this and use the default start method pass From f18287e502cd003ea13f4f0d6ffcf26810510bab Mon Sep 17 00:00:00 2001 From: Hari Vishwanath Date: Tue, 19 Nov 2024 00:22:08 +0530 Subject: [PATCH 3/3] Tests : add and update tests for depends_on functionality --- core/tests/test_container_dependencies.py | 115 +++++++++++++++++++++- 1 file changed, 110 insertions(+), 5 deletions(-) diff --git a/core/tests/test_container_dependencies.py b/core/tests/test_container_dependencies.py index eb062c114..167b083cb 100644 --- a/core/tests/test_container_dependencies.py +++ b/core/tests/test_container_dependencies.py @@ -1,9 +1,13 @@ import pytest from docker.errors import APIError, ImageNotFound from testcontainers.core.container import DockerContainer +from testcontainers.core.exceptions import ContainerStartException def test_single_dependency_starts() -> None: + """ + Test that a container with a single dependency starts correctly. + """ container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") dependency_container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") container.depends_on(dependency_container) @@ -18,17 +22,18 @@ def test_single_dependency_starts() -> None: def test_multiple_dependencies_start() -> None: + """ + Test that a container with multiple dependencies starts correctly. + """ container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") dependency1 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") dependency2 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") container.depends_on([dependency1, dependency2]) - dependency1.start() - dependency2.start() + container.start() + assert dependency1.wait_until_running(), "Dependency 1 did not reach running state" assert dependency2.wait_until_running(), "Dependency 2 did not reach running state" - - container.start() assert container.wait_until_running(), "Container did not reach running state" container.stop() @@ -37,6 +42,9 @@ def test_multiple_dependencies_start() -> None: def test_dependency_failure() -> None: + """ + Test that the container fails to start if a dependency fails to start. + """ container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") failing_dependency = DockerContainer("nonexistent-image") container.depends_on(failing_dependency) @@ -44,10 +52,13 @@ def test_dependency_failure() -> None: with pytest.raises((APIError, ImageNotFound)): container.start() - assert container._container is None + assert container._container is None, "Container should not start if dependency fails" def test_all_dependencies_fail() -> None: + """ + Test that the container fails to start if all dependencies fail. + """ container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") failing_dependency1 = DockerContainer("nonexistent-image1") failing_dependency2 = DockerContainer("nonexistent-image2") @@ -62,6 +73,9 @@ def test_all_dependencies_fail() -> None: def test_dependency_cleanup_on_partial_failure() -> None: + """ + Test that all started dependencies are stopped if one of them fails. + """ container = DockerContainer("alpine:latest").with_command("tail -f /dev/null") dependency1 = DockerContainer("alpine:latest").with_command("tail -f /dev/null") failing_dependency = DockerContainer("nonexistent-image3") @@ -74,3 +88,94 @@ def test_dependency_cleanup_on_partial_failure() -> None: assert dependency1._container is None, "dependency1 was not cleaned up properly" assert failing_dependency._container is None, "failing_dependency was not cleaned up properly" assert container._container is None, "container was not cleaned up properly" + + +def test_circular_dependency_detection() -> None: + """ + Test that adding a circular dependency raises a ContainerStartException. + """ + container_a = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container_b = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + + # Add dependency from A to B + container_a.depends_on(container_b) + + with pytest.raises(ContainerStartException, match="Circular dependency detected"): + container_b.depends_on(container_a) + + +def test_multi_level_circular_dependency_detection() -> None: + """ + Test that a multi-level circular dependency raises a ContainerStartException. + """ + container_a = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container_b = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container_c = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + + # Step 1: A depends on B + container_a.depends_on(container_b) + + # Step 2: B depends on C + container_b.depends_on(container_c) + + # Step 3: Adding the circular dependency: C depends on A + with pytest.raises(ContainerStartException, match="Circular dependency detected"): + container_c.depends_on(container_a) + + +def test_complex_dependency_graph() -> None: + container_a = DockerContainer("alpine:latest").with_name("container_a").with_command("tail -f /dev/null") + container_b = DockerContainer("alpine:latest").with_name("container_b").with_command("tail -f /dev/null") + container_c = DockerContainer("alpine:latest").with_name("container_c").with_command("tail -f /dev/null") + container_d = DockerContainer("alpine:latest").with_name("container_d").with_command("tail -f /dev/null") + container_e = DockerContainer("alpine:latest").with_name("container_e").with_command("tail -f /dev/null") + + # Dependency graph: + # A -> [B, C] + # B -> D + # C -> E + container_a.depends_on([container_b, container_c]) + container_b.depends_on(container_d) + container_c.depends_on(container_e) + + try: + container_a.start() + except Exception as e: + raise e + + assert container_a.wait_until_running(), "Container A did not reach running state" + assert container_b.wait_until_running(), "Container B did not reach running state" + assert container_c.wait_until_running(), "Container C did not reach running state" + assert container_d.wait_until_running(), "Container D did not reach running state" + assert container_e.wait_until_running(), "Container E did not reach running state" + + # Cleanup + container_a.stop() + container_b.stop() + container_c.stop() + container_d.stop() + container_e.stop() + + +def test_dependency_cleanup_on_complex_failure() -> None: + """ + Test that all dependencies are cleaned up in a complex graph if one fails. + """ + container_a = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container_b = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + container_c = DockerContainer("alpine:latest").with_command("tail -f /dev/null") + failing_container = DockerContainer("nonexistent-image") + + # Dependency graph: + # A -> [B, C] + # C -> Failing + container_a.depends_on([container_b, container_c]) + container_c.depends_on(failing_container) + + with pytest.raises(Exception): + container_a.start() + + assert container_b._container is None, "Container B was not cleaned up properly" + assert container_c._container is None, "Container C was not cleaned up properly" + assert failing_container._container is None, "Failing container was not cleaned up properly" + assert container_a._container is None, "Container A was not cleaned up properly"