Repository: reframe-hpc/reframe

Issue:
generalize autodetect setting

On some NERSC machines we are using `/etc/clustername` instead of `/etc/xthostname`.

Could the auto detection method be generalized to support run time specification of which path to check for cluster identity?

cc: @bcfriesen

Discussion:

Patch:
diff --git a/docs/config_reference.rst b/docs/config_reference.rst
index a3f5e047..e2f29c48 100644
--- a/docs/config_reference.rst
+++ b/docs/config_reference.rst
@@ -61,6 +61,29 @@ It consists of the following properties, which we also call conventionally *conf
    A list of `general configuration objects <#general-configuration>`__.
 
 
+.. py:data:: autodetect_methods
+
+   :required: No
+   :default: ``["py::socket.gethostname"]``
+
+   A list of system auto-detection methods for identifying the current system.
+
+   The list can contain two types of methods:
+
+   1. Python methods: These are prefixed with ``py::`` and should point to a Python callable taking zero arguments and returning a string.
+      If the specified Python callable is not prefixed with a module, it will be looked up in the loaded configuration files starting from the last file.
+      If the requested symbol cannot be found, a warning will be issued and the method will be ignored.
+   2. Shell commands: Any string not prefixed with ``py::`` will be treated as a shell command and will be executed *during auto-detection* to retrieve the hostname.
+      The standard output of the command will be used.
+
+   If the :option:`--system` option is not passed, ReFrame will try to autodetect the current system trying the methods in this list successively, until one of them succeeds.
+   The resulting name will be matched against the :attr:`~config.systems.hostnames` patterns of each system and the system that matches first will be used as the current one.
+
+   The auto-detection methods can also be controlled through the :envvar:`RFM_AUTODETECT_METHODS` environment variable.
+
+   .. versionadded:: 4.3
+
+
 .. warning::
    .. versionchanged:: 4.0.0
       The :data:`schedulers` section is removed.
diff --git a/docs/configure.rst b/docs/configure.rst
index 4415cb11..689bfbc7 100644
--- a/docs/configure.rst
+++ b/docs/configure.rst
@@ -311,12 +311,9 @@ As discussed previously, ReFrame's configuration file can store the configuratio
 When launched, ReFrame will pick the first matching configuration and load it.
 
 ReFrame uses an auto-detection mechanism to get information about the host it is running on and uses that information to pick the right system configuration.
-Currently, only one auto-detection method is supported that retrieves the hostname.
-Based on this, ReFrame goes through all the systems in its configuration and tries to match the hostname against any of the patterns defined in each system's ``hostnames`` property.
-The detection process stops at the first match found, and that system's configuration is selected.
-
-The auto-detection process can be controlled through the :envvar:`RFM_AUTODETECT_METHOD`, :envvar:`RFM_AUTODETECT_FQDN` and :envvar:`RFM_AUTODETECT_XTHOSTNAME` environment variables.
-
+The default auto-detection method uses the ``hostname`` command, but you can define more methods by setting either the :attr:`autodetect_methods` configuration parameter or the :envvar:`RFM_AUTODETECT_METHODS` environment variable.
+After having retrieved the hostname, ReFrame goes through all the systems in its configuration and tries to match it against the :attr:`~config.systems.hostnames` patterns defined for each system.
+The first system whose :attr:`~config.systems.hostnames` match will become the current system and its configuration will be loaded.
 
 As soon as a system configuration is selected, all configuration objects that have a ``target_systems`` property are resolved against the selected system, and any configuration object that is not applicable is dropped.
 So, internally, ReFrame keeps an *instantiation* of the site configuration for the selected system only.
diff --git a/docs/manpage.rst b/docs/manpage.rst
index 2730cdf6..7fb33079 100644
--- a/docs/manpage.rst
+++ b/docs/manpage.rst
@@ -1136,6 +1136,9 @@ Whenever an environment variable is associated with a configuration option, its
    .. versionchanged:: 4.0.0
       This variable now defaults to ``0``.
 
+   .. deprecated:: 4.3
+      Please use ``RFM_AUTODETECT_METHODS=py::fqdn`` in the future.
+
 
 .. envvar:: RFM_AUTODETECT_METHOD
 
@@ -1155,6 +1158,16 @@ Whenever an environment variable is associated with a configuration option, its
 
 
    .. versionadded:: 3.11.0
+   .. deprecated:: 4.3
+      This has no effect.
+      For setting multiple auto-detection methods, please use the :envvar:`RFM_AUTODETECT_METHODS`.
+
+.. envvar:: RFM_AUTODETECT_METHODS
+
+   A comma-separated list of system auto-detection methods.
+   Please refer to the :attr:`autodetect_methods` configuration parameter for more information on how to set this variable.
+
+   .. versionadded:: 4.3
 
 
 .. envvar:: RFM_AUTODETECT_XTHOSTNAME
@@ -1179,6 +1192,10 @@ Whenever an environment variable is associated with a configuration option, its
    .. versionchanged:: 4.0.0
       This variable now defaults to ``0``.
 
+   .. deprecated:: 4.3
+      Please use ``RFM_AUTODETECT_METHODS='cat /etc/xthostname,hostname'`` in the future.
+
+
 .. envvar:: RFM_CHECK_SEARCH_PATH
 
    A colon-separated list of filesystem paths where ReFrame should search for tests.
diff --git a/reframe/core/config.py b/reframe/core/config.py
index 9163808b..54d9fd1b 100644
--- a/reframe/core/config.py
+++ b/reframe/core/config.py
@@ -7,18 +7,19 @@ import contextlib
 import copy
 import fnmatch
 import functools
+import importlib
 import itertools
 import json
 import jsonschema
 import os
 import re
-import socket
 
 import reframe
 import reframe.core.settings as settings
 import reframe.utility as util
+import reframe.utility.osext as osext
 from reframe.core.environments import normalize_module_list
-from reframe.core.exceptions import ConfigError, ReframeFatalError
+from reframe.core.exceptions import (ConfigError, ReframeFatalError)
 from reframe.core.logging import getlogger
 from reframe.utility import ScopedDict
 
@@ -60,40 +61,15 @@ def _normalize_syntax(conv):
     return _do_normalize
 
 
-def _hostname(use_fqdn, use_xthostname):
-    '''Return hostname'''
-    if use_xthostname:
-        try:
-            xthostname_file = '/etc/xthostname'
-            getlogger().debug(f'Trying {xthostname_file!r}...')
-            with open(xthostname_file) as fp:
-                return fp.read()
-        except OSError as e:
-            '''Log the error and continue to the next method'''
-            getlogger().debug(f'Failed to read {xthostname_file!r}')
-
-    if use_fqdn:
-        getlogger().debug('Using FQDN...')
-        return socket.getfqdn()
-
-    getlogger().debug('Using standard hostname...')
-    return socket.gethostname()
-
-
 class _SiteConfig:
     def __init__(self):
         self._site_config = None
+        self._config_modules = []
         self._sources = []
         self._subconfigs = {}
         self._local_system = None
         self._sticky_options = {}
-        self._autodetect_meth = 'hostname'
-        self._autodetect_opts = {
-            'hostname': {
-                'use_fqdn': False,
-                'use_xthostname': False,
-            }
-        }
+        self._autodetect_methods = []
         self._definitions = {
             'systems': {},
             'partitions': {},
@@ -195,7 +171,7 @@ class _SiteConfig:
         nc = copy.deepcopy(config)
         if self._site_config is None:
             self._site_config = nc
-            return self
+            return
 
         mergeable_sections = ('general', 'logging', 'schedulers')
         for sec in nc.keys():
@@ -206,10 +182,11 @@ class _SiteConfig:
                     self._site_config[sec], nc[sec]
                 )
             else:
-                if sec == 'systems':
-                    # Systems have to be inserted in the beginning of the list,
-                    # since they are selected by the first matching entry in
-                    # `hostnames`.
+                if sec in ('systems', 'autodetect_methods'):
+                    # Systems have to be inserted in the beginning of the
+                    # list, since they are selected by the first matching
+                    # entry in `hostnames`. Similarly, the autodetection
+                    # methods are tried from left to right.
                     self._site_config[sec] = nc[sec] + self._site_config[sec]
                 else:
                     self._site_config[sec] += nc[sec]
@@ -239,14 +216,8 @@ class _SiteConfig:
     def __getattr__(self, attr):
         return getattr(self._pick_config(), attr)
 
-    def set_autodetect_meth(self, method, **opts):
-        self._autodetect_meth = method
-        try:
-            self._autodetect_opts[method].update(opts)
-        except KeyError:
-            raise ConfigError(
-                f'unknown auto-detection method: {method!r}'
-            ) from None
+    def set_autodetect_methods(self, methods):
+        self._site_config['autodetect_methods'] = list(methods)
 
     @property
     def schema(self):
@@ -364,22 +335,15 @@ class _SiteConfig:
             # import_module_from_file() may raise an ImportError if the
             # configuration file is under ReFrame's top-level directory
             raise ConfigError(
-                f"could not load Python configuration file: '{filename}'"
+                f'could not load Python configuration file: {filename!r}'
             ) from e
 
-        if hasattr(mod, 'settings'):
-            # Looks like an old style config
-            raise ConfigError(
-                f"the syntax of the configuration file {filename!r} "
-                f"is no longer supported"
-            )
-
-        mod = util.import_module_from_file(filename)
         if not hasattr(mod, 'site_configuration'):
             raise ConfigError(
                 f"not a valid Python configuration file: '{filename}'"
             )
 
+        self._config_modules.append(mod)
         self.update_config(mod.site_configuration, filename)
 
     def load_config_json(self, filename):
@@ -393,14 +357,75 @@ class _SiteConfig:
 
         self.update_config(config, filename)
 
-    def _detect_system(self):
-        getlogger().debug(
-            f'Detecting system using method: {self._autodetect_meth!r}'
-        )
-        hostname = _hostname(
-            self._autodetect_opts[self._autodetect_meth]['use_fqdn'],
-            self._autodetect_opts[self._autodetect_meth]['use_xthostname'],
+    def _setup_autodect_methods(self):
+        def _py_meth(m):
+            try:
+                module, symbol = m.rsplit('.', maxsplit=1)
+            except ValueError:
+                # Not enough values to unpack; we assume a single symbol
+                module, symbol = None, m
+
+            try:
+                if module:
+                    mod = importlib.import_module(module)
+                    return getattr(mod, symbol)
+
+                if not self._config_modules:
+                    raise ConfigError(
+                        f'no module context for requested symbol: {symbol!r}'
+                    )
+
+                # Symbol is local to one of the config files; try to resolve
+                # it in reverse order
+                for mod in reversed(self._config_modules):
+                    if hasattr(mod, symbol):
+                        return getattr(mod, symbol)
+
+                raise ConfigError(f'symbol {symbol!r} is not defined in '
+                                  f'any of the loaded configuration files')
+            except (AttributeError, ImportError, ConfigError) as e:
+                getlogger().warning(f"ignoring autodetection method 'py::{m}' "
+                                    f"due to errors: {e}")
+
+        def _sh_meth(m):
+            def _fn():
+                completed = osext.run_command(m, check=True)
+                return completed.stdout.strip()
+
+            return _fn
+
+        methods = self._site_config.get(
+            'autodetect_methods',
+            self._schema['defaults']['autodetect_methods']
         )
+        for m in methods:
+            if m.startswith('py::'):
+                fn = _py_meth(m.lstrip('py::'))
+                if fn is not None:
+                    self._autodetect_methods.append((m, fn))
+            else:
+                self._autodetect_methods.append((m, _sh_meth(m)))
+
+    def _detect_system(self):
+        getlogger().debug('Autodetecting system')
+        if not self._autodetect_methods:
+            self._setup_autodect_methods()
+
+        hostname = None
+        for meth, fn in self._autodetect_methods:
+            getlogger().debug(f'Trying autodetection method: {meth!r}')
+            try:
+                hostname = fn()
+            except Exception as e:
+                getlogger().debug(f'Autodetection method {meth!r} failed: {e}')
+            else:
+                break
+
+        if hostname is None:
+            raise ConfigError('all autodetection methods failed; '
+                              'try passing a system name explicitly using '
+                              'the `--system` option')
+
         getlogger().debug(f'Retrieved hostname: {hostname!r}')
         getlogger().debug(f'Looking for a matching configuration entry')
         for system in self._site_config['systems']:
@@ -412,8 +437,8 @@ class _SiteConfig:
                     )
                     return sysname
 
-        raise ConfigError(f"could not find a configuration entry "
-                          f"for the current system: '{hostname}'")
+        raise ConfigError(f'could not find a configuration entry '
+                          f'for the current system: {hostname!r}')
 
     def validate(self):
         site_config = self._pick_config()
@@ -509,8 +534,8 @@ class _SiteConfig:
         # Create local configuration for the current or the requested system
         local_config['systems'] = systems
         for name, section in site_config.items():
-            if name == 'systems':
-                # The systems sections has already been treated
+            if name in ('systems', 'autodetect_methods'):
+                # These sections have already been treated
                 continue
 
             # Convert section to a scoped dict that will handle correctly and
@@ -625,10 +650,14 @@ def find_config_files(config_path=None, config_file=None):
 
 def load_config(*filenames):
     ret = _SiteConfig()
-    getlogger().debug('Loading the generic configuration')
+    getlogger().debug('Loading the builtin configuration')
     ret.update_config(settings.site_configuration, '<builtin>')
     for f in filenames:
-        getlogger().debug(f'Loading configuration file: {filenames!r}')
+        if f == '<builtin>':
+            # The builtin configuration is always loaded at the beginning
+            continue
+
+        getlogger().debug(f'Loading configuration file: {f!r}')
         _, ext = os.path.splitext(f)
         if ext == '.py':
             ret.load_config_python(f)
diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py
index 1bb2dc80..cd60f369 100644
--- a/reframe/frontend/cli.py
+++ b/reframe/frontend/cli.py
@@ -14,7 +14,6 @@ import sys
 import time
 import traceback
 
-import reframe
 import reframe.core.config as config
 import reframe.core.exceptions as errors
 import reframe.core.logging as logging
@@ -574,14 +573,13 @@ def main():
         dest='autodetect_method',
         envvar='RFM_AUTODETECT_METHOD',
         action='store',
-        default='hostname',
         help='Method to detect the system'
     )
     argparser.add_argument(
-        dest='config_path',
-        envvar='RFM_CONFIG_PATH :',
-        action='append',
-        help='Directories where ReFrame will look for base configuration'
+        dest='autodetect_methods',
+        envvar='RFM_AUTODETECT_METHODS',
+        action='store',
+        help='List of methods for detecting the current system'
     )
     argparser.add_argument(
         dest='autodetect_xthostname',
@@ -591,6 +589,12 @@ def main():
         type=typ.Bool,
         help="Use Cray's xthostname file to retrieve the host name"
     )
+    argparser.add_argument(
+        dest='config_path',
+        envvar='RFM_CONFIG_PATH :',
+        action='append',
+        help='Directories where ReFrame will look for base configuration'
+    )
     argparser.add_argument(
         dest='git_timeout',
         envvar='RFM_GIT_TIMEOUT',
@@ -707,9 +711,7 @@ def main():
     # First configure logging with our generic configuration so as to be able
     # to print pretty messages; logging will be reconfigured by user's
     # configuration later
-    site_config = config.load_config(
-        os.path.join(reframe.INSTALL_PREFIX, 'reframe/core/settings.py')
-    )
+    site_config = config.load_config('<builtin>')
     site_config.select_subconfig('generic')
     options.update_config(site_config)
     logging.configure_logging(site_config)
@@ -739,11 +741,33 @@ def main():
         )
         site_config = config.load_config(*conf_files)
         site_config.validate()
-        site_config.set_autodetect_meth(
-            options.autodetect_method,
-            use_fqdn=options.autodetect_fqdn,
-            use_xthostname=options.autodetect_xthostname
-        )
+
+        if options.autodetect_method:
+            printer.warning('RFM_AUTODETECT_METHOD is deprecated; '
+                            'please use RFM_AUTODETECT_METHODS instead')
+
+        autodetect_methods = []
+        if options.autodetect_methods:
+            autodetect_methods = options.autodetect_methods.split(',')
+        else:
+            if options.autodetect_fqdn:
+                printer.warning(
+                    'RFM_AUTODETECT_FQDN is deprecated; '
+                    'please use RFM_AUTODETECT_METHODS=py::socket.getfqdn '
+                    'instead'
+                )
+                autodetect_methods = ['py::socket.getfqdn']
+            elif options.autodetect_xthostname:
+                printer.warning(
+                    "RFM_AUTODETECT_XTHOSTNAME is deprecated; "
+                    "please use RFM_AUTODETECT_METHODS='cat /etc/xthostname,hostname' "  # noqa: E501
+                    "instead"
+                )
+                autodetect_methods = ['cat /etc/xthostname',
+                                      'py::socket.gethostname']
+
+        if autodetect_methods:
+            site_config.set_autodetect_methods(autodetect_methods)
 
         # We ignore errors about unresolved sections or configuration
         # parameters here, because they might be defined at the individual
diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py
index ee29a87b..d9845d48 100644
--- a/reframe/frontend/loader.py
+++ b/reframe/frontend/loader.py
@@ -20,19 +20,6 @@ from reframe.core.exceptions import NameConflictError, is_severe, what
 from reframe.core.logging import getlogger, time_function
 
 
-class temp_sys_path:
-    def __init__(self, path):
-        self._path = path
-        self._pos = None
-
-    def __enter__(self):
-        self._pos = len(sys.path)
-        sys.path.append(self._path)
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        sys.path.pop(self._pos)
-
-
 class no_op:
     def __enter__(self):
         pass
@@ -206,7 +193,7 @@ class RegressionCheckLoader:
         try:
             dirname = os.path.dirname(filename)
             with osext.change_dir(dirname):
-                with temp_sys_path(dirname):
+                with util.temp_sys_path(dirname):
                     return self.load_from_module(
                         util.import_module_from_file(filename, force)
                     )
diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json
index bee3a273..3589bca9 100644
--- a/reframe/schemas/config.json
+++ b/reframe/schemas/config.json
@@ -234,6 +234,10 @@
     },
     "type": "object",
     "properties": {
+        "autodetect_methods": {
+            "type": "array",
+            "items": {"type": "string"}
+        },
         "systems": {
             "type": "array",
             "items": {
@@ -513,6 +517,7 @@
     "required": ["systems", "environments", "logging"],
     "additionalProperties": false,
     "defaults": {
+        "autodetect_methods": ["py::socket.gethostname"],
         "environments/modules": [],
         "environments/env_vars": [],
         "environments/variables": [],
diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py
index 7eabfbd2..12edb01e 100644
--- a/reframe/utility/__init__.py
+++ b/reframe/utility/__init__.py
@@ -1062,6 +1062,21 @@ class temp_setattr:
         setattr(self._obj, self._attr, self._saved)
 
 
+class temp_sys_path:
+    '''Context manager to temporarily change the py:obj:`sys.path`.'''
+
+    def __init__(self, path):
+        self._path = path
+        self._pos = None
+
+    def __enter__(self):
+        self._pos = len(sys.path)
+        sys.path.append(self._path)
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        sys.path.pop(self._pos)
+
+
 class ScopedDict(UserDict):
     '''This is a special dictionary that imposes scopes on its keys.
 


Test patch:
diff --git a/unittests/resources/config/settings.py b/unittests/resources/config/settings.py
index 7f82bcd8..ad9770b5 100644
--- a/unittests/resources/config/settings.py
+++ b/unittests/resources/config/settings.py
@@ -7,6 +7,11 @@
 # Configuration file just for unit testing
 #
 
+def hostname():
+    '''Custom hostname function for testing auto-detection'''
+    return 'testsys'
+
+
 site_configuration = {
     'systems': [
         {
diff --git a/unittests/test_config.py b/unittests/test_config.py
index 442d5120..c60a3dce 100644
--- a/unittests/test_config.py
+++ b/unittests/test_config.py
@@ -5,6 +5,7 @@
 
 import json
 import pytest
+import sys
 
 import reframe.core.config as config
 import reframe.utility as util
@@ -18,7 +19,8 @@ def generate_partial_configs(tmp_path):
     part2 = tmp_path / 'settings-part2.py'
     part3 = tmp_path / 'settings-part3.py'
     mod = util.import_module_from_file(
-        'unittests/resources/config/settings.py')
+        'unittests/resources/config/settings.py'
+    )
     full_config = mod.site_configuration
 
     config_1 = {
@@ -37,7 +39,11 @@ def generate_partial_configs(tmp_path):
         'logging': full_config['logging'],
         'general': full_config['general'][3:],
     }
-    part1.write_text(f'site_configuration = {config_1!r}')
+
+    # We need to make sure that the custom hostname function is in one of the
+    # files
+    part1.write_text(f'def hostname(): return "testsys"\n'
+                     f'site_configuration = {config_1!r}')
     part2.write_text(f'site_configuration = {config_2!r}')
     part3.write_text(f'site_configuration = {config_3!r}')
     return part1, part2, part3
@@ -488,6 +494,56 @@ def test_variables(tmp_path):
     assert system.partitions[0].local_env.env_vars == {'FOO_GPU': 'yes'}
 
 
+def test_autodetect_meth_python(site_config, tmp_path):
+    moduledir = tmp_path / 'mymod'
+    moduledir.mkdir()
+    with open(moduledir / '__init__.py', 'w') as fp:
+        fp.write('def foo():\n\treturn "sys12"\n')
+
+    site_config.set_autodetect_methods(['py::mymod.foo'])
+    with util.temp_sys_path(str(tmp_path)):
+        site_config.select_subconfig()
+
+    assert site_config.get('systems/0/name') == 'sys0'
+
+    # Uncache the module
+    del sys.modules['mymod']
+
+
+def test_autodetect_meth_python_inline(site_config):
+    site_config.set_autodetect_methods(['py::hostname'])
+    site_config.select_subconfig()
+    assert site_config.get('systems/0/name') == 'testsys'
+
+
+def test_autodetect_meth_python_errors(site_config):
+    site_config.set_autodetect_methods(['py::mymod.foo'])
+    with pytest.raises(ConfigError):
+        site_config.select_subconfig()
+
+    site_config.set_autodetect_methods(['py::foo'])
+    with pytest.raises(ConfigError):
+        site_config.select_subconfig()
+
+
+def test_autodetect_meth_shell(site_config):
+    site_config.set_autodetect_methods(['echo testsys'])
+    site_config.select_subconfig()
+    assert site_config.get('systems/0/name') == 'testsys'
+
+
+def test_autodetect_meth_shell_errors(site_config):
+    site_config.set_autodetect_methods(['xxxxxxxxx'])
+    with pytest.raises(ConfigError):
+        site_config.select_subconfig()
+
+
+def test_autodetect_meth_multiple(site_config):
+    site_config.set_autodetect_methods(['py::foo', 'echo testsys'])
+    site_config.select_subconfig()
+    assert site_config.get('systems/0/name') == 'testsys'
+
+
 def test_hostname_autodetection(site_config):
     # This exercises only the various execution paths
 
@@ -495,7 +551,7 @@ def test_hostname_autodetection(site_config):
     # order to trigger the auto-detection
     for use_xthostname in (True, False):
         for use_fqdn in (True, False):
-            site_config.set_autodetect_meth('hostname',
-                                            use_fqdn=use_fqdn,
-                                            use_xthostname=use_xthostname)
+            site_config.set_autodetect_methods(['py::socket.gethostname',
+                                                'py::socket.getfqdn',
+                                                'cat /etc/xthostname'])
             site_config.select_subconfig()