Skip to content

Commit f7ce90e

Browse files
GitHKAndrei Neagu
andauthored
🐛 sidecar no longer fails when saving data with missing permissions (ITISFoundation#7307)
Co-authored-by: Andrei Neagu <[email protected]>
1 parent 21f2405 commit f7ce90e

File tree

10 files changed

+62
-9
lines changed

10 files changed

+62
-9
lines changed

services/director-v2/src/simcore_service_director_v2/cli/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@
2626
DEFAULT_OUTPUTS_PUSH_ATTEMPTS: Final[int] = 3
2727
DEFAULT_TASK_UPDATE_INTERVAL_S: Final[int] = 1
2828

29-
main = typer.Typer(name=PROJECT_NAME)
29+
main = typer.Typer(
30+
name=PROJECT_NAME,
31+
pretty_exceptions_enable=False,
32+
pretty_exceptions_show_locals=False,
33+
)
3034

3135
_logger = logging.getLogger(__name__)
3236

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ async def runs_docker_compose_down_task(
124124
settings: Annotated[ApplicationSettings, Depends(get_settings)],
125125
shared_store: Annotated[SharedStore, Depends(get_shared_store)],
126126
app: Annotated[FastAPI, Depends(get_application)],
127+
mounted_volumes: Annotated[MountedVolumes, Depends(get_mounted_volumes)],
127128
) -> TaskId:
128129
assert request # nosec
129130

@@ -135,6 +136,7 @@ async def runs_docker_compose_down_task(
135136
app=app,
136137
shared_store=shared_store,
137138
settings=settings,
139+
mounted_volumes=mounted_volumes,
138140
)
139141
except TaskAlreadyRunningError as e:
140142
return cast(str, e.managed_task.task_id) # type: ignore[attr-defined] # pylint:disable=no-member

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/cli.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import asyncio
22
import json
33
import logging
4+
from collections.abc import AsyncIterator
45
from contextlib import asynccontextmanager
5-
from typing import AsyncIterator
66

77
import typer
88
from fastapi import FastAPI
@@ -17,7 +17,11 @@
1717
from .modules.outputs import OutputsManager, setup_outputs
1818

1919
log = logging.getLogger(__name__)
20-
main = typer.Typer(name=PROJECT_NAME)
20+
main = typer.Typer(
21+
name=PROJECT_NAME,
22+
pretty_exceptions_enable=False,
23+
pretty_exceptions_show_locals=False,
24+
)
2125

2226

2327
main.command()(create_settings_command(settings_cls=ApplicationSettings, logger=log))

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@
5353
from ..modules.mounted_fs import MountedVolumes
5454
from ..modules.notifications._notifications_ports import PortNotifier
5555
from ..modules.outputs import OutputsManager, event_propagation_disabled
56-
from .long_running_tasksutils import run_before_shutdown_actions
56+
from .long_running_tasks_utils import (
57+
ensure_read_permissions_on_user_service_data,
58+
run_before_shutdown_actions,
59+
)
5760
from .resource_tracking import send_service_started, send_service_stopped
5861

5962
_logger = logging.getLogger(__name__)
@@ -237,6 +240,7 @@ async def task_runs_docker_compose_down(
237240
app: FastAPI,
238241
shared_store: SharedStore,
239242
settings: ApplicationSettings,
243+
mounted_volumes: MountedVolumes,
240244
) -> None:
241245
if shared_store.compose_spec is None:
242246
_logger.warning("No compose-spec was found")
@@ -312,6 +316,8 @@ async def _send_resource_tracking_stop(platform_status: SimcorePlatformStatus):
312316
await _send_resource_tracking_stop(SimcorePlatformStatus.OK)
313317
raise
314318

319+
await ensure_read_permissions_on_user_service_data(mounted_volumes)
320+
315321
await _send_resource_tracking_stop(SimcorePlatformStatus.OK)
316322

317323
# removing compose-file spec
Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import logging
2+
import os
3+
from datetime import timedelta
4+
from typing import Final
25

36
from models_library.callbacks_mapping import UserServiceCommand
47
from servicelib.logging_utils import log_context
@@ -9,10 +12,13 @@
912
ContainerExecTimeoutError,
1013
)
1114
from ..models.shared_store import SharedStore
12-
from ..modules.container_utils import run_command_in_container
15+
from ..modules.mounted_fs import MountedVolumes
16+
from .container_utils import run_command_in_container
1317

1418
_logger = logging.getLogger(__name__)
1519

20+
_TIMEOUT_PERMISSION_CHANGES: Final[timedelta] = timedelta(minutes=5)
21+
1622

1723
async def run_before_shutdown_actions(
1824
shared_store: SharedStore, before_shutdown: list[UserServiceCommand]
@@ -40,3 +46,22 @@ async def run_before_shutdown_actions(
4046
container_name,
4147
exc_info=True,
4248
)
49+
50+
51+
async def ensure_read_permissions_on_user_service_data(
52+
mounted_volumes: MountedVolumes,
53+
) -> None:
54+
# Makes sure sidecar has access to all files in the user services.
55+
# The user could have removed read permissions form a file, which will cause an error.
56+
57+
# NOTE: command runs inside self container since the user service container might not always be running
58+
self_container = os.environ["HOSTNAME"]
59+
for path_to_store in ( # apply changes to otuputs and all state folders
60+
*mounted_volumes.disk_state_paths_iter(),
61+
mounted_volumes.disk_outputs_path,
62+
):
63+
await run_command_in_container(
64+
self_container,
65+
command=f"chmod -R o+rX '{path_to_store}'",
66+
timeout=_TIMEOUT_PERMISSION_CHANGES.total_seconds(),
67+
)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ async def _update_metrics(self):
107107
)
108108
self._metrics_response = MetricsResponse.from_reply(metrics_fetch_result)
109109
except ContainerExecContainerNotFoundError as e:
110-
_logger.info(
111-
"Container %s was not found could nto recover metrics",
110+
_logger.debug(
111+
"Container %s was not found could not recover metrics",
112112
container_name,
113113
)
114114
self._metrics_response = MetricsResponse.from_error(e)
115115
except Exception as e: # pylint: disable=broad-exception-caught
116-
_logger.info("Unexpected exception", exc_info=True)
116+
_logger.debug("Could not recover metrics", exc_info=True)
117117
self._metrics_response = MetricsResponse.from_error(e)
118118

119119
async def _task_metrics_recovery(self) -> None:

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from aiodocker.volumes import DockerVolume
1212
from async_asgi_testclient import TestClient
1313
from fastapi import FastAPI
14+
from pytest_mock.plugin import MockerFixture
1415
from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict
1516
from simcore_service_dynamic_sidecar.core.application import AppState, create_app
1617
from simcore_service_dynamic_sidecar.core.docker_compose_utils import (
@@ -157,3 +158,10 @@ def port_notifier(app: FastAPI) -> PortNotifier:
157158
settings.DY_SIDECAR_PROJECT_ID,
158159
settings.DY_SIDECAR_NODE_ID,
159160
)
161+
162+
163+
@pytest.fixture
164+
def mock_ensure_read_permissions_on_user_service_data(mocker: MockerFixture) -> None:
165+
mocker.patch(
166+
"simcore_service_dynamic_sidecar.modules.long_running_tasks.ensure_read_permissions_on_user_service_data",
167+
)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,7 @@ def _get_awaitable() -> Awaitable:
520520

521521

522522
async def test_containers_down_after_starting(
523+
mock_ensure_read_permissions_on_user_service_data: None,
523524
httpx_async_client: AsyncClient,
524525
client: Client,
525526
compose_spec: str,

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ async def _get_task_id_docker_compose_down(httpx_async_client: AsyncClient) -> T
172172

173173

174174
def _get_resource_tracking_messages(
175-
mock_core_rabbitmq: dict[str, AsyncMock]
175+
mock_core_rabbitmq: dict[str, AsyncMock],
176176
) -> list[RabbitResourceTrackingMessages]:
177177
return [
178178
x[0][1]
@@ -200,6 +200,7 @@ async def _wait_for_containers_to_be_running(app: FastAPI) -> None:
200200

201201

202202
async def test_service_starts_and_closes_as_expected(
203+
mock_ensure_read_permissions_on_user_service_data: None,
203204
mock_core_rabbitmq: dict[str, AsyncMock],
204205
app: FastAPI,
205206
httpx_async_client: AsyncClient,
@@ -383,6 +384,7 @@ async def _mocked_get_container_states(
383384

384385
@pytest.mark.parametrize("expected_platform_state", SimcorePlatformStatus)
385386
async def test_user_services_crash_when_running(
387+
mock_ensure_read_permissions_on_user_service_data: None,
386388
mock_core_rabbitmq: dict[str, AsyncMock],
387389
app: FastAPI,
388390
httpx_async_client: AsyncClient,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ async def logging_event_handler_observer(
125125
],
126126
)
127127
async def test_chown_triggers_event(
128+
mock_ensure_read_permissions_on_user_service_data: None,
128129
logging_event_handler_observer: None,
129130
fake_dy_volumes_mount_dir: Path,
130131
command_template: str,

0 commit comments

Comments
 (0)