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

refactor(core): Standardize configuration and readiness steps in container lifecycle #527

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fc7c466
Include configuration and waiting hooks in base DockerContainer class
santi Apr 2, 2024
2578a97
Inherit directly from DockerContainer in ArangoDB module
santi Apr 2, 2024
fa0d34a
Use new _wait_until_ready hook in Azureite module
santi Apr 2, 2024
482df60
Use new _wait_until_ready hook in Cassandra module
santi Apr 2, 2024
9437d89
Add http waiting strategy for basic HTTP endpoint status codes
santi Apr 2, 2024
d4a0612
Use new _wait_until_ready hook in ChromaDB module
santi Apr 2, 2024
b7e52ba
Create utility for generating db client compatible connection strings
santi Apr 3, 2024
6964d3e
Use new _wait_until_ready hook in clickhouse module
santi Apr 3, 2024
9e43875
Use new _wait_until_ready hook in elasticsearch module
santi Apr 3, 2024
1b3aebc
Use new _wait_until_ready hook in google module
santi Apr 3, 2024
abbbf7d
Fix case for connection_string without username or password
santi Apr 3, 2024
09c10ba
Use new _wait_until_ready hook in influxdb module
santi Apr 3, 2024
bea1372
Use new _wait_until_ready hook in k3s module
santi Apr 3, 2024
d19f6c9
Use new _wait_until_ready hook in kafka module
santi Apr 3, 2024
124727c
Use new _wait_until_ready hook in keycloak module
santi Apr 3, 2024
1831366
Use new _wait_until_ready hook in localstack module
santi Apr 3, 2024
0688858
Use new _wait_until_ready hook in minio module
santi Apr 3, 2024
e5afe88
Use new _wait_until_ready hook in mongodb module
santi Apr 3, 2024
20aead1
Reorder imports for linting
santi Apr 3, 2024
00b2e7f
Use new _wait_until_ready hook in mssql module
santi Apr 3, 2024
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
11 changes: 11 additions & 0 deletions core/testcontainers/core/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,14 @@ def maybe_emulate_amd64(self) -> "DockerContainer":
return self.with_kwargs(platform="linux/amd64")
return self

def _configure(self) -> None:
pass

def _wait_until_ready(self) -> None:
pass

def start(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def start(self):
def start(self) -> Self:

Not sure what version we target (so either whats in python or via an backport), but I saw that this PR removes a lot of overwritten start() methods which just do this to get the proper type passed out, so not doing this here will mean that typing will regress.

self._configure()
if not RYUK_DISABLED and self.image != RYUK_IMAGE:
logger.debug("Creating Ryuk container")
Reaper.get_instance()
Expand All @@ -92,6 +99,10 @@ def start(self):
**self._kwargs,
)
logger.info("Container started: %s", self._container.short_id)

self._wait_until_ready()
logger.info("Container ready: %s", self._container.short_id)

return self

def stop(self, force=True, delete_volume=True) -> None:
Expand Down
11 changes: 1 addition & 10 deletions core/testcontainers/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class DbContainer(DockerContainer):
"""

@wait_container_is_ready(*ADDITIONAL_TRANSIENT_ERRORS)
def _connect(self) -> None:
def _wait_until_ready(self):
import sqlalchemy

engine = sqlalchemy.create_engine(self.get_connection_url())
Expand Down Expand Up @@ -64,12 +64,3 @@ def _create_connection_url(
if dbname:
url = f"{url}/{dbname}"
return url

def start(self) -> "DbContainer":
self._configure()
super().start()
self._connect()
return self

def _configure(self) -> None:
raise NotImplementedError
38 changes: 38 additions & 0 deletions core/testcontainers/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import platform
import subprocess
import sys
from typing import Optional, Union

LINUX = "linux"
MAC = "mac"
Expand Down Expand Up @@ -53,6 +54,43 @@ def inside_container() -> bool:
return os.path.exists("/.dockerenv")


def create_connection_string(

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

create_connection_url -> e.g. you couldn't create a kafka one with this, couldn't you?

dialect: str,
Copy link
Contributor

Choose a reason for hiding this comment

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

If this would be DB API only, I would say "dialect" is fine, but if this should also be used to generate http URLs, I would call this base_scheme.

host: str,
port: Union[str, int, None] = None,
username: Optional[str] = None,
password: Optional[str] = None,
driver: Optional[str] = None,
dbname: Optional[str] = None,
) -> str:
"""
Returns a connection URL following the RFC-1738 format.
Compatible with database clients such as SQLAlchemy and other popular database client libraries.

Example: postgres+psycopg2://myuser:mypassword@localhost:5432/mytestdb
Copy link
Contributor

@jankatins jankatins Apr 26, 2024

Choose a reason for hiding this comment

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

It would be nice to document that setting driver to None or "" will remove the driver from the URL (psycopg v3 does need a url without dialect and any regular http URL also needs that :-))

Or at least add an example without driver.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would actually split this into a generic "create_connection_url" (scheme instead of dialect and driver, path instead of dbname) and one call (create_db_api_connection_url?) which takes db api inputs (like the current one) and calls create_connection_url with the corresponding arguments.

"""
dialect_driver = dialect
if driver:
dialect_driver += f"+{driver}"

username_password = username if username else ""
if password:
username_password += f":{password}"

if username_password:
username_password += "@"

host_port = host
if port:
host_port += f":{port}"

connection_string = f"{dialect_driver}://{username_password}{host_port}"
if dbname:
connection_string += f"/{dbname}"

return connection_string


def default_gateway_ip() -> str:
"""
Returns gateway IP address of the host that testcontainer process is
Expand Down
15 changes: 15 additions & 0 deletions core/testcontainers/core/waiting_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
# under the License.


import contextlib
import re
import time
import traceback
from http.client import HTTPException
from typing import TYPE_CHECKING, Any, Callable, Optional, Union
from urllib.request import urlopen

import wrapt

Expand Down Expand Up @@ -77,6 +80,18 @@ def wait_for(condition: Callable[..., bool]) -> bool:
return condition()


def wait_for_http(url: str, timeout: Optional[float] = config.TIMEOUT, interval: float = 1) -> float:
start = time.time()
while True:
duration = time.time() - start
with contextlib.suppress(HTTPException), urlopen(url) as r:
if r.status < 300:
return duration
if timeout and duration > timeout:
raise TimeoutError(f"Container did not respond to URL check {url} in {timeout:.3f} seconds")
time.sleep(interval)


def wait_for_logs(
container: "DockerContainer", predicate: Union[Callable, str], timeout: Optional[float] = None, interval: float = 1
) -> float:
Expand Down
31 changes: 19 additions & 12 deletions modules/arangodb/testcontainers/arangodb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@
ArangoDB container support.
"""

import typing
from os import environ
from typing import Optional

from typing_extensions import override

from testcontainers.core.config import TIMEOUT
from testcontainers.core.generic import DbContainer
from testcontainers.core.utils import raise_for_deprecated_parameter
from testcontainers.core.container import DockerContainer
from testcontainers.core.utils import create_connection_string, raise_for_deprecated_parameter
from testcontainers.core.waiting_utils import wait_for_logs


class ArangoDbContainer(DbContainer):
class ArangoDbContainer(DockerContainer):
"""
ArangoDB container.

Expand All @@ -26,7 +28,7 @@ class ArangoDbContainer(DbContainer):
>>> from testcontainers.arangodb import ArangoDbContainer
>>> from arango import ArangoClient

>>> with ArangoDbContainer("arangodb:3.11.8") as arango:
>>> with ArangoDbContainer("arangodb:3.12.0") as arango:
... client = ArangoClient(hosts=arango.get_connection_url())
...
... # Connect
Expand All @@ -42,8 +44,8 @@ def __init__(
image: str = "arangodb:latest",
port: int = 8529,
arango_root_password: str = "passwd",
arango_no_auth: typing.Optional[bool] = None,
arango_random_root_password: typing.Optional[bool] = None,
arango_no_auth: Optional[bool] = None,
arango_random_root_password: Optional[bool] = None,
**kwargs,
) -> None:
"""
Expand Down Expand Up @@ -78,16 +80,21 @@ def __init__(
)
)

@override
def _configure(self) -> None:
self.with_env("ARANGO_ROOT_PASSWORD", self.arango_root_password)
if self.arango_no_auth:
self.with_env("ARANGO_NO_AUTH", "1")
if self.arango_random_root_password:
self.with_env("ARANGO_RANDOM_ROOT_PASSWORD", "1")

def get_connection_url(self) -> str:
port = self.get_exposed_port(self.port)
return f"http://{self.get_container_host_ip()}:{port}"

def _connect(self) -> None:
@override
def _wait_until_ready(self) -> None:
wait_for_logs(self, predicate="is ready for business", timeout=TIMEOUT)

def get_connection_url(self) -> str:
return create_connection_string(
dialect="http",
host=self.get_container_host_ip(),
port=self.get_exposed_port(self.port),
)
13 changes: 4 additions & 9 deletions modules/arangodb/tests/test_arangodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,17 @@ def arango_test_ops(arango_client, expeced_version, username="root", password=""
assert len(student_names) == students_to_insert_cnt


def test_docker_run_arango():
"""
Test ArangoDB container with default settings.
"""
image = f"{ARANGODB_IMAGE_NAME}:{IMAGE_VERSION}"
arango_root_password = "passwd"

with ArangoDbContainer(image) as arango:
@pytest.mark.parametrize("version", ["3.12.0", "3.11.8", "3.10.13"])
def test_docker_run_arango(version: str):
with ArangoDbContainer(f"arangodb:{version}") as arango:
client = ArangoClient(hosts=arango.get_connection_url())

# Test invalid auth
sys_db = client.db("_system", username="root", password="notTheRightPass")
with pytest.raises(DatabaseCreateError):
sys_db.create_database("test")

arango_test_ops(arango_client=client, expeced_version=IMAGE_VERSION, password=arango_root_password)
arango_test_ops(arango_client=client, expeced_version=version, password="passwd")


def test_docker_run_arango_without_auth():
Expand Down
12 changes: 3 additions & 9 deletions modules/azurite/testcontainers/azurite/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ class AzuriteContainer(DockerContainer):
>>> from testcontainers.azurite import AzuriteContainer
>>> from azure.storage.blob import BlobServiceClient

>>> with AzuriteContainer() as azurite_container:
... connection_string = azurite_container.get_connection_string()
>>> with AzuriteContainer("mcr.microsoft.com/azure-storage/azurite:3.29.0") as azurite_container:
... client = BlobServiceClient.from_connection_string(
... connection_string,
... azurite_container.get_connection_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

For ArangoDbContainer is get_conenction_url`, here it's '*_string'. Should we unify that?

... api_version="2019-12-12"
... )
"""
Expand Down Expand Up @@ -102,12 +101,7 @@ def get_connection_string(self) -> str:

return connection_string

def start(self) -> "AzuriteContainer":
super().start()
self._connect()
return self

@wait_container_is_ready(OSError)
def _connect(self) -> None:
def _wait_until_ready(self) -> None:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.connect((self.get_container_host_ip(), int(self.get_exposed_port(next(iter(self.ports))))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Might it make sense to also pull this into utils as a wait_for_port? At least I had the need in my code to basically write something similar:

        @wait_container_is_ready(NoSuchPortExposed)
        def _wait_for_port() -> None:
            zks.get_service_port("schemaregistry", 8085)

        _wait_for_port()

4 changes: 2 additions & 2 deletions modules/azurite/tests/test_azurite.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@


def test_docker_run_azurite():
with AzuriteContainer() as azurite_container:
with AzuriteContainer("mcr.microsoft.com/azure-storage/azurite:3.29.0") as azurite_container:
blob_service_client = BlobServiceClient.from_connection_string(
azurite_container.get_connection_string(), api_version="2019-12-12"
azurite_container.get_connection_string(), api_version="2023-11-03"
)

blob_service_client.create_container("test-container")
10 changes: 4 additions & 6 deletions modules/cassandra/testcontainers/cassandra/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from typing_extensions import override

from testcontainers.core.container import DockerContainer
from testcontainers.core.waiting_utils import wait_for_logs

Expand Down Expand Up @@ -47,14 +49,10 @@ def __init__(self, image: str = "cassandra:latest", **kwargs) -> None:
self.with_env("CASSANDRA_ENDPOINT_SNITCH", "GossipingPropertyFileSnitch")
self.with_env("CASSANDRA_DC", self.DEFAULT_LOCAL_DATACENTER)

def _connect(self):
@override
def _wait_until_ready(self):
wait_for_logs(self, "Startup complete")

def start(self) -> "CassandraContainer":
super().start()
self._connect()
return self

def get_contact_points(self) -> list[tuple[str, int]]:
return [(self.get_container_host_ip(), int(self.get_exposed_port(self.CQL_PORT)))]

Expand Down
9 changes: 6 additions & 3 deletions modules/cassandra/tests/test_cassandra.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import pytest

from cassandra.cluster import Cluster, DCAwareRoundRobinPolicy

from testcontainers.cassandra import CassandraContainer


def test_docker_run_cassandra():
with CassandraContainer("cassandra:4.1.4") as cassandra:
@pytest.mark.parametrize("version", ["4.1.4", "3.11.16"])
def test_docker_run_cassandra(version: str):
with CassandraContainer(f"cassandra:{version}") as cassandra:
cluster = Cluster(
cassandra.get_contact_points(),
load_balancing_policy=DCAwareRoundRobinPolicy(cassandra.get_local_datacenter()),
)
session = cluster.connect()
result = session.execute("SELECT release_version FROM system.local;")
assert result.one().release_version == "4.1.4"
assert result.one().release_version == version
29 changes: 6 additions & 23 deletions modules/chroma/testcontainers/chroma/__init__.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
from typing import TYPE_CHECKING

from requests import ConnectionError, get
from typing_extensions import override

from testcontainers.core.container import DockerContainer
from testcontainers.core.utils import raise_for_deprecated_parameter
from testcontainers.core.waiting_utils import wait_container_is_ready

if TYPE_CHECKING:
from requests import Response
from testcontainers.core.waiting_utils import wait_for_http


class ChromaContainer(DockerContainer):
Expand All @@ -22,7 +17,7 @@ class ChromaContainer(DockerContainer):
>>> import chromadb
>>> from testcontainers.chroma import ChromaContainer

>>> with ChromaContainer() as chroma:
>>> with ChromaContainer("chromadb/chroma:0.4.24") as chroma:
... config = chroma.get_config()
... client = chromadb.HttpClient(host=config["host"], port=config["port"])
... col = client.get_or_create_collection("test")
Expand All @@ -48,7 +43,6 @@ def __init__(
self.port = port

self.with_exposed_ports(self.port)
# self.with_command(f"server /data --address :{self.port}")

def get_config(self) -> dict:
"""This method returns the configuration of the Chroma container,
Expand All @@ -65,17 +59,6 @@ def get_config(self) -> dict:
"port": exposed_port,
}

@wait_container_is_ready(ConnectionError)
def _healthcheck(self) -> None:
"""This is an internal method used to check if the Chroma container
is healthy and ready to receive requests."""
url = f"http://{self.get_config()['endpoint']}/api/v1/heartbeat"
response: Response = get(url)
response.raise_for_status()

def start(self) -> "ChromaContainer":
"""This method starts the Chroma container and runs the healthcheck
to verify that the container is ready to use."""
super().start()
self._healthcheck()
return self
@override
def _wait_until_ready(self) -> None:
wait_for_http(f"http://{self.get_config()['endpoint']}/api/v1/heartbeat")
4 changes: 2 additions & 2 deletions modules/chroma/tests/test_chroma.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
def test_docker_run_chroma():
with ChromaContainer(image="chromadb/chroma:0.4.24") as chroma:
client = chromadb.HttpClient(host=chroma.get_config()["host"], port=chroma.get_config()["port"])
col = client.get_or_create_collection("test")
assert col.name == "test"
collection = client.get_or_create_collection("test")
assert collection.name == "test"
Loading