♻️ maintenance on director-v2 and other minors (ITISFoundation#2642)

Author: pcrespov

Makefile

@@ -467,7 +467,19 @@ local-registry: .env ## creates a local docker registry and configure simcore to
 	@echo CATALOG_BACKGROUND_TASK_REST_TIME=1 >> .env
 	# local registry set in $(local_registry):5000
 	# images currently in registry:
-	curl --silent $(local_registry):5000/v2/_catalog | jq
+	@sleep 3
+	curl --silent $(LOCAL_REGISTRY_HOSTNAME):5000/v2/_catalog | jq '.repositories'
+
+info-registry: ## info on local registry (if any)
+	# ping API
+	curl --silent $(LOCAL_REGISTRY_HOSTNAME):5000/v2
+	# list all
+	curl --silent $(LOCAL_REGISTRY_HOSTNAME):5000/v2/_catalog | jq
+	# target detail info (if set)
+	$(if $(target),\
+	@echo Tags for $(target); \
+	curl --silent $(LOCAL_REGISTRY_HOSTNAME):5000/v2/$(target)/tags/list | jq ,\
+	@echo No target set)
 
 
 ## INFO -------------------------------

packages/models-library/src/models_library/service_settings_labels.py

@@ -104,7 +104,7 @@ class Config(_BaseConfig):
         }
 
 
-ComposeSpecLabel = Optional[Dict[str, Any]]
+ComposeSpecLabel = Dict[str, Any]
 
 
 class DynamicSidecarServiceLabels(BaseModel):

packages/models-library/src/models_library/settings/docker_registry.py

@@ -97,11 +97,14 @@ async def create_dynamic_service(
     ),
     scheduler: DynamicSidecarsScheduler = Depends(get_scheduler),
 ) -> Union[DynamicServiceOut, RedirectResponse]:
+
     simcore_service_labels: SimcoreServiceLabels = (
         await director_v0_client.get_service_labels(
             service=ServiceKeyVersion(key=service.key, version=service.version)
         )
     )
+
+    # LEGACY (backwards compatibility)
     if not simcore_service_labels.needs_dynamic_sidecar:
         # forward to director-v0
         redirect_url_with_query = director_v0_client.client.base_url.copy_with(
@@ -118,6 +121,7 @@ async def create_dynamic_service(
         logger.debug("Redirecting %s", redirect_url_with_query)
         return RedirectResponse(str(redirect_url_with_query))
 
+    #
     if not await is_dynamic_service_running(
         service.node_uuid, dynamic_services_settings.DYNAMIC_SIDECAR
     ):

services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py

@@ -159,6 +159,10 @@ class DynamicSidecarSettings(BaseCustomSettings):
 
     DYNAMIC_SIDECAR_PROXY_SETTINGS: DynamicSidecarProxySettings
 
+    DYNAMIC_SIDECAR_DOCKER_COMPOSE_VERSION: str = Field(
+        "3.8", description="docker-compose version used in the compose-specs"
+    )
+
     @validator("DYNAMIC_SIDECAR_IMAGE", pre=True)
     @classmethod
     def strip_leading_slashes(cls, v) -> str:

services/director-v2/src/simcore_service_director_v2/core/settings.py

@@ -15,6 +15,16 @@
     EntrypointContainerNotFoundError,
 )
 
+# PC -> SAN improvements to discuss
+#
+# TODO: Use logger, not logging!
+#      - compose error msgs instead of log functions
+# TODO: Single instance of httpx client for all requests?: https://www.python-httpx.org/advanced/#why-use-a-client
+#      - see services/api-server/src/simcore_service_api_server/utils/client_base.py  (-> move to servicelib/fastapi ?)
+# TODO: context to unify session's error handling and logging
+# TODO: client function names equal/very similar to server handlers
+#
+
 logger = logging.getLogger(__name__)
 
 
@@ -132,7 +142,7 @@ async def start_service_creation(
                 response = await client.post(url, data=compose_spec)
             if response.status_code != status.HTTP_202_ACCEPTED:
                 message = (
-                    f"ERROR during service creation request: "
+                    "ERROR during service creation request: "
                     f"status={response.status_code}, body={response.text}"
                 )
                 logging.warning(message)
@@ -176,7 +186,7 @@ async def service_save_state(self, dynamic_sidecar_endpoint: str) -> None:
                 logging.warning(message)
                 raise DynamicSchedulerException(message)
         except httpx.HTTPError as e:
-            log_httpx_http_error(url, "PUT", traceback.format_exc())
+            log_httpx_http_error(url, "POST", traceback.format_exc())
             raise e
 
     async def service_restore_state(self, dynamic_sidecar_endpoint: str) -> None:
@@ -191,9 +201,9 @@ async def service_restore_state(self, dynamic_sidecar_endpoint: str) -> None:
                 )
                 logging.warning(message)
                 raise DynamicSchedulerException(message)
-        except httpx.HTTPError as e:
-            log_httpx_http_error(url, "PUT", traceback.format_exc())
-            raise e
+        except httpx.HTTPError:
+            log_httpx_http_error(url, "POST", traceback.format_exc())
+            raise
 
     async def service_pull_input_ports(
         self, dynamic_sidecar_endpoint: str, port_keys: Optional[List[str]] = None
@@ -212,7 +222,7 @@ async def service_pull_input_ports(
                 raise DynamicSchedulerException(message)
             return int(response.text)
         except httpx.HTTPError as e:
-            log_httpx_http_error(url, "PUT", traceback.format_exc())
+            log_httpx_http_error(url, "POST", traceback.format_exc())
             raise e
 
     async def service_push_output_ports(
@@ -231,7 +241,7 @@ async def service_push_output_ports(
                 logging.warning(message)
                 raise DynamicSchedulerException(message)
         except httpx.HTTPError as e:
-            log_httpx_http_error(url, "PUT", traceback.format_exc())
+            log_httpx_http_error(url, "POST", traceback.format_exc())
             raise e
 
     async def get_entrypoint_container_name(

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/client_api.py

@@ -1,6 +1,6 @@
 import json
 from copy import deepcopy
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Union
 
 import yaml
 from fastapi.applications import FastAPI
@@ -10,10 +10,6 @@
 from .docker_service_specs import MATCH_SERVICE_VERSION, MATCH_SIMCORE_REGISTRY
 
 CONTAINER_NAME = "container"
-BASE_SERVICE_SPEC: Dict[str, Any] = {
-    "version": "3.8",
-    "services": {CONTAINER_NAME: {}},
-}
 
 
 def _inject_proxy_network_configuration(
@@ -41,32 +37,61 @@ def _inject_proxy_network_configuration(
     target_container_spec["networks"] = container_networks
 
 
+EnvKeyEqValueList = List[str]
+EnvVarsMap = Dict[str, Optional[str]]
+
+
+class _environment_section:
+    """the 'environment' field in a docker-compose can be either a dict (EnvVarsMap)
+    or a list of "key=value" (EnvKeyEqValueList)
+
+    These helpers can resolve parsing and exporting between these formats
+
+    SEE https://docs.docker.com/compose/compose-file/compose-file-v3/#environment
+    """
+
+    @staticmethod
+    def parse(environment: Union[EnvVarsMap, EnvKeyEqValueList]) -> EnvVarsMap:
+        envs = {}
+        if isinstance(environment, list):
+            for key_eq_value in environment:
+                assert isinstance(key_eq_value, str)  # nosec
+                key, value, *_ = key_eq_value.split("=", maxsplit=1) + [
+                    None,
+                ]  # type: ignore
+                envs[key] = value
+        else:
+            assert isinstance(environment, dict)  # nosec
+            envs = deepcopy(environment)
+        return envs
+
+    @staticmethod
+    def export_as_list(environment: EnvVarsMap) -> EnvKeyEqValueList:
+        envs = []
+        for key, value in environment.items():
+            if value is None:
+                envs.append(f"{key}")
+            else:
+                envs.append(f"{key}={value}")
+        return envs
+
+
 def _inject_paths_mappings(
     service_spec: Dict[str, Any], path_mappings: PathMappingsLabel
 ) -> None:
     for service_name in service_spec["services"]:
         service_content = service_spec["services"][service_name]
 
-        environment_vars: List[str] = service_content.get("environment", [])
-        environment_vars.append(f"DY_SIDECAR_PATH_INPUTS={path_mappings.inputs_path}")
-        environment_vars.append(f"DY_SIDECAR_PATH_OUTPUTS={path_mappings.outputs_path}")
-        str_path_mappings = json.dumps([str(x) for x in path_mappings.state_paths])
-        environment_vars.append(f"DY_SIDECAR_STATE_PATHS={str_path_mappings}")
-
-        service_content["environment"] = environment_vars
-
-
-def _assemble_from_service_key_and_tag(
-    resolved_registry_url: str,
-    service_key: str,
-    service_tag: str,
-):
-    service_spec = deepcopy(BASE_SERVICE_SPEC)
-    service_spec["services"][CONTAINER_NAME] = {
-        "image": f"{resolved_registry_url}/{service_key}:{service_tag}"
-    }
+        env_vars: EnvVarsMap = _environment_section.parse(
+            service_content.get("environment", {})
+        )
+        env_vars["DY_SIDECAR_PATH_INPUTS"] = f"{path_mappings.inputs_path}"
+        env_vars["DY_SIDECAR_PATH_OUTPUTS"] = f"{path_mappings.outputs_path}"
+        env_vars[
+            "DY_SIDECAR_STATE_PATHS"
+        ] = f"{json.dumps([f'{p}' for p in path_mappings.state_paths])}"
 
-    return service_spec
+        service_content["environment"] = _environment_section.export_as_list(env_vars)
 
 
 def _replace_env_vars_in_compose_spec(
@@ -81,12 +106,12 @@ def _replace_env_vars_in_compose_spec(
     return stringified_service_spec
 
 
-async def assemble_spec(
+def assemble_spec(
     app: FastAPI,
     service_key: str,
     service_tag: str,
     paths_mapping: PathMappingsLabel,
-    compose_spec: ComposeSpecLabel,
+    compose_spec: Optional[ComposeSpecLabel],
     container_http_entry: Optional[str],
     dynamic_sidecar_network_name: str,
 ) -> str:
@@ -99,18 +124,27 @@ async def assemble_spec(
         app.state.settings.DIRECTOR_V2_DOCKER_REGISTRY
     )
 
-    container_name = container_http_entry
-    service_spec = compose_spec
+    docker_compose_version = (
+        app.state.settings.DYNAMIC_SERVICES.DYNAMIC_SIDECAR.DYNAMIC_SIDECAR_DOCKER_COMPOSE_VERSION
+    )
 
     # when no compose yaml file was provided
-    if service_spec is None:
-        service_spec = _assemble_from_service_key_and_tag(
-            resolved_registry_url=docker_registry_settings.resolved_registry_url,
-            service_key=service_key,
-            service_tag=service_tag,
-        )
+    if compose_spec is None:
+        service_spec: Dict[str, Any] = {
+            # NOTE: latest version does NOT require
+            "version": docker_compose_version,
+            "services": {
+                CONTAINER_NAME: {
+                    "image": f"{docker_registry_settings.resolved_registry_url}/{service_key}:{service_tag}"
+                }
+            },
+        }
         container_name = CONTAINER_NAME
+    else:
+        service_spec = compose_spec
+        container_name = container_http_entry
 
+    assert service_spec is not None  # nosec
     assert container_name is not None  # nosec
 
     _inject_proxy_network_configuration(

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py

@@ -9,7 +9,7 @@
 CPU_1_PERCENT = 10000000
 
 
-async def get_dynamic_proxy_spec(
+def get_dynamic_proxy_spec(
     scheduler_data: SchedulerData,
     dynamic_sidecar_settings: DynamicSidecarSettings,
     dynamic_sidecar_network_id: str,

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/proxy.py

@@ -1,7 +1,7 @@
 import json
 import logging
 from collections import deque
-from typing import Any, Deque, Dict, List, cast
+from typing import Any, Deque, Dict, List, Optional, cast
 
 from models_library.service_settings_labels import (
     ComposeSpecLabel,
@@ -173,7 +173,9 @@ async def _extract_osparc_involved_service_labels(
     docker_image_name_by_services: Dict[str, SimcoreServiceLabels] = {
         _assemble_key(service_key=service_key, service_tag=service_tag): service_labels
     }
-    compose_spec: ComposeSpecLabel = cast(ComposeSpecLabel, service_labels.compose_spec)
+    compose_spec: Optional[ComposeSpecLabel] = cast(
+        ComposeSpecLabel, service_labels.compose_spec
+    )
     if compose_spec is None:
         return docker_image_name_by_services
 

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/settings.py

@@ -57,7 +57,7 @@ def _get_environment_variables(
     }
 
 
-async def get_dynamic_sidecar_spec(
+def get_dynamic_sidecar_spec(
     scheduler_data: SchedulerData,
     dynamic_sidecar_settings: DynamicSidecarSettings,
     dynamic_sidecar_network_id: str,