Merge branch 'baserock/richardmaw-os/unify-namespace-logic'

This removes the last case of global mounting that could cause issues
for being able to run multiple builds in parallel.

There's still concurrent access to caches and git servers that could
cause issues, but with this, it's possible to run morph in the
test-suite in parallel.

Reviewed-by: Daniel Silverstone
Reviewed-by: Sam Thursfield

5 files changed, 141 insertions, 112 deletions

diff --git a/morphlib/builder2.py b/morphlib/builder2.py
index 8615ed59..f71f21db 100644
--- a/morphlib/builder2.py
+++ b/morphlib/builder2.py
@@ -28,7 +28,6 @@ import time
 import traceback
 import subprocess
 import tempfile
-import textwrap
 import gzip
 
 import cliapp
@@ -647,53 +646,6 @@ class SystemBuilder(BuilderBase):  # pragma: no cover
 
         os.chmod(os_release_file, 0644)
 
-    def _chroot_runcmd(self, rootdir, to_mount, env, *args):
-        # We need to do mounts in a different namespace. Unfortunately
-        # this means we have to in-line the mount commands in the
-        # command-line.
-        command = textwrap.dedent(r'''
-        mount --make-rprivate /
-        rootdir="$1"
-        shift
-        ''')
-        cmdargs = [rootdir]
-
-        # We need to mount all the specified mounts in the namespace,
-        # we don't need to unmount them before exiting, as they'll be
-        # unmounted when the namespace is no longer used.
-        command += textwrap.dedent(r'''
-        while true; do
-            case "$1" in
-            --)
-                shift
-                break
-                ;;
-            *)
-                mount_point="$1"
-                mount_type="$2"
-                mount_source="$3"
-                shift 3
-                path="$rootdir/$mount_point"
-                mount -t "$mount_type" "$mount_source" "$path"
-                ;;
-            esac
-        done
-        ''')
-        for mount_opts in to_mount:
-            cmdargs.extend(mount_opts)
-        cmdargs.append('--')
-
-        command += textwrap.dedent(r'''
-        exec chroot "$rootdir" "$@"
-        ''')
-        cmdargs.extend(args)
-
-        # The single - is just a shell convention to fill $0 when using -c,
-        # since ordinarily $0 contains the program name.
-        cmdline = ['unshare', '--mount', '--', 'sh', '-ec', command, '-']
-        cmdline.extend(cmdargs)
-        self.app.runcmd(cmdline, env=env)
-
     def run_system_integration_commands(self, rootdir):  # pragma: no cover
         ''' Run the system integration commands '''
 
@@ -708,18 +660,16 @@ class SystemBuilder(BuilderBase):  # pragma: no cover
         self.app.status(msg='Running the system integration commands')
 
         to_mount = (
-            ('proc',    'proc',  'none'),
             ('dev/shm', 'tmpfs', 'none'),
             ('tmp',     'tmpfs', 'none'),
         )
         try:
-            for mount_point, mount_type, source in to_mount:
-                path = os.path.join(rootdir, mount_point)
-                if not os.path.exists(path):
-                    os.makedirs(path)
             for bin in sorted(os.listdir(sys_integration_dir)):
-                self._chroot_runcmd(rootdir, to_mount, env,
-                                    os.path.join(SYSTEM_INTEGRATION_PATH, bin))
+                self.app.runcmd(
+                    morphlib.util.containerised_cmdline(
+                        [os.path.join(SYSTEM_INTEGRATION_PATH, bin)],
+                        root=rootdir, mounts=to_mount, mount_proc=True),
+                    env=env)
         except BaseException, e:
             self.app.status(
                     msg='Error while running system integration commands',
diff --git a/morphlib/extensions.py b/morphlib/extensions.py
index 6b81e116..b270d304 100644
--- a/morphlib/extensions.py
+++ b/morphlib/extensions.py
@@ -223,10 +223,7 @@ class ExtensionSubprocess(object):
             def close_read_end():
                 os.close(log_read_fd)
             p = subprocess.Popen(
-                # We unshare and mount --make-rprivate so mounts done by write
-                # extensions can't interfere with the rest of the system.
-                ['unshare', '-m', '--', '/bin/sh', '-c',
-                 'mount --make-rprivate / && exec "$@"', '-', filename] + args,
+                morphlib.util.unshared_cmdline([filename] + list(args)),
                 cwd=cwd, env=new_env,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                 preexec_fn=close_read_end)
diff --git a/morphlib/fsutils.py b/morphlib/fsutils.py
index 751f73f6..6d651171 100644
--- a/morphlib/fsutils.py
+++ b/morphlib/fsutils.py
@@ -104,6 +104,11 @@ def invert_paths(tree_walker, paths):
 
     for dirpath, dirnames, filenames in tree_walker:
 
+        if any(p == dirpath for p in paths): # pragma: no cover
+            # Dir is an exact match for a path
+            # don't recurse any further
+            # Don't yield it, since we don't return listed paths
+            continue
         dn_copy = list(dirnames)
         for subdir in dn_copy:
             subdirpath = os.path.join(dirpath, subdir)
diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py
index 25e33b3f..b676d4db 100644
--- a/morphlib/stagingarea.py
+++ b/morphlib/stagingarea.py
@@ -45,7 +45,6 @@ class StagingArea(object):
         self.dirname = dirname
         self.builddirname = None
         self.destdirname = None
-        self.mounted = []
         self._bind_readonly_mount = None
 
         self.use_chroot = use_chroot
@@ -224,23 +223,6 @@ class StagingArea(object):
             os.makedirs(ccache_destdir)
         return ccache_repodir
 
-    def do_mounts(self, setup_mounts):  # pragma: no cover
-        if not setup_mounts:
-            return
-        for mount_point, mount_type, source in self.to_mount:
-            logging.debug('Mounting %s in staging area' % mount_point)
-            path = os.path.join(self.dirname, mount_point)
-            if not os.path.exists(path):
-                os.makedirs(path)
-            morphlib.fsutils.mount(self._app.runcmd, source, path, mount_type)
-            self.mounted.append(path)
-        return
-
-    def do_unmounts(self):  # pragma: no cover
-        for path in reversed(self.mounted):
-            logging.debug('Unmounting %s in staging area' % path)
-            morphlib.fsutils.unmount(self._app.runcmd, path)
-
     def chroot_open(self, source, setup_mounts): # pragma: no cover
         '''Setup staging area for use as a chroot.'''
 
@@ -251,8 +233,6 @@ class StagingArea(object):
         self.builddirname = builddir
         self.destdirname = destdir
 
-        self.do_mounts(setup_mounts)
-
         return builddir, destdir
 
     def chroot_close(self): # pragma: no cover
@@ -261,22 +241,17 @@ class StagingArea(object):
         This should be called after the staging area is no longer needed.
 
         '''
-
-        self.do_unmounts()
+        # No cleanup is currently required
+        pass
 
     def runcmd(self, argv, **kwargs):  # pragma: no cover
         '''Run a command in a chroot in the staging area.'''
         assert 'env' not in kwargs
-        kwargs['env'] = self.env
+        kwargs['env'] = dict(self.env)
         if 'extra_env' in kwargs:
             kwargs['env'].update(kwargs['extra_env'])
             del kwargs['extra_env']
 
-        if 'cwd' in kwargs:
-            cwd = kwargs['cwd']
-            del kwargs['cwd']
-        else:
-            cwd = '/'
         ccache_dir = kwargs.pop('ccache_dir', None)
 
         chroot_dir = self.dirname if self.use_chroot else '/'
@@ -289,40 +264,28 @@ class StagingArea(object):
                              for d in staging_dirs]
         if not self.use_chroot:
             do_not_mount_dirs += [temp_dir]
-
         logging.debug("Not mounting dirs %r" % do_not_mount_dirs)
 
-        real_argv = ['linux-user-chroot', '--chdir', cwd, '--unshare-net']
-        for d in morphlib.fsutils.invert_paths(os.walk(chroot_dir),
-                                               do_not_mount_dirs):
-            if not os.path.islink(d):
-                real_argv += ['--mount-readonly', self.relative(d)]
-
-        if self.use_chroot:
-            proc_target = os.path.join(self.dirname, 'proc')
-            if not os.path.exists(proc_target):
-                os.makedirs(proc_target)
-            real_argv += ['--mount-proc', self.relative(proc_target)]
-
+        mount_proc = self.use_chroot
         if ccache_dir and not self._app.settings['no-ccache']:
             ccache_target = os.path.join(
                     self.dirname, kwargs['env']['CCACHE_DIR'].lstrip('/'))
-            real_argv += ['--mount-bind', ccache_dir,
-                          self.relative(ccache_target)]
-
-        real_argv += [chroot_dir]
-
-        real_argv += argv
+            binds = ((ccache_dir, ccache_target),)
+        else:
+            binds = ()
 
+        cmdline = morphlib.util.containerised_cmdline(
+                    argv, cwd=kwargs.pop('cwd', '/'),
+                    root=chroot_dir, mounts=self.to_mount,
+                    binds=binds, mount_proc=mount_proc,
+                    writable_paths=do_not_mount_dirs)
         try:
-            if 'logfile' in kwargs and kwargs['logfile'] != None:
-                logfile = kwargs['logfile']
-                del kwargs['logfile']
-
+            if kwargs.get('logfile') != None:
+                logfile = kwargs.pop('logfile')
                 teecmd = ['tee', '-a', logfile]
-                return self._app.runcmd(real_argv, teecmd, **kwargs)
+                return self._app.runcmd(cmdline, teecmd, **kwargs)
             else:
-                return self._app.runcmd(real_argv, **kwargs)
+                return self._app.runcmd(cmdline, **kwargs)
         except cliapp.AppException as e:
             raise cliapp.AppException('In staging area %s: running '
                                       'command \'%s\' failed.' % 
diff --git a/morphlib/util.py b/morphlib/util.py
index cc8ce88d..6f735387 100644
--- a/morphlib/util.py
+++ b/morphlib/util.py
@@ -18,6 +18,7 @@ import itertools
 import os
 import re
 import subprocess
+import textwrap
 
 import fs.osfs
 
@@ -512,3 +513,116 @@ def get_data(relative_path): # pragma: no cover
 
     with open(get_data_path(relative_path)) as f:
         return f.read()
+
+
+def unshared_cmdline(args, root='/', mounts=()): # pragma: no cover
+    '''Describe how to run 'args' inside a separate mount namespace.
+    
+    This function wraps 'args' in a rather long commandline that ensures
+    the subprocess cannot see any of the system's mounts other than those
+    listed in 'mounts', and mounts done by that command can only be seen
+    by that subprocess and its children. When the subprocess exits all
+    of its mounts will be unmounted.
+    
+    '''
+    # We need to do mounts in a different namespace. Unfortunately
+    # this means we have to in-line the mount commands in the
+    # command-line.
+    command = textwrap.dedent(r'''
+    mount --make-rprivate /
+    root="$1"
+    shift
+    ''')
+    cmdargs = [root]
+
+    # We need to mount all the specified mounts in the namespace,
+    # we don't need to unmount them before exiting, as they'll be
+    # unmounted when the namespace is no longer used.
+    command += textwrap.dedent(r'''
+    while true; do
+        case "$1" in
+        --)
+            shift
+            break
+            ;;
+        *)
+            mount_point="$1"
+            mount_type="$2"
+            mount_source="$3"
+            shift 3
+            path="$root/$mount_point"
+            mount -t "$mount_type" "$mount_source" "$path"
+            ;;
+        esac
+    done
+    ''')
+    for mount_point, mount_type, source in mounts:
+        path = os.path.join(root, mount_point)
+        if not os.path.exists(path):
+            os.makedirs(path)
+        cmdargs.extend((mount_point, mount_type, source))
+    cmdargs.append('--')
+
+    command += textwrap.dedent(r'''
+    exec "$@"
+    ''')
+    cmdargs.extend(args)
+
+    # The single - is just a shell convention to fill $0 when using -c,
+    # since ordinarily $0 contains the program name.
+    cmdline = ['unshare', '--mount', '--', 'sh', '-ec', command, '-']
+    cmdline.extend(cmdargs)
+    return cmdline
+
+
+def containerised_cmdline(args, cwd='.', root='/', binds=(),
+                          mount_proc=False, unshare_net=False,
+                          writable_paths=None, **kwargs): # pragma: no cover
+    '''
+    Describe how to run 'args' inside a linux-user-chroot container.
+    
+    The subprocess will only be permitted to write to the paths we
+    specifically allow it to write to, listed in 'writeable paths'. All
+    other locations in the file system will be read-only.
+    
+    The 'root' parameter allows running the command in a chroot, allowing
+    the host file system to be hidden completely except for the paths
+    below 'root'.
+    
+    The 'mount_proc' flag enables mounting of /proc inside 'root'.
+    Locations from the file system can be bind-mounted inside 'root' by
+    setting 'binds' to a list of (src, dest) pairs. The 'dest'
+    directory must be inside 'root'.
+    
+    The 'mounts' parameter allows mounting of arbitrary file-systems,
+    such as tmpfs, before running commands, by setting it to a list of
+    (mount_point, mount_type, source) triples.
+    
+    The subprocess will be run in a separate mount namespace. It can
+    optionally be run in a separate network namespace too by setting
+    'unshare_net'.
+    
+    '''
+
+    if not root.endswith('/'):
+        root += '/'
+    if writable_paths is None:
+        writable_paths = (root,)
+
+    cmdargs = ['linux-user-chroot', '--chdir', cwd]
+    if unshare_net:
+        cmdargs.append('--unshare-net')
+    for src, dst in binds:
+        # linux-user-chroot's mount target paths are relative to the chroot
+        cmdargs.extend(('--mount-bind', src, os.path.relpath(dst, root)))
+    for d in morphlib.fsutils.invert_paths(os.walk(root), writable_paths):
+        if not os.path.islink(d):
+            cmdargs.extend(('--mount-readonly', os.path.relpath(d, root)))
+    if mount_proc:
+        proc_target = os.path.join(root, 'proc')
+        if not os.path.exists(proc_target):
+            os.makedirs(proc_target)
+        cmdargs.extend(('--mount-proc', 'proc'))
+    cmdargs.append(root)
+    cmdargs.extend(args)
+    return unshared_cmdline(cmdargs, root=root, **kwargs)