From e6c6b45c5de6cd8c50d6aabfeb5722d13442e36a Mon Sep 17 00:00:00 2001 From: Andrei Neagu <5694077+GitHK@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:31:20 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=91=EF=B8=8F=20fixes=20issue=20with=20?= =?UTF-8?q?env=20vars=20being=20stored=20as=20dict=20or=20list=20(#6052)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrei Neagu --- .../src/servicelib/async_utils.py | 16 +- .../core/validation.py | 16 +- .../tests/mocks/internet_blocked_spec.yaml | 141 ++++++++++++++++++ .../tests/unit/test_core_validation.py | 48 ++++++ 4 files changed, 213 insertions(+), 8 deletions(-) create mode 100644 services/dynamic-sidecar/tests/mocks/internet_blocked_spec.yaml diff --git a/packages/service-library/src/servicelib/async_utils.py b/packages/service-library/src/servicelib/async_utils.py index b25955da95a..103ef641be2 100644 --- a/packages/service-library/src/servicelib/async_utils.py +++ b/packages/service-library/src/servicelib/async_utils.py @@ -42,14 +42,24 @@ class QueueElement: _sequential_jobs_contexts: dict[str, Context] = {} -async def cancel_sequential_workers() -> None: - """Signals all workers to close thus avoiding errors on shutdown""" - for context in _sequential_jobs_contexts.values(): +async def _safe_cancel(context: Context) -> None: + try: await context.in_queue.put(None) if context.task is not None: context.task.cancel() with suppress(asyncio.CancelledError): await context.task + except RuntimeError as e: + if "Event loop is closed" in f"{e}": + logger.warning("event loop is closed and could not cancel %s", context) + else: + raise + + +async def cancel_sequential_workers() -> None: + """Signals all workers to close thus avoiding errors on shutdown""" + for context in _sequential_jobs_contexts.values(): + await _safe_cancel(context) _sequential_jobs_contexts.clear() logger.info("All run_sequentially_in_context pending workers stopped") diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/validation.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/validation.py index 05d34f87005..2737ccdc7da 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/validation.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/validation.py @@ -107,14 +107,20 @@ def _apply_templating_directives( def _merge_env_vars( - compose_spec_env_vars: list[str], settings_env_vars: list[str] + compose_spec_env_vars: list[str] | dict[str, str], + settings_env_vars: list[str] | dict[str, str], ) -> list[str]: def _gen_parts_env_vars( - env_vars: list[str], + env_vars: list[str] | dict[str, str], ) -> Generator[tuple[str, str], None, None]: - for env_var in env_vars: - key, value = env_var.split("=") - yield key, value + assert isinstance(env_vars, list | dict) # nosec + + if isinstance(env_vars, list): + for env_var in env_vars: + key, value = env_var.split("=") + yield key, value + else: + yield from env_vars.items() # pylint: disable=unnecessary-comprehension dict_spec_env_vars = {k: v for k, v in _gen_parts_env_vars(compose_spec_env_vars)} diff --git a/services/dynamic-sidecar/tests/mocks/internet_blocked_spec.yaml b/services/dynamic-sidecar/tests/mocks/internet_blocked_spec.yaml new file mode 100644 index 00000000000..83bc4592b05 --- /dev/null +++ b/services/dynamic-sidecar/tests/mocks/internet_blocked_spec.yaml @@ -0,0 +1,141 @@ +networks: + dy-sidecar_a019b83f-7cce-46bf-90cf-d02f7f0f089a: + driver: overlay + external: true + name: dy-sidecar_a019b83f-7cce-46bf-90cf-d02f7f0f089a + egress-0: + internal: true + production-simcore_interactive_services_subnet: + driver: overlay + external: true + name: production-simcore_interactive_services_subnet + with-internet: + internal: false +services: + egress-0: + command: "fake" + environment: + OSPARC_NODE_ID: a019b83f-7cce-46bf-90cf-d02f7f0f089a + OSPARC_STUDY_ID: ef60eaa6-3f52-11ef-9745-0242ac172e8c + image: envoyproxy/envoy:v1.25-latest + labels: + - io.simcore.runtime.cpu-limit=0 + - io.simcore.runtime.memory-limit=0 + - io.simcore.runtime.node-id=a019b83f-7cce-46bf-90cf-d02f7f0f089a + - io.simcore.runtime.product-name=s4llite + - io.simcore.runtime.project-id=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - io.simcore.runtime.simcore-user-agent=undefined + - io.simcore.runtime.swarm-stack-name=production-simcore + - io.simcore.runtime.user-id=47568 + networks: + egress-0: + aliases: + - ip-10-1-0-25.ec2.internal + with-internet: null + rt-web-lite: + cpus: 0.5 + depends_on: + - s4l-core + environment: + - DY_SIDECAR_PATH_INPUTS=/home/smu/work/inputs + - DY_SIDECAR_PATH_OUTPUTS=/home/smu/work/outputs + - DY_SIDECAR_STATE_PATHS=[""/home/smu/work/workspace""] + - SIMCORE_NANO_CPUS_LIMIT=500000000 + - SIMCORE_MEMORY_BYTES_LIMIT=1073741824 + - OSPARC_STUDY_ID=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - OSPARC_NODE_ID=a019b83f-7cce-46bf-90cf-d02f7f0f089a + image: simcore/services/dynamic/sim4life-lite:2.0.106 + init: true + labels: + - io.simcore.runtime.cpu-limit=0.5 + - io.simcore.runtime.memory-limit=1073741824 + - io.simcore.runtime.node-id=a019b83f-7cce-46bf-90cf-d02f7f0f089a + - io.simcore.runtime.product-name=s4llite + - io.simcore.runtime.project-id=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - io.simcore.runtime.simcore-user-agent=undefined + - io.simcore.runtime.swarm-stack-name=production-simcore + - io.simcore.runtime.user-id=47568 + mem_limit: 1073741824 + mem_reservation: 1073741824 + networks: + dy-sidecar_a019b83f-7cce-46bf-90cf-d02f7f0f089a: null + s4l-core: + cpus: 3.5 + depends_on: + - egress-0 + environment: + - DISPLAY= + - DY_SIDECAR_PATH_INPUTS=/home/smu/work/inputs + - DY_SIDECAR_PATH_OUTPUTS=/home/smu/work/outputs + - DY_SIDECAR_STATE_PATHS=[""/home/smu/work/workspace""] + - SIMCORE_NANO_CPUS_LIMIT=3500000000 + - SIMCORE_MEMORY_BYTES_LIMIT=17179869184 + - OSPARC_STUDY_ID=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - OSPARC_NODE_ID=a019b83f-7cce-46bf-90cf-d02f7f0f089a + image: simcore/services/dynamic/s4l-core-lite:2.0.106 + init: true + labels: + - io.simcore.runtime.cpu-limit=3.5 + - io.simcore.runtime.memory-limit=17179869184 + - io.simcore.runtime.node-id=a019b83f-7cce-46bf-90cf-d02f7f0f089a + - io.simcore.runtime.product-name=s4llite + - io.simcore.runtime.project-id=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - io.simcore.runtime.simcore-user-agent=undefined + - io.simcore.runtime.swarm-stack-name=production-simcore + - io.simcore.runtime.user-id=47568 + mem_limit: 17179869184 + mem_reservation: 17179869184 + networks: + egress-0: null + runtime: nvidia + volumes: + - /tmp/.X11-unix:/tmp/.X11-unix + s4l-core-stream: + cpus: 0.5 + environment: + - DY_SIDECAR_PATH_INPUTS=/home/smu/work/inputs + - DY_SIDECAR_PATH_OUTPUTS=/home/smu/work/outputs + - DY_SIDECAR_STATE_PATHS=[""/home/smu/work/workspace""] + - SIMCORE_NANO_CPUS_LIMIT=500000000 + - SIMCORE_MEMORY_BYTES_LIMIT=1073741824 + - OSPARC_STUDY_ID=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - OSPARC_NODE_ID=a019b83f-7cce-46bf-90cf-d02f7f0f089a + image: simcore/services/dynamic/s4l-core-stream:2.0.106 + init: true + labels: + - io.simcore.runtime.cpu-limit=0.5 + - io.simcore.runtime.memory-limit=1073741824 + - io.simcore.runtime.node-id=a019b83f-7cce-46bf-90cf-d02f7f0f089a + - io.simcore.runtime.product-name=s4llite + - io.simcore.runtime.project-id=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - io.simcore.runtime.simcore-user-agent=undefined + - io.simcore.runtime.swarm-stack-name=production-simcore + - io.simcore.runtime.user-id=47568 + mem_limit: 1073741824 + mem_reservation: 1073741824 + networks: + with-internet: null + sym-server: + cpus: 0.5 + environment: + - DY_SIDECAR_PATH_INPUTS=/home/smu/work/inputs + - DY_SIDECAR_PATH_OUTPUTS=/home/smu/work/outputs + - DY_SIDECAR_STATE_PATHS=[""/home/smu/work/workspace""] + - SIMCORE_NANO_CPUS_LIMIT=500000000 + - SIMCORE_MEMORY_BYTES_LIMIT=2147483648 + - OSPARC_STUDY_ID=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - OSPARC_NODE_ID=a019b83f-7cce-46bf-90cf-d02f7f0f089a + image: simcore/services/dynamic/sym-server-dy:2.0.106 + init: true + labels: + - io.simcore.runtime.cpu-limit=0.5 + - io.simcore.runtime.memory-limit=2147483648 + - io.simcore.runtime.node-id=a019b83f-7cce-46bf-90cf-d02f7f0f089a + - io.simcore.runtime.product-name=s4llite + - io.simcore.runtime.project-id=ef60eaa6-3f52-11ef-9745-0242ac172e8c + - io.simcore.runtime.simcore-user-agent=undefined + - io.simcore.runtime.swarm-stack-name=production-simcore + - io.simcore.runtime.user-id=47568 + mem_limit: 2147483648 + mem_reservation: 2147483648 +version: \'2.3\' diff --git a/services/dynamic-sidecar/tests/unit/test_core_validation.py b/services/dynamic-sidecar/tests/unit/test_core_validation.py index 3775a0ff674..886f729dd30 100644 --- a/services/dynamic-sidecar/tests/unit/test_core_validation.py +++ b/services/dynamic-sidecar/tests/unit/test_core_validation.py @@ -2,13 +2,20 @@ # pylint: disable=unused-argument from inspect import signature +from pathlib import Path import pytest +from fastapi import FastAPI +from models_library.projects_nodes_io import NodeID +from models_library.services_types import RunID +from pytest_mock import MockerFixture from servicelib.docker_constants import DEFAULT_USER_SERVICES_NETWORK_NAME from simcore_service_dynamic_sidecar.core.validation import ( _connect_user_services, parse_compose_spec, + validate_compose_spec, ) +from simcore_service_dynamic_sidecar.modules.mounted_fs import MountedVolumes @pytest.fixture @@ -129,3 +136,44 @@ def test_inject_backend_networking( DEFAULT_USER_SERVICES_NETWORK_NAME in parsed_compose_spec["services"]["iseg-web"]["networks"] ) + + +@pytest.fixture +def mock_get_volume_by_label(mocker: MockerFixture) -> None: + mocker.patch( + "simcore_service_dynamic_sidecar.modules.mounted_fs.get_volume_by_label", + autospec=True, + return_value={"Mountpoint": "/fake/mount"}, + ) + + +@pytest.fixture +def no_internet_spec(project_tests_dir: Path) -> str: + no_intenret_file = project_tests_dir / "mocks" / "internet_blocked_spec.yaml" + return no_intenret_file.read_text() + + +@pytest.fixture +def fake_mounted_volumes() -> MountedVolumes: + return MountedVolumes( + run_id=RunID.create(), + node_id=NodeID("a019b83f-7cce-46bf-90cf-d02f7f0f089a"), + inputs_path=Path("/"), + outputs_path=Path("/"), + user_preferences_path=None, + state_paths=[], + state_exclude=set(), + compose_namespace="", + dy_volumes=Path("/"), + ) + + +async def test_regression_validate_compose_spec( + mock_get_volume_by_label: None, + app: FastAPI, + no_internet_spec: str, + fake_mounted_volumes: MountedVolumes, +): + await validate_compose_spec( + app.state.settings, no_internet_spec, fake_mounted_volumes + )