From 816cc0a65d2cf591afc5f0d3fa40e9fdf0f636b0 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 20 Apr 2023 08:51:33 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"=E2=99=BB=EF=B8=8F=20Volumes=20keep?= =?UTF-8?q?=20track=20if=20the=20data=20requires=20saving=20=F0=9F=9A=A8?= =?UTF-8?q?=20(#3974)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 5b9e9bc44d1c108d672ec7a3fad6519ea4248424. --- .../src/models_library/users.py | 6 +- .../src/models_library/volumes.py | 30 ---- .../service-library/requirements/_base.in | 5 +- .../src/servicelib/enum_utils.py | 13 -- .../src/servicelib/file_constants.py | 5 - .../src/servicelib/volumes_utils.py | 34 ----- .../tests/test_volumes_utils.py | 34 ----- .../simcore_service_agent/core/settings.py | 3 +- .../unit/test_modules_volumes_cleanup_s3.py | 10 +- .../api/routes/dynamic_services.py | 1 - .../models/domains/dynamic_services.py | 4 - .../schemas/dynamic_services/scheduler.py | 5 +- .../dynamic_sidecar/api_client/_public.py | 15 -- .../dynamic_sidecar/api_client/_thin.py | 14 -- .../modules/dynamic_sidecar/scheduler/_abc.py | 1 - .../scheduler/_core/_events_utils.py | 38 ----- .../scheduler/_core/_scheduler.py | 2 - .../dynamic_sidecar/scheduler/_task.py | 3 - .../02/test_dynamic_services_routes.py | 1 - .../director-v2/tests/integration/02/utils.py | 9 +- .../tests/mocks/fake_scheduler_data.json | 3 +- .../fake_scheduler_data_compose_spec.json | 3 +- services/director-v2/tests/unit/conftest.py | 7 - .../unit/test_api_route_dynamic_scheduler.py | 4 +- ...dules_dynamic_sidecar_client_api_public.py | 24 --- ...modules_dynamic_sidecar_client_api_thin.py | 27 ---- ...es_dynamic_sidecar_docker_service_specs.py | 2 +- .../unit/with_dbs/test_modules_node_rights.py | 1 + services/dynamic-sidecar/openapi.json | 77 ---------- .../api/_routing.py | 13 +- .../api/volumes.py | 29 ---- .../core/application.py | 9 +- .../core/utils.py | 17 +++ .../modules/mounted_fs.py | 27 ++-- .../modules/nodeports.py | 6 +- .../modules/volume_files.py | 139 ------------------ .../tests/unit/test_api_volumes.py | 53 ------- .../unit/test_modules_outputs_manager.py | 3 +- .../unit/test_modules_outputs_watcher.py | 7 +- .../tests/unit/test_modules_volume_files.py | 63 -------- .../director_v2_core_dynamic_services.py | 2 - .../projects/projects_api.py | 10 -- .../02/test_projects_handlers__open_close.py | 4 - 43 files changed, 55 insertions(+), 708 deletions(-) delete mode 100644 packages/models-library/src/models_library/volumes.py delete mode 100644 packages/service-library/src/servicelib/enum_utils.py delete mode 100644 packages/service-library/src/servicelib/file_constants.py delete mode 100644 packages/service-library/src/servicelib/volumes_utils.py delete mode 100644 packages/service-library/tests/test_volumes_utils.py delete mode 100644 services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/volumes.py delete mode 100644 services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/volume_files.py delete mode 100644 services/dynamic-sidecar/tests/unit/test_api_volumes.py delete mode 100644 services/dynamic-sidecar/tests/unit/test_modules_volume_files.py diff --git a/packages/models-library/src/models_library/users.py b/packages/models-library/src/models_library/users.py index 15e736359b4..9847e22c809 100644 --- a/packages/models-library/src/models_library/users.py +++ b/packages/models-library/src/models_library/users.py @@ -1,6 +1,4 @@ -from typing import TypeAlias - from pydantic import PositiveInt -UserID: TypeAlias = PositiveInt -GroupID: TypeAlias = PositiveInt +UserID = PositiveInt +GroupID = PositiveInt diff --git a/packages/models-library/src/models_library/volumes.py b/packages/models-library/src/models_library/volumes.py deleted file mode 100644 index 44be98e3661..00000000000 --- a/packages/models-library/src/models_library/volumes.py +++ /dev/null @@ -1,30 +0,0 @@ -from enum import auto - -from .utils.enums import StrAutoEnum - - -class VolumeCategory(StrAutoEnum): - """ - These uniquely identify volumes which are mounted by - the dynamic-sidecar and user services. - - This is primarily used to keep track of the status of - each individual volume on the volumes. - - The status is ingested by the agent and processed - when the volume is removed. - """ - - # contains data relative to output ports - OUTPUTS = auto() - - # contains data relative to input ports - INPUTS = auto() - - # contains files which represent the state of the service - # usually the user's workspace - STATES = auto() - - # contains dynamic-sidecar data required to maintain state - # between restarts - SHARED_STORE = auto() diff --git a/packages/service-library/requirements/_base.in b/packages/service-library/requirements/_base.in index 33f9e7939ae..0a47104d50d 100644 --- a/packages/service-library/requirements/_base.in +++ b/packages/service-library/requirements/_base.in @@ -4,14 +4,13 @@ --constraint ../../../requirements/constraints.txt --constraint ./constraints.txt - -aio-pika aiodebug aiofiles +aio-pika arrow # date/time +redis pydantic pyinstrument pyyaml -redis tenacity tqdm diff --git a/packages/service-library/src/servicelib/enum_utils.py b/packages/service-library/src/servicelib/enum_utils.py deleted file mode 100644 index 6399a60cb04..00000000000 --- a/packages/service-library/src/servicelib/enum_utils.py +++ /dev/null @@ -1,13 +0,0 @@ -# NOTE: this is a duplicate, but currently we don't want to enforce a dependency between -# service-library and models-library -# should go away and removed when the following case is solved -# https://github.com/ITISFoundation/osparc-simcore/issues/4013 - -from enum import Enum, unique - - -@unique -class StrAutoEnum(str, Enum): - @staticmethod - def _generate_next_value_(name, start, count, last_values): - return name.upper() diff --git a/packages/service-library/src/servicelib/file_constants.py b/packages/service-library/src/servicelib/file_constants.py deleted file mode 100644 index fdf3aacbd31..00000000000 --- a/packages/service-library/src/servicelib/file_constants.py +++ /dev/null @@ -1,5 +0,0 @@ -from typing import Final - -HIDDEN_FILE_NAME: Final[str] = ".hidden_do_not_remove" -AGENT_FILE_NAME: Final[str] = ".agent" -KEY_VALUE_FILE_NAME: Final[str] = "key_values.json" diff --git a/packages/service-library/src/servicelib/volumes_utils.py b/packages/service-library/src/servicelib/volumes_utils.py deleted file mode 100644 index 34d52ee4ada..00000000000 --- a/packages/service-library/src/servicelib/volumes_utils.py +++ /dev/null @@ -1,34 +0,0 @@ -from datetime import datetime -from enum import auto -from pathlib import Path - -import aiofiles -import arrow -from pydantic import BaseModel, Field - -from .enum_utils import StrAutoEnum - - -class VolumeStatus(StrAutoEnum): - CONTENT_NEEDS_TO_BE_SAVED = auto() - CONTENT_WAS_SAVED = auto() - CONTENT_NO_SAVE_REQUIRED = auto() - - -class VolumeState(BaseModel): - status: VolumeStatus - last_changed: datetime = Field(default_factory=lambda: arrow.utcnow().datetime) - - def __eq__(self, other: "VolumeState") -> bool: - # only include status for equality last_changed is not important - return self.status == other.status - - -async def load_volume_state(agent_file_path: Path) -> VolumeState: - async with aiofiles.open(agent_file_path, mode="r") as f: - return VolumeState.parse_raw(await f.read()) - - -async def save_volume_state(agent_file_path: Path, volume_state: VolumeState) -> None: - async with aiofiles.open(agent_file_path, mode="w") as f: - await f.write(volume_state.json()) diff --git a/packages/service-library/tests/test_volumes_utils.py b/packages/service-library/tests/test_volumes_utils.py deleted file mode 100644 index 792b71fb58b..00000000000 --- a/packages/service-library/tests/test_volumes_utils.py +++ /dev/null @@ -1,34 +0,0 @@ -# pylint: disable=redefined-outer-name - -from pathlib import Path - -import pytest -from pytest import FixtureRequest -from servicelib.volumes_utils import ( - VolumeState, - VolumeStatus, - load_volume_state, - save_volume_state, -) - - -@pytest.fixture -def agent_file_path(tmp_path: Path) -> Path: - return tmp_path / "fake_agent_file" - - -@pytest.fixture(params=VolumeStatus) -def status(request: FixtureRequest) -> VolumeStatus: - return request.param - - -async def test_save_load_volume_state(agent_file_path: Path, status: VolumeStatus): - to_save_volume_state = VolumeState(status=status) - await save_volume_state(agent_file_path, to_save_volume_state) - assert await load_volume_state(agent_file_path) == to_save_volume_state - - -def test_volume_state_equality(status: VolumeStatus): - assert VolumeState(status=status) == VolumeState(status=status) - schema_property_count = len(VolumeState.schema()["properties"]) - assert len(VolumeState(status=status).dict()) == schema_property_count diff --git a/services/agent/src/simcore_service_agent/core/settings.py b/services/agent/src/simcore_service_agent/core/settings.py index 573ed0a92bf..a17811bd276 100644 --- a/services/agent/src/simcore_service_agent/core/settings.py +++ b/services/agent/src/simcore_service_agent/core/settings.py @@ -2,7 +2,6 @@ from models_library.basic_types import BootModeEnum, LogLevel from pydantic import Field, NonNegativeInt, validator -from servicelib.file_constants import AGENT_FILE_NAME, HIDDEN_FILE_NAME from settings_library.base import BaseCustomSettings from settings_library.r_clone import S3Provider from settings_library.utils_logging import MixinLoggingSettings @@ -33,7 +32,7 @@ class ApplicationSettings(BaseCustomSettings, MixinLoggingSettings): 5, description="parallel transfers to s3" ) AGENT_VOLUMES_CLEANUP_EXCLUDE_FILES: list[str] = Field( - [AGENT_FILE_NAME, HIDDEN_FILE_NAME, "key_values.json"], + [".hidden_do_not_remove", "key_values.json"], description="Files to ignore when syncing to s3", ) AGENT_VOLUMES_CLEANUP_INTERVAL_S: NonNegativeInt = Field( diff --git a/services/agent/tests/unit/test_modules_volumes_cleanup_s3.py b/services/agent/tests/unit/test_modules_volumes_cleanup_s3.py index bb4978bda19..29639d4aadd 100644 --- a/services/agent/tests/unit/test_modules_volumes_cleanup_s3.py +++ b/services/agent/tests/unit/test_modules_volumes_cleanup_s3.py @@ -11,11 +11,6 @@ from aiodocker.volumes import DockerVolume from pydantic import HttpUrl from pytest import LogCaptureFixture -from servicelib.file_constants import ( - AGENT_FILE_NAME, - HIDDEN_FILE_NAME, - KEY_VALUE_FILE_NAME, -) from simcore_service_agent.core.settings import ApplicationSettings from simcore_service_agent.modules.volumes_cleanup._s3 import ( S3Provider, @@ -80,9 +75,8 @@ async def _download_files_from_bucket( def _create_data(folder: Path) -> None: for file in { # pylint:disable=use-sequence-for-iteration - AGENT_FILE_NAME, - HIDDEN_FILE_NAME, - KEY_VALUE_FILE_NAME, + ".hidden_do_not_remove", + "key_values.json", "f1.txt", "f2.txt", "f3.txt", diff --git a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py index 463bacb8eb0..0a2cbd9e77f 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py +++ b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py @@ -142,7 +142,6 @@ async def create_dynamic_service( request_dns=x_dynamic_sidecar_request_dns, request_scheme=x_dynamic_sidecar_request_scheme, request_simcore_user_agent=x_simcore_user_agent, - can_save=service.can_save, ) return cast(DynamicServiceGet, await scheduler.get_stack_status(service.node_uuid)) diff --git a/services/director-v2/src/simcore_service_director_v2/models/domains/dynamic_services.py b/services/director-v2/src/simcore_service_director_v2/models/domains/dynamic_services.py index 855b9fcb7f9..612c88660ec 100644 --- a/services/director-v2/src/simcore_service_director_v2/models/domains/dynamic_services.py +++ b/services/director-v2/src/simcore_service_director_v2/models/domains/dynamic_services.py @@ -38,9 +38,6 @@ class DynamicServiceCreate(ServiceDetails): service_resources: ServiceResourcesDict product_name: str = Field(..., description="Current product name") - can_save: bool = Field( - ..., description="the service data must be saved when closing" - ) class Config: schema_extra = { @@ -52,7 +49,6 @@ class Config: "node_uuid": "75c7f3f4-18f9-4678-8610-54a2ade78eaa", "basepath": "/x/75c7f3f4-18f9-4678-8610-54a2ade78eaa", "product_name": "osparc", - "can_save": True, "service_resources": ServiceResourcesDictHelpers.Config.schema_extra[ "examples" ][0], diff --git a/services/director-v2/src/simcore_service_director_v2/models/schemas/dynamic_services/scheduler.py b/services/director-v2/src/simcore_service_director_v2/models/schemas/dynamic_services/scheduler.py index 4321d7fefac..3b81310d9d5 100644 --- a/services/director-v2/src/simcore_service_director_v2/models/schemas/dynamic_services/scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/models/schemas/dynamic_services/scheduler.py @@ -438,13 +438,12 @@ def from_http_request( request_dns: str, request_scheme: str, request_simcore_user_agent: str, - can_save: bool, run_id: UUID | None = None, ) -> "SchedulerData": # This constructor method sets current product names_helper = DynamicSidecarNamesHelper.make(service.node_uuid) - obj_dict: dict[str, Any] = dict( + obj_dict = dict( service_name=names_helper.service_name_dynamic_sidecar, hostname=names_helper.service_name_dynamic_sidecar, port=port, @@ -465,7 +464,7 @@ def from_http_request( request_scheme=request_scheme, proxy_service_name=names_helper.proxy_service_name, request_simcore_user_agent=request_simcore_user_agent, - dynamic_sidecar={"service_removal_state": {"can_save": can_save}}, + dynamic_sidecar={}, ) if run_id: obj_dict["run_id"] = run_id diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_public.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_public.py index 4542f7cb731..ac21ec6842e 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_public.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_public.py @@ -8,7 +8,6 @@ from models_library.projects import ProjectID from models_library.projects_networks import DockerNetworkAlias from models_library.projects_nodes_io import NodeID -from models_library.volumes import VolumeCategory from pydantic import AnyHttpUrl, PositiveFloat from servicelib.fastapi.long_running_tasks.client import ( Client, @@ -20,7 +19,6 @@ ) from servicelib.logging_utils import log_context from servicelib.utils import logged_gather -from servicelib.volumes_utils import VolumeStatus from simcore_service_director_v2.core.settings import DynamicSidecarSettings from ....models.schemas.dynamic_services import SchedulerData @@ -403,19 +401,6 @@ async def restart_containers(self, dynamic_sidecar_endpoint: AnyHttpUrl) -> None _debug_progress_callback, ) - @log_decorator(logger=logger) - async def update_volume_state( - self, - dynamic_sidecar_endpoint: AnyHttpUrl, - volume_category: VolumeCategory, - volume_status: VolumeStatus, - ) -> None: - await self._thin_client.put_volumes( - dynamic_sidecar_endpoint, - volume_category=volume_category, - volume_status=volume_status, - ) - async def setup(app: FastAPI) -> None: with log_context(logger, logging.DEBUG, "dynamic-sidecar api client setup"): diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_thin.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_thin.py index 5c80dc23ca8..42a0b83e2a5 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_thin.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/api_client/_thin.py @@ -4,10 +4,8 @@ from fastapi import FastAPI, status from httpx import Response, Timeout -from models_library.volumes import VolumeCategory from pydantic import AnyHttpUrl from servicelib.docker_constants import SUFFIX_EGRESS_PROXY_NAME -from servicelib.volumes_utils import VolumeStatus from ....core.settings import DynamicSidecarSettings from ._base import BaseThinClient, expect_status, retry_on_errors @@ -227,15 +225,3 @@ async def post_containers_tasks_restart( ) -> Response: url = self._get_url(dynamic_sidecar_endpoint, "/containers:restart") return await self.client.post(url) - - @retry_on_errors - @expect_status(status.HTTP_204_NO_CONTENT) - async def put_volumes( - self, - dynamic_sidecar_endpoint: AnyHttpUrl, - volume_category: VolumeCategory, - volume_status: VolumeStatus, - ) -> Response: - url = self._get_url(dynamic_sidecar_endpoint, f"/volumes/{volume_category}") - - return await self.client.put(url, json={"status": volume_status}) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_abc.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_abc.py index 975021f445b..4db87e1e348 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_abc.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_abc.py @@ -79,7 +79,6 @@ async def add_service( request_dns: str, request_scheme: str, request_simcore_user_agent: str, - can_save: bool, ) -> None: """ Adds a new service. diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py index 522b16f40f8..187e819880c 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py @@ -12,14 +12,12 @@ from models_library.rabbitmq_messages import InstrumentationRabbitMessage from models_library.service_settings_labels import SimcoreServiceLabels from models_library.services import ServiceKeyVersion -from models_library.volumes import VolumeCategory from servicelib.fastapi.long_running_tasks.client import ( ProgressCallback, TaskClientResultError, ) from servicelib.fastapi.long_running_tasks.server import TaskProgress from servicelib.utils import logged_gather -from servicelib.volumes_utils import VolumeStatus from simcore_postgres_database.models.comp_tasks import NodeClass from tenacity import TryAgain from tenacity._asyncio import AsyncRetrying @@ -139,11 +137,6 @@ async def service_save_state( await dynamic_sidecar_client.save_service_state( scheduler_data.endpoint, progress_callback=progress_callback ) - await dynamic_sidecar_client.update_volume_state( - scheduler_data.endpoint, - volume_category=VolumeCategory.STATES, - volume_status=VolumeStatus.CONTENT_WAS_SAVED, - ) async def service_push_outputs( @@ -156,11 +149,6 @@ async def service_push_outputs( await dynamic_sidecar_client.push_service_output_ports( scheduler_data.endpoint, progress_callback=progress_callback ) - await dynamic_sidecar_client.update_volume_state( - scheduler_data.endpoint, - volume_category=VolumeCategory.OUTPUTS, - volume_status=VolumeStatus.CONTENT_WAS_SAVED, - ) async def service_remove_sidecar_proxy_docker_networks_and_volumes( @@ -420,32 +408,6 @@ async def prepare_services_environment( dynamic_sidecar_client = get_dynamic_sidecar_client(app, scheduler_data.node_uuid) dynamic_sidecar_endpoint = scheduler_data.endpoint - # update if volume requires saving - def _get_state_params(can_save: bool | None) -> dict[str, VolumeStatus]: - return ( - {"volume_status": VolumeStatus.CONTENT_WAS_SAVED} - if can_save - else {"volume_status": VolumeStatus.CONTENT_NO_SAVE_REQUIRED} - ) - - update_volume_state_params = _get_state_params( - scheduler_data.dynamic_sidecar.service_removal_state.can_save - ) - await logged_gather( - *( - dynamic_sidecar_client.update_volume_state( - scheduler_data.endpoint, - volume_category=VolumeCategory.STATES, - **update_volume_state_params, - ), - dynamic_sidecar_client.update_volume_state( - scheduler_data.endpoint, - volume_category=VolumeCategory.OUTPUTS, - **update_volume_state_params, - ), - ) - ) - async def _pull_outputs_and_state(): tasks = [ dynamic_sidecar_client.pull_service_output_ports(dynamic_sidecar_endpoint) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler.py index 03f89ab6813..beecce21a72 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler.py @@ -149,7 +149,6 @@ async def add_service( request_dns: str, request_scheme: str, request_simcore_user_agent: str, - can_save: bool, ) -> None: """Invoked before the service is started""" scheduler_data = SchedulerData.from_http_request( @@ -159,7 +158,6 @@ async def add_service( request_dns=request_dns, request_scheme=request_scheme, request_simcore_user_agent=request_simcore_user_agent, - can_save=can_save, ) await self._add_service(scheduler_data) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_task.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_task.py index 61d15d886eb..b43266c4ac4 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_task.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_task.py @@ -70,9 +70,7 @@ async def add_service( request_dns: str, request_scheme: str, request_simcore_user_agent: str, - can_save: bool, ) -> None: - return await self._scheduler.add_service( service, simcore_service_labels, @@ -80,7 +78,6 @@ async def add_service( request_dns, request_scheme, request_simcore_user_agent, - can_save, ) def is_service_tracked(self, node_uuid: NodeID) -> bool: diff --git a/services/director-v2/tests/integration/02/test_dynamic_services_routes.py b/services/director-v2/tests/integration/02/test_dynamic_services_routes.py index 1297f8ed174..c93c32a7dfe 100644 --- a/services/director-v2/tests/integration/02/test_dynamic_services_routes.py +++ b/services/director-v2/tests/integration/02/test_dynamic_services_routes.py @@ -119,7 +119,6 @@ def start_request_data( service_version=dy_static_file_server_dynamic_sidecar_service["image"]["tag"], request_scheme="http", request_dns="localhost:50000", - can_save=True, settings=[ { "name": "resources", diff --git a/services/director-v2/tests/integration/02/utils.py b/services/director-v2/tests/integration/02/utils.py index 386b15737fd..6aa25c70b19 100644 --- a/services/director-v2/tests/integration/02/utils.py +++ b/services/director-v2/tests/integration/02/utils.py @@ -5,7 +5,7 @@ import logging import os import urllib.parse -from typing import Any +from typing import Any, Optional import aiodocker import httpx @@ -121,7 +121,7 @@ async def _wait_for_service(service_name: str) -> None: async def _get_service_published_port( - service_name: str, target_port: int | None = None + service_name: str, target_port: Optional[int] = None ) -> int: # it takes a bit of time for the port to be auto generated # keep trying until it is there @@ -215,7 +215,7 @@ async def patch_dynamic_service_url(app: FastAPI, node_uuid: str) -> str: # patch the endppoint inside the scheduler scheduler: DynamicSidecarsScheduler = app.state.dynamic_sidecar_scheduler - endpoint: str | None = None + endpoint: Optional[str] = None async with scheduler._scheduler._lock: # pylint: disable=protected-access for ( scheduler_data @@ -267,7 +267,7 @@ async def assert_start_service( service_key: str, service_version: str, service_uuid: str, - basepath: str | None, + basepath: Optional[str], catalog_url: URL, ) -> None: service_resources: ServiceResourcesDict = await _get_service_resources( @@ -282,7 +282,6 @@ async def assert_start_service( service_version=service_version, service_uuid=service_uuid, basepath=basepath, - can_save=True, service_resources=ServiceResourcesDictHelpers.create_jsonable( service_resources ), diff --git a/services/director-v2/tests/mocks/fake_scheduler_data.json b/services/director-v2/tests/mocks/fake_scheduler_data.json index 5f309a4fb39..3efbfb9b8c9 100644 --- a/services/director-v2/tests/mocks/fake_scheduler_data.json +++ b/services/director-v2/tests/mocks/fake_scheduler_data.json @@ -68,6 +68,5 @@ "request_dns": "localhost", "request_scheme": "http", "proxy_service_name": "dy-proxy_3e68d1f6-be3e-414e-a468-4a2bf415f756", - "request_simcore_user_agent": "", - "product": "osparc" + "request_simcore_user_agent": "" } diff --git a/services/director-v2/tests/mocks/fake_scheduler_data_compose_spec.json b/services/director-v2/tests/mocks/fake_scheduler_data_compose_spec.json index 28fc6abfe29..ff14ebb3247 100644 --- a/services/director-v2/tests/mocks/fake_scheduler_data_compose_spec.json +++ b/services/director-v2/tests/mocks/fake_scheduler_data_compose_spec.json @@ -98,6 +98,5 @@ "request_dns": "localhost", "request_scheme": "http", "proxy_service_name": "dy-proxy_dd2b8ceb-4408-4bfb-a953-46178836e12d", - "request_simcore_user_agent": "", - "product": "osparc" + "request_simcore_user_agent": "" } diff --git a/services/director-v2/tests/unit/conftest.py b/services/director-v2/tests/unit/conftest.py index bfc06a9d422..9cfafd947bb 100644 --- a/services/director-v2/tests/unit/conftest.py +++ b/services/director-v2/tests/unit/conftest.py @@ -94,11 +94,6 @@ def request_scheme() -> str: return "http" -@pytest.fixture -def can_save() -> bool: - return True - - @pytest.fixture def request_simcore_user_agent() -> str: return "python/test" @@ -112,7 +107,6 @@ def scheduler_data_from_http_request( request_dns: str, request_scheme: str, request_simcore_user_agent: str, - can_save: bool, run_id: RunID, ) -> SchedulerData: return SchedulerData.from_http_request( @@ -122,7 +116,6 @@ def scheduler_data_from_http_request( request_dns=request_dns, request_scheme=request_scheme, request_simcore_user_agent=request_simcore_user_agent, - can_save=can_save, run_id=run_id, ) diff --git a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py index cf8205144fb..829be0c8bfa 100644 --- a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py +++ b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py @@ -83,7 +83,6 @@ async def observed_service( dynamic_sidecar_port: int, request_dns: str, request_scheme: str, - can_save: bool, ) -> SchedulerData: await dynamic_sidecar_scheduler.add_service( dynamic_service_create, @@ -91,8 +90,7 @@ async def observed_service( dynamic_sidecar_port, request_dns, request_scheme, - "", - can_save, + request_simcore_user_agent="", ) # pylint:disable=protected-access return dynamic_sidecar_scheduler._scheduler.get_scheduler_data( diff --git a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_public.py b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_public.py index a8d4ca02ad5..a221c087eb2 100644 --- a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_public.py +++ b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_public.py @@ -9,12 +9,10 @@ from faker import Faker from fastapi import FastAPI, status from httpx import HTTPError, Response -from models_library.volumes import VolumeCategory from pydantic import AnyHttpUrl, parse_obj_as from pytest import LogCaptureFixture, MonkeyPatch from pytest_mock import MockerFixture from pytest_simcore.helpers.typing_env import EnvVarsDict -from servicelib.volumes_utils import VolumeStatus from simcore_service_director_v2.core.settings import AppSettings from simcore_service_director_v2.modules.dynamic_sidecar.api_client._errors import ( ClientHttpError, @@ -333,25 +331,3 @@ async def test_detach_container_from_network( ) is None ) - - -@pytest.mark.parametrize("volume_category", VolumeCategory) -@pytest.mark.parametrize("volume_status", VolumeStatus) -async def test_update_volume_state( - get_patched_client: Callable, - dynamic_sidecar_endpoint: AnyHttpUrl, - volume_category: VolumeCategory, - volume_status: VolumeStatus, -) -> None: - with get_patched_client( - "put_volumes", - return_value=Response(status_code=status.HTTP_204_NO_CONTENT), - ) as client: - assert ( - await client.update_volume_state( - dynamic_sidecar_endpoint, - volume_category=volume_category, - volume_status=volume_status, - ) - is None - ) diff --git a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_thin.py b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_thin.py index d4a49dc4d7e..a1617dedcb3 100644 --- a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_thin.py +++ b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_client_api_thin.py @@ -7,14 +7,12 @@ import pytest from fastapi import FastAPI, status from httpx import Response -from models_library.volumes import VolumeCategory from pydantic import AnyHttpUrl, parse_obj_as from pytest import MonkeyPatch from pytest_simcore.helpers.typing_env import EnvVarsDict from respx import MockRouter, Route from respx.types import SideEffectTypes from servicelib.docker_constants import SUFFIX_EGRESS_PROXY_NAME -from servicelib.volumes_utils import VolumeStatus from simcore_service_director_v2.core.settings import AppSettings from simcore_service_director_v2.modules.dynamic_sidecar.api_client._thin import ( ThinDynamicSidecarClient, @@ -246,31 +244,6 @@ async def test_post_containers_networks_detach( assert_responses(mock_response, response) -@pytest.mark.parametrize("volume_category", VolumeCategory) -@pytest.mark.parametrize("volume_status", VolumeStatus) -async def test_put_volumes( - thin_client: ThinDynamicSidecarClient, - dynamic_sidecar_endpoint: AnyHttpUrl, - mock_request: MockRequestType, - volume_category: str, - volume_status: VolumeStatus, -) -> None: - mock_response = Response(status.HTTP_204_NO_CONTENT) - mock_request( - "PUT", - f"{dynamic_sidecar_endpoint}/{thin_client.API_VERSION}/volumes/{volume_category}", - mock_response, - None, - ) - - response = await thin_client.put_volumes( - dynamic_sidecar_endpoint, - volume_category=volume_category, - volume_status=volume_status, - ) - assert_responses(mock_response, response) - - @pytest.mark.parametrize( "handler_name, mock_endpoint, extra_kwargs", [ diff --git a/services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_service_specs.py b/services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_service_specs.py index e83ba8de67e..9b38e77a1d7 100644 --- a/services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_service_specs.py +++ b/services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_service_specs.py @@ -116,7 +116,7 @@ def expected_dynamic_sidecar_spec( "is_service_environment_ready": False, "service_removal_state": { "can_remove": False, - "can_save": True, + "can_save": None, "was_removed": False, }, "status": {"current": "ok", "info": ""}, diff --git a/services/director-v2/tests/unit/with_dbs/test_modules_node_rights.py b/services/director-v2/tests/unit/with_dbs/test_modules_node_rights.py index 879f0d2524f..7dd61faf374 100644 --- a/services/director-v2/tests/unit/with_dbs/test_modules_node_rights.py +++ b/services/director-v2/tests/unit/with_dbs/test_modules_node_rights.py @@ -86,6 +86,7 @@ async def minimal_app( @pytest.fixture async def node_rights_manager(minimal_app: FastAPI) -> AsyncIterable[NodeRightsManager]: + redis_lock_manger = await NodeRightsManager.instance(minimal_app) await redis_lock_manger.redis_client_sdk.redis.flushall() yield redis_lock_manger diff --git a/services/dynamic-sidecar/openapi.json b/services/dynamic-sidecar/openapi.json index d6da3d31c3c..8ed16f4a533 100644 --- a/services/dynamic-sidecar/openapi.json +++ b/services/dynamic-sidecar/openapi.json @@ -658,50 +658,6 @@ } } } - }, - "/v1/volumes/{id}": { - "put": { - "tags": [ - "volumes" - ], - "summary": "Updates the state of the volume", - "operationId": "put_volume_state_v1_volumes__id__put", - "parameters": [ - { - "required": true, - "schema": { - "$ref": "#/components/schemas/VolumeCategory" - }, - "name": "id", - "in": "path" - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PutVolumeItem" - } - } - }, - "required": true - }, - "responses": { - "204": { - "description": "Successful Response" - }, - "422": { - "description": "Validation Error", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/HTTPValidationError" - } - } - } - } - } - } } }, "components": { @@ -812,18 +768,6 @@ } } }, - "PutVolumeItem": { - "title": "PutVolumeItem", - "required": [ - "status" - ], - "type": "object", - "properties": { - "status": { - "$ref": "#/components/schemas/VolumeStatus" - } - } - }, "SelectBox": { "title": "SelectBox", "required": [ @@ -983,27 +927,6 @@ } } }, - "VolumeCategory": { - "title": "VolumeCategory", - "enum": [ - "OUTPUTS", - "INPUTS", - "STATES", - "SHARED_STORE" - ], - "type": "string", - "description": "These uniquely identify volumes which are mounted by\nthe dynamic-sidecar and user services.\n\nThis is primarily used to keep track of the status of\neach individual volume on the volumes.\n\nThe status is ingested by the agent and processed\nwhen the volume is removed." - }, - "VolumeStatus": { - "title": "VolumeStatus", - "enum": [ - "CONTENT_NEEDS_TO_BE_SAVED", - "CONTENT_WAS_SAVED", - "CONTENT_NO_SAVE_REQUIRED" - ], - "type": "string", - "description": "An enumeration." - }, "Widget": { "title": "Widget", "required": [ diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/_routing.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/_routing.py index 9f6d04e676a..b731b86a466 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/_routing.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/_routing.py @@ -6,13 +6,7 @@ from fastapi import APIRouter from .._meta import API_VTAG -from . import ( - containers, - containers_extension, - containers_long_running_tasks, - health, - volumes, -) +from . import containers, containers_extension, containers_long_running_tasks, health main_router = APIRouter() main_router.include_router(health.router) @@ -31,10 +25,5 @@ tags=["containers"], prefix=f"/{API_VTAG}", ) -main_router.include_router( - volumes.router, - tags=["volumes"], - prefix=f"/{API_VTAG}", -) __all__: tuple[str, ...] = ("main_router",) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/volumes.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/volumes.py deleted file mode 100644 index 7ad90582164..00000000000 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/volumes.py +++ /dev/null @@ -1,29 +0,0 @@ -from fastapi import APIRouter, Depends -from fastapi import Path as PathParam -from fastapi import status -from models_library.volumes import VolumeCategory -from pydantic import BaseModel -from servicelib.volumes_utils import VolumeStatus - -from ..modules.mounted_fs import MountedVolumes -from ..modules.volume_files import set_volume_state -from ._dependencies import get_mounted_volumes - -router = APIRouter() - - -class PutVolumeItem(BaseModel): - status: VolumeStatus - - -@router.put( - "/volumes/{id}", - summary="Updates the state of the volume", - status_code=status.HTTP_204_NO_CONTENT, -) -async def put_volume_state( - item: PutVolumeItem, - volume_category: VolumeCategory = PathParam(..., alias="id"), - mounted_volumes: MountedVolumes = Depends(get_mounted_volumes), -) -> None: - await set_volume_state(mounted_volumes, volume_category, status=item.status) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/application.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/application.py index abc7b2e2007..0374e7501d7 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/application.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/application.py @@ -19,10 +19,6 @@ from ..modules.attribute_monitor import setup_attribute_monitor from ..modules.mounted_fs import MountedVolumes, setup_mounted_fs from ..modules.outputs import setup_outputs -from ..modules.volume_files import ( - create_agent_file_on_all_volumes, - create_hidden_file_on_all_volumes, -) from .docker_compose_utils import docker_compose_down from .docker_logs import setup_background_log_fetcher from .error_handlers import http_error_handler, node_not_found_error_handler @@ -30,7 +26,7 @@ from .rabbitmq import setup_rabbitmq from .remote_debug import setup as remote_debug_setup from .settings import ApplicationSettings -from .utils import login_registry +from .utils import login_registry, volumes_fix_permissions logger = logging.getLogger(__name__) @@ -171,8 +167,7 @@ async def _on_startup() -> None: app_state = AppState(app) await login_registry(app_state.settings.REGISTRY_SETTINGS) - await create_hidden_file_on_all_volumes(app_state.mounted_volumes) - await create_agent_file_on_all_volumes(app_state.mounted_volumes) + await volumes_fix_permissions(app_state.mounted_volumes) # STARTED print(APP_STARTED_BANNER_MSG, flush=True) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/utils.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/utils.py index bf4e667adc4..1384ee8afc7 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/utils.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/utils.py @@ -24,6 +24,10 @@ from tenacity.stop import stop_after_delay from tenacity.wait import wait_fixed +from ..modules.mounted_fs import MountedVolumes + +HIDDEN_FILE_NAME = ".hidden_do_not_remove" + logger = logging.getLogger(__name__) @@ -204,3 +208,16 @@ def assemble_container_names(validated_compose_content: str) -> list[str]: service_data["container_name"] for service_data in parsed_compose_spec["services"].values() ] + + +async def volumes_fix_permissions(mounted_volumes: MountedVolumes) -> None: + # NOTE: by creating a hidden file on all mounted volumes + # the same permissions are ensured and avoids + # issues when starting the services + for volume_path in mounted_volumes.all_disk_paths(): + hidden_file = volume_path / HIDDEN_FILE_NAME + hidden_file.write_text( + f"Directory must not be empty.\nCreated by {__file__}.\n" + "Required by oSPARC internals to properly enforce permissions on this " + "directory and all its files" + ) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/mounted_fs.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/mounted_fs.py index 2823dc93304..611b9a83840 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/mounted_fs.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/mounted_fs.py @@ -42,7 +42,6 @@ def __init__( outputs_path: Path, state_paths: list[Path], state_exclude: set[str], - shared_store_path: Path, compose_namespace: str, dy_volumes: Path, ) -> None: @@ -52,9 +51,8 @@ def __init__( self.outputs_path: Path = outputs_path self.state_paths: list[Path] = state_paths self.state_exclude: set[str] = state_exclude - self.shared_store_path: Path = shared_store_path - self.compose_namespace: str = compose_namespace - self._dy_volumes: Path = dy_volumes + self.compose_namespace = compose_namespace + self._dy_volumes = dy_volumes self._ensure_directories() @@ -88,27 +86,23 @@ def disk_inputs_path(self) -> Path: def disk_outputs_path(self) -> Path: return _ensure_path(self._dy_volumes / self.outputs_path.relative_to("/")) - @cached_property - def disk_shared_store_path(self) -> Path: - return _ensure_path(self._dy_volumes / self.shared_store_path.relative_to("/")) - def disk_state_paths(self) -> Iterator[Path]: for state_path in self.state_paths: yield _ensure_path(self._dy_volumes / state_path.relative_to("/")) + def all_disk_paths(self) -> Iterator[Path]: + # PC: keeps iterator to follow same style as disk_state_paths but IMO it is overreaching + yield self.disk_inputs_path + yield self.disk_outputs_path + yield from self.disk_state_paths() + def _ensure_directories(self) -> None: """ Creates directories on its file system, these will be mounted by the user services. """ - for path in ( - self._dy_volumes, - self.disk_shared_store_path, - self.disk_inputs_path, - self.disk_outputs_path, - ): + _ensure_path(self._dy_volumes) + for path in self.all_disk_paths(): _ensure_path(path) - for state_path in self.disk_state_paths(): - _ensure_path(state_path) @staticmethod async def _get_bind_path_from_label(label: str, run_id: RunID) -> Path: @@ -149,7 +143,6 @@ def setup_mounted_fs(app: FastAPI) -> MountedVolumes: outputs_path=settings.DY_SIDECAR_PATH_OUTPUTS, state_paths=settings.DY_SIDECAR_STATE_PATHS, state_exclude=settings.DY_SIDECAR_STATE_EXCLUDE, - shared_store_path=settings.DYNAMIC_SIDECAR_SHARED_STORE_DIR, compose_namespace=settings.DYNAMIC_SIDECAR_COMPOSE_NAMESPACE, dy_volumes=settings.DYNAMIC_SIDECAR_DY_VOLUMES_MOUNT_DIR, ) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py index 1273a6e4d29..1c2c006ba31 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py @@ -18,7 +18,6 @@ from pydantic import ByteSize from servicelib.archiving_utils import PrunableFolder, archive_dir, unarchive_dir from servicelib.async_utils import run_sequentially_in_context -from servicelib.file_constants import KEY_VALUE_FILE_NAME from servicelib.file_utils import remove_directory from servicelib.logging_utils import log_context from servicelib.progress_bar import ProgressBarData @@ -41,6 +40,7 @@ class PortTypeName(str, Enum): _FILE_TYPE_PREFIX = "data:" +_KEY_VALUE_FILE_NAME = "key_values.json" logger = logging.getLogger(__name__) @@ -161,7 +161,7 @@ async def upload_outputs( ), ) else: - data_file = outputs_path / KEY_VALUE_FILE_NAME + data_file = outputs_path / _KEY_VALUE_FILE_NAME if data_file.exists(): data = json.loads(data_file.read_text()) if port.key in data and data[port.key] is not None: @@ -299,7 +299,7 @@ async def download_target_ports( # create/update the json file with the new values if data: - data_file = target_dir / KEY_VALUE_FILE_NAME + data_file = target_dir / _KEY_VALUE_FILE_NAME if data_file.exists(): current_data = json.loads(data_file.read_text()) # merge data diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/volume_files.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/volume_files.py deleted file mode 100644 index 03c7f02d4cd..00000000000 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/volume_files.py +++ /dev/null @@ -1,139 +0,0 @@ -import asyncio -import os -import stat -from dataclasses import dataclass -from functools import partial -from pathlib import Path -from typing import Union - -import aiofiles -from aiofiles import os as aiofiles_os -from models_library.volumes import VolumeCategory -from servicelib.file_constants import AGENT_FILE_NAME, HIDDEN_FILE_NAME -from servicelib.volumes_utils import VolumeState, VolumeStatus, save_volume_state - -from .mounted_fs import MountedVolumes - -chmod = aiofiles_os.wrap(os.chmod) # type: ignore - - -@dataclass -class _MountedVolumesLocalPaths: - outputs: Path - inputs: Path - states: tuple[Path, ...] - shared_store: Path - - @classmethod - def from_mounted_volumes( - cls, mounted_volumes: MountedVolumes - ) -> "_MountedVolumesLocalPaths": - return cls( - inputs=mounted_volumes.disk_inputs_path, - outputs=mounted_volumes.disk_outputs_path, - states=tuple(mounted_volumes.disk_state_paths()), - shared_store=mounted_volumes.disk_shared_store_path, - ) - - def paths_from_volume_category(self, volume_category: VolumeCategory) -> list[Path]: - result: Union[Path, tuple[Path, ...]] = self.__getattribute__( - volume_category.lower() - ) - if isinstance(result, Path): - return [result] - return list(result) - - -async def _create_file_with_restricted_permissions(file: Path) -> None: - """only allows the user who created the files to change it""" - - # NOTE: the `stat.S_IWGRP`, group write permission, should not be here. - # when the user services start they chown and chmod and all the existing - # files in the work directory. - # For now this is required otherwise the dynamic-sidecar will not be able to - # write back the created file any longer. - - file_touch = partial( - file.touch, - mode=(stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IWGRP | stat.S_IROTH), - exist_ok=True, - ) - await asyncio.get_event_loop().run_in_executor(None, file_touch) - - # NOTE: After the file creation, ideally the file should be set as - # `immutable`. There is an issue with docker https://github.com/moby/moby/issues/45177 - # await async_command(f"chattr +i {hidden_file}", timeout=5) - # if above issue is fixed, a context manager that disables - # the file's immutability can be used by the dynamic-sidecar. This also allows - # to have protected files. - - -async def create_hidden_file_on_all_volumes(mounted_volumes: MountedVolumes) -> None: - # NOTE: by creating a hidden file on all mounted volumes - # the same permissions are ensured and avoids - # issues when starting the services - - volumes_local_paths = _MountedVolumesLocalPaths.from_mounted_volumes( - mounted_volumes - ) - for volume_path in volumes_local_paths.states + ( - volumes_local_paths.inputs, - volumes_local_paths.outputs, - volumes_local_paths.shared_store, - ): - hidden_file = volume_path / HIDDEN_FILE_NAME - - # restrict permissions - await _create_file_with_restricted_permissions(hidden_file) - - # write content - async with aiofiles.open(hidden_file, mode="w") as f: - await f.write( - f"Directory must not be empty.\nCreated by {__file__}.\n" - "Required by oSPARC internals to properly enforce permissions on this " - "directory and all its files" - ) - - -async def create_agent_file_on_all_volumes(mounted_volumes: MountedVolumes) -> None: - volumes_local_paths = _MountedVolumesLocalPaths.from_mounted_volumes( - mounted_volumes - ) - - # volumes which do not require saving - for path in (volumes_local_paths.inputs, volumes_local_paths.shared_store): - agent_file_path = path / AGENT_FILE_NAME - await _create_file_with_restricted_permissions(agent_file_path) - - await save_volume_state( - agent_file_path=agent_file_path, - volume_state=VolumeState(status=VolumeStatus.CONTENT_NO_SAVE_REQUIRED), - ) - - # volumes which require saving - for path in volumes_local_paths.states + (volumes_local_paths.outputs,): - agent_file_path = path / AGENT_FILE_NAME - await _create_file_with_restricted_permissions(agent_file_path) - - await save_volume_state( - agent_file_path=agent_file_path, - volume_state=VolumeState(status=VolumeStatus.CONTENT_NEEDS_TO_BE_SAVED), - ) - - -async def set_volume_state( - mounted_volumes: MountedVolumes, - volume_category: VolumeCategory, - status: VolumeStatus, -) -> None: - volumes_local_paths = _MountedVolumesLocalPaths.from_mounted_volumes( - mounted_volumes - ) - volume_paths: list[Path] = volumes_local_paths.paths_from_volume_category( - volume_category - ) - for volume_path in volume_paths: - await save_volume_state( - agent_file_path=volume_path / AGENT_FILE_NAME, - volume_state=VolumeState(status=status), - ) diff --git a/services/dynamic-sidecar/tests/unit/test_api_volumes.py b/services/dynamic-sidecar/tests/unit/test_api_volumes.py deleted file mode 100644 index 6e0701db266..00000000000 --- a/services/dynamic-sidecar/tests/unit/test_api_volumes.py +++ /dev/null @@ -1,53 +0,0 @@ -from pathlib import Path - -import pytest -from fastapi import status -from fastapi.testclient import TestClient -from models_library.volumes import VolumeCategory -from servicelib.file_constants import AGENT_FILE_NAME -from servicelib.volumes_utils import VolumeState, VolumeStatus, load_volume_state -from simcore_service_dynamic_sidecar._meta import API_VTAG -from simcore_service_dynamic_sidecar.modules.mounted_fs import MountedVolumes - - -@pytest.mark.parametrize( - "volume_category", [VolumeCategory.STATES, VolumeCategory.OUTPUTS] -) -async def test_volumes_state_saved_ok(test_client: TestClient, volume_category: str): - mounted_volumes: MountedVolumes = test_client.application.state.mounted_volumes - - volumes_path_map: dict[str, list[Path]] = { - VolumeCategory.STATES: list(mounted_volumes.disk_state_paths()), - VolumeCategory.OUTPUTS: [mounted_volumes.disk_outputs_path], - } - - for path in volumes_path_map[volume_category]: - assert await load_volume_state(path / AGENT_FILE_NAME) == VolumeState( - status=VolumeStatus.CONTENT_NEEDS_TO_BE_SAVED - ) - - response = await test_client.put( - f"/{API_VTAG}/volumes/{volume_category}", - json={"status": VolumeStatus.CONTENT_WAS_SAVED}, - ) - assert response.status_code == status.HTTP_204_NO_CONTENT, response.text - - for path in volumes_path_map[volume_category]: - assert await load_volume_state(path / AGENT_FILE_NAME) == VolumeState( - status=VolumeStatus.CONTENT_WAS_SAVED - ) - - -@pytest.mark.parametrize("invalid_volume_category", ["outputs", "outputS"]) -async def test_volumes_state_saved_error( - test_client: TestClient, invalid_volume_category: str -): - response = await test_client.put( - f"/{API_VTAG}/volumes/{invalid_volume_category}", - json={"status": VolumeStatus.CONTENT_WAS_SAVED}, - ) - assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY, response.text - json_response = response.json() - assert ( - invalid_volume_category not in json_response["detail"][0]["ctx"]["enum_values"] - ) diff --git a/services/dynamic-sidecar/tests/unit/test_modules_outputs_manager.py b/services/dynamic-sidecar/tests/unit/test_modules_outputs_manager.py index 1eb407e456d..a9d7b2106d1 100644 --- a/services/dynamic-sidecar/tests/unit/test_modules_outputs_manager.py +++ b/services/dynamic-sidecar/tests/unit/test_modules_outputs_manager.py @@ -272,6 +272,7 @@ async def test_port_key_tracker_add_pending( async def test_port_key_tracker_are_pending_ports_uploading( port_key_tracker_with_ports: _PortKeyTracker, port_keys: list[str] ): + await port_key_tracker_with_ports.move_all_ports_to_uploading() assert await port_key_tracker_with_ports.are_pending_ports_uploading() is False @@ -372,7 +373,6 @@ async def test_regression_io_log_redirect_cb( outputs_path=Path("/"), state_paths=[], state_exclude=set(), - shared_store_path=Path("/"), compose_namespace="", dy_volumes=Path("/"), ) @@ -389,6 +389,7 @@ async def test_regression_io_log_redirect_cb( setup_outputs_manager(app) async with TestClient(app): # runs setup handlers + outputs_manager: OutputsManager = app.state.outputs_manager assert outputs_manager.io_log_redirect_cb is not None diff --git a/services/dynamic-sidecar/tests/unit/test_modules_outputs_watcher.py b/services/dynamic-sidecar/tests/unit/test_modules_outputs_watcher.py index 3f93122d44b..61ce2893788 100644 --- a/services/dynamic-sidecar/tests/unit/test_modules_outputs_watcher.py +++ b/services/dynamic-sidecar/tests/unit/test_modules_outputs_watcher.py @@ -59,9 +59,7 @@ @pytest.fixture -def mounted_volumes( - faker: Faker, tmp_path: Path, shared_store_dir: Path -) -> Iterator[MountedVolumes]: +def mounted_volumes(faker: Faker, tmp_path: Path) -> Iterator[MountedVolumes]: mounted_volumes = MountedVolumes( run_id=faker.uuid4(cast_to=None), node_id=faker.uuid4(cast_to=None), @@ -69,8 +67,7 @@ def mounted_volumes( outputs_path=tmp_path / "outputs", state_paths=[], state_exclude=set(), - shared_store_path=Path("/"), - compose_namespace=shared_store_dir, + compose_namespace="", dy_volumes=tmp_path, ) yield mounted_volumes diff --git a/services/dynamic-sidecar/tests/unit/test_modules_volume_files.py b/services/dynamic-sidecar/tests/unit/test_modules_volume_files.py deleted file mode 100644 index a37c5036375..00000000000 --- a/services/dynamic-sidecar/tests/unit/test_modules_volume_files.py +++ /dev/null @@ -1,63 +0,0 @@ -# pylint: disable=redefined-outer-name - -from pathlib import Path - -import pytest -from faker import Faker -from models_library.volumes import VolumeCategory -from servicelib.file_constants import AGENT_FILE_NAME -from servicelib.volumes_utils import VolumeState, VolumeStatus, load_volume_state -from simcore_service_dynamic_sidecar.modules.mounted_fs import MountedVolumes -from simcore_service_dynamic_sidecar.modules.volume_files import ( - create_agent_file_on_all_volumes, - create_hidden_file_on_all_volumes, - set_volume_state, -) - - -@pytest.fixture -def mounted_volumes(tmp_path: Path, faker: Faker) -> MountedVolumes: - return MountedVolumes( - run_id=faker.uuid4(cast_to=None), - node_id=faker.uuid4(cast_to=None), - inputs_path=tmp_path / "inputs", - outputs_path=tmp_path / "outputs", - state_paths=[tmp_path / "state"], - state_exclude=set(), - shared_store_path=tmp_path / "shared_store", - compose_namespace="test", - dy_volumes=Path("/"), - ) - - -async def test_create_hidden_file_on_all_volumes(mounted_volumes: MountedVolumes): - await create_hidden_file_on_all_volumes(mounted_volumes) - - -async def test_create_agent_file_on_all_volumes(mounted_volumes: MountedVolumes): - await create_agent_file_on_all_volumes(mounted_volumes) - - paths_to_check: list[Path] = list(mounted_volumes.disk_state_paths()) + [ - mounted_volumes.disk_outputs_path - ] - - for path in paths_to_check: - assert await load_volume_state(path / AGENT_FILE_NAME) == VolumeState( - status=VolumeStatus.CONTENT_NEEDS_TO_BE_SAVED - ) - - # mark as saved - await set_volume_state( - mounted_volumes, VolumeCategory.OUTPUTS, status=VolumeStatus.CONTENT_WAS_SAVED - ) - await set_volume_state( - mounted_volumes, - VolumeCategory.STATES, - status=VolumeStatus.CONTENT_WAS_SAVED, - ) - - # ensure these paths required saving and were saved - for path in paths_to_check: - assert await load_volume_state(path / AGENT_FILE_NAME) == VolumeState( - status=VolumeStatus.CONTENT_WAS_SAVED - ) diff --git a/services/web/server/src/simcore_service_webserver/director_v2_core_dynamic_services.py b/services/web/server/src/simcore_service_webserver/director_v2_core_dynamic_services.py index 3e5e3a29290..9ae99883629 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2_core_dynamic_services.py +++ b/services/web/server/src/simcore_service_webserver/director_v2_core_dynamic_services.py @@ -82,7 +82,6 @@ async def run_dynamic_service( *, app: web.Application, product_name: str, - save_state: bool, user_id: PositiveInt, project_id: str, service_key: str, @@ -100,7 +99,6 @@ async def run_dynamic_service( """ data = { "product_name": product_name, - "can_save": save_state, "user_id": user_id, "project_id": project_id, "key": service_key, diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index fb30c2c4968..7514af280a4 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -228,15 +228,6 @@ async def _start_dynamic_service( # this is a dynamic node, let's gather its resources and start it - save_state = False - user_role: UserRole = await get_user_role(request.app, user_id) - if user_role > UserRole.GUEST: - save_state = await ProjectDBAPI.get_from_app_context( - request.app - ).has_permission( - user_id=user_id, project_uuid=f"{project_uuid}", permission="write" - ) - lock_key = _nodes_utils.get_service_start_lock_key(user_id, project_uuid) redis_client_sdk = get_redis_lock_manager_client_sdk(request.app) project_settings: ProjectsSettings = get_plugin_settings(request.app) @@ -270,7 +261,6 @@ async def _start_dynamic_service( await director_v2_api.run_dynamic_service( app=request.app, product_name=product_name, - save_state=save_state, project_id=f"{project_uuid}", user_id=user_id, service_key=service_key, diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_handlers__open_close.py b/services/web/server/tests/unit/with_dbs/02/test_projects_handlers__open_close.py index 532751e691f..b0bcea1ca70 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_handlers__open_close.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_handlers__open_close.py @@ -315,7 +315,6 @@ async def test_open_project( mock_orphaned_services: mock.Mock, mock_catalog_api: dict[str, mock.Mock], osparc_product_name: str, - user_role: UserRole, ): # POST /v0/projects/{project_id}:open # open project @@ -347,7 +346,6 @@ async def test_open_project( simcore_user_agent=UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE, request_dns=request_dns, product_name=osparc_product_name, - save_state=user_role > UserRole.GUEST, service_resources=ServiceResourcesDictHelpers.create_jsonable( mock_service_resources ), @@ -378,7 +376,6 @@ async def test_open_template_project_for_edition( mock_orphaned_services: mock.Mock, mock_catalog_api: dict[str, mock.Mock], osparc_product_name: str, - user_role: UserRole, ): # POST /v0/projects/{project_id}:open # open project @@ -417,7 +414,6 @@ async def test_open_template_project_for_edition( mock_service_resources ), product_name=osparc_product_name, - save_state=user_role > UserRole.GUEST, ) ) mocked_director_v2_api["director_v2_api.run_dynamic_service"].assert_has_calls(