From 11006aa8353a9396d6723bdf18539ed3a2b41caf Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 17 Mar 2023 15:42:48 +0100 Subject: [PATCH 01/18] added extra labels to services --- .../src/models_library/docker.py | 3 ++ packages/models-library/tests/test_docker.py | 43 +++++++++++++------ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index 22cfce217a8..b7865ad71ed 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -2,6 +2,7 @@ from typing import Optional from models_library.generated_models.docker_rest_api import Task +from models_library.products import ProductName from models_library.projects import ProjectID from models_library.projects_nodes import NodeID from models_library.users import UserID @@ -37,6 +38,8 @@ class SimcoreServiceDockerLabelKeys(BaseModel): user_id: UserID = Field(..., alias="user_id") project_id: ProjectID = Field(..., alias="study_id") node_id: NodeID = Field(..., alias="uuid") + product_name: Optional[ProductName] = None + user_agent: Optional[str] = None def to_docker_labels(self) -> dict[str, str]: """returns a dictionary of strings as required by docker""" diff --git a/packages/models-library/tests/test_docker.py b/packages/models-library/tests/test_docker.py index ff25a72ef1e..c6e238ca1e2 100644 --- a/packages/models-library/tests/test_docker.py +++ b/packages/models-library/tests/test_docker.py @@ -3,6 +3,8 @@ # pylint: disable=unused-variable +from typing import Any + import pytest from faker import Faker from models_library.docker import ( @@ -12,6 +14,8 @@ ) from pydantic import ValidationError, parse_obj_as +faker = Faker() + @pytest.mark.parametrize( "label_key, valid", @@ -101,18 +105,33 @@ def test_docker_generic_tag(image_name: str, valid: bool): parse_obj_as(DockerGenericTag, image_name) -@pytest.fixture -def osparc_docker_label_keys( - faker: Faker, -) -> SimcoreServiceDockerLabelKeys: - return SimcoreServiceDockerLabelKeys.parse_obj( - dict(user_id=faker.pyint(), project_id=faker.uuid4(), node_id=faker.uuid4()) +@pytest.mark.parametrize( + "obj_data", + [ + pytest.param( + { + "user_id": faker.pyint(), + "project_id": faker.uuid4(), + "node_id": faker.uuid4(), + }, + id="parse_existing_service_labels", + ), + pytest.param( + { + "user_id": faker.pyint(), + "project_id": faker.uuid4(), + "node_id": faker.uuid4(), + "product": "test_p", + "user_agent": "test/python", + }, + id="parse_new_service_labels", + ), + ], +) +def test_simcore_service_docker_label_keys(obj_data: dict[str, Any]): + simcore_service_docker_label_keys = SimcoreServiceDockerLabelKeys.parse_obj( + obj_data ) - - -def test_osparc_docker_label_keys_to_docker_labels( - osparc_docker_label_keys: SimcoreServiceDockerLabelKeys, -): - exported_dict = osparc_docker_label_keys.to_docker_labels() + exported_dict = simcore_service_docker_label_keys.to_docker_labels() assert all(isinstance(v, str) for v in exported_dict.values()) assert parse_obj_as(SimcoreServiceDockerLabelKeys, exported_dict) From fd164777972d3b527d6113e6c178b1cf2ade21ab Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 17 Mar 2023 15:43:42 +0100 Subject: [PATCH 02/18] inject user_agent to user services --- .../api/routes/dynamic_services.py | 5 ++--- .../models/schemas/dynamic_services/scheduler.py | 5 +++++ .../dynamic_sidecar/docker_compose_specs.py | 14 ++++++++++++-- .../modules/dynamic_sidecar/scheduler/_abc.py | 1 + .../dynamic_sidecar/scheduler/_core/_events.py | 2 +- .../dynamic_sidecar/scheduler/_core/_scheduler.py | 2 ++ .../modules/dynamic_sidecar/scheduler/_task.py | 8 +++++++- 7 files changed, 30 insertions(+), 7 deletions(-) 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 f9d4cde7ed5..6aee71d7808 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 @@ -101,13 +101,13 @@ async def create_dynamic_service( service: DynamicServiceCreate, x_dynamic_sidecar_request_dns: str = Header(...), x_dynamic_sidecar_request_scheme: str = Header(...), + x_dynamic_sidecar_request_user_agent: str = Header(...), director_v0_client: DirectorV0Client = Depends(get_director_v0_client), dynamic_services_settings: DynamicServicesSettings = Depends( get_dynamic_services_settings ), scheduler: DynamicSidecarsScheduler = Depends(get_scheduler), ) -> Union[DynamicServiceGet, RedirectResponse]: - simcore_service_labels: SimcoreServiceLabels = ( await director_v0_client.get_service_labels( service=ServiceKeyVersion(key=service.key, version=service.version) @@ -141,6 +141,7 @@ async def create_dynamic_service( port=dynamic_services_settings.DYNAMIC_SIDECAR.DYNAMIC_SIDECAR_PORT, request_dns=x_dynamic_sidecar_request_dns, request_scheme=x_dynamic_sidecar_request_scheme, + request_user_agent=x_dynamic_sidecar_request_user_agent, ) return cast(DynamicServiceGet, await scheduler.get_stack_status(service.node_uuid)) @@ -156,7 +157,6 @@ async def get_dynamic_sidecar_status( director_v0_client: DirectorV0Client = Depends(get_director_v0_client), scheduler: DynamicSidecarsScheduler = Depends(get_scheduler), ) -> Union[DynamicServiceGet, RedirectResponse]: - try: return cast(DynamicServiceGet, await scheduler.get_stack_status(node_uuid)) except DynamicSidecarNotFoundError: @@ -317,7 +317,6 @@ async def update_projects_networks( director_v0_client: DirectorV0Client = Depends(get_director_v0_client), rabbitmq_client: RabbitMQClient = Depends(get_rabbitmq_client), ) -> None: - await projects_networks.update_from_workbench( projects_networks_repository=projects_networks_repository, projects_repository=projects_repository, 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 20937393abe..195184080b8 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 @@ -397,6 +397,9 @@ def endpoint(self) -> AnyHttpUrl: request_scheme: str = Field( ..., description="used when configuring the CORS options on the proxy" ) + request_user_agent: str = Field( + ..., description="used as label to filter out from the metrics" + ) proxy_service_name: str = Field(None, description="service name given to the proxy") product_name: Optional[str] = Field( @@ -414,6 +417,7 @@ def from_http_request( port: PortInt, request_dns: str, request_scheme: str, + request_user_agent: str, run_id: Optional[UUID] = None, ) -> "SchedulerData": # This constructor method sets current product @@ -440,6 +444,7 @@ def from_http_request( request_dns=request_dns, request_scheme=request_scheme, proxy_service_name=names_helper.proxy_service_name, + request_user_agent=request_user_agent, dynamic_sidecar={}, ) if run_id: diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py index 74246a12b39..950918f3eb1 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py @@ -4,6 +4,7 @@ from fastapi.applications import FastAPI from models_library.docker import SimcoreServiceDockerLabelKeys +from models_library.products import ProductName from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID from models_library.service_settings_labels import ( @@ -192,12 +193,18 @@ def _update_container_labels( user_id: UserID, project_id: ProjectID, node_id: NodeID, + user_agent: str, + product_name: ProductName, ) -> None: for spec in service_spec["services"].values(): labels: list[str] = spec.setdefault("labels", []) label_keys = SimcoreServiceDockerLabelKeys( - user_id=user_id, study_id=project_id, uuid=node_id + user_id=user_id, + study_id=project_id, + uuid=node_id, + user_agent=user_agent, + product_name=product_name, ) docker_labels = [f"{k}={v}" for k, v in label_keys.to_docker_labels().items()] @@ -219,10 +226,11 @@ def assemble_spec( service_resources: ServiceResourcesDict, simcore_service_labels: SimcoreServiceLabels, allow_internet_access: bool, - product_name: str, + product_name: ProductName, user_id: UserID, project_id: ProjectID, node_id: NodeID, + user_agent: str, ) -> str: """ returns a docker-compose spec used by @@ -286,6 +294,8 @@ def assemble_spec( user_id=user_id, project_id=project_id, node_id=node_id, + product_name=product_name, + user_agent=user_agent, ) # TODO: will be used in next PR 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 7ca0d80c323..edc803ef83f 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,6 +79,7 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, + request_user_agent: str, ) -> None: """ Adds a new service. diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py index 981422b82cd..304e06e684d 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py @@ -106,7 +106,6 @@ async def will_trigger(cls, app: FastAPI, scheduler_data: SchedulerData) -> bool @classmethod async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None: - # instrumentation message = InstrumentationRabbitMessage( metrics="service_started", @@ -482,6 +481,7 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None: user_id=scheduler_data.user_id, project_id=scheduler_data.project_id, node_id=scheduler_data.node_uuid, + user_agent=scheduler_data.request_user_agent, ) logger.debug( 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 4753f849402..7e60276b931 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 @@ -155,6 +155,7 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, + request_user_agent: str, ) -> None: """Invoked before the service is started""" scheduler_data = SchedulerData.from_http_request( @@ -163,6 +164,7 @@ async def add_service( port=port, request_dns=request_dns, request_scheme=request_scheme, + request_user_agent=request_user_agent, ) 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 8ba2858d816..53ebc712deb 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,15 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, + request_user_agent: str, ) -> None: return await self._scheduler.add_service( - service, simcore_service_labels, port, request_dns, request_scheme + service, + simcore_service_labels, + port, + request_dns, + request_scheme, + request_user_agent, ) def is_service_tracked(self, node_uuid: NodeID) -> bool: From dcad36d47cf266a20dee1ac6f4246de4c455283c Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 17 Mar 2023 15:44:14 +0100 Subject: [PATCH 03/18] propagate user_agent upon request --- .../director/director_api.py | 14 ++++++++------ .../director_v2_core_dynamic_services.py | 2 ++ .../projects/projects_api.py | 2 +- .../tests/unit/isolated/test_director_api.py | 19 ++++++++----------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/director/director_api.py b/services/web/server/src/simcore_service_webserver/director/director_api.py index 4292f495be3..8c30fd8e5dd 100644 --- a/services/web/server/src/simcore_service_webserver/director/director_api.py +++ b/services/web/server/src/simcore_service_webserver/director/director_api.py @@ -6,7 +6,7 @@ import logging import urllib.parse import warnings -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Optional from aiohttp import ClientSession, web from servicelib.aiohttp.client_session import get_client_session @@ -21,7 +21,7 @@ warnings.warn("Director-v0 is deprecated, please use Director-v2", DeprecationWarning) -def _get_director_client(app: web.Application) -> Tuple[ClientSession, URL]: +def _get_director_client(app: web.Application) -> tuple[ClientSession, URL]: settings: DirectorSettings = get_plugin_settings(app) api_endpoint = settings.base_url session = get_client_session(app) @@ -32,7 +32,7 @@ async def get_running_interactive_services( app: web.Application, user_id: Optional[str] = None, project_id: Optional[str] = None, -) -> List[Dict[str, Any]]: +) -> list[dict[str, Any]]: session, api_endpoint = _get_director_client(app) params = {} @@ -59,7 +59,8 @@ async def start_service( service_uuid: str, request_dns: str, request_scheme: str, -) -> Optional[Dict]: + request_user_agent: str, +) -> Optional[dict]: session, api_endpoint = _get_director_client(app) params = { @@ -74,6 +75,7 @@ async def start_service( headers = { "X-Dynamic-Sidecar-Request-DNS": request_dns, "X-Dynamic-Sidecar-Request-Scheme": request_scheme, + "X-Dynamic-Sidecar-Request-User-Agent": request_user_agent, } url = (api_endpoint / "running_interactive_services").with_query(params) @@ -126,7 +128,7 @@ async def stop_services( async def get_service_by_key_version( app: web.Application, service_key: str, service_version: str -) -> Optional[Dict]: +) -> Optional[dict]: session, api_endpoint = _get_director_client(app) url = ( @@ -147,7 +149,7 @@ async def get_service_by_key_version( async def get_services_extras( app: web.Application, service_key: str, service_version: str -) -> Optional[Dict]: +) -> Optional[dict]: session, api_endpoint = _get_director_client(app) url = ( 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 4ff7b0453e3..d30bb4457eb 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 @@ -83,6 +83,7 @@ async def run_dynamic_service( service_uuid: str, request_dns: str, request_scheme: str, + request_user_agent: str, service_resources: ServiceResourcesDict, ) -> DataType: """ @@ -106,6 +107,7 @@ async def run_dynamic_service( headers = { "X-Dynamic-Sidecar-Request-DNS": request_dns, "X-Dynamic-Sidecar-Request-Scheme": request_scheme, + "X-Dynamic-Sidecar-Request-User-Agent": request_user_agent, } settings: DirectorV2Settings = get_plugin_settings(app) 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 8d9c30c5a0e..bf44557d1e3 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 @@ -233,6 +233,7 @@ async def _start_dynamic_service( service_uuid=f"{node_uuid}", request_dns=extract_dns_without_default_port(request.url), request_scheme=request.headers.get("X-Forwarded-Proto", request.url.scheme), + request_user_agent=request.headers.get("User-Agent", ""), service_resources=service_resources, ) @@ -591,7 +592,6 @@ async def try_open_project_for_user( await get_user_name(app, user_id), notify_users=False, ): - with managed_resource(user_id, client_session_id, app) as rt: # NOTE: if max_number_of_studies_per_user is set, the same # project shall still be openable if the tab was closed diff --git a/services/web/server/tests/unit/isolated/test_director_api.py b/services/web/server/tests/unit/isolated/test_director_api.py index 0e3056b176a..015810e768b 100644 --- a/services/web/server/tests/unit/isolated/test_director_api.py +++ b/services/web/server/tests/unit/isolated/test_director_api.py @@ -11,7 +11,7 @@ import json import re from pathlib import Path -from typing import Any, Callable, Dict, List, Tuple +from typing import Any, Callable import pytest import yaml @@ -36,14 +36,14 @@ def director_openapi_dir(osparc_simcore_root_dir: Path) -> Path: @pytest.fixture(scope="session") -def director_openapi_specs(director_openapi_dir: Path) -> Dict[str, Any]: +def director_openapi_specs(director_openapi_dir: Path) -> dict[str, Any]: openapi_path = director_openapi_dir / "openapi.yaml" openapi_specs = yaml.safe_load(openapi_path.read_text()) return openapi_specs @pytest.fixture(scope="session") -def running_service_model_schema(osparc_simcore_root_dir: Path) -> Dict: +def running_service_model_schema(osparc_simcore_root_dir: Path) -> dict: # SEE: https://github.com/ITISFoundation/osparc-simcore/tree/master/api/specs/common/schemas/running_service.yaml#L30 content = yaml.safe_load( ( @@ -60,7 +60,7 @@ def running_service_model_schema(osparc_simcore_root_dir: Path) -> Dict: @pytest.fixture(scope="session") -def registry_service_model_schema(osparc_simcore_root_dir: Path) -> Dict: +def registry_service_model_schema(osparc_simcore_root_dir: Path) -> dict: # SEE: https://github.com/ITISFoundation/osparc-simcore/tree/master/api/specs/common/schemas/services.yaml#L11 # https://github.com/ITISFoundation/osparc-simcore/tree/master/api/specs/common/schemas/node-meta-v0.0.1.json schema = json.loads( @@ -81,7 +81,7 @@ def model_fake_factory(random_json_from_schema: Callable) -> Callable: Adapter to create fake data instances of a mo """ - def _create(schema: Dict, **override_attrs): + def _create(schema: dict, **override_attrs): model_instance = random_json_from_schema(json.dumps(schema)) model_instance.update(override_attrs) @@ -128,9 +128,8 @@ def mock_director_service( registry_service_model_schema, user_id: int, project_id: str, - project_nodes: List[Tuple[str, ...]], + project_nodes: list[tuple[str, ...]], ): - # helpers def fake_registry_service_model(**overrides): return model_fake_factory(registry_service_model_schema, **overrides) @@ -156,7 +155,6 @@ def _get_running(): # Mocks director's service API api/specs/director/openapi.yaml # with aioresponses() as mock: - # GET /running_interactive_services ------------------------------------------------- url_pattern = ( r"^http://[a-z\-_]*director:[0-9]+/v0/running_interactive_services\?.*$" @@ -332,9 +330,8 @@ async def test_director_workflow( app_mock, user_id: int, project_id: str, - project_nodes: List[Tuple[str, ...]], + project_nodes: list[tuple[str, ...]], ): - app = app_mock # After app's setup, the director config should be in place @@ -353,7 +350,6 @@ async def test_director_workflow( assert not running_services for service_key, service_version, service_uuid in project_nodes: - # service is in registry service = await get_service_by_key_version(app, service_key, service_version) assert service @@ -374,6 +370,7 @@ async def test_director_workflow( service_uuid, "localhost", "http", + "test", ) # now is running From 9eee29c39bc31888801f1f7f78b9899ba4525dad Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 17 Mar 2023 16:18:10 +0100 Subject: [PATCH 04/18] pylint --- services/director-v2/tests/unit/conftest.py | 7 +++++++ .../test_modules_dynamic_sidecar_docker_compose_specs.py | 6 +++++- .../director_v2_core_dynamic_services.py | 1 + .../src/simcore_service_webserver/projects/projects_api.py | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/services/director-v2/tests/unit/conftest.py b/services/director-v2/tests/unit/conftest.py index 79d3566d392..edfbe7e73c1 100644 --- a/services/director-v2/tests/unit/conftest.py +++ b/services/director-v2/tests/unit/conftest.py @@ -94,6 +94,11 @@ def request_scheme() -> str: return "http" +@pytest.fixture +def request_user_agent() -> str: + return "python/test" + + @pytest.fixture def scheduler_data_from_http_request( dynamic_service_create: DynamicServiceCreate, @@ -101,6 +106,7 @@ def scheduler_data_from_http_request( dynamic_sidecar_port: int, request_dns: str, request_scheme: str, + request_user_agent: str, run_id: RunID, ) -> SchedulerData: return SchedulerData.from_http_request( @@ -109,6 +115,7 @@ def scheduler_data_from_http_request( port=dynamic_sidecar_port, request_dns=request_dns, request_scheme=request_scheme, + request_user_agent=request_user_agent, run_id=run_id, ) diff --git a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py index dca0a996563..2240ca2adf9 100644 --- a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py +++ b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py @@ -163,6 +163,8 @@ def test_regression_service_has_no_reservations(): USER_ID: UserID = 1 PROJECT_ID: ProjectID = uuid4() NODE_ID: NodeID = uuid4() +USER_AGENT: str = "python/test" +PRODUCT_NAME: str = "osparc" @pytest.mark.parametrize( @@ -199,6 +201,8 @@ def test_regression_service_has_no_reservations(): f"user_id={USER_ID}", f"study_id={PROJECT_ID}", f"uuid={NODE_ID}", + f"user_agent={USER_AGENT}", + f"product_name={PRODUCT_NAME}", ] }, } @@ -211,6 +215,6 @@ async def test_update_container_labels( service_spec: dict[str, Any], expected_result: dict[str, Any] ): docker_compose_specs._update_container_labels( - service_spec, USER_ID, PROJECT_ID, NODE_ID + service_spec, USER_ID, PROJECT_ID, NODE_ID, USER_AGENT, PRODUCT_NAME ) assert service_spec == expected_result 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 d30bb4457eb..f5d8c913986 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 @@ -74,6 +74,7 @@ async def get_dynamic_service(app: web.Application, node_uuid: str) -> DataType: @log_decorator(logger=log) async def run_dynamic_service( + *, app: web.Application, product_name: str, user_id: PositiveInt, 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 bf44557d1e3..76d9361b74f 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 @@ -224,7 +224,7 @@ async def _start_dynamic_service( ) await director_v2_api.run_dynamic_service( - request.app, + app=request.app, product_name=product_name, project_id=f"{project_uuid}", user_id=user_id, From 45c66928514ffdbd7fc82b643fbd9a0ed201e9a8 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 20 Mar 2023 08:57:55 +0100 Subject: [PATCH 05/18] deprecation request --- packages/models-library/src/models_library/docker.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index b7865ad71ed..b9b62472a4e 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -38,6 +38,10 @@ class SimcoreServiceDockerLabelKeys(BaseModel): user_id: UserID = Field(..., alias="user_id") project_id: ProjectID = Field(..., alias="study_id") node_id: NodeID = Field(..., alias="uuid") + + # NOTE: `simcore_user_agent` and `product_name` should become + # mandatory once below PR reaches production + # https://github.com/ITISFoundation/osparc-simcore/pull/3990 product_name: Optional[ProductName] = None user_agent: Optional[str] = None From f6a3bb95e9b491aac0af6de2752aa1233633a591 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 20 Mar 2023 09:28:34 +0100 Subject: [PATCH 06/18] refactor --- .../models-library/src/models_library/docker.py | 2 +- packages/models-library/tests/test_docker.py | 16 ++++++++-------- .../api/routes/dynamic_services.py | 4 ++-- .../models/schemas/dynamic_services/scheduler.py | 6 +++--- .../dynamic_sidecar/docker_compose_specs.py | 8 ++++---- .../modules/dynamic_sidecar/scheduler/_abc.py | 2 +- .../dynamic_sidecar/scheduler/_core/_events.py | 2 +- .../scheduler/_core/_scheduler.py | 5 +++-- .../modules/dynamic_sidecar/scheduler/_task.py | 4 ++-- services/director-v2/tests/unit/conftest.py | 6 +++--- ...dules_dynamic_sidecar_docker_compose_specs.py | 6 +++--- .../director/director_api.py | 5 +++-- .../director_v2_core_dynamic_services.py | 5 +++-- .../projects/projects_api.py | 3 ++- 14 files changed, 39 insertions(+), 35 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index d3e927f0d66..82406b1ce32 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -35,7 +35,7 @@ class SimcoreServiceDockerLabelKeys(BaseModel): # mandatory once below PR reaches production # https://github.com/ITISFoundation/osparc-simcore/pull/3990 product_name: Optional[ProductName] = None - user_agent: Optional[str] = None + simcore_user_agent: Optional[str] = None def to_docker_labels(self) -> dict[str, str]: """returns a dictionary of strings as required by docker""" diff --git a/packages/models-library/tests/test_docker.py b/packages/models-library/tests/test_docker.py index c6e238ca1e2..066502c03a0 100644 --- a/packages/models-library/tests/test_docker.py +++ b/packages/models-library/tests/test_docker.py @@ -14,7 +14,7 @@ ) from pydantic import ValidationError, parse_obj_as -faker = Faker() +_faker = Faker() @pytest.mark.parametrize( @@ -110,19 +110,19 @@ def test_docker_generic_tag(image_name: str, valid: bool): [ pytest.param( { - "user_id": faker.pyint(), - "project_id": faker.uuid4(), - "node_id": faker.uuid4(), + "user_id": _faker.pyint(), + "project_id": _faker.uuid4(), + "node_id": _faker.uuid4(), }, id="parse_existing_service_labels", ), pytest.param( { - "user_id": faker.pyint(), - "project_id": faker.uuid4(), - "node_id": faker.uuid4(), + "user_id": _faker.pyint(), + "project_id": _faker.uuid4(), + "node_id": _faker.uuid4(), "product": "test_p", - "user_agent": "test/python", + "simcore_user_agent": "a-test-puppet", }, id="parse_new_service_labels", ), 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 6aee71d7808..f13aae3005e 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 @@ -101,7 +101,7 @@ async def create_dynamic_service( service: DynamicServiceCreate, x_dynamic_sidecar_request_dns: str = Header(...), x_dynamic_sidecar_request_scheme: str = Header(...), - x_dynamic_sidecar_request_user_agent: str = Header(...), + x_simcore_user_agent: str = Header(...), director_v0_client: DirectorV0Client = Depends(get_director_v0_client), dynamic_services_settings: DynamicServicesSettings = Depends( get_dynamic_services_settings @@ -141,7 +141,7 @@ async def create_dynamic_service( port=dynamic_services_settings.DYNAMIC_SIDECAR.DYNAMIC_SIDECAR_PORT, request_dns=x_dynamic_sidecar_request_dns, request_scheme=x_dynamic_sidecar_request_scheme, - request_user_agent=x_dynamic_sidecar_request_user_agent, + request_simcore_user_agent=x_simcore_user_agent, ) return cast(DynamicServiceGet, await scheduler.get_stack_status(service.node_uuid)) 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 195184080b8..b8b241d95f3 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 @@ -397,7 +397,7 @@ def endpoint(self) -> AnyHttpUrl: request_scheme: str = Field( ..., description="used when configuring the CORS options on the proxy" ) - request_user_agent: str = Field( + request_simcore_user_agent: str = Field( ..., description="used as label to filter out from the metrics" ) proxy_service_name: str = Field(None, description="service name given to the proxy") @@ -417,7 +417,7 @@ def from_http_request( port: PortInt, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, run_id: Optional[UUID] = None, ) -> "SchedulerData": # This constructor method sets current product @@ -444,7 +444,7 @@ def from_http_request( request_dns=request_dns, request_scheme=request_scheme, proxy_service_name=names_helper.proxy_service_name, - request_user_agent=request_user_agent, + request_simcore_user_agent=request_simcore_user_agent, dynamic_sidecar={}, ) if run_id: diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py index 950918f3eb1..6d49a788b34 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py @@ -193,7 +193,7 @@ def _update_container_labels( user_id: UserID, project_id: ProjectID, node_id: NodeID, - user_agent: str, + simcore_user_agent: str, product_name: ProductName, ) -> None: for spec in service_spec["services"].values(): @@ -203,7 +203,7 @@ def _update_container_labels( user_id=user_id, study_id=project_id, uuid=node_id, - user_agent=user_agent, + simcore_user_agent=simcore_user_agent, product_name=product_name, ) docker_labels = [f"{k}={v}" for k, v in label_keys.to_docker_labels().items()] @@ -230,7 +230,7 @@ def assemble_spec( user_id: UserID, project_id: ProjectID, node_id: NodeID, - user_agent: str, + simcore_user_agent: str, ) -> str: """ returns a docker-compose spec used by @@ -295,7 +295,7 @@ def assemble_spec( project_id=project_id, node_id=node_id, product_name=product_name, - user_agent=user_agent, + simcore_user_agent=simcore_user_agent, ) # TODO: will be used in next PR 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 edc803ef83f..8093ff3f707 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,7 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, ) -> None: """ Adds a new service. diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py index 304e06e684d..86900388830 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events.py @@ -481,7 +481,7 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None: user_id=scheduler_data.user_id, project_id=scheduler_data.project_id, node_id=scheduler_data.node_uuid, - user_agent=scheduler_data.request_user_agent, + simcore_user_agent=scheduler_data.request_simcore_user_agent, ) logger.debug( 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 7e60276b931..380dae8c35d 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 @@ -155,7 +155,7 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, ) -> None: """Invoked before the service is started""" scheduler_data = SchedulerData.from_http_request( @@ -164,7 +164,7 @@ async def add_service( port=port, request_dns=request_dns, request_scheme=request_scheme, - request_user_agent=request_user_agent, + request_simcore_user_agent=request_simcore_user_agent, ) await self._add_service(scheduler_data) @@ -216,6 +216,7 @@ def list_services( all_tracked_service_uuids = list(self._inverse_search_mapping.keys()) if user_id is None and project_id is None: return all_tracked_service_uuids + # let's filter def _is_scheduled(node_id: NodeID) -> bool: try: 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 53ebc712deb..103b9dd60e0 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,7 +70,7 @@ async def add_service( port: PortInt, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, ) -> None: return await self._scheduler.add_service( service, @@ -78,7 +78,7 @@ async def add_service( port, request_dns, request_scheme, - request_user_agent, + request_simcore_user_agent, ) def is_service_tracked(self, node_uuid: NodeID) -> bool: diff --git a/services/director-v2/tests/unit/conftest.py b/services/director-v2/tests/unit/conftest.py index edfbe7e73c1..9c482eaece3 100644 --- a/services/director-v2/tests/unit/conftest.py +++ b/services/director-v2/tests/unit/conftest.py @@ -95,7 +95,7 @@ def request_scheme() -> str: @pytest.fixture -def request_user_agent() -> str: +def request_simcore_user_agent() -> str: return "python/test" @@ -106,7 +106,7 @@ def scheduler_data_from_http_request( dynamic_sidecar_port: int, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, run_id: RunID, ) -> SchedulerData: return SchedulerData.from_http_request( @@ -115,7 +115,7 @@ def scheduler_data_from_http_request( port=dynamic_sidecar_port, request_dns=request_dns, request_scheme=request_scheme, - request_user_agent=request_user_agent, + request_simcore_user_agent=request_simcore_user_agent, run_id=run_id, ) diff --git a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py index 2240ca2adf9..3505cfbeb92 100644 --- a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py +++ b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py @@ -163,7 +163,7 @@ def test_regression_service_has_no_reservations(): USER_ID: UserID = 1 PROJECT_ID: ProjectID = uuid4() NODE_ID: NodeID = uuid4() -USER_AGENT: str = "python/test" +SIMCORE_USER_AGENT: str = "a-puppet" PRODUCT_NAME: str = "osparc" @@ -201,7 +201,7 @@ def test_regression_service_has_no_reservations(): f"user_id={USER_ID}", f"study_id={PROJECT_ID}", f"uuid={NODE_ID}", - f"user_agent={USER_AGENT}", + f"user_agent={SIMCORE_USER_AGENT}", f"product_name={PRODUCT_NAME}", ] }, @@ -215,6 +215,6 @@ async def test_update_container_labels( service_spec: dict[str, Any], expected_result: dict[str, Any] ): docker_compose_specs._update_container_labels( - service_spec, USER_ID, PROJECT_ID, NODE_ID, USER_AGENT, PRODUCT_NAME + service_spec, USER_ID, PROJECT_ID, NODE_ID, SIMCORE_USER_AGENT, PRODUCT_NAME ) assert service_spec == expected_result diff --git a/services/web/server/src/simcore_service_webserver/director/director_api.py b/services/web/server/src/simcore_service_webserver/director/director_api.py index 8c30fd8e5dd..8ba773e7a2f 100644 --- a/services/web/server/src/simcore_service_webserver/director/director_api.py +++ b/services/web/server/src/simcore_service_webserver/director/director_api.py @@ -10,6 +10,7 @@ from aiohttp import ClientSession, web from servicelib.aiohttp.client_session import get_client_session +from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER from servicelib.utils import logged_gather from yarl import URL @@ -59,7 +60,7 @@ async def start_service( service_uuid: str, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, ) -> Optional[dict]: session, api_endpoint = _get_director_client(app) @@ -75,7 +76,7 @@ async def start_service( headers = { "X-Dynamic-Sidecar-Request-DNS": request_dns, "X-Dynamic-Sidecar-Request-Scheme": request_scheme, - "X-Dynamic-Sidecar-Request-User-Agent": request_user_agent, + SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, } url = (api_endpoint / "running_interactive_services").with_query(params) 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 f5d8c913986..44c97ac9e24 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 @@ -18,6 +18,7 @@ ServiceResourcesDictHelpers, ) from pydantic.types import NonNegativeFloat, PositiveInt +from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER from servicelib.logging_utils import log_decorator from servicelib.progress_bar import ProgressBarData from servicelib.rabbitmq import RabbitMQClient @@ -84,7 +85,7 @@ async def run_dynamic_service( service_uuid: str, request_dns: str, request_scheme: str, - request_user_agent: str, + request_simcore_user_agent: str, service_resources: ServiceResourcesDict, ) -> DataType: """ @@ -108,7 +109,7 @@ async def run_dynamic_service( headers = { "X-Dynamic-Sidecar-Request-DNS": request_dns, "X-Dynamic-Sidecar-Request-Scheme": request_scheme, - "X-Dynamic-Sidecar-Request-User-Agent": request_user_agent, + SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, } settings: DirectorV2Settings = get_plugin_settings(app) 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 76d9361b74f..a13955ad136 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 @@ -40,6 +40,7 @@ APP_JSONSCHEMA_SPECS_KEY, ) from servicelib.aiohttp.jsonschema_validation import validate_instance +from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER from servicelib.json_serialization import json_dumps from servicelib.logging_utils import log_context from servicelib.utils import fire_and_forget_task, logged_gather @@ -233,7 +234,7 @@ async def _start_dynamic_service( service_uuid=f"{node_uuid}", request_dns=extract_dns_without_default_port(request.url), request_scheme=request.headers.get("X-Forwarded-Proto", request.url.scheme), - request_user_agent=request.headers.get("User-Agent", ""), + request_simcore_user_agent=request.headers.get(SIMCORE_USER_AGENT_HEADER, ""), service_resources=service_resources, ) From 2f05d1d8ac5acd559cfbc0d1adc865086679a884 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 20 Mar 2023 12:43:25 +0100 Subject: [PATCH 07/18] fixed broken tests --- .../unit/with_dbs/02/test_projects_handlers__open_close.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 548a866fbcc..0298fe10eed 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 @@ -335,13 +335,14 @@ async def test_open_project( for service_uuid, service in dynamic_services.items(): calls.append( call( - client.app, + app=client.app, project_id=user_project["uuid"], service_key=service["key"], service_uuid=service_uuid, service_version=service["version"], user_id=logged_user["id"], request_scheme=request_scheme, + request_simcore_user_agent="", request_dns=request_dns, product_name=osparc_product_name, service_resources=ServiceResourcesDictHelpers.create_jsonable( @@ -399,13 +400,14 @@ async def test_open_template_project_for_edition( for service_uuid, service in dynamic_services.items(): calls.append( call( - client.app, + app=client.app, project_id=template_project["uuid"], service_key=service["key"], service_uuid=service_uuid, service_version=service["version"], user_id=logged_user["id"], request_scheme=request_scheme, + request_simcore_user_agent="", request_dns=request_dns, service_resources=ServiceResourcesDictHelpers.create_jsonable( mock_service_resources From 4c6aea073701dc17c1cc4d6757864bd1a5e95a90 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 20 Mar 2023 13:33:01 +0100 Subject: [PATCH 08/18] fixed broken tests --- .../schemas/dynamic_services/scheduler.py | 21 ++- .../tests/mocks/fake_scheduler_data.json | 133 +++++++++--------- .../fake_scheduler_data_compose_spec.json | 3 +- .../unit/test_api_route_dynamic_scheduler.py | 1 + 4 files changed, 90 insertions(+), 68 deletions(-) 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 b8b241d95f3..e479d275861 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 @@ -408,6 +408,26 @@ def endpoint(self) -> AnyHttpUrl: "If set to None, the current product is undefined. Mostly for backwards compatibility", ) + @root_validator(pre=True) + @classmethod + def _ensure_legacy_format_compatibility(cls, values): + warnings.warn( + ( + "Once https://github.com/ITISFoundation/osparc-simcore/pull/3990 " + "reaches production this entire root_validator function " + "can be safely removed. Please check " + "https://github.com/ITISFoundation/osparc-simcore/issues/3996" + ), + DeprecationWarning, + stacklevel=2, + ) + request_simcore_user_agent: Optional[str] = values.get( + "request_simcore_user_agent" + ) + if not request_simcore_user_agent: + values["request_simcore_user_agent"] = "" + return values + @classmethod def from_http_request( # pylint: disable=too-many-arguments @@ -421,7 +441,6 @@ def from_http_request( run_id: Optional[UUID] = None, ) -> "SchedulerData": # This constructor method sets current product - assert service.product_name is not None # nosec names_helper = DynamicSidecarNamesHelper.make(service.node_uuid) obj_dict = dict( diff --git a/services/director-v2/tests/mocks/fake_scheduler_data.json b/services/director-v2/tests/mocks/fake_scheduler_data.json index c27951bd5b2..3efbfb9b8c9 100644 --- a/services/director-v2/tests/mocks/fake_scheduler_data.json +++ b/services/director-v2/tests/mocks/fake_scheduler_data.json @@ -1,71 +1,72 @@ { - "paths_mapping": { - "inputs_path": "/home/jovyan/work/inputs", - "outputs_path": "/home/jovyan/work/outputs", - "state_paths": [ - "/home/jovyan/work/workspace" - ], - "state_exclude": null + "paths_mapping": { + "inputs_path": "/home/jovyan/work/inputs", + "outputs_path": "/home/jovyan/work/outputs", + "state_paths": [ + "/home/jovyan/work/workspace" + ], + "state_exclude": null + }, + "compose_spec": "null", + "container_http_entry": null, + "restart_policy": "no-restart", + "key": "simcore/services/dynamic/jupyter-math", + "version": "2.0.4", + "user_id": 1, + "project_id": "c440556a-b4d9-11ec-9a17-02420a000016", + "node_uuid": "3e68d1f6-be3e-414e-a468-4a2bf415f756", + "service_name": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", + "hostname": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", + "port": 8000, + "dynamic_sidecar": { + "status": { + "current": "ok", + "info": "" }, - "compose_spec": "null", - "container_http_entry": null, - "restart_policy": "no-restart", - "key": "simcore/services/dynamic/jupyter-math", - "version": "2.0.4", - "user_id": 1, - "project_id": "c440556a-b4d9-11ec-9a17-02420a000016", - "node_uuid": "3e68d1f6-be3e-414e-a468-4a2bf415f756", - "service_name": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", - "hostname": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", - "port": 8000, - "dynamic_sidecar": { - "status": { - "current": "ok", - "info": "" - }, - "is_ready": true, - "was_compose_spec_submitted": true, - "containers_inspect": [ - { - "status": "running", - "name": "/dy-sidecar-3e68d1f6-be3e-414e-a468-4a2bf415f756-0-container", - "id": "c2f5f363cfa1bd4d16beb2086d2b7573fd4ebfb4fcf72bc7cda235efaaf5540b", - "container_state": {} - } - ], - "was_dynamic_sidecar_started": true, - "were_containers_created": true, - "is_project_network_attached": true, - "is_service_environment_ready": true, - "service_removal_state": { - "can_remove": false, - "can_save": null, - "was_removed": false - }, - "dynamic_sidecar_id": "sbig36r7lmciw0qvmbyjq8l87", - "dynamic_sidecar_network_id": "u6zunjtlawom4iimotk1hpe5j", - "swarm_network_id": "viqys7lieb0zjqoj00h0w5662", - "swarm_network_name": "master-simcore_interactive_services_subnet" + "is_ready": true, + "was_compose_spec_submitted": true, + "containers_inspect": [ + { + "status": "running", + "name": "/dy-sidecar-3e68d1f6-be3e-414e-a468-4a2bf415f756-0-container", + "id": "c2f5f363cfa1bd4d16beb2086d2b7573fd4ebfb4fcf72bc7cda235efaaf5540b", + "container_state": {} + } + ], + "was_dynamic_sidecar_started": true, + "were_containers_created": true, + "is_project_network_attached": true, + "is_service_environment_ready": true, + "service_removal_state": { + "can_remove": false, + "can_save": null, + "was_removed": false }, - "dynamic_sidecar_network_name": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", - "simcore_traefik_zone": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", - "service_port": "8888", - "service_resources": { - "container": { - "image": "simcore/services/dynamic/jupyter-math:2.0.5", - "resources": { - "CPU": { - "limit": 4, - "reservation": 0.1 - }, - "RAM": { - "limit": 103079215104, - "reservation": 536870912 - } - } + "dynamic_sidecar_id": "sbig36r7lmciw0qvmbyjq8l87", + "dynamic_sidecar_network_id": "u6zunjtlawom4iimotk1hpe5j", + "swarm_network_id": "viqys7lieb0zjqoj00h0w5662", + "swarm_network_name": "master-simcore_interactive_services_subnet" + }, + "dynamic_sidecar_network_name": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", + "simcore_traefik_zone": "dy-sidecar_3e68d1f6-be3e-414e-a468-4a2bf415f756", + "service_port": "8888", + "service_resources": { + "container": { + "image": "simcore/services/dynamic/jupyter-math:2.0.5", + "resources": { + "CPU": { + "limit": 4, + "reservation": 0.1 + }, + "RAM": { + "limit": 103079215104, + "reservation": 536870912 } - }, - "request_dns": "localhost", - "request_scheme": "http", - "proxy_service_name": "dy-proxy_3e68d1f6-be3e-414e-a468-4a2bf415f756" + } + } + }, + "request_dns": "localhost", + "request_scheme": "http", + "proxy_service_name": "dy-proxy_3e68d1f6-be3e-414e-a468-4a2bf415f756", + "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 1109600e014..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 @@ -97,5 +97,6 @@ }, "request_dns": "localhost", "request_scheme": "http", - "proxy_service_name": "dy-proxy_dd2b8ceb-4408-4bfb-a953-46178836e12d" + "proxy_service_name": "dy-proxy_dd2b8ceb-4408-4bfb-a953-46178836e12d", + "request_simcore_user_agent": "" } 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 6aee77c72bc..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 @@ -90,6 +90,7 @@ async def observed_service( dynamic_sidecar_port, request_dns, request_scheme, + request_simcore_user_agent="", ) # pylint:disable=protected-access return dynamic_sidecar_scheduler._scheduler.get_scheduler_data( From 116278e29f88afd94aef8bc0cc869cbdfd1c2cfa Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 09:55:58 +0100 Subject: [PATCH 09/18] using validator for defaults and made labels mandatory --- .../src/models_library/docker.py | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index 82406b1ce32..764050dcde4 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -1,12 +1,12 @@ import re -from typing import Optional +from typing import Any, Optional from models_library.generated_models.docker_rest_api import Task from models_library.products import ProductName from models_library.projects import ProjectID from models_library.projects_nodes import NodeID from models_library.users import UserID -from pydantic import BaseModel, ConstrainedStr, Field +from pydantic import BaseModel, ConstrainedStr, Field, root_validator from .basic_regex import DOCKER_GENERIC_TAG_KEY_RE, DOCKER_LABEL_KEY_REGEX @@ -31,11 +31,21 @@ class SimcoreServiceDockerLabelKeys(BaseModel): project_id: ProjectID = Field(..., alias="study_id") node_id: NodeID = Field(..., alias="uuid") - # NOTE: `simcore_user_agent` and `product_name` should become - # mandatory once below PR reaches production + product_name: ProductName + simcore_user_agent: str + + # NOTE: `simcore_user_agent` and `product_name` no longer required + # defaults after this PR reaches production. This can be removed # https://github.com/ITISFoundation/osparc-simcore/pull/3990 - product_name: Optional[ProductName] = None - simcore_user_agent: Optional[str] = None + # related issue https://github.com/ITISFoundation/osparc-simcore/issues/3993 + @root_validator(pre=True) + @classmethod + def check_owner_has_access_rights(cls, values: dict[str, Any]) -> dict[str, Any]: + if values.get("product_name", None) is None: + values["product_name"] = "opsarc" + if values.get("simcore_user_agent", None) is None: + values["simcore_user_agent"] = "" + return values def to_docker_labels(self) -> dict[str, str]: """returns a dictionary of strings as required by docker""" From 658a87c6cb9be8bb59739b1e0ea459c8220b114b Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 10:08:06 +0100 Subject: [PATCH 10/18] refactor --- .../models/schemas/dynamic_services/scheduler.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 e479d275861..9cba54d9cc7 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 @@ -398,11 +398,12 @@ def endpoint(self) -> AnyHttpUrl: ..., description="used when configuring the CORS options on the proxy" ) request_simcore_user_agent: str = Field( - ..., description="used as label to filter out from the metrics" + ..., + description="used as label to filter out the metrics from the cAdvisor prometheus metrics", ) proxy_service_name: str = Field(None, description="service name given to the proxy") - product_name: Optional[str] = Field( + product_name: str = Field( None, description="Current product upon which this service is scheduled. " "If set to None, the current product is undefined. Mostly for backwards compatibility", From 0dae4d13f4b808fa8fab6605d31cc084040650bc Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 10:16:53 +0100 Subject: [PATCH 11/18] refactor header names --- .../src/servicelib/common_headers.py | 4 ++++ .../integration/02/test_dynamic_services_routes.py | 9 ++++++--- services/director-v2/tests/integration/02/utils.py | 14 ++++++++------ .../with_dbs/test_api_route_dynamic_services.py | 8 ++++++-- .../director/director_api.py | 8 ++++++-- .../director_v2_core_dynamic_services.py | 8 ++++++-- 6 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 packages/service-library/src/servicelib/common_headers.py diff --git a/packages/service-library/src/servicelib/common_headers.py b/packages/service-library/src/servicelib/common_headers.py new file mode 100644 index 00000000000..c451813c446 --- /dev/null +++ b/packages/service-library/src/servicelib/common_headers.py @@ -0,0 +1,4 @@ +from typing import Final + +X_DYNAMIC_SIDECAR_REQUEST_DNS: Final[str] = "X-Dynamic-Sidecar-Request-DNS" +X_DYNAMIC_SIDECAR_REQUEST_SCHEME: Final[str] = "X-Dynamic-Sidecar-Request-Scheme" 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 1831423d2f4..154ac91fd9f 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 @@ -25,6 +25,10 @@ from pytest_mock.plugin import MockerFixture from pytest_simcore.helpers.typing_env import EnvVarsDict from pytest_simcore.helpers.utils_docker import get_localhost_ip +from servicelib.common_headers import ( + X_DYNAMIC_SIDECAR_REQUEST_DNS, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME, +) from settings_library.rabbit import RabbitSettings from settings_library.redis import RedisSettings from simcore_service_director_v2.core.application import init_app @@ -244,7 +248,6 @@ async def key_version_expected( dy_static_file_server_service: dict, docker_registry_image_injector: Callable, ) -> list[tuple[ServiceKeyVersion, bool]]: - results: list[tuple[ServiceKeyVersion, bool]] = [] sleeper_service = docker_registry_image_injector( @@ -281,8 +284,8 @@ async def test_start_status_stop( "/v2/dynamic_services", json=start_request_data, headers={ - "x-dynamic-sidecar-request-dns": start_request_data["request_dns"], - "x-dynamic-sidecar-request-scheme": start_request_data["request_scheme"], + X_DYNAMIC_SIDECAR_REQUEST_DNS: start_request_data["request_dns"], + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: start_request_data["request_scheme"], }, ) assert response.status_code == 201, response.text diff --git a/services/director-v2/tests/integration/02/utils.py b/services/director-v2/tests/integration/02/utils.py index d60b10c21eb..011147b76ff 100644 --- a/services/director-v2/tests/integration/02/utils.py +++ b/services/director-v2/tests/integration/02/utils.py @@ -18,6 +18,10 @@ from models_library.users import UserID from pydantic import PositiveInt, parse_obj_as from pytest_simcore.helpers.utils_docker import get_localhost_ip +from servicelib.common_headers import ( + X_DYNAMIC_SIDECAR_REQUEST_DNS, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME, +) from simcore_service_director_v2.core.settings import AppSettings from simcore_service_director_v2.models.schemas.constants import ( DYNAMIC_PROXY_SERVICE_PREFIX, @@ -265,7 +269,6 @@ async def assert_start_service( basepath: Optional[str], catalog_url: URL, ) -> None: - service_resources: ServiceResourcesDict = await _get_service_resources( catalog_url=catalog_url, service_key=service_key, @@ -284,8 +287,8 @@ async def assert_start_service( product_name=product_name, ) headers = { - "x-dynamic-sidecar-request-dns": director_v2_client.base_url.host, - "x-dynamic-sidecar-request-scheme": director_v2_client.base_url.scheme, + X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.host, + X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.scheme, } result = await director_v2_client.post( @@ -300,7 +303,6 @@ async def get_service_data( service_uuid: str, node_data: Node, ) -> dict[str, Any]: - # result = response = await director_v2_client.get( f"/v2/dynamic_services/{service_uuid}", follow_redirects=False @@ -358,8 +360,8 @@ async def assert_retrieve_service( director_v2_client: httpx.AsyncClient, service_uuid: str ) -> None: headers = { - "x-dynamic-sidecar-request-dns": director_v2_client.base_url.host, - "x-dynamic-sidecar-request-scheme": director_v2_client.base_url.scheme, + X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.host, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: director_v2_client.base_url.scheme, } result = await director_v2_client.post( diff --git a/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py b/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py index f815d48f18f..885225df5ee 100644 --- a/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py +++ b/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py @@ -20,6 +20,10 @@ from pytest_mock.plugin import MockerFixture from pytest_simcore.helpers.typing_env import EnvVarsDict from respx import MockRouter +from servicelib.common_headers import ( + X_DYNAMIC_SIDECAR_REQUEST_DNS, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME, +) from simcore_service_director_v2.models.domains.dynamic_services import ( DynamicServiceCreate, RetrieveDataOutEnveloped, @@ -72,8 +76,8 @@ def minimal_config( @pytest.fixture(scope="session") def dynamic_sidecar_headers() -> dict[str, str]: return { - "X-Dynamic-Sidecar-Request-DNS": "", - "X-Dynamic-Sidecar-Request-Scheme": "", + X_DYNAMIC_SIDECAR_REQUEST_DNS: "", + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: "", } diff --git a/services/web/server/src/simcore_service_webserver/director/director_api.py b/services/web/server/src/simcore_service_webserver/director/director_api.py index 8ba773e7a2f..bcb0d155416 100644 --- a/services/web/server/src/simcore_service_webserver/director/director_api.py +++ b/services/web/server/src/simcore_service_webserver/director/director_api.py @@ -11,6 +11,10 @@ from aiohttp import ClientSession, web from servicelib.aiohttp.client_session import get_client_session from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER +from servicelib.common_headers import ( + X_DYNAMIC_SIDECAR_REQUEST_DNS, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME, +) from servicelib.utils import logged_gather from yarl import URL @@ -74,8 +78,8 @@ async def start_service( } headers = { - "X-Dynamic-Sidecar-Request-DNS": request_dns, - "X-Dynamic-Sidecar-Request-Scheme": request_scheme, + X_DYNAMIC_SIDECAR_REQUEST_DNS: request_dns, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: request_scheme, SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, } 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 44c97ac9e24..5355ad2155e 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 @@ -19,6 +19,10 @@ ) from pydantic.types import NonNegativeFloat, PositiveInt from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER +from servicelib.common_headers import ( + X_DYNAMIC_SIDECAR_REQUEST_DNS, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME, +) from servicelib.logging_utils import log_decorator from servicelib.progress_bar import ProgressBarData from servicelib.rabbitmq import RabbitMQClient @@ -107,8 +111,8 @@ async def run_dynamic_service( } headers = { - "X-Dynamic-Sidecar-Request-DNS": request_dns, - "X-Dynamic-Sidecar-Request-Scheme": request_scheme, + X_DYNAMIC_SIDECAR_REQUEST_DNS: request_dns, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: request_scheme, SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, } From 1992f7588723efdc9124a8e5e87cce68f4aa6400 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 10:20:26 +0100 Subject: [PATCH 12/18] refactor X-Forwarded-Proto --- packages/service-library/src/servicelib/common_headers.py | 1 + .../src/simcore_service_webserver/projects/projects_api.py | 3 ++- .../server/src/simcore_service_webserver/storage_handlers.py | 3 ++- .../web/server/src/simcore_service_webserver/utils_aiohttp.py | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/service-library/src/servicelib/common_headers.py b/packages/service-library/src/servicelib/common_headers.py index c451813c446..37edf4ef4c7 100644 --- a/packages/service-library/src/servicelib/common_headers.py +++ b/packages/service-library/src/servicelib/common_headers.py @@ -2,3 +2,4 @@ X_DYNAMIC_SIDECAR_REQUEST_DNS: Final[str] = "X-Dynamic-Sidecar-Request-DNS" X_DYNAMIC_SIDECAR_REQUEST_SCHEME: Final[str] = "X-Dynamic-Sidecar-Request-Scheme" +X_FORWARDED_PROTO: Final[str] = "X-Forwarded-Proto" 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 a13955ad136..f48455265dc 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 @@ -41,6 +41,7 @@ ) from servicelib.aiohttp.jsonschema_validation import validate_instance from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER +from servicelib.common_headers import X_FORWARDED_PROTO from servicelib.json_serialization import json_dumps from servicelib.logging_utils import log_context from servicelib.utils import fire_and_forget_task, logged_gather @@ -233,7 +234,7 @@ async def _start_dynamic_service( service_version=service_version, service_uuid=f"{node_uuid}", request_dns=extract_dns_without_default_port(request.url), - request_scheme=request.headers.get("X-Forwarded-Proto", request.url.scheme), + request_scheme=request.headers.get(X_FORWARDED_PROTO, request.url.scheme), request_simcore_user_agent=request.headers.get(SIMCORE_USER_AGENT_HEADER, ""), service_resources=service_resources, ) diff --git a/services/web/server/src/simcore_service_webserver/storage_handlers.py b/services/web/server/src/simcore_service_webserver/storage_handlers.py index 0e4bcf8eed8..7a45eae9a0e 100644 --- a/services/web/server/src/simcore_service_webserver/storage_handlers.py +++ b/services/web/server/src/simcore_service_webserver/storage_handlers.py @@ -15,6 +15,7 @@ from servicelib.aiohttp.client_session import get_client_session from servicelib.aiohttp.rest_responses import create_data_response, unwrap_envelope from servicelib.aiohttp.rest_utils import extract_and_validate +from servicelib.common_headers import X_FORWARDED_PROTO from servicelib.request_keys import RQT_USERID_KEY from yarl import URL @@ -86,7 +87,7 @@ def _unresolve_storage_url(request: web.Request, storage_url: AnyUrl) -> AnyUrl: prefix = f"/{_get_storage_vtag(request.app)}" converted_url = request.url.with_path( f"/v0/storage{storage_url.path.removeprefix(prefix)}" - ).with_scheme(request.headers.get("X-Forwarded-Proto", request.url.scheme)) + ).with_scheme(request.headers.get(X_FORWARDED_PROTO, request.url.scheme)) return parse_obj_as(AnyUrl, f"{converted_url}") diff --git a/services/web/server/src/simcore_service_webserver/utils_aiohttp.py b/services/web/server/src/simcore_service_webserver/utils_aiohttp.py index ebb32b1b488..45347ded92f 100644 --- a/services/web/server/src/simcore_service_webserver/utils_aiohttp.py +++ b/services/web/server/src/simcore_service_webserver/utils_aiohttp.py @@ -8,6 +8,7 @@ from models_library.generics import Envelope from pydantic import Field from pydantic.generics import GenericModel +from servicelib.common_headers import X_FORWARDED_PROTO from servicelib.json_serialization import json_dumps from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -46,7 +47,7 @@ def url_for(route_name: str, **params: dict[str, Any]) -> str: .with_scheme( # Custom header by traefik. See labels in docker-compose as: # - traefik.http.middlewares.${SWARM_STACK_NAME_NO_HYPHEN}_sslheader.headers.customrequestheaders.X-Forwarded-Proto=http - request.headers.get("X-Forwarded-Proto", request.url.scheme) + request.headers.get(X_FORWARDED_PROTO, request.url.scheme) ) .with_path(str(rel_url)) ) From 591a1a08970349e433a7bbd82f93dea003a5b677 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 10:25:40 +0100 Subject: [PATCH 13/18] fixing deprecation messages --- .../models-library/src/models_library/docker.py | 17 ++++++++++++----- .../schemas/dynamic_services/scheduler.py | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index 764050dcde4..59d664604bb 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -1,4 +1,5 @@ import re +import warnings from typing import Any, Optional from models_library.generated_models.docker_rest_api import Task @@ -34,13 +35,19 @@ class SimcoreServiceDockerLabelKeys(BaseModel): product_name: ProductName simcore_user_agent: str - # NOTE: `simcore_user_agent` and `product_name` no longer required - # defaults after this PR reaches production. This can be removed - # https://github.com/ITISFoundation/osparc-simcore/pull/3990 - # related issue https://github.com/ITISFoundation/osparc-simcore/issues/3993 @root_validator(pre=True) @classmethod - def check_owner_has_access_rights(cls, values: dict[str, Any]) -> dict[str, Any]: + def ensure_defaults(cls, values: dict[str, Any]) -> dict[str, Any]: + warnings.warn( + ( + "Once https://github.com/ITISFoundation/osparc-simcore/pull/3990 " + "reaches production this entire root_validator function " + "can be safely removed. Please check " + "https://github.com/ITISFoundation/osparc-simcore/issues/3996" + ), + DeprecationWarning, + stacklevel=2, + ) if values.get("product_name", None) is None: values["product_name"] = "opsarc" if values.get("simcore_user_agent", None) is None: 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 4d7b82fcc76..25c354068c1 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 @@ -1,5 +1,6 @@ import json import logging +import warnings from enum import Enum from functools import cached_property from typing import Any, Mapping, Optional From 216abcb265e23aa14ef5464dcdb0cc7cf0862021 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 14:52:40 +0100 Subject: [PATCH 14/18] fixed missing --- .../models/schemas/dynamic_services/scheduler.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 25c354068c1..1303d2e4021 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 @@ -17,7 +17,15 @@ ) from models_library.services import RunID from models_library.services_resources import ServiceResourcesDict -from pydantic import AnyHttpUrl, BaseModel, Extra, Field, constr, parse_obj_as +from pydantic import ( + AnyHttpUrl, + BaseModel, + Extra, + Field, + constr, + parse_obj_as, + root_validator, +) from servicelib.error_codes import ErrorCodeStr from servicelib.exception_utils import DelayedExceptionHandler From 4cf3710a023bec6d9cf1a56a3ba0b34a82ada8b0 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 15:27:09 +0100 Subject: [PATCH 15/18] fix broken test --- .../src/models_library/docker.py | 2 +- ...es_dynamic_sidecar_docker_compose_specs.py | 38 ++++++------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index 59d664604bb..dfc94fbe90c 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -57,7 +57,7 @@ def ensure_defaults(cls, values: dict[str, Any]) -> dict[str, Any]: def to_docker_labels(self) -> dict[str, str]: """returns a dictionary of strings as required by docker""" std_export = self.dict(by_alias=True) - return {k: f"{v}" for k, v in std_export.items()} + return {k: f"{v}" for k, v in sorted(std_export.items())} @classmethod def from_docker_task(cls, docker_task: Task) -> "SimcoreServiceDockerLabelKeys": diff --git a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py index 3505cfbeb92..2324b38c3eb 100644 --- a/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py +++ b/services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_compose_specs.py @@ -166,45 +166,29 @@ def test_regression_service_has_no_reservations(): SIMCORE_USER_AGENT: str = "a-puppet" PRODUCT_NAME: str = "osparc" +EXPECTED_LABELS: list[str] = [ + f"product_name={PRODUCT_NAME}", + f"simcore_user_agent={SIMCORE_USER_AGENT}", + f"study_id={PROJECT_ID}", + f"user_id={USER_ID}", + f"uuid={NODE_ID}", +] + @pytest.mark.parametrize( "service_spec, expected_result", [ pytest.param( {"services": {"service-1": {}}}, - { - "services": { - "service-1": { - "labels": [ - f"user_id={USER_ID}", - f"study_id={PROJECT_ID}", - f"uuid={NODE_ID}", - ] - } - } - }, + {"services": {"service-1": {"labels": EXPECTED_LABELS}}}, id="single_service", ), pytest.param( {"services": {"service-1": {}, "service-2": {}}}, { "services": { - "service-1": { - "labels": [ - f"user_id={USER_ID}", - f"study_id={PROJECT_ID}", - f"uuid={NODE_ID}", - ] - }, - "service-2": { - "labels": [ - f"user_id={USER_ID}", - f"study_id={PROJECT_ID}", - f"uuid={NODE_ID}", - f"user_agent={SIMCORE_USER_AGENT}", - f"product_name={PRODUCT_NAME}", - ] - }, + "service-1": {"labels": EXPECTED_LABELS}, + "service-2": {"labels": EXPECTED_LABELS}, } }, id="multiple_services", From c32e81138df4218f2e29a09f275e9116b24b2389 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 15:54:01 +0100 Subject: [PATCH 16/18] fixed broken test --- .../test_modules_dynamic_sidecar_docker_service_specs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 1186d6dc323..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 @@ -88,7 +88,7 @@ def simcore_service_labels() -> SimcoreServiceLabels: @pytest.fixture def expected_dynamic_sidecar_spec( - run_id: RunID, osparc_product_name: str + run_id: RunID, osparc_product_name: str, request_simcore_user_agent: str ) -> dict[str, Any]: return { "endpoint_spec": {}, @@ -141,6 +141,7 @@ def expected_dynamic_sidecar_spec( "proxy_service_name": "dy-proxy_75c7f3f4-18f9-4678-8610-54a2ade78eaa", "request_dns": "test-endpoint", "request_scheme": "http", + "request_simcore_user_agent": request_simcore_user_agent, "restart_policy": "on-inputs-downloaded", "service_name": "dy-sidecar_75c7f3f4-18f9-4678-8610-54a2ade78eaa", "service_port": 65534, From 76ff72265e897be4bca317fc4b5cc4061373de8a Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 22 Mar 2023 16:01:40 +0100 Subject: [PATCH 17/18] refactor headers --- .../src/servicelib/aiohttp/monitoring.py | 14 ++++---------- .../src/servicelib/common_headers.py | 1 + .../tests/aiohttp/test_monitoring.py | 9 +++------ .../with_dbs/test_api_route_dynamic_services.py | 2 ++ .../director/director_api.py | 4 ++-- .../director_v2_core_dynamic_services.py | 4 ++-- .../projects/projects_api.py | 5 ++--- 7 files changed, 16 insertions(+), 23 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/monitoring.py b/packages/service-library/src/servicelib/aiohttp/monitoring.py index 4c12fdd1e10..7bbed9c196b 100644 --- a/packages/service-library/src/servicelib/aiohttp/monitoring.py +++ b/packages/service-library/src/servicelib/aiohttp/monitoring.py @@ -21,6 +21,7 @@ from prometheus_client.registry import CollectorRegistry from servicelib.aiohttp.typing_extension import Handler +from ..common_headers import X_SIMCORE_USER_AGENT from ..logging_utils import log_catch log = logging.getLogger(__name__) @@ -112,7 +113,6 @@ kPLATFORM_COLLECTOR = f"{__name__}.collector_platform" kGC_COLLECTOR = f"{__name__}.collector_gc" -SIMCORE_USER_AGENT_HEADER: Final[str] = "X-Simcore-User-Agent" UNDEFINED_REGULAR_USER_AGENT: Final[str] = "undefined" @@ -168,16 +168,12 @@ async def middleware_handler(request: web.Request, handler: Handler): app_name, request.method, canonical_endpoint, - request.headers.get( - SIMCORE_USER_AGENT_HEADER, UNDEFINED_REGULAR_USER_AGENT - ), + request.headers.get(X_SIMCORE_USER_AGENT, UNDEFINED_REGULAR_USER_AGENT), ).track_inprogress(), response_summary.labels( app_name, request.method, canonical_endpoint, - request.headers.get( - SIMCORE_USER_AGENT_HEADER, UNDEFINED_REGULAR_USER_AGENT - ), + request.headers.get(X_SIMCORE_USER_AGENT, UNDEFINED_REGULAR_USER_AGENT), ).time(): resp = await handler(request) @@ -213,9 +209,7 @@ async def middleware_handler(request: web.Request, handler: Handler): request.method, canonical_endpoint, resp.status, - request.headers.get( - SIMCORE_USER_AGENT_HEADER, UNDEFINED_REGULAR_USER_AGENT - ), + request.headers.get(X_SIMCORE_USER_AGENT, UNDEFINED_REGULAR_USER_AGENT), ).inc() if exit_middleware_cb: diff --git a/packages/service-library/src/servicelib/common_headers.py b/packages/service-library/src/servicelib/common_headers.py index 37edf4ef4c7..842e9d14986 100644 --- a/packages/service-library/src/servicelib/common_headers.py +++ b/packages/service-library/src/servicelib/common_headers.py @@ -3,3 +3,4 @@ X_DYNAMIC_SIDECAR_REQUEST_DNS: Final[str] = "X-Dynamic-Sidecar-Request-DNS" X_DYNAMIC_SIDECAR_REQUEST_SCHEME: Final[str] = "X-Dynamic-Sidecar-Request-Scheme" X_FORWARDED_PROTO: Final[str] = "X-Forwarded-Proto" +X_SIMCORE_USER_AGENT: Final[str] = "X-Simcore-User-Agent" diff --git a/packages/service-library/tests/aiohttp/test_monitoring.py b/packages/service-library/tests/aiohttp/test_monitoring.py index d9d78e5c79b..18b6043b658 100644 --- a/packages/service-library/tests/aiohttp/test_monitoring.py +++ b/packages/service-library/tests/aiohttp/test_monitoring.py @@ -11,11 +11,8 @@ from aiohttp.test_utils import TestClient from faker import Faker from prometheus_client.parser import text_string_to_metric_families -from servicelib.aiohttp.monitoring import ( - SIMCORE_USER_AGENT_HEADER, - UNDEFINED_REGULAR_USER_AGENT, - setup_monitoring, -) +from servicelib.aiohttp.monitoring import UNDEFINED_REGULAR_USER_AGENT, setup_monitoring +from servicelib.common_headers import X_SIMCORE_USER_AGENT @pytest.fixture @@ -120,7 +117,7 @@ async def test_request_with_simcore_user_agent(client: TestClient, faker: Faker) faker_simcore_user_agent = faker.name() response = await client.get( "/monitored_request", - headers={SIMCORE_USER_AGENT_HEADER: faker_simcore_user_agent}, + headers={X_SIMCORE_USER_AGENT: faker_simcore_user_agent}, ) assert response.status == web.HTTPOk.status_code diff --git a/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py b/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py index 885225df5ee..2cfee21cf47 100644 --- a/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py +++ b/services/director-v2/tests/unit/with_dbs/test_api_route_dynamic_services.py @@ -23,6 +23,7 @@ from servicelib.common_headers import ( X_DYNAMIC_SIDECAR_REQUEST_DNS, X_DYNAMIC_SIDECAR_REQUEST_SCHEME, + X_SIMCORE_USER_AGENT, ) from simcore_service_director_v2.models.domains.dynamic_services import ( DynamicServiceCreate, @@ -78,6 +79,7 @@ def dynamic_sidecar_headers() -> dict[str, str]: return { X_DYNAMIC_SIDECAR_REQUEST_DNS: "", X_DYNAMIC_SIDECAR_REQUEST_SCHEME: "", + X_SIMCORE_USER_AGENT: "", } diff --git a/services/web/server/src/simcore_service_webserver/director/director_api.py b/services/web/server/src/simcore_service_webserver/director/director_api.py index bcb0d155416..4c0502726c8 100644 --- a/services/web/server/src/simcore_service_webserver/director/director_api.py +++ b/services/web/server/src/simcore_service_webserver/director/director_api.py @@ -10,10 +10,10 @@ from aiohttp import ClientSession, web from servicelib.aiohttp.client_session import get_client_session -from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER from servicelib.common_headers import ( X_DYNAMIC_SIDECAR_REQUEST_DNS, X_DYNAMIC_SIDECAR_REQUEST_SCHEME, + X_SIMCORE_USER_AGENT, ) from servicelib.utils import logged_gather from yarl import URL @@ -80,7 +80,7 @@ async def start_service( headers = { X_DYNAMIC_SIDECAR_REQUEST_DNS: request_dns, X_DYNAMIC_SIDECAR_REQUEST_SCHEME: request_scheme, - SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, + X_SIMCORE_USER_AGENT: request_simcore_user_agent, } url = (api_endpoint / "running_interactive_services").with_query(params) 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 5355ad2155e..fc35a8a7f42 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 @@ -18,10 +18,10 @@ ServiceResourcesDictHelpers, ) from pydantic.types import NonNegativeFloat, PositiveInt -from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER from servicelib.common_headers import ( X_DYNAMIC_SIDECAR_REQUEST_DNS, X_DYNAMIC_SIDECAR_REQUEST_SCHEME, + X_SIMCORE_USER_AGENT, ) from servicelib.logging_utils import log_decorator from servicelib.progress_bar import ProgressBarData @@ -113,7 +113,7 @@ async def run_dynamic_service( headers = { X_DYNAMIC_SIDECAR_REQUEST_DNS: request_dns, X_DYNAMIC_SIDECAR_REQUEST_SCHEME: request_scheme, - SIMCORE_USER_AGENT_HEADER: request_simcore_user_agent, + X_SIMCORE_USER_AGENT: request_simcore_user_agent, } settings: DirectorV2Settings = get_plugin_settings(app) 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 8ec884b21bb..4a4484ec560 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 @@ -40,8 +40,7 @@ APP_JSONSCHEMA_SPECS_KEY, ) from servicelib.aiohttp.jsonschema_validation import validate_instance -from servicelib.aiohttp.monitoring import SIMCORE_USER_AGENT_HEADER -from servicelib.common_headers import X_FORWARDED_PROTO +from servicelib.common_headers import X_FORWARDED_PROTO, X_SIMCORE_USER_AGENT from servicelib.json_serialization import json_dumps from servicelib.logging_utils import log_context from servicelib.utils import fire_and_forget_task, logged_gather @@ -236,7 +235,7 @@ async def _start_dynamic_service( service_uuid=f"{node_uuid}", request_dns=extract_dns_without_default_port(request.url), request_scheme=request.headers.get(X_FORWARDED_PROTO, request.url.scheme), - request_simcore_user_agent=request.headers.get(SIMCORE_USER_AGENT_HEADER, ""), + request_simcore_user_agent=request.headers.get(X_SIMCORE_USER_AGENT, ""), service_resources=service_resources, ) From 939b26bae481208a2bfb41a0ecdef51804916d43 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 23 Mar 2023 11:42:41 +0100 Subject: [PATCH 18/18] added missing headers --- .../tests/integration/02/test_dynamic_services_routes.py | 2 ++ services/director-v2/tests/integration/02/utils.py | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) 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 154ac91fd9f..95087e29978 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 @@ -28,6 +28,7 @@ from servicelib.common_headers import ( X_DYNAMIC_SIDECAR_REQUEST_DNS, X_DYNAMIC_SIDECAR_REQUEST_SCHEME, + X_SIMCORE_USER_AGENT, ) from settings_library.rabbit import RabbitSettings from settings_library.redis import RedisSettings @@ -286,6 +287,7 @@ async def test_start_status_stop( headers={ X_DYNAMIC_SIDECAR_REQUEST_DNS: start_request_data["request_dns"], X_DYNAMIC_SIDECAR_REQUEST_SCHEME: start_request_data["request_scheme"], + X_SIMCORE_USER_AGENT: "", }, ) assert response.status_code == 201, response.text diff --git a/services/director-v2/tests/integration/02/utils.py b/services/director-v2/tests/integration/02/utils.py index 011147b76ff..6aa25c70b19 100644 --- a/services/director-v2/tests/integration/02/utils.py +++ b/services/director-v2/tests/integration/02/utils.py @@ -21,6 +21,7 @@ from servicelib.common_headers import ( X_DYNAMIC_SIDECAR_REQUEST_DNS, X_DYNAMIC_SIDECAR_REQUEST_SCHEME, + X_SIMCORE_USER_AGENT, ) from simcore_service_director_v2.core.settings import AppSettings from simcore_service_director_v2.models.schemas.constants import ( @@ -288,7 +289,8 @@ async def assert_start_service( ) headers = { X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.host, - X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.scheme, + X_DYNAMIC_SIDECAR_REQUEST_SCHEME: director_v2_client.base_url.scheme, + X_SIMCORE_USER_AGENT: "", } result = await director_v2_client.post( @@ -362,6 +364,7 @@ async def assert_retrieve_service( headers = { X_DYNAMIC_SIDECAR_REQUEST_DNS: director_v2_client.base_url.host, X_DYNAMIC_SIDECAR_REQUEST_SCHEME: director_v2_client.base_url.scheme, + X_SIMCORE_USER_AGENT: "", } result = await director_v2_client.post(