summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Maw <richard.maw@codethink.co.uk>2018-10-08 11:34:29 +0100
committerRichard Maw <richard.maw@codethink.co.uk>2018-11-14 13:30:34 +0000
commite7633500995376be9d727c902408f03180d56169 (patch)
tree8846718150c1d17fa5257a83f376a2f0d71b3a98
parentda735e56e77846f6ea6e80809fc0bd5de7b3b657 (diff)
downloadbuildstream-e7633500995376be9d727c902408f03180d56169.tar.gz
buildstream/sandbox/_sandboxbwrap.py: Distinguish sandbox failure from command failure
If `bwrap` fails to set up the sandbox and start the payload command it won't write an exit-code in --json-status-fd, so we can report if it was a sandboxing failure if we don't get exit-code status and a payload command failure if we do and it's non-zero. Closes https://gitlab.com/BuildStream/buildstream/issues/286
-rw-r--r--buildstream/_platform/linux.py3
-rw-r--r--buildstream/sandbox/_sandboxbwrap.py69
2 files changed, 52 insertions, 20 deletions
diff --git a/buildstream/_platform/linux.py b/buildstream/_platform/linux.py
index be1e9d933..33f3966c1 100644
--- a/buildstream/_platform/linux.py
+++ b/buildstream/_platform/linux.py
@@ -44,10 +44,12 @@ class Linux(Platform):
self._bwrap_exists = False
self._have_good_bwrap = False
self._die_with_parent_available = False
+ self._json_status_available = False
else:
self._bwrap_exists = True
self._have_good_bwrap = (0, 1, 2) <= bwrap_version
self._die_with_parent_available = (0, 1, 8) <= bwrap_version
+ self._json_status_available = (0, 3, 2) <= bwrap_version
self._local_sandbox_available = self._have_fuse and self._have_good_bwrap
@@ -97,6 +99,7 @@ class Linux(Platform):
# Inform the bubblewrap sandbox as to whether it can use user namespaces or not
kwargs['user_ns_available'] = self._user_ns_available
kwargs['die_with_parent_available'] = self._die_with_parent_available
+ kwargs['json_status_available'] = self._json_status_available
return SandboxBwrap(*args, **kwargs)
def _check_user_ns_available(self):
diff --git a/buildstream/sandbox/_sandboxbwrap.py b/buildstream/sandbox/_sandboxbwrap.py
index 066932f5c..839780f95 100644
--- a/buildstream/sandbox/_sandboxbwrap.py
+++ b/buildstream/sandbox/_sandboxbwrap.py
@@ -17,6 +17,8 @@
# Authors:
# Andrew Leeming <andrew.leeming@codethink.co.uk>
# Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
+import collections
+import json
import os
import sys
import time
@@ -24,7 +26,8 @@ import errno
import signal
import subprocess
import shutil
-from contextlib import ExitStack
+from contextlib import ExitStack, suppress
+from tempfile import TemporaryFile
import psutil
@@ -53,6 +56,7 @@ class SandboxBwrap(Sandbox):
super().__init__(*args, **kwargs)
self.user_ns_available = kwargs['user_ns_available']
self.die_with_parent_available = kwargs['die_with_parent_available']
+ self.json_status_available = kwargs['json_status_available']
def run(self, command, flags, *, cwd=None, env=None):
stdout, stderr = self._get_output()
@@ -160,24 +164,31 @@ class SandboxBwrap(Sandbox):
gid = self._get_config().build_gid
bwrap_command += ['--uid', str(uid), '--gid', str(gid)]
- # Add the command
- bwrap_command += command
-
- # bwrap might create some directories while being suid
- # and may give them to root gid, if it does, we'll want
- # to clean them up after, so record what we already had
- # there just in case so that we can safely cleanup the debris.
- #
- existing_basedirs = {
- directory: os.path.exists(os.path.join(root_directory, directory))
- for directory in ['tmp', 'dev', 'proc']
- }
-
- # Use the MountMap context manager to ensure that any redirected
- # mounts through fuse layers are in context and ready for bwrap
- # to mount them from.
- #
with ExitStack() as stack:
+ pass_fds = ()
+ # Improve error reporting with json-status if available
+ if self.json_status_available:
+ json_status_file = stack.enter_context(TemporaryFile())
+ pass_fds = (json_status_file.fileno(),)
+ bwrap_command += ['--json-status-fd', str(json_status_file.fileno())]
+
+ # Add the command
+ bwrap_command += command
+
+ # bwrap might create some directories while being suid
+ # and may give them to root gid, if it does, we'll want
+ # to clean them up after, so record what we already had
+ # there just in case so that we can safely cleanup the debris.
+ #
+ existing_basedirs = {
+ directory: os.path.exists(os.path.join(root_directory, directory))
+ for directory in ['tmp', 'dev', 'proc']
+ }
+
+ # Use the MountMap context manager to ensure that any redirected
+ # mounts through fuse layers are in context and ready for bwrap
+ # to mount them from.
+ #
stack.enter_context(mount_map.mounted(self))
# If we're interactive, we want to inherit our stdin,
@@ -190,7 +201,7 @@ class SandboxBwrap(Sandbox):
# Run bubblewrap !
exit_code = self.run_bwrap(bwrap_command, stdin, stdout, stderr,
- (flags & SandboxFlags.INTERACTIVE))
+ (flags & SandboxFlags.INTERACTIVE), pass_fds)
# Cleanup things which bwrap might have left behind, while
# everything is still mounted because bwrap can be creating
@@ -238,10 +249,27 @@ class SandboxBwrap(Sandbox):
# a bug, bwrap mounted a tempfs here and when it exits, that better be empty.
pass
+ if self.json_status_available:
+ json_status_file.seek(0, 0)
+ child_exit_code = None
+ # The JSON status file's output is a JSON object per line
+ # with the keys present identifying the type of message.
+ # The only message relevant to us now is the exit-code of the subprocess.
+ for line in json_status_file:
+ with suppress(json.decoder.JSONDecodeError):
+ o = json.loads(line)
+ if isinstance(o, collections.abc.Mapping) and 'exit-code' in o:
+ child_exit_code = o['exit-code']
+ break
+ if child_exit_code is None:
+ raise SandboxError("`bwrap' terminated during sandbox setup with exitcode {}".format(exit_code),
+ reason="bwrap-sandbox-fail")
+ exit_code = child_exit_code
+
self._vdir._mark_changed()
return exit_code
- def run_bwrap(self, argv, stdin, stdout, stderr, interactive):
+ def run_bwrap(self, argv, stdin, stdout, stderr, interactive, pass_fds):
# Wrapper around subprocess.Popen() with common settings.
#
# This function blocks until the subprocess has terminated.
@@ -317,6 +345,7 @@ class SandboxBwrap(Sandbox):
# The default is to share file descriptors from the parent process
# to the subprocess, which is rarely good for sandboxing.
close_fds=True,
+ pass_fds=pass_fds,
stdin=stdin,
stdout=stdout,
stderr=stderr,