🐛⚠️ New R_CLONE_ENABLED env var and fix contraints in compose-spec for side-car (ITISFoundation#3084)

Co-authored-by: Andrei Neagu <[email protected]>

Author: pcrespov

packages/settings-library/src/settings_library/r_clone.py

@@ -13,5 +13,17 @@ class S3Provider(str, Enum):
 
 
 class RCloneSettings(BaseCustomSettings):
+    # TODO: PC this flag is actually ONLY used by the dynamic sidecar.
+    # It determines how the dynamic sidecar sets up node-ports storage
+    # mechanism. I propose it is added as an extra independent variable in
+    # simcore_service_dynamic_idecar.core.settings.Settings instead of here.
+    R_CLONE_ENABLED: bool = Field(
+        True,
+        description=(
+            "simple way to enable/disable the usage of rclone "
+            "in parts of the system where it is optional "
+            "eg: dynamic-sidecar"
+        ),
+    )
     R_CLONE_S3: S3Settings = Field(auto_default_from_env=True)
     R_CLONE_PROVIDER: S3Provider

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

@@ -60,7 +60,7 @@
 
 SUPPORTED_TRAEFIK_LOG_LEVELS: Set[str] = {"info", "debug", "warn", "error"}
 
-PlacementConstraint = constr(
+PlacementConstraintStr = constr(
     strip_whitespace=True, regex=r"^[a-zA-Z0-9. ]*(!=|==){1}[a-zA-Z0-9. ]*$"
 )
 
@@ -443,7 +443,7 @@ class AppSettings(BaseCustomSettings, MixinLoggingSettings):
 
     # This is just a service placement constraint, see
     # https://docs.docker.com/engine/swarm/services/#control-service-placement.
-    DIRECTOR_V2_SERVICES_CUSTOM_CONSTRAINTS: List[PlacementConstraint] = Field(
+    DIRECTOR_V2_SERVICES_CUSTOM_CONSTRAINTS: List[PlacementConstraintStr] = Field(
         default_factory=list,
         example='["node.labels.region==east", "one!=yes"]',
     )

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

@@ -73,6 +73,7 @@ def _parse_env_settings(settings: List[str]) -> Dict:
 
 
 # pylint: disable=too-many-branches
+# TODO: PC->ANE: i tend to agree with pylint, perhaps we can refactor this together
 def update_service_params_from_settings(
     labels_service_settings: SimcoreServiceSettingsLabel,
     create_service_params: Dict[str, Any],
@@ -122,13 +123,17 @@ def update_service_params_from_settings(
 
         # placement constraints
         elif param.name == "constraints":  # python-API compatible
+
             create_service_params["task_template"]["Placement"][
                 "Constraints"
             ] += param.value
+
         elif param.setting_type == "Constraints":  # REST-API compatible
+
             create_service_params["task_template"]["Placement"][
                 "Constraints"
             ] += param.value
+
         elif param.name == "env":
             log.debug("Found env parameter %s", param.value)
             env_settings = _parse_env_settings(param.value)
@@ -153,6 +158,13 @@ def update_service_params_from_settings(
         create_service_params["task_template"]["Resources"]["Limits"]["MemoryBytes"]
     )
 
+    # Cleanup repeated constraints.
+    # Observed in deploy how constraint 'node.platform.os == linux' was appended many many times
+    constraints = create_service_params["task_template"]["Placement"]["Constraints"]
+    if constraints:
+        assert isinstance(constraints, list)  # nosec
+        constraints = list(set(constraints))
+
 
 def _assemble_key(service_key: str, service_tag: str) -> str:
     return f"{service_key}:{service_tag}"

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

@@ -73,6 +73,7 @@ def _get_environment_variables(
         "S3_BUCKET_NAME": r_clone_settings.R_CLONE_S3.S3_BUCKET_NAME,
         "S3_SECURE": f"{r_clone_settings.R_CLONE_S3.S3_SECURE}",
         "R_CLONE_PROVIDER": r_clone_settings.R_CLONE_PROVIDER,
+        "R_CLONE_ENABLED": f"{r_clone_settings.R_CLONE_ENABLED}",
     }
 
 

services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_service_specs.py

@@ -202,6 +202,7 @@ def expected_dynamic_sidecar_spec() -> dict[str, Any]:
                     "REGISTRY_URL": "registry.osparc-master.speag.com",
                     "REGISTRY_USER": "test",
                     "R_CLONE_PROVIDER": "MINIO",
+                    "R_CLONE_ENABLED": "True",
                     "S3_ACCESS_KEY": "12345678",
                     "S3_BUCKET_NAME": "simcore",
                     "S3_ENDPOINT": "http://172.17.0.1:9001",

services/director-v2/tests/unit/test_modules_dynamic_sidecar_docker_service_specs_sidecar.py

@@ -60,6 +60,7 @@
     "SIMCORE_HOST_NAME",
     "STORAGE_ENDPOINT",
     "R_CLONE_PROVIDER",
+    "R_CLONE_ENABLED",
     "S3_ENDPOINT",
     "S3_ACCESS_KEY",
     "S3_SECRET_KEY",

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py

@@ -105,6 +105,18 @@ def is_development_mode(self) -> bool:
     def loglevel(self) -> int:
         return int(getattr(logging, self.LOG_LEVEL))
 
+    @property
+    def rclone_settings_for_nodeports(self) -> Optional[RCloneSettings]:
+        """
+        If R_CLONE_ENABLED is False it returns None which indicates
+        nodeports to disable rclone and fallback to the previous storage mechanim.
+        """
+        return (
+            self.DY_SIDECAR_R_CLONE_SETTINGS
+            if self.DY_SIDECAR_R_CLONE_SETTINGS.R_CLONE_ENABLED
+            else None
+        )
+
 
 @lru_cache
 def get_settings() -> DynamicSidecarSettings:

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/data_manager.py

@@ -76,6 +76,6 @@ async def upload_path_if_exists(path: Path, state_exclude: List[str]) -> None:
             project_id=str(settings.DY_SIDECAR_PROJECT_ID),
             node_uuid=str(settings.DY_SIDECAR_NODE_ID),
             file_or_folder=path,
-            r_clone_settings=settings.DY_SIDECAR_R_CLONE_SETTINGS,
+            r_clone_settings=settings.rclone_settings_for_nodeports,
         )
     logger.info("Finished upload of %s", path)

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py

@@ -61,7 +61,7 @@ async def upload_outputs(outputs_path: Path, port_keys: list[str]) -> None:
         user_id=settings.DY_SIDECAR_USER_ID,
         project_id=str(settings.DY_SIDECAR_PROJECT_ID),
         node_uuid=str(settings.DY_SIDECAR_NODE_ID),
-        r_clone_settings=settings.DY_SIDECAR_R_CLONE_SETTINGS,
+        r_clone_settings=settings.rclone_settings_for_nodeports,
     )
 
     # let's gather the tasks
@@ -254,7 +254,7 @@ async def download_target_ports(
         user_id=settings.DY_SIDECAR_USER_ID,
         project_id=str(settings.DY_SIDECAR_PROJECT_ID),
         node_uuid=str(settings.DY_SIDECAR_NODE_ID),
-        r_clone_settings=settings.DY_SIDECAR_R_CLONE_SETTINGS,
+        r_clone_settings=settings.rclone_settings_for_nodeports,
     )
 
     # let's gather all the data