♻️🐛 docker engine cleans up volumes when removing dynamic-sidecars (ITISFoundation#2915)

* volume removal is handled by the docker engine

* refactor and fix test

* removed outdated

* fixed test

* fix tests

* retriggering CI

* fix test

* removed unused

* fix broken test

* bringing back uuid

* renaming

* added better exception handling

* moved to global space

* fix imports

* codestyle and fix missing imports

* moved mounted_volumes to application from global

* disabled rclone test for now

* fix labels

* fixed test

Co-authored-by: Andrei Neagu <[email protected]>

Author: GitHK

services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py

@@ -26,8 +26,8 @@
 )
 from ...models.schemas.dynamic_services import SchedulerData
 from ...modules import projects_networks
-from ...modules.db.repositories.projects_networks import ProjectsNetworksRepository
 from ...modules.db.repositories.projects import ProjectsRepository
+from ...modules.db.repositories.projects_networks import ProjectsNetworksRepository
 from ...modules.dynamic_sidecar.docker_api import (
     is_dynamic_service_running,
     list_dynamic_sidecar_services,

services/director-v2/src/simcore_service_director_v2/models/schemas/constants.py

@@ -11,5 +11,5 @@
     r"(^([a-zA-Z0-9:.-]+)/)?(dynamic-sidecar):([a-zA-Z0-9.-]+$)"
 )
 
-REGEX_DY_SERVICE_SIDECAR = fr"^{DYNAMIC_SIDECAR_SERVICE_PREFIX}_[a-zA-Z0-9-_]*"
-REGEX_DY_SERVICE_PROXY = fr"^{DYNAMIC_PROXY_SERVICE_PREFIX}_[a-zA-Z0-9-_]*"
+REGEX_DY_SERVICE_SIDECAR = rf"^{DYNAMIC_SIDECAR_SERVICE_PREFIX}_[a-zA-Z0-9-_]*"
+REGEX_DY_SERVICE_PROXY = rf"^{DYNAMIC_PROXY_SERVICE_PREFIX}_[a-zA-Z0-9-_]*"

services/director-v2/src/simcore_service_director_v2/modules/db/repositories/projects_networks.py

@@ -2,8 +2,8 @@
 
 import sqlalchemy as sa
 from aiopg.sa.result import RowProxy
-from models_library.projects_networks import NetworksWithAliases, ProjectsNetworks
 from models_library.projects import ProjectID
+from models_library.projects_networks import NetworksWithAliases, ProjectsNetworks
 from sqlalchemy.dialects.postgresql import insert as pg_insert
 
 from ....core.errors import ProjectNotFoundError

services/director-v2/src/simcore_service_director_v2/modules/db/tables.py

@@ -1,8 +1,8 @@
 from simcore_postgres_database.models.comp_pipeline import comp_pipeline
 from simcore_postgres_database.models.comp_runs import comp_runs
 from simcore_postgres_database.models.comp_tasks import NodeClass, StateType, comp_tasks
-from simcore_postgres_database.models.projects_networks import projects_networks
 from simcore_postgres_database.models.projects import ProjectType, projects
+from simcore_postgres_database.models.projects_networks import projects_networks
 
 __all__ = [
     "StateType",

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/client_api.py

@@ -5,8 +5,8 @@
 
 import httpx
 from fastapi import FastAPI
-from models_library.projects_networks import DockerNetworkAlias
 from models_library.projects import ProjectID
+from models_library.projects_networks import DockerNetworkAlias
 from servicelib.utils import logged_gather
 from starlette import status
 

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/events.py

@@ -4,8 +4,8 @@
 
 import httpx
 from fastapi import FastAPI
-from models_library.projects_networks import ProjectsNetworks
 from models_library.projects import ProjectAtDB
+from models_library.projects_networks import ProjectsNetworks
 from models_library.projects_nodes import Node
 from models_library.service_settings_labels import (
     SimcoreServiceLabels,
@@ -22,8 +22,8 @@
 from ....core.settings import AppSettings, DynamicSidecarSettings
 from ....models.schemas.dynamic_services import DynamicSidecarStatus, SchedulerData
 from ....modules.director_v0 import DirectorV0Client
-from ...db.repositories.projects_networks import ProjectsNetworksRepository
 from ...db.repositories.projects import ProjectsRepository
+from ...db.repositories.projects_networks import ProjectsNetworksRepository
 from .._namepsace import get_compose_namespace
 from ..client_api import DynamicSidecarClient, get_dynamic_sidecar_client
 from ..docker_api import (
@@ -586,7 +586,6 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
         await app.state.dynamic_sidecar_scheduler.finish_service_removal(
             scheduler_data.node_uuid
         )
-
         scheduler_data.dynamic_sidecar.service_removal_state.mark_removed()
 
 

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/volumes_resolver.py

@@ -92,10 +92,14 @@ def mount_entry(
         dynamic-sidecar) is running.
         """
         return {
-            "Source": cls.source(compose_namespace, path),
             "Target": cls.target(path),
             "Type": "volume",
-            "VolumeOptions": {"Labels": {"uuid": f"{node_uuid}"}},
+            "VolumeOptions": {
+                "Labels": {
+                    "source": cls.source(compose_namespace, path),
+                    "uuid": f"{node_uuid}",
+                }
+            },
         }
 
     @classmethod
@@ -108,11 +112,11 @@ def mount_r_clone(
         r_clone_settings: RCloneSettings,
     ) -> Dict[str, Any]:
         return {
-            "Source": cls.source(compose_namespace, path),
             "Target": cls.target(path),
             "Type": "volume",
             "VolumeOptions": {
                 "Labels": {
+                    "source": cls.source(compose_namespace, path),
                     "uuid": f"{node_uuid}",
                 },
                 "DriverConfig": _get_s3_volume_driver_config(

services/director-v2/src/simcore_service_director_v2/modules/projects_networks.py

@@ -3,6 +3,7 @@
 from typing import NamedTuple, Set
 from uuid import UUID
 
+from models_library.projects import ProjectAtDB, ProjectID, Workbench
 from models_library.projects_networks import (
     PROJECT_NETWORK_PREFIX,
     ContainerAliases,
@@ -11,7 +12,6 @@
     NetworksWithAliases,
     ProjectsNetworks,
 )
-from models_library.projects import ProjectAtDB, ProjectID, Workbench
 from models_library.projects_nodes_io import NodeIDStr
 from models_library.rabbitmq_messages import LoggerRabbitMessage
 from models_library.service_settings_labels import SimcoreServiceLabels
@@ -23,8 +23,8 @@
 from simcore_service_director_v2.modules.rabbitmq import RabbitMQClient
 
 from ..api.dependencies.director_v0 import DirectorV0Client
-from ..modules.db.repositories.projects_networks import ProjectsNetworksRepository
 from ..modules.db.repositories.projects import ProjectsRepository
+from ..modules.db.repositories.projects_networks import ProjectsNetworksRepository
 from ..modules.dynamic_sidecar.scheduler import DynamicSidecarsScheduler
 
 logger = logging.getLogger(__name__)

services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py

@@ -281,7 +281,16 @@ def _is_docker_r_clone_plugin_installed() -> bool:
     return is_plugin_installed
 
 
-@pytest.fixture(scope="session", params={"true", "false"})
+@pytest.fixture(
+    scope="session",
+    params={
+        # NOTE: There is an issue with the docker rclone volume plugin:
+        # SEE https://github.com/rclone/rclone/issues/6059
+        # Disabling rclone test until this is fixed.
+        # "true",
+        "false",
+    },
+)
 def dev_features_enabled(request) -> str:
     if request.param == "true" and not _is_docker_r_clone_plugin_installed():
         pytest.skip("Required docker plugin `rclone` not installed.")

services/director-v2/tests/unit/test_modules_dynamic_sidecar_volumes_resolver.py

@@ -3,7 +3,7 @@
 import os
 from pathlib import Path
 from typing import Any, Callable, Dict, List
-from uuid import uuid4
+from uuid import UUID
 
 import pytest
 from faker import Faker
@@ -16,29 +16,28 @@
 
 
 # FIXTURES
-@pytest.fixture(scope="module")
-def compose_namespace() -> str:
-    return f"{DYNAMIC_SIDECAR_SERVICE_PREFIX}_{uuid4()}"
+@pytest.fixture
+def compose_namespace(faker: Faker) -> str:
+    return f"{DYNAMIC_SIDECAR_SERVICE_PREFIX}_{faker.uuid4()}"
 
 
-@pytest.fixture(scope="module")
-def node_uuid() -> str:
-    return f"{uuid4()}"
+@pytest.fixture
+def node_uuid(faker: Faker) -> UUID:
+    return faker.uuid4(cast_to=None)
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture
 def state_paths() -> List[Path]:
     return [Path(f"/tmp/asd/asd/{x}") for x in range(10)]
 
 
 @pytest.fixture
-def expect(node_uuid: str) -> Callable[[str, str], Dict[str, Any]]:
+def expected_volume_config(node_uuid: UUID) -> Callable[[str, str], Dict[str, Any]]:
     def _callable(source: str, target: str) -> Dict[str, Any]:
         return {
-            "Source": source,
             "Target": target,
             "Type": "volume",
-            "VolumeOptions": {"Labels": {"uuid": node_uuid}},
+            "VolumeOptions": {"Labels": {"source": source, "uuid": f"{node_uuid}"}},
         }
 
     return _callable
@@ -49,24 +48,24 @@ def _callable(source: str, target: str) -> Dict[str, Any]:
 
 def test_expected_paths(
     compose_namespace: str,
-    node_uuid: str,
+    node_uuid: UUID,
     state_paths: List[Path],
-    expect: Callable[[str, str], Dict[str, Any]],
+    expected_volume_config: Callable[[str, str], Dict[str, Any]],
 ) -> None:
     fake = Faker()
 
     inputs_path = Path(fake.file_path(depth=3)).parent
     assert DynamicSidecarVolumesPathsResolver.mount_entry(
         compose_namespace, inputs_path, node_uuid
-    ) == expect(
+    ) == expected_volume_config(
         source=f"{compose_namespace}{f'{inputs_path}'.replace('/', '_')}",
         target=str(Path("/dy-volumes") / inputs_path.relative_to("/")),
     )
 
     outputs_path = Path(fake.file_path(depth=3)).parent
     assert DynamicSidecarVolumesPathsResolver.mount_entry(
         compose_namespace, outputs_path, node_uuid
-    ) == expect(
+    ) == expected_volume_config(
         source=f"{compose_namespace}{f'{outputs_path}'.replace('/', '_')}",
         target=str(Path("/dy-volumes") / outputs_path.relative_to("/")),
     )
@@ -75,7 +74,7 @@ def test_expected_paths(
         name_from_path = f"{path}".replace(os.sep, "_")
         assert DynamicSidecarVolumesPathsResolver.mount_entry(
             compose_namespace, path, node_uuid
-        ) == expect(
+        ) == expected_volume_config(
             source=f"{compose_namespace}{name_from_path}",
             target=str(Path("/dy-volumes/") / path.relative_to("/")),
         )