diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index 4d74e60e7c3..8edce477cc7 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -8,7 +8,7 @@ async def removal_policy_task(app: FastAPI) -> None: - _logger.info("Removal policy task started") + _logger.info("FAKE Removal policy task started (not yet implemented)") # After X days of inactivity remove data from EFS # Probably use `last_modified_data` in the project DB table diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py index 58e019ef7f9..0946b82177b 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py @@ -27,7 +27,7 @@ class EfsGuardianBackgroundTask(TypedDict): ] -def on_app_startup(app: FastAPI) -> Callable[[], Awaitable[None]]: +def _on_app_startup(app: FastAPI) -> Callable[[], Awaitable[None]]: async def _startup() -> None: with log_context( _logger, logging.INFO, msg="Efs Guardian startup.." @@ -49,7 +49,7 @@ async def _startup() -> None: return _startup -def on_app_shutdown( +def _on_app_shutdown( _app: FastAPI, ) -> Callable[[], Awaitable[None]]: async def _stop() -> None: @@ -69,5 +69,5 @@ async def _stop() -> None: def setup(app: FastAPI) -> None: - app.add_event_handler("startup", on_app_startup(app)) - app.add_event_handler("shutdown", on_app_shutdown(app)) + app.add_event_handler("startup", _on_app_startup(app)) + app.add_event_handler("shutdown", _on_app_shutdown(app)) diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/efs_manager.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/efs_manager.py index 42cb0839564..be0460b7e64 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/efs_manager.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/efs_manager.py @@ -77,8 +77,7 @@ async def get_project_node_data_size( / f"{node_id}" ) - service_size = await efs_manager_utils.get_size_bash_async(_dir_path) - return service_size + return await efs_manager_utils.get_size_bash_async(_dir_path) async def remove_project_node_data_write_permissions( self, project_id: ProjectID, node_id: NodeID diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py index 0f1e87fc13c..11c7781bbae 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py @@ -3,6 +3,7 @@ from fastapi import FastAPI from models_library.rabbitmq_messages import DynamicServiceRunningMessage from pydantic import parse_raw_as +from servicelib.logging_utils import log_context from simcore_service_efs_guardian.services.modules.redis import get_redis_lock_client from ..core.settings import get_application_settings @@ -50,22 +51,16 @@ async def process_dynamic_service_running_message(app: FastAPI, data: bytes) -> ) if size > settings.EFS_DEFAULT_USER_SERVICE_SIZE_BYTES: - _logger.warning( - "Removing write permissions inside of EFS starts for project ID: %s, node ID: %s, current user: %s, size: %s, upper limit: %s", - rabbit_message.project_id, - rabbit_message.node_id, - rabbit_message.user_id, - size, - settings.EFS_DEFAULT_USER_SERVICE_SIZE_BYTES, - ) - redis = get_redis_lock_client(app) - async with redis.lock_context( - f"efs_remove_write_permissions-{rabbit_message.project_id=}-{rabbit_message.node_id=}", - blocking=True, - blocking_timeout_s=10, - ): - await efs_manager.remove_project_node_data_write_permissions( - project_id=rabbit_message.project_id, node_id=rabbit_message.node_id - ) + msg = f"Removing write permissions inside of EFS starts for project ID: {rabbit_message.project_id}, node ID: {rabbit_message.node_id}, current user: {rabbit_message.user_id}, size: {size}, upper limit: {settings.EFS_DEFAULT_USER_SERVICE_SIZE_BYTES}" + with log_context(_logger, logging.WARNING, msg=msg): + redis = get_redis_lock_client(app) + async with redis.lock_context( + f"efs_remove_write_permissions-{rabbit_message.project_id=}-{rabbit_message.node_id=}", + blocking=True, + blocking_timeout_s=10, + ): + await efs_manager.remove_project_node_data_write_permissions( + project_id=rabbit_message.project_id, node_id=rabbit_message.node_id + ) return True diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py index 3c174586fd3..d879d189157 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py @@ -15,7 +15,11 @@ _logger = logging.getLogger(__name__) -_RUT_MESSAGE_TTL_IN_MS = 2 * 60 * 60 * 1000 # 2 hours +_SEC = 1000 # in ms +_MIN = 60 * _SEC # in ms +_HOUR = 60 * _MIN # in ms + +_EFS_MESSAGE_TTL_IN_MS = 2 * _HOUR async def _subscribe_to_rabbitmq(app) -> str: @@ -27,12 +31,12 @@ async def _subscribe_to_rabbitmq(app) -> str: process_dynamic_service_running_message, app ), exclusive_queue=False, - message_ttl=_RUT_MESSAGE_TTL_IN_MS, + message_ttl=_EFS_MESSAGE_TTL_IN_MS, ) return subscribed_queue -def on_app_startup(app: FastAPI) -> Callable[[], Awaitable[None]]: +def _on_app_startup(app: FastAPI) -> Callable[[], Awaitable[None]]: async def _startup() -> None: with log_context( _logger, logging.INFO, msg="setup resource tracker" @@ -48,7 +52,7 @@ async def _startup() -> None: return _startup -def on_app_shutdown( +def _on_app_shutdown( _app: FastAPI, ) -> Callable[[], Awaitable[None]]: async def _stop() -> None: @@ -58,5 +62,5 @@ async def _stop() -> None: def setup(app: FastAPI) -> None: - app.add_event_handler("startup", on_app_startup(app)) - app.add_event_handler("shutdown", on_app_shutdown(app)) + app.add_event_handler("startup", _on_app_startup(app)) + app.add_event_handler("shutdown", _on_app_shutdown(app)) diff --git a/services/efs-guardian/tests/unit/conftest.py b/services/efs-guardian/tests/unit/conftest.py index bb1bc1473f2..da4196ea859 100644 --- a/services/efs-guardian/tests/unit/conftest.py +++ b/services/efs-guardian/tests/unit/conftest.py @@ -2,7 +2,10 @@ # pylint:disable=unused-argument # pylint:disable=redefined-outer-name +import os import re +import shutil +import stat from collections.abc import AsyncIterator, Callable from pathlib import Path from typing import Awaitable @@ -18,6 +21,7 @@ from pytest_mock import MockerFixture from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict from servicelib.rabbitmq import RabbitMQRPCClient +from settings_library.efs import AwsEfsSettings from settings_library.rabbit import RabbitSettings from simcore_service_efs_guardian.core.application import create_app from simcore_service_efs_guardian.core.settings import ApplicationSettings @@ -28,6 +32,7 @@ "pytest_simcore.docker_registry", "pytest_simcore.docker_swarm", "pytest_simcore.environment_configs", + "pytest_simcore.faker_projects_data", "pytest_simcore.pydantic_models", "pytest_simcore.pytest_global_environs", "pytest_simcore.rabbit_service", @@ -148,3 +153,22 @@ async def rpc_client( async def mocked_redis_server(mocker: MockerFixture) -> None: mock_redis = FakeRedis() mocker.patch("redis.asyncio.from_url", return_value=mock_redis) + + +@pytest.fixture +async def cleanup(app: FastAPI): + + yield + + aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS + _dir_path = Path(aws_efs_settings.EFS_MOUNTED_PATH) + if _dir_path.exists(): + for root, dirs, files in os.walk(_dir_path): + for name in dirs + files: + file_path = Path(root, name) + # Get the current permissions of the file or directory + current_permissions = Path.stat(file_path).st_mode + # Add write permission for the owner (user) + Path.chmod(file_path, current_permissions | stat.S_IWUSR) + + shutil.rmtree(_dir_path) diff --git a/services/efs-guardian/tests/unit/test_api_health.py b/services/efs-guardian/tests/unit/test_api_health.py index 5b801de21bb..8b42d559e7f 100644 --- a/services/efs-guardian/tests/unit/test_api_health.py +++ b/services/efs-guardian/tests/unit/test_api_health.py @@ -30,7 +30,7 @@ def app_environment( async def test_healthcheck( rabbit_service: RabbitSettings, - mocked_redis_server, + mocked_redis_server: None, client: httpx.AsyncClient, ): response = await client.get("/") diff --git a/services/efs-guardian/tests/unit/test_efs_guardian_rpc.py b/services/efs-guardian/tests/unit/test_efs_guardian_rpc.py index 9e2d6711c2a..48474a69d45 100644 --- a/services/efs-guardian/tests/unit/test_efs_guardian_rpc.py +++ b/services/efs-guardian/tests/unit/test_efs_guardian_rpc.py @@ -10,6 +10,8 @@ import pytest from faker import Faker from fastapi import FastAPI +from models_library.projects import ProjectID +from models_library.projects_nodes import NodeID from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from servicelib.rabbitmq import RabbitMQRPCClient @@ -40,20 +42,21 @@ async def test_rpc_create_project_specific_data_dir( rpc_client: RabbitMQRPCClient, faker: Faker, app: FastAPI, + project_id: ProjectID, + node_id: NodeID, + cleanup: None, ): aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS - _project_id = faker.uuid4() - _node_id = faker.uuid4() - _storage_directory_name = faker.name() + _storage_directory_name = faker.word() with patch( "simcore_service_efs_guardian.services.efs_manager.os.chown" ) as mocked_chown: result = await efs_manager.create_project_specific_data_dir( rpc_client, - project_id=_project_id, - node_id=_node_id, + project_id=project_id, + node_id=node_id, storage_directory_name=_storage_directory_name, ) mocked_chown.assert_called_once() @@ -62,8 +65,8 @@ async def test_rpc_create_project_specific_data_dir( _expected_path = ( aws_efs_settings.EFS_MOUNTED_PATH / aws_efs_settings.EFS_PROJECT_SPECIFIC_DATA_DIRECTORY - / _project_id - / _node_id + / f"{project_id}" + / f"{node_id}" / _storage_directory_name ) assert _expected_path == result diff --git a/services/efs-guardian/tests/unit/test_efs_manager.py b/services/efs-guardian/tests/unit/test_efs_manager.py index e041cb79b44..5c5c57cf3ab 100644 --- a/services/efs-guardian/tests/unit/test_efs_manager.py +++ b/services/efs-guardian/tests/unit/test_efs_manager.py @@ -4,8 +4,6 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable -import os -import shutil import stat from pathlib import Path from unittest.mock import patch @@ -41,32 +39,13 @@ def app_environment( ) -@pytest.fixture -async def cleanup(app: FastAPI): - - yield - - aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS - _dir_path = Path(aws_efs_settings.EFS_MOUNTED_PATH) - if Path.exists(_dir_path): - for root, dirs, files in os.walk(_dir_path): - for name in dirs + files: - file_path = os.path.join(root, name) - # Get the current permissions of the file or directory - current_permissions = os.stat(file_path).st_mode - # Add write permission for the owner (user) - os.chmod(file_path, current_permissions | stat.S_IWUSR) - - shutil.rmtree(_dir_path) - - def assert_permissions( file_path: Path, expected_readable: bool, expected_writable: bool, expected_executable: bool, ): - file_stat = os.stat(file_path) + file_stat = Path.stat(file_path) file_permissions = file_stat.st_mode is_readable = bool(file_permissions & stat.S_IRUSR) is_writable = bool(file_permissions & stat.S_IWUSR) @@ -88,17 +67,17 @@ async def test_remove_write_access_rights( mocked_redis_server: None, app: FastAPI, cleanup: None, + project_id: ProjectID, + node_id: NodeID, ): aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS - _project_id = ProjectID(faker.uuid4()) - _node_id = NodeID(faker.uuid4()) _storage_directory_name = faker.word() _dir_path = ( aws_efs_settings.EFS_MOUNTED_PATH / aws_efs_settings.EFS_PROJECT_SPECIFIC_DATA_DIRECTORY - / f"{_project_id}" - / f"{_node_id}" + / f"{project_id}" + / f"{node_id}" / f"{_storage_directory_name}" ) @@ -106,7 +85,7 @@ async def test_remove_write_access_rights( assert ( await efs_manager.check_project_node_data_directory_exits( - project_id=_project_id, node_id=_node_id + project_id=project_id, node_id=node_id ) is False ) @@ -115,31 +94,31 @@ async def test_remove_write_access_rights( "simcore_service_efs_guardian.services.efs_manager.os.chown" ) as mocked_chown: await efs_manager.create_project_specific_data_dir( - project_id=_project_id, - node_id=_node_id, + project_id=project_id, + node_id=node_id, storage_directory_name=_storage_directory_name, ) + assert mocked_chown.called assert ( await efs_manager.check_project_node_data_directory_exits( - project_id=_project_id, node_id=_node_id + project_id=project_id, node_id=node_id ) is True ) size_before = await efs_manager.get_project_node_data_size( - project_id=_project_id, node_id=_node_id + project_id=project_id, node_id=node_id ) file_paths = [] for i in range(3): # Let's create 3 small files for testing file_path = Path(_dir_path, f"test_file_{i}.txt") - with open(file_path, "w") as f: - f.write(f"This is file {i}") + file_path.write_text(f"This is file {i}") file_paths.append(file_path) size_after = await efs_manager.get_project_node_data_size( - project_id=_project_id, node_id=_node_id + project_id=project_id, node_id=node_id ) assert size_after > size_before @@ -153,7 +132,7 @@ async def test_remove_write_access_rights( ) await efs_manager.remove_project_node_data_write_permissions( - project_id=_project_id, node_id=_node_id + project_id=project_id, node_id=node_id ) for file_path in file_paths: