Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
26 changes: 26 additions & 0 deletions packages/pytest-simcore/src/pytest_simcore/docker_api_proxy.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import logging
from collections.abc import Callable
from contextlib import AsyncExitStack

import aiodocker
import pytest
from aiohttp import BasicAuth, ClientSession, ClientTimeout
from fastapi import FastAPI
from pydantic import TypeAdapter
from pytest_mock.plugin import MockerFixture
from settings_library.docker_api_proxy import DockerApiProxysettings
from tenacity import before_sleep_log, retry, stop_after_delay, wait_fixed

Expand Down Expand Up @@ -58,3 +63,24 @@ async def docker_api_proxy_settings(
await _wait_till_docker_api_proxy_is_responsive(settings)

return settings


@pytest.fixture
async def mock_setup_remote_docker_client(mocker: MockerFixture) -> Callable[[str], None]:
def _(target_setup_to_replace: str) -> None:
def _setup(app: FastAPI, settings: DockerApiProxysettings) -> None:
_ = settings
exit_stack = AsyncExitStack()

async def on_startup() -> None:
app.state.remote_docker_client = await exit_stack.enter_async_context(aiodocker.Docker())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

1 question: did you check what happens if the connection to the docker engine is broken. does this client reconnects?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's based on HTTP not TCP, so there is no such issue. If the connection comes back this will work as expcted


async def on_shutdown() -> None:
await exit_stack.aclose()

app.add_event_handler("startup", on_startup)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not using lifespan events in the fast app instead of these deprecate events https://fastapi.tiangolo.com/advanced/events/

if new code keeps the deprecated events ... we will never get the code up to date. Right now we have a nasty melange of all of these events :-(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because not all services support them. And we cannot mix them easily. Either the entire service is lifespan based or setup based.

app.add_event_handler("shutdown", on_shutdown)

mocker.patch(target_setup_to_replace, new=_setup)

return _
25 changes: 25 additions & 0 deletions packages/service-library/src/servicelib/fastapi/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,31 @@ async def remote_docker_client_lifespan(app: FastAPI, state: State) -> AsyncIter
yield {}


def setup_remote_docker_client(app: FastAPI, settings: DockerApiProxysettings) -> None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this not be using the new lifespan mechanisms?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should not be a "remote_docker_client" but the "docker_client".
and if there is no settings it should create the default client with the unix socket.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a lifespan, for services which use it. In this situation it is required since not all of them use it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The intent of the name is to avoid any possible confusion that this is a docker client that uses the local docker socket.
I want to keep this very obvious since mixing them is bad. Especially since we only have 1 machine for development and not multiple node machines.

This is client is intended to use docker swarm API and runs only on maser nodes.

If you are on a worker and need to list the containers, this will not work, since the connection will point to a docker master.

If you don't like the name, I suggest to make it even more obvious. Something like swarm_master_node_docker_client. It has to be obvious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the reasons above, this should never user the local docker socket

exit_stack = AsyncExitStack()

async def on_startup() -> None:
session = await exit_stack.enter_async_context(
ClientSession(
auth=aiohttp.BasicAuth(
login=settings.DOCKER_API_PROXY_USER,
password=settings.DOCKER_API_PROXY_PASSWORD.get_secret_value(),
)
)
)

app.state.remote_docker_client = await exit_stack.enter_async_context(
aiodocker.Docker(url=settings.base_url, session=session)
)
await wait_till_docker_api_proxy_is_responsive(app)

async def on_shutdown() -> None:
await exit_stack.aclose()

app.add_event_handler("startup", on_startup)
Comment thread
GitHK marked this conversation as resolved.
app.add_event_handler("shutdown", on_shutdown)


@tenacity.retry(
wait=tenacity.wait_fixed(5),
stop=tenacity.stop_after_delay(60),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from common_library.json_serialization import json_dumps
from fastapi import FastAPI
from servicelib.fastapi.docker import setup_remote_docker_client
from servicelib.fastapi.tracing import (
initialize_fastapi_app_tracing,
setup_tracing,
Expand Down Expand Up @@ -61,6 +62,10 @@ def create_app(settings: ApplicationSettings, tracing_config: TracingConfig) ->

setup_instrumentation(app)
setup_api_routes(app)

if settings.AUTOSCALING_DOCKER_API_PROXY:
setup_remote_docker_client(app, settings.AUTOSCALING_DOCKER_API_PROXY)

setup_docker(app)
setup_rabbitmq(app)
setup_ec2(app)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from servicelib.logging_utils import LogLevelInt
from settings_library.application import BaseApplicationSettings
from settings_library.base import BaseCustomSettings
from settings_library.docker_api_proxy import DockerApiProxysettings
from settings_library.docker_registry import RegistrySettings
from settings_library.ec2 import EC2Settings
from settings_library.rabbit import RabbitSettings
Expand Down Expand Up @@ -367,6 +368,11 @@ class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings):
Field(json_schema_extra={"auto_default_from_env": True}),
]

AUTOSCALING_DOCKER_API_PROXY: Annotated[
DockerApiProxysettings | None,
Field(json_schema_extra={"auto_default_from_env": True}),
]

AUTOSCALING_PROMETHEUS_INSTRUMENTATION_ENABLED: bool = True

AUTOSCALING_DRAIN_NODES_WITH_LABELS: Annotated[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

import aiodocker
from fastapi import FastAPI
from servicelib.fastapi.docker import get_remote_docker_client
from tenacity.asyncio import AsyncRetrying
from tenacity.before_sleep import before_sleep_log
from tenacity.stop import stop_after_delay
from tenacity.wait import wait_random_exponential

from ..core.settings import ApplicationSettings

logger = logging.getLogger(__name__)


Expand All @@ -21,8 +24,20 @@ async def ping(self) -> bool:


def setup(app: FastAPI) -> None:
settings: ApplicationSettings = app.state.settings

async def on_startup() -> None:
app.state.docker_client = client = AutoscalingDocker()
# Get the remote docker client configured by servicelib
if settings.AUTOSCALING_DOCKER_API_PROXY:
client = get_remote_docker_client(app)
# Promote to AutoscalingDocker, works safely because
# AutoscalingDocker does not add any new attributes, only methods
client.__class__ = AutoscalingDocker
assert type(client) is AutoscalingDocker # nosec
else:
Comment thread
GitHK marked this conversation as resolved.
client = AutoscalingDocker()

app.state.docker_client = client

async for attempt in AsyncRetrying(
reraise=True,
Expand Down
7 changes: 5 additions & 2 deletions services/autoscaling/tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,15 @@

pytest_plugins = [
"pytest_simcore.asyncio_event_loops",
"pytest_simcore.aws_server",
"pytest_simcore.aws_ec2_service",
"pytest_simcore.aws_iam_service",
"pytest_simcore.aws_server",
"pytest_simcore.aws_ssm_service",
"pytest_simcore.dask_scheduler",
"pytest_simcore.docker",
"pytest_simcore.docker_api_proxy",
"pytest_simcore.docker_compose",
"pytest_simcore.docker_swarm",
"pytest_simcore.docker",
"pytest_simcore.environment_configs",
"pytest_simcore.logging",
"pytest_simcore.rabbit_service",
Expand Down Expand Up @@ -234,13 +235,15 @@ def external_ec2_instances_allowed_types(

@pytest.fixture
def app_environment(
mock_setup_remote_docker_client: Callable[[str], None],
mock_env_devel_environment: EnvVarsDict,
monkeypatch: pytest.MonkeyPatch,
faker: Faker,
aws_allowed_ec2_instance_type_names: list[InstanceTypeType],
ec2_instance_custom_tags: dict[str, str],
external_envfile_dict: EnvVarsDict,
) -> EnvVarsDict:
mock_setup_remote_docker_client("simcore_service_autoscaling.core.application.setup_remote_docker_client")
# SEE https://faker.readthedocs.io/en/master/providers/faker.providers.internet.html?highlight=internet#faker-providers-internet
if external_envfile_dict:
delenvs_from_dict(monkeypatch, mock_env_devel_environment, raising=False)
Expand Down
6 changes: 6 additions & 0 deletions services/director-v2/.env-devel
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ DIRECTOR_V2_SELF_SIGNED_SSL_FILENAME=filename

DIRECTOR_V2_GENERIC_RESOURCE_PLACEMENT_CONSTRAINTS_SUBSTITUTIONS='{}'

DOCKER_API_PROXY_HOST=docker-api-proxy
DOCKER_API_PROXY_PASSWORD=admin
DOCKER_API_PROXY_PORT=8888
DOCKER_API_PROXY_SECURE=False
DOCKER_API_PROXY_USER=admin

LOG_LEVEL=DEBUG

POSTGRES_USER=test
Expand Down
1 change: 1 addition & 0 deletions services/director-v2/requirements/_test.in
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
--constraint _base.txt

aio_pika
aioboto3
alembic # migration due to pytest_simcore.postgres_service2
asgi_lifespan
async-asgi-testclient # replacement for fastapi.testclient.TestClient [see b) below]
Expand Down
54 changes: 54 additions & 0 deletions services/director-v2/requirements/_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,33 @@ aio-pika==9.5.8
# via
# -c requirements/_base.txt
# -r requirements/_test.in
aioboto3==15.5.0
# via -r requirements/_test.in
aiobotocore==2.25.1
# via aioboto3
aiofiles==25.1.0
# via
# -c requirements/_base.txt
# aioboto3
aiohappyeyeballs==2.6.1
# via
# -c requirements/_base.txt
# aiohttp
aiohttp==3.13.2
# via
# -c requirements/../../../requirements/constraints.txt
# -c requirements/_base.txt
# aiobotocore
aioitertools==0.13.0
# via aiobotocore
aiormq==6.9.2
# via
# -c requirements/_base.txt
# aio-pika
aiosignal==1.4.0
# via
# -c requirements/_base.txt
# aiohttp
alembic==1.17.2
# via
# -c requirements/_base.txt
Expand All @@ -23,9 +46,17 @@ async-asgi-testclient==1.4.11
attrs==25.4.0
# via
# -c requirements/_base.txt
# aiohttp
# pytest-docker
bokeh==3.8.1
# via dask
boto3==1.40.61
# via aiobotocore
botocore==1.40.61
# via
# aiobotocore
# boto3
# s3transfer
certifi==2025.11.12
# via
# -c requirements/../../../requirements/constraints.txt
Expand Down Expand Up @@ -70,6 +101,11 @@ fakeredis==2.32.1
# via -r requirements/_test.in
flaky==3.8.1
# via -r requirements/_test.in
frozenlist==1.8.0
# via
# -c requirements/_base.txt
# aiohttp
# aiosignal
fsspec==2025.10.0
# via
# -c requirements/_base.txt
Expand Down Expand Up @@ -109,6 +145,11 @@ jinja2==3.1.6
# bokeh
# dask
# distributed
jmespath==1.1.0
# via
# aiobotocore
# boto3
# botocore
locket==1.0.0
# via
# -c requirements/_base.txt
Expand All @@ -133,6 +174,8 @@ msgpack==1.1.2
multidict==6.7.0
# via
# -c requirements/_base.txt
# aiobotocore
# aiohttp
# async-asgi-testclient
# yarl
mypy==1.18.2
Expand Down Expand Up @@ -178,6 +221,7 @@ pprintpp==0.4.0
propcache==0.4.1
# via
# -c requirements/_base.txt
# aiohttp
# yarl
psutil==7.1.3
# via
Expand Down Expand Up @@ -213,6 +257,8 @@ pytest-xdist==3.8.0
python-dateutil==2.9.0.post0
# via
# -c requirements/_base.txt
# aiobotocore
# botocore
# pandas
pytz==2025.2
# via pandas
Expand All @@ -235,6 +281,8 @@ requests==2.32.5
# docker
respx==0.22.0
# via -r requirements/_test.in
s3transfer==0.14.0
# via boto3
six==1.17.0
# via
# -c requirements/_base.txt
Expand Down Expand Up @@ -293,15 +341,21 @@ urllib3==2.6.3
# via
# -c requirements/../../../requirements/constraints.txt
# -c requirements/_base.txt
# botocore
# distributed
# docker
# requests
wrapt==1.17.3
# via
# -c requirements/_base.txt
# aiobotocore
xyzservices==2025.10.0
# via bokeh
yarl==1.22.0
# via
# -c requirements/_base.txt
# aio-pika
# aiohttp
# aiormq
zict==3.0.0
# via
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ async def list_tracked_dynamic_services(
) -> list[DynamicServiceGet]:
legacy_running_services = await director_v0_client.get_running_services(user_id, project_id)

get_stack_status_tasks = [
get_stack_statuses_tasks = [
scheduler.get_stack_status(service_uuid)
for service_uuid in scheduler.list_services(user_id=user_id, project_id=project_id)
]

# NOTE: Review error handling https://github.com/ITISFoundation/osparc-simcore/issues/3194
dynamic_sidecar_running_services = await asyncio.gather(*get_stack_status_tasks)
dynamic_sidecar_running_services = await asyncio.gather(*get_stack_statuses_tasks)

return legacy_running_services + dynamic_sidecar_running_services

Expand All @@ -94,6 +94,7 @@ async def list_tracked_dynamic_services(
)
@log_decorator(logger=logger)
async def create_dynamic_service(
request: Request,
service: DynamicServiceCreate,
catalog_client: Annotated[CatalogClient, Depends(get_catalog_client)],
director_v0_client: Annotated[DirectorV0Client, Depends(get_director_v0_client)],
Expand Down Expand Up @@ -122,7 +123,9 @@ async def create_dynamic_service(
logger.debug("Redirecting %s", redirect_url_with_query)
return RedirectResponse(str(redirect_url_with_query))

if not await is_sidecar_running(service.node_uuid, dynamic_services_settings.DYNAMIC_SCHEDULER.SWARM_STACK_NAME):
if not await is_sidecar_running(
request.app, service.node_uuid, dynamic_services_settings.DYNAMIC_SCHEDULER.SWARM_STACK_NAME
):
await scheduler.add_service(
service=service,
simcore_service_labels=simcore_service_labels,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from fastapi import FastAPI, HTTPException, status
from fastapi.exceptions import RequestValidationError
from fastapi_lifespan_manager import LifespanManager
from servicelib.fastapi.docker import setup_remote_docker_client
from servicelib.fastapi.lifespan_utils import Lifespan
from servicelib.fastapi.logging_lifespan import create_logging_shutdown_event
from servicelib.fastapi.openapi import (
Expand Down Expand Up @@ -184,6 +185,8 @@ def create_app( # noqa: C901
tracing_settings=settings.DIRECTOR_V2_TRACING,
)

setup_remote_docker_client(app, settings.DIRECTOR_V2_DOCKER_API_PROXY)

db.setup(app, settings.POSTGRES)

if get_tracing_config(app).tracing_enabled:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ class MissingComputationalResourcesError(TaskSchedulingError): # pylint: disabl

class InsufficientComputationalResourcesError(TaskSchedulingError): # pylint: disable=too-many-ancestors
msg_template: str = (
"Insufficient computational resources to run {service_name}:{service_version} with {service_requested_resources} on cluster."
"Insufficient computational resources to run {service_name}:{service_version} "
"with {service_requested_resources} on cluster."
"Cluster available workers: {cluster_available_resources}"
"TIP: Reduce service required resources or contact oSparc support"
)
Expand Down
Loading
Loading