[global] Align sysroot determination and usage across sos

The determination of sysroot - being automatic, user-specified, or
controlled via environment variables in a container - has gotten muddied
over time. This has resulted in different parts of the project;
`Policy`, `Plugin`, `SoSComponent`, etc... to not always be in sync when
sysroot is not `/`, thus causing varying and unexpected/unintended
behavior.

Fix this by only determining sysroot within `Policy()` initialization,
and then using that determination across all aspects of the project that
use or reference sysroot.

This results in several changes:

- `PackageManager()` will now (again) correctly reference host package
  lists when sos is run in a container.

- `ContainerRuntime()` is now able to activate when sos is running in a
  container.

- Plugins will now properly use sysroot for _all_ plugin enablement
  triggers.

- Plugins, Policy, and SoSComponents now all reference the
  `self.sysroot` variable, rather than changing between `sysroot`.
`_host_sysroot`, and `commons['sysroot']`. `_host_sysroot` has been
removed from `Policy`.

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

12 files changed, 64 insertions, 56 deletions

diff --git a/sos/archive.py b/sos/archive.py
index b02b2475..e3c68b77 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -153,7 +153,7 @@ class FileCacheArchive(Archive):
         return (os.path.join(self._archive_root, name))
 
     def join_sysroot(self, path):
-        if path.startswith(self.sysroot):
+        if not self.sysroot or path.startswith(self.sysroot):
             return path
         if path[0] == os.sep:
             path = path[1:]
diff --git a/sos/component.py b/sos/component.py
index 5ac6e47f..dba0aabf 100644
--- a/sos/component.py
+++ b/sos/component.py
@@ -109,7 +109,7 @@ class SoSComponent():
             try:
                 import sos.policies
                 self.policy = sos.policies.load(sysroot=self.opts.sysroot)
-                self.sysroot = self.policy.host_sysroot()
+                self.sysroot = self.policy.sysroot
             except KeyboardInterrupt:
                 self._exit(0)
             self._is_root = self.policy.is_root()
diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py
index fb8db1d7..ef9188de 100644
--- a/sos/policies/__init__.py
+++ b/sos/policies/__init__.py
@@ -110,7 +110,6 @@ any third party.
     presets = {"": PresetDefaults()}
     presets_path = PRESETS_PATH
     _in_container = False
-    _host_sysroot = '/'
 
     def __init__(self, sysroot=None, probe_runtime=True):
         """Subclasses that choose to override this initializer should call
@@ -124,7 +123,7 @@ any third party.
         self.package_manager = PackageManager()
         self.valid_subclasses = [IndependentPlugin]
         self.set_exec_path()
-        self._host_sysroot = sysroot
+        self.sysroot = sysroot
         self.register_presets(GENERIC_PRESETS)
 
     def check(self, remote=''):
@@ -177,14 +176,6 @@ any third party.
         """
         return self._in_container
 
-    def host_sysroot(self):
-        """Get the host's default sysroot
-
-        :returns: Host sysroot
-        :rtype: ``str`` or ``None``
-        """
-        return self._host_sysroot
-
     def dist_version(self):
         """
         Return the OS version
diff --git a/sos/policies/distros/__init__.py b/sos/policies/distros/__init__.py
index 7bdc81b8..c69fc1e7 100644
--- a/sos/policies/distros/__init__.py
+++ b/sos/policies/distros/__init__.py
@@ -71,19 +71,18 @@ class LinuxPolicy(Policy):
     def __init__(self, sysroot=None, init=None, probe_runtime=True):
         super(LinuxPolicy, self).__init__(sysroot=sysroot,
                                           probe_runtime=probe_runtime)
-        self.init_kernel_modules()
 
-        # need to set _host_sysroot before PackageManager()
         if sysroot:
-            self._container_init()
-            self._host_sysroot = sysroot
+            self.sysroot = sysroot
         else:
-            sysroot = self._container_init()
+            self.sysroot = self._container_init()
+
+        self.init_kernel_modules()
 
         if init is not None:
             self.init_system = init
         elif os.path.isdir("/run/systemd/system/"):
-            self.init_system = SystemdInit(chroot=sysroot)
+            self.init_system = SystemdInit(chroot=self.sysroot)
         else:
             self.init_system = InitSystem()
 
@@ -149,27 +148,30 @@ class LinuxPolicy(Policy):
             if os.environ[ENV_CONTAINER] in ['docker', 'oci', 'podman']:
                 self._in_container = True
         if ENV_HOST_SYSROOT in os.environ:
-            self._host_sysroot = os.environ[ENV_HOST_SYSROOT]
-        use_sysroot = self._in_container and self._host_sysroot is not None
+            _host_sysroot = os.environ[ENV_HOST_SYSROOT]
+        use_sysroot = self._in_container and _host_sysroot is not None
         if use_sysroot:
-            host_tmp_dir = os.path.abspath(self._host_sysroot + self._tmp_dir)
+            host_tmp_dir = os.path.abspath(_host_sysroot + self._tmp_dir)
             self._tmp_dir = host_tmp_dir
-        return self._host_sysroot if use_sysroot else None
+        return _host_sysroot if use_sysroot else None
 
     def init_kernel_modules(self):
         """Obtain a list of loaded kernel modules to reference later for plugin
         enablement and SoSPredicate checks
         """
         self.kernel_mods = []
+        release = os.uname().release
 
         # first load modules from lsmod
-        lines = shell_out("lsmod", timeout=0).splitlines()
+        lines = shell_out("lsmod", timeout=0, chroot=self.sysroot).splitlines()
         self.kernel_mods.extend([
             line.split()[0].strip() for line in lines[1:]
         ])
 
         # next, include kernel builtins
-        builtins = "/usr/lib/modules/%s/modules.builtin" % os.uname().release
+        builtins = self.join_sysroot(
+            "/usr/lib/modules/%s/modules.builtin" % release
+        )
         try:
             with open(builtins, "r") as mfile:
                 for line in mfile:
@@ -186,7 +188,7 @@ class LinuxPolicy(Policy):
             'dm_mod': 'CONFIG_BLK_DEV_DM'
         }
 
-        booted_config = "/boot/config-%s" % os.uname().release
+        booted_config = self.join_sysroot("/boot/config-%s" % release)
         kconfigs = []
         try:
             with open(booted_config, "r") as kfile:
@@ -200,6 +202,11 @@ class LinuxPolicy(Policy):
             if config_strings[builtin] in kconfigs:
                 self.kernel_mods.append(builtin)
 
+    def join_sysroot(self, path):
+        if self.sysroot and self.sysroot != '/':
+            path = os.path.join(self.sysroot, path.lstrip('/'))
+        return path
+
     def pre_work(self):
         # this method will be called before the gathering begins
 
diff --git a/sos/policies/distros/debian.py b/sos/policies/distros/debian.py
index 95b389a6..639fd5eb 100644
--- a/sos/policies/distros/debian.py
+++ b/sos/policies/distros/debian.py
@@ -27,7 +27,7 @@ class DebianPolicy(LinuxPolicy):
                  remote_exec=None):
         super(DebianPolicy, self).__init__(sysroot=sysroot, init=init,
                                            probe_runtime=probe_runtime)
-        self.package_manager = DpkgPackageManager(chroot=sysroot,
+        self.package_manager = DpkgPackageManager(chroot=self.sysroot,
                                                   remote_exec=remote_exec)
         self.valid_subclasses += [DebianPlugin]
 
diff --git a/sos/policies/distros/redhat.py b/sos/policies/distros/redhat.py
index eb442407..4b14abaf 100644
--- a/sos/policies/distros/redhat.py
+++ b/sos/policies/distros/redhat.py
@@ -42,7 +42,6 @@ class RedHatPolicy(LinuxPolicy):
     _redhat_release = '/etc/redhat-release'
     _tmp_dir = "/var/tmp"
     _in_container = False
-    _host_sysroot = '/'
     default_scl_prefix = '/opt/rh'
     name_pattern = 'friendly'
     upload_url = None
@@ -57,7 +56,7 @@ class RedHatPolicy(LinuxPolicy):
                                            probe_runtime=probe_runtime)
         self.usrmove = False
 
-        self.package_manager = RpmPackageManager(chroot=sysroot,
+        self.package_manager = RpmPackageManager(chroot=self.sysroot,
                                                  remote_exec=remote_exec)
 
         self.valid_subclasses += [RedHatPlugin]
diff --git a/sos/policies/runtimes/__init__.py b/sos/policies/runtimes/__init__.py
index f28d6a1d..2e60ad23 100644
--- a/sos/policies/runtimes/__init__.py
+++ b/sos/policies/runtimes/__init__.py
@@ -64,7 +64,7 @@ class ContainerRuntime():
         :returns: ``True`` if the runtime is active, else ``False``
         :rtype: ``bool``
         """
-        if is_executable(self.binary):
+        if is_executable(self.binary, self.policy.sysroot):
             self.active = True
             return True
         return False
@@ -78,7 +78,7 @@ class ContainerRuntime():
         containers = []
         _cmd = "%s ps %s" % (self.binary, '-a' if get_all else '')
         if self.active:
-            out = sos_get_command_output(_cmd)
+            out = sos_get_command_output(_cmd, chroot=self.policy.sysroot)
             if out['status'] == 0:
                 for ent in out['output'].splitlines()[1:]:
                     ent = ent.split()
@@ -112,8 +112,10 @@ class ContainerRuntime():
         images = []
         fmt = '{{lower .Repository}}:{{lower .Tag}} {{lower .ID}}'
         if self.active:
-            out = sos_get_command_output("%s images --format '%s'"
-                                         % (self.binary, fmt))
+            out = sos_get_command_output(
+                "%s images --format '%s'" % (self.binary, fmt),
+                chroot=self.policy.sysroot
+            )
             if out['status'] == 0:
                 for ent in out['output'].splitlines():
                     ent = ent.split()
@@ -129,7 +131,10 @@ class ContainerRuntime():
         """
         vols = []
         if self.active:
-            out = sos_get_command_output("%s volume ls" % self.binary)
+            out = sos_get_command_output(
+                "%s volume ls" % self.binary,
+                chroot=self.policy.sysroot
+            )
             if out['status'] == 0:
                 for ent in out['output'].splitlines()[1:]:
                     ent = ent.split()
diff --git a/sos/policies/runtimes/docker.py b/sos/policies/runtimes/docker.py
index 759dfaf6..e81f580e 100644
--- a/sos/policies/runtimes/docker.py
+++ b/sos/policies/runtimes/docker.py
@@ -18,9 +18,9 @@ class DockerContainerRuntime(ContainerRuntime):
     name = 'docker'
     binary = 'docker'
 
-    def check_is_active(self):
+    def check_is_active(self, sysroot=None):
         # the daemon must be running
-        if (is_executable('docker') and
+        if (is_executable('docker', sysroot) and
                 (self.policy.init_system.is_running('docker') or
                  self.policy.init_system.is_running('snap.docker.dockerd'))):
             self.active = True
diff --git a/sos/report/__init__.py b/sos/report/__init__.py
index a4c92acc..a6c72778 100644
--- a/sos/report/__init__.py
+++ b/sos/report/__init__.py
@@ -173,14 +173,12 @@ class SoSReport(SoSComponent):
         self._set_directories()
 
         msg = "default"
-        host_sysroot = self.policy.host_sysroot()
+        self.sysroot = self.policy.sysroot
         # set alternate system root directory
         if self.opts.sysroot:
             msg = "cmdline"
-            self.sysroot = self.opts.sysroot
-        elif self.policy.in_container() and host_sysroot != os.sep:
+        elif self.policy.in_container() and self.sysroot != os.sep:
             msg = "policy"
-            self.sysroot = host_sysroot
         self.soslog.debug("set sysroot to '%s' (%s)" % (self.sysroot, msg))
 
         if self.opts.chroot not in chroot_modes:
diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py
index 46028bb1..e180ae17 100644
--- a/sos/report/plugins/__init__.py
+++ b/sos/report/plugins/__init__.py
@@ -724,7 +724,7 @@ class Plugin():
         """
         if not self.use_sysroot():
             return path
-        if path.startswith(self.sysroot):
+        if self.sysroot and path.startswith(self.sysroot):
             return path[len(self.sysroot):]
         return path
 
@@ -743,8 +743,10 @@ class Plugin():
                   ``False``
         :rtype: ``bool``
         """
-        paths = [self.sysroot, self.archive.get_tmp_dir()]
-        return os.path.commonprefix(paths) == self.sysroot
+        # if sysroot is still None, that implies '/'
+        _sysroot = self.sysroot or '/'
+        paths = [_sysroot, self.archive.get_tmp_dir()]
+        return os.path.commonprefix(paths) == _sysroot
 
     def is_installed(self, package_name):
         """Is the package $package_name installed?
@@ -2621,7 +2623,7 @@ class Plugin():
         return list(set(expanded))
 
     def _collect_copy_specs(self):
-        for path in self.copy_paths:
+        for path in sorted(self.copy_paths, reverse=True):
             self._log_info("collecting path '%s'" % path)
             self._do_copy_path(path)
         self.generate_copyspec_tags()
@@ -2749,7 +2751,7 @@ class Plugin():
 
         return ((any(self.path_exists(fname) for fname in files) or
                 any(self.is_installed(pkg) for pkg in packages) or
-                any(is_executable(cmd) for cmd in commands) or
+                any(is_executable(cmd, self.sysroot) for cmd in commands) or
                 any(self.is_module_loaded(mod) for mod in self.kernel_mods) or
                 any(self.is_service(svc) for svc in services) or
                 any(self.container_exists(cntr) for cntr in containers)) and
@@ -2817,7 +2819,7 @@ class Plugin():
         :returns:           True if the path exists in sysroot, else False
         :rtype:             ``bool``
         """
-        return path_exists(path, self.commons['cmdlineopts'].sysroot)
+        return path_exists(path, self.sysroot)
 
     def path_isdir(self, path):
         """Helper to call the sos.utilities wrapper that allows the
@@ -2830,7 +2832,7 @@ class Plugin():
         :returns:           True if the path is a dir, else False
         :rtype:             ``bool``
         """
-        return path_isdir(path, self.commons['cmdlineopts'].sysroot)
+        return path_isdir(path, self.sysroot)
 
     def path_isfile(self, path):
         """Helper to call the sos.utilities wrapper that allows the
@@ -2843,7 +2845,7 @@ class Plugin():
         :returns:           True if the path is a file, else False
         :rtype:             ``bool``
         """
-        return path_isfile(path, self.commons['cmdlineopts'].sysroot)
+        return path_isfile(path, self.sysroot)
 
     def path_islink(self, path):
         """Helper to call the sos.utilities wrapper that allows the
@@ -2856,7 +2858,7 @@ class Plugin():
         :returns:           True if the path is a link, else False
         :rtype:             ``bool``
         """
-        return path_islink(path, self.commons['cmdlineopts'].sysroot)
+        return path_islink(path, self.sysroot)
 
     def listdir(self, path):
         """Helper to call the sos.utilities wrapper that allows the
@@ -2869,7 +2871,7 @@ class Plugin():
         :returns:           Contents of path, if it is a directory
         :rtype:             ``list``
         """
-        return listdir(path, self.commons['cmdlineopts'].sysroot)
+        return listdir(path, self.sysroot)
 
     def path_join(self, path, *p):
         """Helper to call the sos.utilities wrapper that allows the
diff --git a/sos/report/plugins/unpackaged.py b/sos/report/plugins/unpackaged.py
index 772b1d1f..24203c4b 100644
--- a/sos/report/plugins/unpackaged.py
+++ b/sos/report/plugins/unpackaged.py
@@ -58,10 +58,11 @@ class Unpackaged(Plugin, RedHatPlugin):
             """
             expanded = []
             for f in files:
-                if self.path_islink(f):
-                    expanded.append("{} -> {}".format(f, os.readlink(f)))
+                fp = self.path_join(f)
+                if self.path_islink(fp):
+                    expanded.append("{} -> {}".format(fp, os.readlink(fp)))
                 else:
-                    expanded.append(f)
+                    expanded.append(fp)
             return expanded
 
         # Check command predicate to avoid costly processing
diff --git a/sos/utilities.py b/sos/utilities.py
index b7575153..d6630933 100644
--- a/sos/utilities.py
+++ b/sos/utilities.py
@@ -96,11 +96,15 @@ def grep(pattern, *files_or_paths):
     return matches
 
 
-def is_executable(command):
+def is_executable(command, sysroot=None):
     """Returns if a command matches an executable on the PATH"""
 
     paths = os.environ.get("PATH", "").split(os.path.pathsep)
     candidates = [command] + [os.path.join(p, command) for p in paths]
+    if sysroot:
+        candidates += [
+            os.path.join(sysroot, c.lstrip('/')) for c in candidates
+        ]
     return any(os.access(path, os.X_OK) for path in candidates)
 
 
@@ -216,8 +220,9 @@ def get_human_readable(size, precision=2):
 
 
 def _os_wrapper(path, sysroot, method, module=os.path):
-    if sysroot not in [None, '/']:
-        path = os.path.join(sysroot, path.lstrip('/'))
+    if sysroot and sysroot != os.sep:
+        if not path.startswith(sysroot):
+            path = os.path.join(sysroot, path.lstrip('/'))
     _meth = getattr(module, method)
     return _meth(path)
 
@@ -243,7 +248,7 @@ def listdir(path, sysroot):
 
 
 def path_join(path, *p, sysroot=os.sep):
-    if not path.startswith(sysroot):
+    if sysroot and not path.startswith(sysroot):
         path = os.path.join(sysroot, path.lstrip(os.sep))
     return os.path.join(path, *p)