Refactor NS resolver logic for clarity and consistency

The NS resolver munging was previously scattered across multiple
locations in the Mock codebase.  This duplication made the logic
difficult to follow and led to bugs, such as the one addressed
in rpm-software-management#1697.  This change simplifies and consolidates the code.

Follow-up-for: rpm-software-management#1697

Author: praiskup

mock/py/mock.py

@@ -766,7 +766,6 @@ def main():
         # allow bootstrap buildroot to access the network for getting packages
         bootstrap_buildroot_config['rpmbuild_networking'] = True
         bootstrap_buildroot_config['use_host_resolv'] = True
-        util.setup_host_resolv(bootstrap_buildroot_config)
 
         # disable updating bootstrap chroot
         bootstrap_buildroot_config['update_before_build'] = False
@@ -812,10 +811,6 @@ def main():
             mount_point = BindMountPoint(srcpath=key_dir, bindpath=chroot_dir)
             bootstrap_buildroot.mounts.add(mount_point)
 
-    # this changes config_opts['nspawn_args'], so do it after initializing
-    # bootstrap chroot to not inherit the changes there
-    util.setup_host_resolv(config_opts)
-
     buildroot = Buildroot(config_opts, uidManager, state, plugins, bootstrap_buildroot)
 
     for baseurl in options.repos:

mock/py/mockbuild/buildroot.py

@@ -14,6 +14,7 @@
 import stat
 import tempfile
 import uuid
+from textwrap import dedent
 
 from . import file_util
 from . import mounts
@@ -491,9 +492,28 @@ def _copy_config(self, filename, symlink=False, warn=True):
 
     @traceLog()
     def _setup_resolver_config(self):
+        """
+        There are a few things to think about when reading/modifying this
+        method.
+        """
         if self.config['use_host_resolv'] and self.config['rpmbuild_networking']:
             self._copy_config('resolv.conf')
             self._copy_config('hosts')
+        else:
+            # If we don't copy host's resolv.conf, we at least want to resolve
+            # our own hostname.  See commit 28027fc26d.
+            if 'etc/hosts' not in self.config['files']:
+                self.config['files']['etc/hosts'] = dedent('''\
+                    127.0.0.1 localhost localhost.localdomain
+                    ::1       localhost localhost.localdomain localhost6 localhost6.localdomain6
+                    ''')
+            # We want to have empty resolv.conf to speedup name resolution
+            # failure (see commit 3f939785bb).
+            rconf = self.make_chroot_path("/etc/resolv.conf")
+            file_util.unlink_if_exists(rconf)
+            file_util.touch(rconf)
+
+        util.temporary_nspawn_resolver_hack(self.config)
 
     @traceLog()
     def _setup_katello_ca(self):

mock/py/mockbuild/util.py

@@ -27,7 +27,6 @@
 import tempfile
 # pylint: disable=wrong-import-order
 import termios
-from textwrap import dedent
 import time
 import uuid
 
@@ -782,9 +781,6 @@ def _prepare_nspawn_command(chrootPath, user, cmd, nspawn_args=None, env=None,
     if not USE_NSPAWN_SECCOMP:
         env['SYSTEMD_SECCOMP'] = '0'
 
-    if _check_nspawn_resolv_conf():
-        nspawn_argv.append("--resolv-conf=off")
-
     # The '/bin/sh -c' wrapper is explicitly requested (--shell).  In this case
     # we shrink the list of arguments into one shell command, so the command is
     # completely shell-expanded.
@@ -859,30 +855,21 @@ def clean_env():
 
 
 @traceLog()
-def setup_host_resolv(config_opts):
-    log = getLog()
-    if not config_opts['use_host_resolv']:
-        # If we don't copy host's resolv.conf, we at least want to resolve
-        # our own hostname.  See commit 28027fc26d.
-        if 'etc/hosts' not in config_opts['files']:
-            config_opts['files']['etc/hosts'] = dedent('''\
-                127.0.0.1 localhost localhost.localdomain
-                ::1       localhost localhost.localdomain localhost6 localhost6.localdomain6
-                ''')
-
+def temporary_nspawn_resolver_hack(config_opts):
+    """
+    Remove once we stop supporting RHEL 8 hosts.  RHEL 9+ has --resolv-conf=off
+    which can be added into 'nspawn_args' unconditionally (even if nspawn is
+    unused).
+    """
     if not USE_NSPAWN:
-        # Not using nspawn -> don't touch /etc/resolv.conf; we already have
-        # a valid file prepared by Buildroot._init() (if user requested).
+        # Not using nspawn, no need for additional hacks.
         return
 
-    if config_opts['rpmbuild_networking'] and not config_opts['use_host_resolv']:
-        # keep the default systemd-nspawn's /etc/resolv.conf
+    if _check_nspawn_resolv_conf():
+        # do not let nspawn override the files we create above
+        config_opts["nspawn_args"] += ["--resolv-conf=off"]
         return
 
-    # Either we want to have empty resolv.conf to speedup name resolution
-    # failure (rpmbuild_networking is off, see commit 3f939785bb), or we want
-    # to copy hosts resolv.conf file.
-
     resolv_path = (tempfile.mkstemp(prefix="mock-resolv."))[1]
     atexit.register(_nspawnTempResolvAtExit, resolv_path)
 
@@ -893,9 +880,10 @@ def setup_host_resolv(config_opts):
         try:
             shutil.copyfile('/etc/resolv.conf', resolv_path)
         except FileNotFoundError:
+            log = getLog()
             log.warning("Non-existing resolv.conf on host, using an empty one")
 
-    config_opts['nspawn_args'] += ['--bind={0}:/etc/resolv.conf'.format(resolv_path)]
+    config_opts['nspawn_args'] += [f'--bind={resolv_path}:/etc/resolv.conf']
 
 
 def pretty_getcwd():

releng/release-notes-next/resolv-conf-hacks-on-one-place.bugfix.md

@@ -0,0 +1,4 @@
+The NS resolver munging was previously scattered across multiple locations in
+the Mock codebase.  This duplication made the logic difficult to follow and led
+to bugs, such as the one addressed in [PR#1697][].  This change simplifies and
+consolidates the code.