From 16ca562131458d4dc181af5385cf0e1438e30b02 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Tue, 15 Feb 2022 15:26:12 -0800 Subject: [PATCH 1/7] Refactor singularity module --- packages/e4s_cl/cf/containers/singularity.py | 41 ++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/packages/e4s_cl/cf/containers/singularity.py b/packages/e4s_cl/cf/containers/singularity.py index e380b1a2..5c5c6da3 100644 --- a/packages/e4s_cl/cf/containers/singularity.py +++ b/packages/e4s_cl/cf/containers/singularity.py @@ -37,13 +37,24 @@ def __setup__(self): #self.bind_file('/dev', option=FileOptions.READ_WRITE) #self.bind_file('/tmp', option=FileOptions.READ_WRITE) + def _format_bound(self): + """ + Format a list of files to a compatible bind option of singularity + """ + + def _format(): + for source, dest, options_val in self.bound: + yield f"{source}:{dest}:{OPTION_STRINGS[options_val]}" + + self.env.update({"SINGULARITY_BIND": ','.join(_format())}) + def _prepare(self, command) -> list[str]: self.add_ld_library_path("/.singularity.d/libs") self.env.update( {'SINGULARITYENV_LD_PRELOAD': ":".join(self.ld_preload)}) self.env.update( {'SINGULARITYENV_LD_LIBRARY_PATH': ":".join(self.ld_lib_path)}) - self.format_bound() + self._format_bound() nvidia_flag = ['--nv'] if self._has_nvidia() else [] return [ @@ -51,25 +62,6 @@ def _prepare(self, command) -> list[str]: self.image, *command ] - def run(self, command): - if not which(self.executable): - raise BackendNotAvailableError(self.executable) - - container_cmd = self._prepare(command) - - return run_subprocess(container_cmd, env=self.env) - - def format_bound(self): - """ - Format a list of files to a compatible bind option of singularity - """ - - def _format(): - for source, dest, options_val in self.bound: - yield f"{source}:{dest}:{OPTION_STRINGS[options_val]}" - - self.env.update({"SINGULARITY_BIND": ','.join(_format())}) - def bind_env_var(self, key, value): self.env.update({f"SINGULARITYENV_{key}": value}) @@ -79,5 +71,14 @@ def _has_nvidia(self): return False return True + def run(self, command): + if not which(self.executable): + raise BackendNotAvailableError(self.executable) + + container_cmd = self._prepare(command) + + return run_subprocess(container_cmd, env=self.env) + + CLASS = SingularityContainer From 6a19900c7f50bc1dc3044906d12459873f9134a7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Tue, 15 Feb 2022 15:26:29 -0800 Subject: [PATCH 2/7] Introduced podman support --- packages/e4s_cl/cf/containers/podman.py | 76 +++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 packages/e4s_cl/cf/containers/podman.py diff --git a/packages/e4s_cl/cf/containers/podman.py b/packages/e4s_cl/cf/containers/podman.py new file mode 100644 index 00000000..aba7bbd9 --- /dev/null +++ b/packages/e4s_cl/cf/containers/podman.py @@ -0,0 +1,76 @@ +import os +from pathlib import Path +from e4s_cl.error import InternalError +from e4s_cl.util import which, run_subprocess +from e4s_cl.logger import get_logger +from e4s_cl.cf.pipe import NamedPipe, ENV_VAR_NAMED +from e4s_cl.cf.containers import Container, FileOptions, BackendNotAvailableError + +LOGGER = get_logger(__name__) + +NAME = 'podman' +EXECUTABLES = ['podman'] +MIMES = [] + + +class PodmanContainer(Container): + pipe_manager = NamedPipe + + def _fd_number(self): + max_good_fd = 2 + fd_number = len(list(Path('/proc/self/fd').glob('*'))) + for fd in range(3, fd_number): + try: + os.set_inheritable(fd, True) + except OSError as err: + if err.errno != 9: + raise InternalError( + f"Failed to set the inheritable flag on fd {fd}: {str(err)}" + ) from err + else: + max_good_fd = fd + + return max_good_fd - 3 + + def _working_dir(self): + return ['--workdir', os.getcwd()] + + def _format_bound(self): + + def _format(): + fifo = os.environ.get(ENV_VAR_NAMED, '') + if fifo: + yield f"--mount=type=bind,src={fifo},dst={fifo},ro=false" + + for src, dst, opt in self.bound: + yield f"--mount=type=bind,src={src.as_posix()},dst={dst.as_posix()}{',ro=true' if (opt == FileOptions.READ_ONLY) else ''}" + + return list(_format()) + + def _prepare(self, command): + + return [ + self.executable, # absolute path to podman + 'run', # Run a container + '--env-host', # Pass host environment /!\ + f"--preserve-fds={self._fd_number()}", # Inherit file descriptors /!\ + *self._working_dir(), # Work in the same CWD + *self._format_bound(), # Bound files options + self.image, + *command + ] + + def run(self, command): + """ + def run(self, command: list[str]): + """ + + if not which(self.executable): + raise BackendNotAvailableError(self.executable) + + container_cmd = self._prepare(command) + + return run_subprocess(container_cmd, env=self.env) + + +CLASS = PodmanContainer From d24ef666a16d8f5bcd5fb9da285ce08909110b2b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Thu, 17 Feb 2022 10:36:01 -0800 Subject: [PATCH 3/7] podman: Fill file descriptor slots Attempt to fill file descriptors before running podman --- packages/e4s_cl/cf/containers/podman.py | 83 +++++++++++++++++++---- packages/e4s_cl/cli/commands/__execute.py | 3 + packages/e4s_cl/util.py | 2 + 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/packages/e4s_cl/cf/containers/podman.py b/packages/e4s_cl/cf/containers/podman.py index aba7bbd9..776834f3 100644 --- a/packages/e4s_cl/cf/containers/podman.py +++ b/packages/e4s_cl/cf/containers/podman.py @@ -1,4 +1,5 @@ import os +from shlex import join from pathlib import Path from e4s_cl.error import InternalError from e4s_cl.util import which, run_subprocess @@ -13,24 +14,77 @@ MIMES = [] -class PodmanContainer(Container): - pipe_manager = NamedPipe +def opened_fds(): + fds = [] - def _fd_number(self): - max_good_fd = 2 - fd_number = len(list(Path('/proc/self/fd').glob('*'))) - for fd in range(3, fd_number): + for file in Path('/proc/self/fd').glob('*'): + if not file.exists(): + continue + + try: + fd_no = int(file.name) + except: + continue + + fds.append(fd_no) + + return fds + + +class FDFiller: + + def __init__(self): + self.__opened_files = [] + + def __enter__(self): + fds = opened_fds() + + for fd in fds: try: os.set_inheritable(fd, True) except OSError as err: - if err.errno != 9: - raise InternalError( - f"Failed to set the inheritable flag on fd {fd}: {str(err)}" - ) from err - else: - max_good_fd = fd + if err.errno == 9: + continue + + missing = set(range(max(fds))) - set(fds) + + while missing: + null = open('/dev/null', 'w') + + if null.fileno() not in missing: + raise InternalError(f"Unexpected fileno: {null.fileno()}") + + try: + os.set_inheritable(null.fileno(), True) + except OSError as err: + if err.errno == 9: + continue + + missing.discard(null.fileno()) + + self.__opened_files.append(null) + LOGGER.debug("Created %d file descriptors: %s", len(self.__opened_files), [f.fileno() for f in self.__opened_files]) + + def __exit__(self, type_, value, traceback): + for file in self.__opened_files: + file.close() + + +class PodmanContainer(Container): + pipe_manager = NamedPipe + + def _fd_number(self): + """ + Podman requires the --preserve-fds=K option to pass file descriptors; + K being the amount (in addition of 0,1,2) of fds to pass. It also is + strict on the inheritance flag of those descriptors, and will not + function if any one of them is invalid/uninheritable. To abide to this, + we go through all the opened file descriptors, manually set the + inheritance flag to true, and return as soon as a file descriptor refuses + """ - return max_good_fd - 3 + LOGGER.debug("Max fd: %d (%s)", max(opened_fds()), opened_fds()) + return max(opened_fds()) - 3 def _working_dir(self): return ['--workdir', os.getcwd()] @@ -70,7 +124,8 @@ def run(self, command: list[str]): container_cmd = self._prepare(command) - return run_subprocess(container_cmd, env=self.env) + with FDFiller(): + return run_subprocess(container_cmd, env=self.env) CLASS = PodmanContainer diff --git a/packages/e4s_cl/cli/commands/__execute.py b/packages/e4s_cl/cli/commands/__execute.py index 3d141498..9fd93057 100644 --- a/packages/e4s_cl/cli/commands/__execute.py +++ b/packages/e4s_cl/cli/commands/__execute.py @@ -258,6 +258,9 @@ def _path(library: HostLibrary): code = container.run(command) + if code: + LOGGER.critical("Container command failed with error code %d", code) + params.teardown() return code diff --git a/packages/e4s_cl/util.py b/packages/e4s_cl/util.py index 2242b2e3..a2f1bd5d 100644 --- a/packages/e4s_cl/util.py +++ b/packages/e4s_cl/util.py @@ -154,6 +154,8 @@ def run_subprocess(cmd: list[str], if log_file := getattr(process_logger.handlers[0], 'baseFilename', None): LOGGER.error("See %s for details.", log_file) + else: + LOGGER.debug("Process %d returned %d", pid, returncode) del process_logger From eeca3c0b2dd7f736a1175b37b99cc0548c507e42 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Thu, 17 Feb 2022 13:35:09 -0800 Subject: [PATCH 4/7] Functionning podman module --- packages/e4s_cl/cf/containers/podman.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e4s_cl/cf/containers/podman.py b/packages/e4s_cl/cf/containers/podman.py index 776834f3..1e86e9ae 100644 --- a/packages/e4s_cl/cf/containers/podman.py +++ b/packages/e4s_cl/cf/containers/podman.py @@ -63,7 +63,9 @@ def __enter__(self): missing.discard(null.fileno()) self.__opened_files.append(null) - LOGGER.debug("Created %d file descriptors: %s", len(self.__opened_files), [f.fileno() for f in self.__opened_files]) + LOGGER.debug("Created %d file descriptors: %s", + len(self.__opened_files), + [f.fileno() for f in self.__opened_files]) def __exit__(self, type_, value, traceback): for file in self.__opened_files: @@ -106,6 +108,8 @@ def _prepare(self, command): return [ self.executable, # absolute path to podman 'run', # Run a container + '--rm', # Remove when done + '--ipc=host', # Use host IPC /!\ '--env-host', # Pass host environment /!\ f"--preserve-fds={self._fd_number()}", # Inherit file descriptors /!\ *self._working_dir(), # Work in the same CWD From 062976839ec311ba612dcccc6ba7c1d3ec07b267 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Thu, 17 Feb 2022 13:39:06 -0800 Subject: [PATCH 5/7] Added podman to the compatible software list --- docs/compatibility/software.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/compatibility/software.rst b/docs/compatibility/software.rst index d21f5322..8876fc41 100644 --- a/docs/compatibility/software.rst +++ b/docs/compatibility/software.rst @@ -4,14 +4,13 @@ Software Compatibility Container backends ------------------- -As of now, `singularity `_, `shifter `_ and `docker `_ are supported in **e4s-cl**. +As of now, `singularity `_, `shifter `_, `docker `_ and `podman `_ are supported in **e4s-cl**. More container technologies can be supported. Create an issue on github or write a dedicated module in :code:`e4s_cl/cf/containers`. Refer to :code:`e4s_cl/cf/containers/__init__.py` for details. .. warning:: Using **docker** with MPI - Several MPI implementations expect their processes to inherit opened file descriptors; because of docker's client-daemon architecture, this is not possible. - + Several MPI implementations expect their processes to inherit opened file descriptors; because of docker's client-daemon architecture, this is not possible.To use docker images with MPI, it is encouraged to used podman `_. Process launchers ------------------ From 25010bca51ecf3a261bd6bdece7d5c22d520ee9a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Thu, 17 Feb 2022 13:58:49 -0800 Subject: [PATCH 6/7] Linted podman module --- packages/e4s_cl/cf/containers/podman.py | 42 +++++++++++++++++++------ 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/e4s_cl/cf/containers/podman.py b/packages/e4s_cl/cf/containers/podman.py index 1e86e9ae..640efbec 100644 --- a/packages/e4s_cl/cf/containers/podman.py +++ b/packages/e4s_cl/cf/containers/podman.py @@ -1,5 +1,8 @@ +""" +Podman container manager support +""" + import os -from shlex import join from pathlib import Path from e4s_cl.error import InternalError from e4s_cl.util import which, run_subprocess @@ -15,6 +18,12 @@ def opened_fds(): + """ + -> set[int] + + Returns a list of all the opened file descriptors opened by the current + process + """ fds = [] for file in Path('/proc/self/fd').glob('*'): @@ -23,7 +32,7 @@ def opened_fds(): try: fd_no = int(file.name) - except: + except ValueError: continue fds.append(fd_no) @@ -32,13 +41,24 @@ def opened_fds(): class FDFiller: + """ + Context manager that will "fill" the opened file descriptors to have a + contiguous list, and make every fd inheritable + """ def __init__(self): + """ + Initialize by creating a buffer of opened files + """ self.__opened_files = [] def __enter__(self): + """ + Create as many open files as necessary + """ fds = opened_fds() + # Make every existing file descriptor inheritable for fd in fds: try: os.set_inheritable(fd, True) @@ -46,23 +66,27 @@ def __enter__(self): if err.errno == 9: continue + # Compute all the missing numbers in the list missing = set(range(max(fds))) - set(fds) while missing: - null = open('/dev/null', 'w') + # Open files towards /dev/null + null = open('/dev/null', 'w', encoding='utf-8') if null.fileno() not in missing: raise InternalError(f"Unexpected fileno: {null.fileno()}") try: + # Set the file as inheritable os.set_inheritable(null.fileno(), True) except OSError as err: if err.errno == 9: continue + # It is not missing anymore missing.discard(null.fileno()) - self.__opened_files.append(null) + LOGGER.debug("Created %d file descriptors: %s", len(self.__opened_files), [f.fileno() for f in self.__opened_files]) @@ -73,16 +97,16 @@ def __exit__(self, type_, value, traceback): class PodmanContainer(Container): - pipe_manager = NamedPipe + """ + Podman container object + """ def _fd_number(self): """ Podman requires the --preserve-fds=K option to pass file descriptors; K being the amount (in addition of 0,1,2) of fds to pass. It also is - strict on the inheritance flag of those descriptors, and will not - function if any one of them is invalid/uninheritable. To abide to this, - we go through all the opened file descriptors, manually set the - inheritance flag to true, and return as soon as a file descriptor refuses + strict on the existence and inheritance flag of those descriptors, and + will not function if any one of them is invalid/uninheritable. """ LOGGER.debug("Max fd: %d (%s)", max(opened_fds()), opened_fds()) From 986a35a47a07c0c56de31984948b92c3f9e83ac0 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Thu, 17 Feb 2022 14:13:54 -0800 Subject: [PATCH 7/7] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 64d13ce9..9ea7e137 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ Additions: - Added barebones module awareness - Added logging output in dedicated logging folders - Added docker backend module +- Added podman backend module Removed: