✨Comp backend: allow to change to s3 links on on demand clusters (⚠️ devops) (#4740)

sanderegg · web-flow · commit d68a9bec616f · 2023-09-14T09:29:39.000+02:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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}

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ test() {`
`21`	`21`	`# shellcheck source=/dev/null`
`22`	`22`	`source .venv/bin/activate`
`23`	`23`	`pushd services/director-v2`
`24`		`- make test-ci-integration test-subfolder="$1"`
	`24`	`+ make test-ci-integration test-path="$1"`
`25`	`25`	`popd`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ test() {`
`23`	`23`	`pushd services/director-v2`
`24`	`24`	`make test-ci-unit pytest-parameters="--numprocesses=auto --ignore-glob=/with_dbs/"`
`25`	`25`	`# these tests cannot be run in parallel`
`26`		`- make test-ci-unit test-subfolder=with_dbs`
	`26`	`+ make test-ci-unit test-path=with_dbs`
`27`	`27`	`popd`
`28`	`28`	`}`
`29`	`29`
Original file line number	Diff line number	Diff line change
`@@ -126,6 +126,7 @@ def set_default_thumbnail_if_empty(cls, v, values):`
`126`	`126`	`default_thumbnails = {`
`127`	`127`	`ClusterTypeInModel.AWS.value: "https://upload.wikimedia.org/wikipedia/commons/thumb/9/93/Amazon_Web_Services_Logo.svg/250px-Amazon_Web_Services_Logo.svg.png",`
`128`	`128`	`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",`
	`129`	`+ 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",`
`129`	`130`	`}`
`130`	`131`	`return default_thumbnails[cluster_type]`
`131`	`132`	`return v`