From d68a9bec616f83674c1ff8c1060dfb2ba188bf23 Mon Sep 17 00:00:00 2001 From: Sylvain <35365065+sanderegg@users.noreply.github.com> Date: Thu, 14 Sep 2023 09:29:39 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8Comp=20backend:=20allow=20to=20change?= =?UTF-8?q?=20to=20s3=20links=20on=20on=20demand=20clusters=20(=E2=9A=A0?= =?UTF-8?q?=EF=B8=8F=20devops)=20(#4740)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .env-devel | 3 ++ .../integration-testing/director-v2.bash | 2 +- ci/github/integration-testing/webserver.bash | 2 +- ci/github/unit-testing/director-v2.bash | 2 +- ci/github/unit-testing/webserver.bash | 4 +-- .../api_schemas_directorv2/clusters.py | 1 + .../src/models_library/clusters.py | 3 +- .../models-library/tests/test_clusters.py | 34 +++++++++++++++++-- scripts/common-service.Makefile | 10 +++--- .../core/settings.py | 4 +++ .../modules/clusters_keeper.py | 2 +- .../modules/dask_clients_pool.py | 6 +++- .../02/test_dynamic_services_routes.py | 1 + ...t_dynamic_sidecar_nodeports_integration.py | 1 + services/docker-compose.yml | 3 ++ 15 files changed, 62 insertions(+), 16 deletions(-) diff --git a/.env-devel b/.env-devel index 6e15a490cf4..567674ee153 100644 --- a/.env-devel +++ b/.env-devel @@ -44,6 +44,9 @@ DIRECTOR_REGISTRY_CACHING_TTL=900 DIRECTOR_REGISTRY_CACHING=True COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_URL=tcp://dask-scheduler:8786 +COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_FILE_LINK_TYPE=S3 +COMPUTATIONAL_BACKEND_DEFAULT_FILE_LINK_TYPE=PRESIGNED +COMPUTATIONAL_BACKEND_ON_DEMAND_CLUSTERS_FILE_LINK_TYPE=PRESIGNED DIRECTOR_V2_DEV_FEATURES_ENABLED=0 DYNAMIC_SIDECAR_IMAGE=${DOCKER_REGISTRY:-itisfoundation}/dynamic-sidecar:${DOCKER_IMAGE_TAG:-latest} diff --git a/ci/github/integration-testing/director-v2.bash b/ci/github/integration-testing/director-v2.bash index 80c3020adf2..55f4ce2a835 100755 --- a/ci/github/integration-testing/director-v2.bash +++ b/ci/github/integration-testing/director-v2.bash @@ -21,7 +21,7 @@ test() { # shellcheck source=/dev/null source .venv/bin/activate pushd services/director-v2 - make test-ci-integration test-subfolder="$1" + make test-ci-integration test-path="$1" popd } diff --git a/ci/github/integration-testing/webserver.bash b/ci/github/integration-testing/webserver.bash index 2cfec244808..a39e21d3555 100755 --- a/ci/github/integration-testing/webserver.bash +++ b/ci/github/integration-testing/webserver.bash @@ -21,7 +21,7 @@ test() { # shellcheck source=/dev/null source .venv/bin/activate pushd services/web/server - make test-ci-integration test-subfolder="$1" + make test-ci-integration test-path="$1" popd } diff --git a/ci/github/unit-testing/director-v2.bash b/ci/github/unit-testing/director-v2.bash index f1b73b1ec1d..a28fc7724d4 100755 --- a/ci/github/unit-testing/director-v2.bash +++ b/ci/github/unit-testing/director-v2.bash @@ -23,7 +23,7 @@ test() { pushd services/director-v2 make test-ci-unit pytest-parameters="--numprocesses=auto --ignore-glob=**/with_dbs/**" # these tests cannot be run in parallel - make test-ci-unit test-subfolder=with_dbs + make test-ci-unit test-path=with_dbs popd } diff --git a/ci/github/unit-testing/webserver.bash b/ci/github/unit-testing/webserver.bash index 4cf936a9304..b2f721ff080 100755 --- a/ci/github/unit-testing/webserver.bash +++ b/ci/github/unit-testing/webserver.bash @@ -27,7 +27,7 @@ test_isolated() { # shellcheck source=/dev/null source .venv/bin/activate pushd services/web/server - make test-ci-unit test-subfolder=isolated pytest-parameters="--numprocesses=auto" + make test-ci-unit test-path=isolated pytest-parameters="--numprocesses=auto" popd } @@ -36,7 +36,7 @@ test_with_db() { source .venv/bin/activate pushd services/web/server echo "testing in services/web/server/tests/unit/with_dbs/$1" - make test-ci-unit test-subfolder="with_dbs/$1" + make test-ci-unit test-path="with_dbs/$1" popd } diff --git a/packages/models-library/src/models_library/api_schemas_directorv2/clusters.py b/packages/models-library/src/models_library/api_schemas_directorv2/clusters.py index 2aa49f03ccb..49f87097109 100644 --- a/packages/models-library/src/models_library/api_schemas_directorv2/clusters.py +++ b/packages/models-library/src/models_library/api_schemas_directorv2/clusters.py @@ -126,6 +126,7 @@ def set_default_thumbnail_if_empty(cls, v, values): default_thumbnails = { ClusterTypeInModel.AWS.value: "https://upload.wikimedia.org/wikipedia/commons/thumb/9/93/Amazon_Web_Services_Logo.svg/250px-Amazon_Web_Services_Logo.svg.png", ClusterTypeInModel.ON_PREMISE.value: "https://upload.wikimedia.org/wikipedia/commons/thumb/a/ac/Crystal_Clear_app_network_local.png/120px-Crystal_Clear_app_network_local.png", + ClusterTypeInModel.ON_DEMAND.value: "https://upload.wikimedia.org/wikipedia/commons/thumb/9/93/Amazon_Web_Services_Logo.svg/250px-Amazon_Web_Services_Logo.svg.png", } return default_thumbnails[cluster_type] return v diff --git a/packages/models-library/src/models_library/clusters.py b/packages/models-library/src/models_library/clusters.py index 094216732eb..87b266b06d7 100644 --- a/packages/models-library/src/models_library/clusters.py +++ b/packages/models-library/src/models_library/clusters.py @@ -19,10 +19,11 @@ class ClusterTypeInModel(StrAutoEnum): - # This enum is equivalent to `simcore_postgres_database.models.clusters.ClusterType` + # This enum contains more types than its equivalent to `simcore_postgres_database.models.clusters.ClusterType` # SEE models-library/tests/test__pydantic_models_and_enums.py AWS = auto() ON_PREMISE = auto() + ON_DEMAND = auto() class ClusterAccessRights(BaseModel): diff --git a/packages/models-library/tests/test_clusters.py b/packages/models-library/tests/test_clusters.py index 4aff7b8fd95..842b5929236 100644 --- a/packages/models-library/tests/test_clusters.py +++ b/packages/models-library/tests/test_clusters.py @@ -9,13 +9,17 @@ CLUSTER_USER_RIGHTS, DEFAULT_CLUSTER_ID, Cluster, + ClusterTypeInModel, ) from pydantic import BaseModel, ValidationError +from simcore_postgres_database.models.clusters import ClusterType @pytest.mark.parametrize( "model_cls", - (Cluster,), + [ + Cluster, + ], ) def test_cluster_access_rights_correctly_created_when_owner_access_rights_not_present( model_cls: type[BaseModel], model_cls_examples: dict[str, dict[str, Any]] @@ -35,7 +39,9 @@ def test_cluster_access_rights_correctly_created_when_owner_access_rights_not_pr @pytest.mark.parametrize( "model_cls", - (Cluster,), + [ + Cluster, + ], ) def test_cluster_fails_when_owner_has_no_admin_rights_unless_default_cluster( model_cls: type[BaseModel], @@ -61,7 +67,9 @@ def test_cluster_fails_when_owner_has_no_admin_rights_unless_default_cluster( @pytest.mark.parametrize( "model_cls", - (Cluster,), + [ + Cluster, + ], ) def test_cluster_fails_when_owner_has_no_user_rights_if_default_cluster( model_cls: type[BaseModel], @@ -82,3 +90,23 @@ def test_cluster_fails_when_owner_has_no_user_rights_if_default_cluster( modified_example["access_rights"][owner_gid] = CLUSTER_ADMIN_RIGHTS with pytest.raises(ValidationError): model_cls(**modified_example) + + +def test_cluster_type_in_model_includes_postgres_database_model(): + models_library_cluster_types_names: set[str] = { + t.name for t in set(ClusterTypeInModel) + } + postgres_library_cluster_types_names: set[str] = {t.name for t in set(ClusterType)} + assert postgres_library_cluster_types_names.issubset( + models_library_cluster_types_names + ) + + models_library_cluster_types_values: set[str] = { + t.value for t in set(ClusterTypeInModel) + } # type: ignore + postgres_library_cluster_types_values: set[str] = { + t.value for t in set(ClusterType) + } + assert postgres_library_cluster_types_values.issubset( + models_library_cluster_types_values + ) diff --git a/scripts/common-service.Makefile b/scripts/common-service.Makefile index 7bd3ce5bf89..b57380c1c45 100644 --- a/scripts/common-service.Makefile +++ b/scripts/common-service.Makefile @@ -42,16 +42,16 @@ install-dev install-prod install-ci: _check_venv_active ## install app in develo .PHONY: test-dev-unit test-ci-unit test-dev-integration test-ci-integration test-dev -TEST_SUBFOLDER := $(if $(test-subfolder),/$(test-subfolder),) -test-dev-unit test-ci-unit: _check_venv_active +TEST_PATH := $(if $(test-path),/$(patsubst tests/integration/%,%, $(patsubst tests/unit/%,%, $(patsubst %/,%,$(test-path)))),) +test-dev-unit test-ci-unit: _check_venv_active ## run app unit tests (specifying test-path can restrict to a folder) # Targets tests/unit folder - @make --no-print-directory _run-$(subst -unit,,$@) target=$(CURDIR)/tests/unit$(TEST_SUBFOLDER) + @make --no-print-directory _run-$(subst -unit,,$@) target=$(CURDIR)/tests/unit$(TEST_PATH) -test-dev-integration test-ci-integration: +test-dev-integration test-ci-integration: ## run app integration tests (specifying test-path can restrict to a folder) # Targets tests/integration folder using local/$(image-name):production images @export DOCKER_REGISTRY=local; \ export DOCKER_IMAGE_TAG=production; \ - make --no-print-directory _run-$(subst -integration,,$@) target=$(CURDIR)/tests/integration$(TEST_SUBFOLDER) + make --no-print-directory _run-$(subst -integration,,$@) target=$(CURDIR)/tests/integration$(TEST_PATH) test-dev: test-dev-unit test-dev-integration ## runs unit and integration tests for development (e.g. w/ pdb) diff --git a/services/director-v2/src/simcore_service_director_v2/core/settings.py b/services/director-v2/src/simcore_service_director_v2/core/settings.py index d77bb963af8..4a138a3e9e5 100644 --- a/services/director-v2/src/simcore_service_director_v2/core/settings.py +++ b/services/director-v2/src/simcore_service_director_v2/core/settings.py @@ -471,6 +471,10 @@ class ComputationalBackendSettings(BaseCustomSettings): FileLinkType.PRESIGNED, description=f"Default file link type to use with computational backend '{list(FileLinkType)}'", ) + COMPUTATIONAL_BACKEND_ON_DEMAND_CLUSTERS_FILE_LINK_TYPE: FileLinkType = Field( + FileLinkType.PRESIGNED, + description=f"Default file link type to use with computational backend on-demand clusters '{list(FileLinkType)}'", + ) @cached_property def default_cluster(self) -> Cluster: diff --git a/services/director-v2/src/simcore_service_director_v2/modules/clusters_keeper.py b/services/director-v2/src/simcore_service_director_v2/modules/clusters_keeper.py index 9dc4243bb48..8811dcb54c0 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/clusters_keeper.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/clusters_keeper.py @@ -51,7 +51,7 @@ async def get_or_create_on_demand_cluster( return BaseCluster( name=f"{user_id=}on-demand-cluster", - type=ClusterTypeInModel.AWS, + type=ClusterTypeInModel.ON_DEMAND, owner=user_id, endpoint=returned_cluster.endpoint, authentication=returned_cluster.authentication, diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dask_clients_pool.py b/services/director-v2/src/simcore_service_director_v2/modules/dask_clients_pool.py index a59ff62be6d..3ab9fd05ead 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dask_clients_pool.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dask_clients_pool.py @@ -6,7 +6,7 @@ from typing import TypeAlias from fastapi import FastAPI -from models_library.clusters import BaseCluster +from models_library.clusters import BaseCluster, ClusterTypeInModel from pydantic import AnyUrl from ..core.errors import ( @@ -80,6 +80,10 @@ async def _concurently_safe_acquire_client() -> DaskClient: tasks_file_link_type = ( self.settings.COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_FILE_LINK_TYPE ) + if cluster.type is ClusterTypeInModel.ON_DEMAND: + tasks_file_link_type = ( + self.settings.COMPUTATIONAL_BACKEND_ON_DEMAND_CLUSTERS_FILE_LINK_TYPE + ) self._cluster_to_client_map[ cluster.endpoint ] = dask_client = await DaskClient.create( 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 f8b727f6cdb..cbca1b381e6 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 @@ -279,6 +279,7 @@ async def key_version_expected( return results +@pytest.mark.flaky(max_runs=3) async def test_start_status_stop( director_v2_client: TestClient, node_uuid: str, diff --git a/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py b/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py index b659198b6c6..0e671311926 100644 --- a/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py +++ b/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py @@ -846,6 +846,7 @@ async def _assert_retrieve_completed( ), "TIP: Message missing suggests that the data was never uploaded: look in services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py" +@pytest.mark.flaky(max_runs=3) async def test_nodeports_integration( minimal_configuration: None, cleanup_services_and_networks: None, diff --git a/services/docker-compose.yml b/services/docker-compose.yml index 3db9771e3d3..883fddf3a3c 100644 --- a/services/docker-compose.yml +++ b/services/docker-compose.yml @@ -151,6 +151,9 @@ services: - CATALOG_HOST=${CATALOG_HOST:-catalog} - CATALOG_PORT=${CATALOG_PORT:-8000} - COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_URL=${COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_URL:-tcp://dask-scheduler:8786} + - COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_FILE_LINK_TYPE=${COMPUTATIONAL_BACKEND_DEFAULT_CLUSTER_FILE_LINK_TYPE} + - COMPUTATIONAL_BACKEND_DEFAULT_FILE_LINK_TYPE=${COMPUTATIONAL_BACKEND_DEFAULT_FILE_LINK_TYPE} + - COMPUTATIONAL_BACKEND_ON_DEMAND_CLUSTERS_FILE_LINK_TYPE=${COMPUTATIONAL_BACKEND_ON_DEMAND_CLUSTERS_FILE_LINK_TYPE} - DIRECTOR_HOST=${DIRECTOR_HOST:-director} - DIRECTOR_PORT=${DIRECTOR_PORT:-8080} - DIRECTOR_SELF_SIGNED_SSL_FILENAME=${DIRECTOR_SELF_SIGNED_SSL_FILENAME}