🐛 Fix issue with agent and volume permissions when backing up (#8214)

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

Author: GitHK

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/container_utils.py renamed to packages/service-library/src/servicelib/container_utils.py

@@ -1,19 +1,35 @@
 import asyncio
 import logging
 from collections.abc import Sequence
-from typing import Any
+from typing import Any, Final
 
 from aiodocker import Docker, DockerError
 from aiodocker.execs import Exec
 from aiodocker.stream import Stream
+from common_library.errors_classes import OsparcErrorMixin
 from pydantic import NonNegativeFloat
-from starlette import status
 
-from ..core.errors import (
-    ContainerExecCommandFailedError,
-    ContainerExecContainerNotFoundError,
-    ContainerExecTimeoutError,
-)
+
+class BaseContainerUtilsError(OsparcErrorMixin, Exception):
+    pass
+
+
+class ContainerExecContainerNotFoundError(BaseContainerUtilsError):
+    msg_template = "Container '{container_name}' was not found"
+
+
+class ContainerExecTimeoutError(BaseContainerUtilsError):
+    msg_template = "Timed out after {timeout} while executing: '{command}'"
+
+
+class ContainerExecCommandFailedError(BaseContainerUtilsError):
+    msg_template = (
+        "Command '{command}' exited with code '{exit_code}'"
+        "and output: '{command_result}'"
+    )
+
+
+_HTTP_404_NOT_FOUND: Final[int] = 404
 
 _logger = logging.getLogger(__name__)
 
@@ -77,10 +93,10 @@ async def run_command_in_container(
             _execute_command(container_name, command), timeout
         )
     except DockerError as e:
-        if e.status == status.HTTP_404_NOT_FOUND:
+        if e.status == _HTTP_404_NOT_FOUND:
             raise ContainerExecContainerNotFoundError(
                 container_name=container_name
             ) from e
         raise
-    except asyncio.TimeoutError as e:
+    except TimeoutError as e:
         raise ContainerExecTimeoutError(timeout=timeout, command=command) from e

services/dynamic-sidecar/tests/unit/test_modules_container_utils.py renamed to packages/service-library/tests/test_container_utils.py

@@ -1,11 +1,10 @@
 # pylint: disable=redefined-outer-name
 
-import contextlib
 from collections.abc import AsyncIterable
 
 import aiodocker
 import pytest
-from simcore_service_dynamic_sidecar.modules.container_utils import (
+from servicelib.container_utils import (
     ContainerExecCommandFailedError,
     ContainerExecContainerNotFoundError,
     ContainerExecTimeoutError,
@@ -26,9 +25,7 @@ async def running_container_name() -> AsyncIterable[str]:
 
         yield container_inspect["Name"][1:]
 
-        with contextlib.suppress(aiodocker.DockerError):
-            await container.kill()
-        await container.delete()
+        await container.delete(force=True)
 
 
 async def test_run_command_in_container_container_not_found():

services/agent/src/simcore_service_agent/services/backup.py

@@ -1,18 +1,23 @@
 import asyncio
 import logging
+import os
 import tempfile
 from asyncio.streams import StreamReader
+from datetime import timedelta
 from pathlib import Path
 from textwrap import dedent
 from typing import Final
 from uuid import uuid4
 
 from fastapi import FastAPI
+from servicelib.container_utils import run_command_in_container
 from settings_library.utils_r_clone import resolve_provider
 
 from ..core.settings import ApplicationSettings
 from ..models.volumes import DynamicServiceVolumeLabels, VolumeDetails
 
+_TIMEOUT_PERMISSION_CHANGES: Final[timedelta] = timedelta(minutes=5)
+
 _logger = logging.getLogger(__name__)
 
 
@@ -107,6 +112,15 @@ def _log_expected_operation(
     _logger.log(log_level, formatted_message)
 
 
+async def _ensure_permissions_on_source_dir(source_dir: Path) -> None:
+    self_container = os.environ["HOSTNAME"]
+    await run_command_in_container(
+        self_container,
+        command=f"chmod -R o+rX '{source_dir}'",
+        timeout=_TIMEOUT_PERMISSION_CHANGES.total_seconds(),
+    )
+
+
 async def _store_in_s3(
     settings: ApplicationSettings, volume_name: str, volume_details: VolumeDetails
 ) -> None:
@@ -148,6 +162,8 @@ async def _store_in_s3(
         volume_details.labels, s3_path, r_clone_ls_output, volume_name
     )
 
+    await _ensure_permissions_on_source_dir(source_dir)
+
     # sync files via rclone
     r_clone_sync = [
         "rclone",

services/agent/tests/unit/test_services_backup.py

@@ -1,12 +1,14 @@
 # pylint: disable=redefined-outer-name
+# pylint: disable=unused-argument
 
 import asyncio
-from collections.abc import Awaitable, Callable
+from collections.abc import AsyncIterable, Awaitable, Callable
 from pathlib import Path
 from typing import Final
 from uuid import uuid4
 
 import aioboto3
+import aiodocker
 import pytest
 from fastapi import FastAPI
 from models_library.projects import ProjectID
@@ -37,6 +39,28 @@ def volume_content(tmpdir: Path) -> Path:
     return path
 
 
+@pytest.fixture
+async def mock_container_with_data(
+    volume_content: Path, monkeypatch: pytest.MonkeyPatch
+) -> AsyncIterable[None]:
+    async with aiodocker.Docker() as client:
+        container = await client.containers.run(
+            config={
+                "Image": "alpine:latest",
+                "Cmd": ["/bin/ash", "-c", "sleep 10000"],
+                "HostConfig": {"Binds": [f"{volume_content}:{volume_content}:rw"]},
+            }
+        )
+        container_inspect = await container.show()
+
+        container_name = container_inspect["Name"][1:]
+        monkeypatch.setenv("HOSTNAME", container_name)
+
+        yield None
+
+        await container.delete(force=True)
+
+
 @pytest.fixture
 def downlaoded_from_s3(tmpdir: Path) -> Path:
     path = Path(tmpdir) / "downloaded_from_s3"
@@ -45,6 +69,7 @@ def downlaoded_from_s3(tmpdir: Path) -> Path:
 
 
 async def test_backup_volume(
+    mock_container_with_data: None,
     volume_content: Path,
     project_id: ProjectID,
     swarm_stack_name: str,

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers.py

@@ -14,14 +14,15 @@
     ActivityInfoOrNone,
 )
 from pydantic import TypeAdapter, ValidationError
-from servicelib.fastapi.requests_decorators import cancel_on_disconnect
-
-from ...core.docker_utils import docker_client
-from ...core.errors import (
+from servicelib.container_utils import (
     ContainerExecCommandFailedError,
     ContainerExecContainerNotFoundError,
     ContainerExecTimeoutError,
+    run_command_in_container,
 )
+from servicelib.fastapi.requests_decorators import cancel_on_disconnect
+
+from ...core.docker_utils import docker_client
 from ...core.settings import ApplicationSettings
 from ...core.validation import (
     ComposeSpecValidation,
@@ -30,7 +31,6 @@
 )
 from ...models.schemas.containers import ContainersComposeSpec
 from ...models.shared_store import SharedStore
-from ...modules.container_utils import run_command_in_container
 from ...modules.mounted_fs import MountedVolumes
 from ._dependencies import (
     get_container_restart_lock,

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/errors.py

@@ -14,18 +14,3 @@ class VolumeNotFoundError(BaseDynamicSidecarError):
 
 class UnexpectedDockerError(BaseDynamicSidecarError):
     msg_template = "An unexpected Docker error occurred status_code={status_code}, message={message}"
-
-
-class ContainerExecContainerNotFoundError(BaseDynamicSidecarError):
-    msg_template = "Container '{container_name}' was not found"
-
-
-class ContainerExecTimeoutError(BaseDynamicSidecarError):
-    msg_template = "Timed out after {timeout} while executing: '{command}'"
-
-
-class ContainerExecCommandFailedError(BaseDynamicSidecarError):
-    msg_template = (
-        "Command '{command}' exited with code '{exit_code}'"
-        "and output: '{command_result}'"
-    )

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/long_running_tasks_utils.py

@@ -5,16 +5,16 @@
 
 from aiodocker import DockerError
 from models_library.callbacks_mapping import UserServiceCommand
-from servicelib.logging_utils import log_context
-
-from ..core.errors import (
+from servicelib.container_utils import (
     ContainerExecCommandFailedError,
     ContainerExecContainerNotFoundError,
     ContainerExecTimeoutError,
+    run_command_in_container,
 )
+from servicelib.logging_utils import log_context
+
 from ..models.shared_store import SharedStore
 from ..modules.mounted_fs import MountedVolumes
-from .container_utils import run_command_in_container
 
 _logger = logging.getLogger(__name__)
 

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/prometheus_metrics.py

@@ -10,14 +10,14 @@
 from fastapi import FastAPI, status
 from models_library.callbacks_mapping import CallbacksMapping, UserServiceCommand
 from pydantic import BaseModel, NonNegativeFloat, NonNegativeInt
-from servicelib.logging_utils import log_context
-from servicelib.sequences_utils import pairwise
-from simcore_service_dynamic_sidecar.core.errors import (
+from servicelib.container_utils import (
     ContainerExecContainerNotFoundError,
+    run_command_in_container,
 )
+from servicelib.logging_utils import log_context
+from servicelib.sequences_utils import pairwise
 
 from ..models.shared_store import SharedStore
-from .container_utils import run_command_in_container
 
 _logger = logging.getLogger(__name__)
 

services/dynamic-sidecar/tests/unit/test_core_docker_utils.py

@@ -2,7 +2,6 @@
 # pylint: disable=unused-argument
 # pylint: disable=unused-variable
 from collections.abc import AsyncIterable, AsyncIterator
-from contextlib import suppress
 
 import aiodocker
 import pytest
@@ -73,9 +72,7 @@ async def started_services(container_names: list[str]) -> AsyncIterator[None]:
         yield
 
         for container in started_containers:
-            with suppress(aiodocker.DockerError):
-                await container.kill()
-            await container.delete()
+            await container.delete(force=True)
 
 
 async def test_volume_with_label(