fix: container launch issues (#4163)

There were issues running Fluent containers through PyFluent introduced
at some point since version 0.30.5 (after which the
`simba-plugin-fluent` tests started failing). The
mount_source/mount_target/working_dir logic was not working properly,
see first commit below for test that was showing the failure:
270495d

Notes:
- Main change: revised mount_source/mount_target logic, see code
comments in `fluent_container.py` for more details
- Took the opportunity to do some refactoring:
- New `dict_to_str` utility function to provide "pretty printed" string
for dicts, also aware of hide log secrets env var for CI runs
  - Revised docstring for mount_source/mount_target
- New test, and test fixes related to this. In summary: we should not
assume that the full paths that Fluent sees inside the container match
exactly the full paths that PyFluent sees on the host system

---------

Co-authored-by: pyansys-ci-bot <[email protected]>

Author: raph-luc

doc/changelog.d/4163.fixed.md

@@ -0,0 +1 @@
+container launch issues

src/ansys/fluent/core/launcher/container_launcher.py

@@ -44,6 +44,7 @@
 from ansys.fluent.core.fluent_connection import FluentConnection
 from ansys.fluent.core.launcher.fluent_container import (
     configure_container_dict,
+    dict_to_str,
     start_fluent_container,
 )
 from ansys.fluent.core.launcher.launch_options import (
@@ -199,23 +200,20 @@ def __init__(
             self._args.append(" -meshing")
 
     def __call__(self):
+
         if self.argvals["dry_run"]:
             config_dict, *_ = configure_container_dict(
                 self._args, **self.argvals["container_dict"]
             )
-            from pprint import pprint
-
+            dict_str = dict_to_str(config_dict)
             print("\nDocker container run configuration:\n")
             print("config_dict = ")
-            if os.getenv("PYFLUENT_HIDE_LOG_SECRETS") != "1":
-                pprint(config_dict)
-            else:
-                config_dict_h = config_dict.copy()
-                config_dict_h.pop("environment")
-                pprint(config_dict_h)
-                del config_dict_h
+            print(dict_str)
             return config_dict
 
+        logger.debug(f"Fluent container launcher args: {self._args}")
+        logger.debug(f"Fluent container launcher argvals:\n{dict_to_str(self.argvals)}")
+
         if is_compose():
             port, config_dict, container = start_fluent_container(
                 self._args, self.argvals["container_dict"]

src/ansys/fluent/core/launcher/fluent_container.py

@@ -74,6 +74,7 @@
 import logging
 import os
 from pathlib import Path, PurePosixPath
+from pprint import pformat
 import tempfile
 from typing import Any, List
 
@@ -118,6 +119,20 @@ def __init__(self):
         )
 
 
+def dict_to_str(dict: dict) -> str:
+    """Converts the dict to string while hiding the 'environment' argument from the dictionary,
+    if the environment variable 'PYFLUENT_HIDE_LOG_SECRETS' is '1'.
+    This is useful for logging purposes, to avoid printing sensitive information such as license server details.
+    """
+
+    if "environment" in dict and os.getenv("PYFLUENT_HIDE_LOG_SECRETS") == "1":
+        modified_dict = dict.copy()
+        modified_dict.pop("environment")
+        return pformat(modified_dict)
+    else:
+        return pformat(dict)
+
+
 @all_deprecators(
     deprecate_arg_mappings=[
         {
@@ -158,9 +173,12 @@ def configure_container_dict(
     args : List[str]
         List of Fluent launch arguments.
     mount_source : str | Path, optional
-        Existing path in the host operating system that will be mounted to ``mount_target``.
+        Path on the host system to mount into the container. This directory will serve as the working directory
+        for the Fluent process inside the container. If not specified, PyFluent's current working directory will
+        be used.
     mount_target : str | Path, optional
-        Path inside the container where ``mount_source`` will be mounted to.
+        Path inside the container where ``mount_source`` will be mounted. This will be the working directory path
+        visible to the Fluent process running inside the container.
     timeout : int, optional
         Time limit  for the Fluent container to start, in seconds. By default, 30 seconds.
     port : int, optional
@@ -213,71 +231,85 @@ def configure_container_dict(
     See also :func:`start_fluent_container`.
     """
 
-    if (
-        container_dict
-        and "environment" in container_dict
-        and os.getenv("PYFLUENT_HIDE_LOG_SECRETS") == "1"
-    ):
-        container_dict_h = container_dict.copy()
-        container_dict_h.pop("environment")
-        logger.debug(f"container_dict before processing: {container_dict_h}")
-        del container_dict_h
-    else:
-        logger.debug(f"container_dict before processing: {container_dict}")
+    logger.debug(f"container_dict before processing:\n{dict_to_str(container_dict)}")
+
+    # Starting with 'mount_source' because it is not tied to the 'working_dir'.
+    # The intended 'mount_source' logic is as follows, if it is not directly specified:
+    # 1. If 'file_transfer_service' is provided, use its 'mount_source'.
+    # 2. Try to use the environment variable 'PYFLUENT_CONTAINER_MOUNT_SOURCE', if it is set.
+    # 3. Use the value from 'pyfluent.CONTAINER_MOUNT_SOURCE', if it is set.
+    # 4. If 'volumes' is specified in 'container_dict', try to infer the value from it.
+    # 5. Finally, use the current working directory, which is always available.
 
     if not mount_source:
         if file_transfer_service:
             mount_source = file_transfer_service.mount_source
         else:
             mount_source = os.getenv(
                 "PYFLUENT_CONTAINER_MOUNT_SOURCE",
-                pyfluent.CONTAINER_MOUNT_SOURCE or os.getcwd(),
+                pyfluent.CONTAINER_MOUNT_SOURCE,
             )
 
-    elif "volumes" in container_dict:
-        logger.warning(
-            "'volumes' keyword specified in 'container_dict', but "
-            "it is going to be overwritten by specified 'mount_source'."
-        )
-        container_dict.pop("volumes")
+    if "volumes" in container_dict:
+        if len(container_dict["volumes"]) != 1:
+            logger.warning(
+                "Multiple volumes being mounted in the Docker container, "
+                "Assuming the first mount is the working directory for Fluent."
+            )
+        volumes_string = container_dict["volumes"][0]
+        if mount_source:
+            logger.warning(
+                "'volumes' keyword specified in 'container_dict', but "
+                "it is going to be overwritten by specified 'mount_source'."
+            )
+        else:
+            mount_source = volumes_string.split(":")[0]
+            logger.debug(f"mount_source: {mount_source}")
+        inferred_mount_target = volumes_string.split(":")[1]
+        logger.debug(f"inferred_mount_target: {inferred_mount_target}")
+
+    if not mount_source:
+        logger.debug("No container 'mount_source' specified, using default value.")
+        mount_source = os.getcwd()
 
-    if not os.path.exists(mount_source):
-        os.makedirs(mount_source)
+    # The intended 'mount_target' logic is as follows, if it is not directly specified:
+    # 1. If 'working_dir' is specified in 'container_dict', use it as 'mount_target'.
+    # 2. Use the environment variable 'PYFLUENT_CONTAINER_MOUNT_TARGET', if it is set.
+    # 3. Try to infer the value from the 'volumes' keyword in 'container_dict', if available.
+    # 4. Finally, use the value from 'pyfluent.CONTAINER_MOUNT_TARGET', which is always set.
 
     if not mount_target:
-        mount_target = os.getenv(
-            "PYFLUENT_CONTAINER_MOUNT_TARGET", pyfluent.CONTAINER_MOUNT_TARGET
-        )
-    elif "volumes" in container_dict:
-        logger.warning(
-            "'volumes' keyword specified in 'container_dict', but "
-            "it is going to be overwritten by specified 'mount_target'."
-        )
-        container_dict.pop("volumes")
+        if "working_dir" in container_dict:
+            mount_target = container_dict["working_dir"]
+        else:
+            mount_target = os.getenv("PYFLUENT_CONTAINER_MOUNT_TARGET")
+
+    if "working_dir" in container_dict and mount_target:
+        # working_dir will be set later to the final value of mount_target
+        container_dict.pop("working_dir")
+
+    if not mount_target and "volumes" in container_dict:
+        mount_target = inferred_mount_target
+
+    if not mount_target:
+        logger.debug("No container 'mount_target' specified, using default value.")
+        mount_target = pyfluent.CONTAINER_MOUNT_TARGET
 
     if "volumes" not in container_dict:
         container_dict.update(volumes=[f"{mount_source}:{mount_target}"])
     else:
-        logger.debug(f"container_dict['volumes']: {container_dict['volumes']}")
-        if len(container_dict["volumes"]) != 1:
-            logger.warning(
-                "Multiple volumes being mounted in the Docker container, "
-                "using the first mount as the working directory for Fluent."
-            )
-        volumes_string = container_dict["volumes"][0]
-        mount_target = ""
-        for c in reversed(volumes_string):
-            if c == ":":
-                break
-            else:
-                mount_target += c
-        mount_target = mount_target[::-1]
-        mount_source = volumes_string.replace(":" + mount_target, "")
-        logger.debug(f"mount_source: {mount_source}")
-        logger.debug(f"mount_target: {mount_target}")
+        container_dict["volumes"][0] = f"{mount_source}:{mount_target}"
+
     logger.warning(
-        f"Starting Fluent container mounted to {mount_source}, with this path available as {mount_target} for the Fluent session running inside the container."
+        f"Configuring Fluent container to mount to {mount_source}, "
+        f"with this path available as {mount_target} for the Fluent session running inside the container."
     )
+
+    if "working_dir" not in container_dict:
+        container_dict.update(
+            working_dir=mount_target,
+        )
+
     port_mapping = {port: port} if port else {}
     if not port_mapping and "ports" in container_dict:
         # take the specified 'port', OR the first port value from the specified 'ports', for Fluent to use
@@ -315,11 +347,7 @@ def configure_container_dict(
             labels={"test_name": test_name},
         )
 
-    if "working_dir" not in container_dict:
-        container_dict.update(
-            working_dir=mount_target,
-        )
-
+    # Find the server info file name from the command line arguments
     if "command" in container_dict:
         for v in container_dict["command"]:
             if v.startswith("-sifile="):
@@ -343,6 +371,20 @@ def configure_container_dict(
         os.close(fd)
         container_server_info_file = PurePosixPath(mount_target) / Path(sifile).name
 
+    logger.debug(
+        f"Using server info file '{container_server_info_file}' for Fluent container."
+    )
+
+    # If the 'command' had already been specified in the 'container_dict',
+    # maintain other 'command' arguments but update the '-sifile' argument,
+    # as the 'mount_target' or 'working_dir' may have changed.
+    if "command" in container_dict:
+        for i, item in enumerate(container_dict["command"]):
+            if item.startswith("-sifile="):
+                container_dict["command"][i] = f"-sifile={container_server_info_file}"
+    else:
+        container_dict["command"] = args + [f"-sifile={container_server_info_file}"]
+
     if not fluent_image:
         if not image_tag:
             image_tag = os.getenv(
@@ -384,16 +426,13 @@ def configure_container_dict(
             container_dict["environment"] = {}
         container_dict["environment"]["FLUENT_LAUNCHED_FROM_PYFLUENT"] = "1"
 
-    fluent_commands = [f"-sifile={container_server_info_file}"] + args
-
-    container_dict_default = {}
-    container_dict_default.update(
-        command=fluent_commands,
+    container_dict_base = {}
+    container_dict_base.update(
         detach=True,
         auto_remove=True,
     )
 
-    for k, v in container_dict_default.items():
+    for k, v in container_dict_base.items():
         if k not in container_dict:
             container_dict[k] = v
 
@@ -404,6 +443,13 @@ def configure_container_dict(
         container_dict["mount_source"] = mount_source
         container_dict["mount_target"] = mount_target
 
+    logger.debug(
+        f"Fluent container timeout: {timeout}, container_grpc_port: {container_grpc_port}, "
+        f"host_server_info_file: '{host_server_info_file}', "
+        f"remove_server_info_file: {remove_server_info_file}"
+    )
+    logger.debug(f"container_dict after processing:\n{dict_to_str(container_dict)}")
+
     return (
         container_dict,
         timeout,
@@ -458,21 +504,6 @@ def start_fluent_container(
         remove_server_info_file,
     ) = container_vars
 
-    if os.getenv("PYFLUENT_HIDE_LOG_SECRETS") != "1":
-        logger.debug(f"container_vars: {container_vars}")
-    else:
-        config_dict_h = config_dict.copy()
-        config_dict_h.pop("environment")
-        container_vars_tmp = (
-            config_dict_h,
-            timeout,
-            port,
-            host_server_info_file,
-            remove_server_info_file,
-        )
-        logger.debug(f"container_vars: {container_vars_tmp}")
-        del container_vars_tmp
-
     try:
         if is_compose():
             config_dict["fluent_port"] = port

tests/conftest.py

@@ -177,7 +177,6 @@ def run_before_each_test(
     monkeypatch.setenv("PYFLUENT_TEST_NAME", request.node.name)
     monkeypatch.setenv("PYFLUENT_CODEGEN_SKIP_BUILTIN_SETTINGS", "1")
     pyfluent.CONTAINER_MOUNT_SOURCE = pyfluent.EXAMPLES_PATH
-    pyfluent.CONTAINER_MOUNT_TARGET = pyfluent.EXAMPLES_PATH
     original_cwd = os.getcwd()
     monkeypatch.chdir(tmp_path)
     yield

tests/test_builtin_settings.py

@@ -547,8 +547,8 @@ def test_builtin_settings(mixing_elbow_case_data_session):
     else:
         with pytest.raises(RuntimeError):
             CustomVectors(settings_source=solver)
-    tmp_save_path = tempfile.mkdtemp(dir=pyfluent.EXAMPLES_PATH)
-    project_file = Path(tmp_save_path) / "mixing_elbow_param.flprj"
+    tmp_save_path = Path(tempfile.mkdtemp(dir=pyfluent.EXAMPLES_PATH))
+    project_file = Path(tmp_save_path.parts[-1]) / "mixing_elbow_param.flprj"
     solver.settings.parametric_studies.initialize(project_filename=str(project_file))
     assert ParametricStudies(settings_source=solver) == solver.parametric_studies
     assert (