diff --git a/runners/mlcube_singularity/mlcube_singularity/singularity_client.py b/runners/mlcube_singularity/mlcube_singularity/singularity_client.py index 17d2036..e66dffd 100644 --- a/runners/mlcube_singularity/mlcube_singularity/singularity_client.py +++ b/runners/mlcube_singularity/mlcube_singularity/singularity_client.py @@ -8,6 +8,7 @@ from pathlib import Path from shlex import shlex +import omegaconf import requests import semver @@ -236,21 +237,50 @@ class Client: """ @classmethod - def from_env(cls) -> "Client": + def from_env( + cls, preferred_exec: t.Optional[str] = None, platform_name: t.Optional[str] = None + ) -> "Client": + """Create Singularity client trying different commands to run it, return first that succeeds. + + Args: + preferred_exec: First executable to try (e.g., the one from system settings file or MLCube config). + platform_name: Try executable in system settings file for this platform. Has lower priority than + `preferred_exec` if both specified. + Return: + Instance of singularity client that this system can run. + """ + # TODO sergey: what if there are multiple different ways to run singularity (like different wrapper scripts + # with pre/post actions, and user wants to use this particular runner) - think about adding `platform` - + # the one that user specified on a command line. + # TODO sergey: what if by trying to run it below we trigger these unwanted pre/post condition actions? # Check if system settings file contains information about singularity. executables = ["singularity", "sudo singularity", "apptainer", "sudo apptainer"] - system_settings = SystemSettings() - if "singularity" in system_settings.platforms: - executables = [ - system_settings.platforms["singularity"]["singularity"] - ] + executables - logger.info( - "Client.from_env found singularity platform config in MLCube system settings file " - "(file=%s, platform=%s). Will try it first for detecting singularity CLI client.", - system_settings.path.as_posix(), - system_settings.platforms["singularity"], - ) + def _add(_exec: str) -> None: + if _exec in executables: + executables.remove(_exec) + executables.insert(0, _exec) + + # Check if preferred executable is provided, and is not one of those above. + if preferred_exec is not None: + logger.debug("Client.from_env adding preferred singularity executable '%s'.", preferred_exec) + _add(preferred_exec) + + if platform_name: + system_settings = SystemSettings() + # TODO sergey: we can have other `singularity` and `apptainer` platforms named differently. Need probably to + # look at `value -> runner` that contains actual runner type. + if platform_name in system_settings.platforms: + preconfigured_platform: omegaconf.DictConfig = system_settings.platforms[platform_name] + if "singularity" in preconfigured_platform: + _add(preconfigured_platform["singularity"]) + logger.info( + "Client.from_env found singularity platform config in MLCube system settings file " + "(file=%s, platform=%s). Will try it first or second for detecting singularity CLI client.", + system_settings.path.as_posix(), + preconfigured_platform["singularity"], + ) + logger.debug( "Client.from_env will try candidate executables in the following order " "(first available will be selected): %s", diff --git a/runners/mlcube_singularity/mlcube_singularity/singularity_run.py b/runners/mlcube_singularity/mlcube_singularity/singularity_run.py index b6d0235..7b3128e 100644 --- a/runners/mlcube_singularity/mlcube_singularity/singularity_run.py +++ b/runners/mlcube_singularity/mlcube_singularity/singularity_run.py @@ -71,15 +71,15 @@ def merge(mlcube: DictConfig) -> None: # This maybe useful to automatically detect singularity executable try: - client: t.Optional[Client] = Client.from_env() + client: t.Optional[Client] = Client.from_env(mlcube.runner.singularity) - singularity_exec = s_cfg.singularity if "singularity" in s_cfg else mlcube.runner.singularity - client_singularity = " ".join(client.singularity) - if singularity_exec != client_singularity: + exec_from_cfg = s_cfg.singularity if "singularity" in s_cfg else mlcube.runner.singularity + exec_from_env = " ".join(client.singularity) + if exec_from_cfg != exec_from_env: # Not updating for now since this is consistent with previous implementation. logger.warning( - "Config.merge singularity executable from config (%s) is not the one MLCube can run (%s).", - singularity_exec, client_singularity + "Config.merge singularity executable from configuration '(%s)' is not the one MLCube detected " + "from environment '(%s)'.", exec_from_cfg, exec_from_env ) except ExecutionError: client = None @@ -140,22 +140,30 @@ def merge(mlcube: DictConfig) -> None: # The idea is that we can use the remote docker image as a source for the build process, automatically # generating an image name in a local environment. Key here is that the source has a scheme - `docker://` # The --fakeroot switch is useful and is supported in singularity version >= 3.5 + # TODO sergey: why do we even think that this case is special - why not rely on system settings that we can + # automatically configure (e.g., detect correct (executable, fakeroot or no fakeroot) pairs? extra_args = {} if client is not None: if "singularity" not in s_cfg: + # TODO sergey: is this really a good idea to (maybe) replace exec from system settings if they differ? extra_args["singularity"] = " ".join(client.singularity) if "build_args" not in s_cfg: if client.supports_fakeroot(): - logger.info( - "Config.merge [build_args] will use --fakeroot CLI switch (CLI client seems to be " - "supporting it)." - ) - extra_args["build_args"] = "--fakeroot" + if 'fakeroot' not in mlcube.runner.build_args: + logger.warning( + "Config.merge build_args for runner configuration in system settings does not contain " + "fakeroot, while singularity runtime seems to be supporting this. Previous implementation " + "would override this and would use '--fakeroot' in some corner cases (like this), but this " + "was not intended behavior (a bug). If you believe your singularity runtime needs " + "`--fakeroot`, please update your system settings file (for the platform you are " + "using now)." + ) else: - logger.warning( - "Config.merge [build_args] will not use --fakeroot CLI switch (CLI client too old or " - "version unknown)" - ) + if 'fakeroot' in mlcube.runner.build_args: + logger.warning( + "Config.merge you singularity client seems not to be supporting fakeroot, but the runner " + "in system settings file is configured to use this switch." + ) build_file = "docker://" + d_cfg["image"] if "tar_file" in d_cfg: