Add wrapper driver execution context

We recently began altering the mount map used by the wrapper driver
for each execution run (so that we can only include the current
playbook).  However, the setMountsMap method operates on the global
driver object rather than an object more closely bound to the lifetime
of the playbook run.  The fact that this works at all is just luck
(executing process is slow enough that hitting a race condition where
the wrong directories are mounted is unlikely).

To correct this, add a new layer which contains the context for the
current playbook execution.

Change-Id: I3a06f19e88435a49c7b9aea4e1221b812f5a43d0

tests/unit/test_bubblewrap.py

@@ -32,12 +32,13 @@ class TestBubblewrap(testtools.TestCase):
 
     def test_bubblewrap_wraps(self):
         bwrap = bubblewrap.BubblewrapDriver()
+        context = bwrap.getExecutionContext()
         work_dir = tempfile.mkdtemp()
         ssh_agent = SshAgent()
         self.addCleanup(ssh_agent.stop)
         ssh_agent.start()
-        po = bwrap.getPopen(work_dir=work_dir,
-                            ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
+        po = context.getPopen(work_dir=work_dir,
+                              ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
         self.assertTrue(po.passwd_r > 2)
         self.assertTrue(po.group_r > 2)
         self.assertTrue(work_dir in po.command)
@@ -55,14 +56,15 @@ class TestBubblewrap(testtools.TestCase):
 
     def test_bubblewrap_leak(self):
         bwrap = bubblewrap.BubblewrapDriver()
+        context = bwrap.getExecutionContext()
         work_dir = tempfile.mkdtemp()
         ansible_dir = tempfile.mkdtemp()
         ssh_agent = SshAgent()
         self.addCleanup(ssh_agent.stop)
         ssh_agent.start()
-        po = bwrap.getPopen(work_dir=work_dir,
-                            ansible_dir=ansible_dir,
-                            ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
+        po = context.getPopen(work_dir=work_dir,
+                              ansible_dir=ansible_dir,
+                              ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
         leak_time = 60
         # Use hexadecimal notation to avoid false-positive
         true_proc = po(['bash', '-c', 'sleep 0x%X & disown' % leak_time])

zuul/driver/__init__.py

@@ -258,25 +258,19 @@ class WrapperInterface(object, metaclass=abc.ABCMeta):
     """
 
     @abc.abstractmethod
-    def getPopen(self, **kwargs):
-        """Create and return a subprocess.Popen factory wrapped however the
-        driver sees fit.
+    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+        """Create and return an execution context.
 
-        This method is required by the interface
-
-        :arg dict kwargs: key/values for use by driver as needed
-
-        :returns: a callable that takes the same args as subprocess.Popen
-        :rtype: Callable
-        """
-        pass
+        The execution context is meant to be used for a single
+        invocation of a command.
 
-    @abc.abstractmethod
-    def setMountsMap(self, state_dir, ro_paths=None, rw_paths=None):
-        """Add additional mount point to the execution environment.
+        This method is required by the interface
 
-        :arg str state_dir: the state directory to be read write
         :arg list ro_paths: read only files or directories to bind mount
         :arg list rw_paths: read write files or directories to bind mount
+
+        :returns: a new ExecutionContext object.
+        :rtype: BaseExecutionContext
+
         """
         pass

zuul/driver/bubblewrap/__init__.py

@@ -27,6 +27,7 @@ import re
 from typing import Dict, List  # flake8: noqa
 
 from zuul.driver import (Driver, WrapperInterface)
+from zuul.execution_context import BaseExecutionContext
 
 
 class WrappedPopen(object):
@@ -69,27 +70,11 @@ class WrappedPopen(object):
             self.group_r = None
 
 
-class BubblewrapDriver(Driver, WrapperInterface):
-    name = 'bubblewrap'
-    log = logging.getLogger("zuul.BubblewrapDriver")
-
-    mounts_map = {'rw': [], 'ro': []}  # type: Dict[str, List]
-    release_file_re = re.compile('^\W+-release$')
-
-    def __init__(self):
-        self.bwrap_command = self._bwrap_command()
-
-    def reconfigure(self, tenant):
-        pass
+class BubblewrapExecutionContext(BaseExecutionContext):
+    log = logging.getLogger("zuul.BubblewrapExecutionContext")
 
-    def stop(self):
-        pass
-
-    def setMountsMap(self, ro_paths=None, rw_paths=None):
-        if not ro_paths:
-            ro_paths = []
-        if not rw_paths:
-            rw_paths = []
+    def __init__(self, bwrap_command, ro_paths, rw_paths):
+        self.bwrap_command = bwrap_command
         self.mounts_map = {'ro': ro_paths, 'rw': rw_paths}
 
     def getPopen(self, **kwargs):
@@ -145,6 +130,22 @@ class BubblewrapDriver(Driver, WrapperInterface):
 
         return wrapped_popen
 
+
+class BubblewrapDriver(Driver, WrapperInterface):
+    log = logging.getLogger("zuul.BubblewrapDriver")
+    name = 'bubblewrap'
+
+    release_file_re = re.compile('^\W+-release$')
+
+    def __init__(self):
+        self.bwrap_command = self._bwrap_command()
+
+    def reconfigure(self, tenant):
+        pass
+
+    def stop(self):
+        pass
+
     def _bwrap_command(self):
         bwrap_command = [
             'bwrap',
@@ -185,6 +186,15 @@ class BubblewrapDriver(Driver, WrapperInterface):
 
         return bwrap_command
 
+    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+        if not ro_paths:
+            ro_paths = []
+        if not rw_paths:
+            rw_paths = []
+        return BubblewrapExecutionContext(
+            self.bwrap_command,
+            ro_paths, rw_paths)
+
 
 def main(args=None):
     logging.basicConfig(level=logging.DEBUG)
@@ -192,18 +202,19 @@ def main(args=None):
     driver = BubblewrapDriver()
 
     parser = argparse.ArgumentParser()
-    parser.add_argument('--ro-bind', nargs='+')
-    parser.add_argument('--rw-bind', nargs='+')
+    parser.add_argument('--ro-paths', nargs='+')
+    parser.add_argument('--rw-paths', nargs='+')
     parser.add_argument('work_dir')
     parser.add_argument('run_args', nargs='+')
     cli_args = parser.parse_args()
 
     ssh_auth_sock = os.environ.get('SSH_AUTH_SOCK')
 
-    driver.setMountsMap(cli_args.ro_bind, cli_args.rw_bind)
+    context = driver.getExecutionContext(
+        cli_args.ro_paths, cli_args.rw_paths)
 
-    popen = driver.getPopen(work_dir=cli_args.work_dir,
-                            ssh_auth_sock=ssh_auth_sock)
+    popen = context.getPopen(work_dir=cli_args.work_dir,
+                             ssh_auth_sock=ssh_auth_sock)
     x = popen(cli_args.run_args)
     x.wait()
 

zuul/driver/nullwrap/__init__.py

@@ -18,14 +18,19 @@ import logging
 import subprocess
 
 from zuul.driver import (Driver, WrapperInterface)
+from zuul.execution_context import BaseExecutionContext
 
 
-class NullwrapDriver(Driver, WrapperInterface):
-    name = 'nullwrap'
-    log = logging.getLogger("zuul.NullwrapDriver")
+class NullExecutionContext(BaseExecutionContext):
+    log = logging.getLogger("zuul.NullExecutionContext")
 
     def getPopen(self, **kwargs):
         return subprocess.Popen
 
-    def setMountsMap(self, **kwargs):
-        pass
+
+class NullwrapDriver(Driver, WrapperInterface):
+    name = 'nullwrap'
+    log = logging.getLogger("zuul.NullwrapDriver")
+
+    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+        return NullExecutionContext()

zuul/execution_context/__init__.py

@@ -0,0 +1,40 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import abc
+
+
+class BaseExecutionContext(object, metaclass=abc.ABCMeta):
+    """The execution interface returned by a wrapper.
+
+    Wrapper drivers return instances which implement this interface.
+
+    It is used to hold information and aid in the execution of a
+    single command.
+
+    """
+
+    @abc.abstractmethod
+    def getPopen(self, **kwargs):
+        """Create and return a subprocess.Popen factory wrapped however the
+        driver sees fit.
+
+        This method is required by the interface
+
+        :arg dict kwargs: key/values for use by driver as needed
+
+        :returns: a callable that takes the same args as subprocess.Popen
+        :rtype: Callable
+        """
+        pass

zuul/executor/server.py

@@ -1509,10 +1509,10 @@ class AnsibleJob(object):
         if self.executor_variables_file:
             ro_paths.append(self.executor_variables_file)
 
-        self.executor_server.execution_wrapper.setMountsMap(ro_paths,
-                                                            rw_paths)
+        context = self.executor_server.execution_wrapper.getExecutionContext(
+            ro_paths, rw_paths)
 
-        popen = self.executor_server.execution_wrapper.getPopen(
+        popen = context.getPopen(
             work_dir=self.jobdir.work_root,
             ssh_auth_sock=env_copy.get('SSH_AUTH_SOCK'))