[Notes] [Git][BuildStream/buildstream][richardmaw/distinguish-sandboxing-build-fail] 4 commits: _platform/linux.py: Move get_bwrap_version into _site.py

[richardmaw-codethink <gitlab mg gitlab com>]

Title: GitLab

richardmaw-codethink pushed to branch richardmaw/distinguish-sandboxing-build-fail at BuildStream / buildstream

Commits:

• 1eeb84b4
  by Richard Maw at 2018-11-08T17:35:00Z

  _platform/linux.py: Move get_bwrap_version into _site.py
  
  It's inconvenient to have to create a Linux platform to parse the bwrap version
  and we want to get the version in a consistent manner.
  

• a4d032a3
  by Richard Maw at 2018-11-08T17:35:00Z

  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
  

• fea2f55c
  by Richard Maw at 2018-11-08T17:35:00Z

  tests/testutils/site.py: Check for bwrap supporting --json-status-fd
  

• 06cfa0c6
  by Richard Maw at 2018-11-08T17:35:00Z

  tests/integration/sandbox-bwrap.py: Test distinguishing sandbox exit code from command
  

8 changed files:

• buildstream/_platform/linux.py
• buildstream/_site.py
• buildstream/sandbox/_sandboxbwrap.py
• + tests/integration/project/elements/sandbox-bwrap/break-shell.bst
• + tests/integration/project/elements/sandbox-bwrap/command-exit-42.bst
• + tests/integration/project/elements/sandbox-bwrap/non-executable-shell.bst
• tests/integration/sandbox-bwrap.py
• tests/testutils/site.py

Changes:

buildstream/_platform/linux.py

---

...	...	@@ -18,9 +18,9 @@
18	18	# Tristan Maat <tristan maat codethink co uk>
19	19
20	20	import os
21		-import shutil
22	21	import subprocess
23	22
	23	+from .. import _site
24	24	from .. import utils
25	25	from ..sandbox import SandboxDummy
26	26
...	...	@@ -38,16 +38,18 @@ class Linux(Platform):
38	38
39	39	self._have_fuse = os.path.exists("/dev/fuse")
40	40
41		- bwrap_version = self._get_bwrap_version()
	41	+ bwrap_version = _site.get_bwrap_version()
42	42
43	43	if bwrap_version is None:
44	44	self._bwrap_exists = False
45	45	self._have_good_bwrap = False
46	46	self._die_with_parent_available = False
	47	+ self._json_status_available = False
47	48	else:
48	49	self._bwrap_exists = True
49	50	self._have_good_bwrap = (0, 1, 2) <= bwrap_version
50	51	self._die_with_parent_available = (0, 1, 8) <= bwrap_version
	52	+ self._json_status_available = (0, 3, 2) <= bwrap_version
51	53
52	54	self._local_sandbox_available = self._have_fuse and self._have_good_bwrap
53	55
...	...	@@ -97,6 +99,7 @@ class Linux(Platform):
97	99	# Inform the bubblewrap sandbox as to whether it can use user namespaces or not
98	100	kwargs['user_ns_available'] = self._user_ns_available
99	101	kwargs['die_with_parent_available'] = self._die_with_parent_available
	102	+ kwargs['json_status_available'] = self._json_status_available
100	103	return SandboxBwrap(*args, **kwargs)
101	104
102	105	def _check_user_ns_available(self):
...	...	@@ -119,21 +122,3 @@ class Linux(Platform):
119	122	output = ''
120	123
121	124	return output == 'root'
122		-
123		- def _get_bwrap_version(self):
124		- # Get the current bwrap version
125		- #
126		- # returns None if no bwrap was found
127		- # otherwise returns a tuple of 3 int: major, minor, patch
128		- bwrap_path = shutil.which('bwrap')
129		-
130		- if not bwrap_path:
131		- return None
132		-
133		- cmd = [bwrap_path, "--version"]
134		- try:
135		- version = str(subprocess.check_output(cmd).split()[1], "utf-8")
136		- except subprocess.CalledProcessError:
137		- return None
138		-
139		- return tuple(int(x) for x in version.split("."))

buildstream/_site.py

---

...	...	@@ -18,6 +18,8 @@
18	18	# Tristan Van Berkom <tristan vanberkom codethink co uk>
19	19
20	20	import os
	21	+import shutil
	22	+import subprocess
21	23
22	24	#
23	25	# Private module declaring some info about where the buildstream
...	...	@@ -44,3 +46,22 @@ build_all_template = os.path.join(root, 'data', 'build-all.sh.in')
44	46
45	47	# Module building script template
46	48	build_module_template = os.path.join(root, 'data', 'build-module.sh.in')
	49	+
	50	+
	51	+def get_bwrap_version():
	52	+ # Get the current bwrap version
	53	+ #
	54	+ # returns None if no bwrap was found
	55	+ # otherwise returns a tuple of 3 int: major, minor, patch
	56	+ bwrap_path = shutil.which('bwrap')
	57	+
	58	+ if not bwrap_path:
	59	+ return None
	60	+
	61	+ cmd = [bwrap_path, "--version"]
	62	+ try:
	63	+ version = str(subprocess.check_output(cmd).split()[1], "utf-8")
	64	+ except subprocess.CalledProcessError:
	65	+ return None
	66	+
	67	+ return tuple(int(x) for x in version.split("."))

buildstream/sandbox/_sandboxbwrap.py

---

...	...	@@ -17,6 +17,8 @@
17	17	# Authors:
18	18	# Andrew Leeming <andrew leeming codethink co uk>
19	19	# Tristan Van Berkom <tristan vanberkom codethink co uk>
	20	+import collections
	21	+import json
20	22	import os
21	23	import sys
22	24	import time
...	...	@@ -24,7 +26,8 @@ import errno
24	26	import signal
25	27	import subprocess
26	28	import shutil
27		-from contextlib import ExitStack
	29	+from contextlib import ExitStack, suppress
	30	+from tempfile import TemporaryFile
28	31
29	32	import psutil
30	33
...	...	@@ -53,6 +56,7 @@ class SandboxBwrap(Sandbox):
53	56	super().__init__(*args, **kwargs)
54	57	self.user_ns_available = kwargs['user_ns_available']
55	58	self.die_with_parent_available = kwargs['die_with_parent_available']
	59	+ self.json_status_available = kwargs['json_status_available']
56	60
57	61	def run(self, command, flags, *, cwd=None, env=None):
58	62	stdout, stderr = self._get_output()
...	...	@@ -160,24 +164,31 @@ class SandboxBwrap(Sandbox):
160	164	gid = self._get_config().build_gid
161	165	bwrap_command += ['--uid', str(uid), '--gid', str(gid)]
162	166
163		- # Add the command
164		- bwrap_command += command
165		-
166		- # bwrap might create some directories while being suid
167		- # and may give them to root gid, if it does, we'll want
168		- # to clean them up after, so record what we already had
169		- # there just in case so that we can safely cleanup the debris.
170		- #
171		- existing_basedirs = {
172		- directory: os.path.exists(os.path.join(root_directory, directory))
173		- for directory in ['tmp', 'dev', 'proc']
174		- }
175		-
176		- # Use the MountMap context manager to ensure that any redirected
177		- # mounts through fuse layers are in context and ready for bwrap
178		- # to mount them from.
179		- #
180	167	with ExitStack() as stack:
	168	+ pass_fds = ()
	169	+ # Improve error reporting with json-status if available
	170	+ if self.json_status_available:
	171	+ json_status_file = stack.enter_context(TemporaryFile())
	172	+ pass_fds = (json_status_file.fileno(),)
	173	+ bwrap_command += ['--json-status-fd', str(json_status_file.fileno())]
	174	+
	175	+ # Add the command
	176	+ bwrap_command += command
	177	+
	178	+ # bwrap might create some directories while being suid
	179	+ # and may give them to root gid, if it does, we'll want
	180	+ # to clean them up after, so record what we already had
	181	+ # there just in case so that we can safely cleanup the debris.
	182	+ #
	183	+ existing_basedirs = {
	184	+ directory: os.path.exists(os.path.join(root_directory, directory))
	185	+ for directory in ['tmp', 'dev', 'proc']
	186	+ }
	187	+
	188	+ # Use the MountMap context manager to ensure that any redirected
	189	+ # mounts through fuse layers are in context and ready for bwrap
	190	+ # to mount them from.
	191	+ #
181	192	stack.enter_context(mount_map.mounted(self))
182	193
183	194	# If we're interactive, we want to inherit our stdin,
...	...	@@ -190,7 +201,7 @@ class SandboxBwrap(Sandbox):
190	201
191	202	# Run bubblewrap !
192	203	exit_code = self.run_bwrap(bwrap_command, stdin, stdout, stderr,
193		- (flags & SandboxFlags.INTERACTIVE))
	204	+ (flags & SandboxFlags.INTERACTIVE), pass_fds)
194	205
195	206	# Cleanup things which bwrap might have left behind, while
196	207	# everything is still mounted because bwrap can be creating
...	...	@@ -238,10 +249,24 @@ class SandboxBwrap(Sandbox):
238	249	# a bug, bwrap mounted a tempfs here and when it exits, that better be empty.
239	250	pass
240	251
	252	+ if self.json_status_available:
	253	+ json_status_file.seek(0, 0)
	254	+ child_exit_code = None
	255	+ for line in json_status_file:
	256	+ with suppress(json.decoder.JSONDecodeError):
	257	+ o = json.loads(line)
	258	+ if isinstance(o, collections.abc.Mapping) and 'exit-code' in o:
	259	+ child_exit_code = o['exit-code']
	260	+ break
	261	+ if child_exit_code is None:
	262	+ raise SandboxError("`bwrap' terminated during sandbox setup with exitcode {}".format(exit_code),
	263	+ reason="bwrap-sandbox-fail")
	264	+ exit_code = child_exit_code
	265	+
241	266	self._vdir._mark_changed()
242	267	return exit_code
243	268
244		- def run_bwrap(self, argv, stdin, stdout, stderr, interactive):
	269	+ def run_bwrap(self, argv, stdin, stdout, stderr, interactive, pass_fds):
245	270	# Wrapper around subprocess.Popen() with common settings.
246	271	#
247	272	# This function blocks until the subprocess has terminated.
...	...	@@ -317,6 +342,7 @@ class SandboxBwrap(Sandbox):
317	342	# The default is to share file descriptors from the parent process
318	343	# to the subprocess, which is rarely good for sandboxing.
319	344	close_fds=True,
	345	+ pass_fds=pass_fds,
320	346	stdin=stdin,
321	347	stdout=stdout,
322	348	stderr=stderr,

tests/integration/project/elements/sandbox-bwrap/break-shell.bst

---

	1	+kind: manual
	2	+depends:
	3	+ - base/base-alpine.bst
	4	+
	5	+public:
	6	+ bst:
	7	+ integration-commands:
	8	+ - |
	9	+ chmod a-x /bin/sh

tests/integration/project/elements/sandbox-bwrap/command-exit-42.bst

---

	1	+kind: manual
	2	+depends:
	3	+ - base/base-alpine.bst
	4	+
	5	+config:
	6	+ build-commands:
	7	+ - |
	8	+ exit 42

tests/integration/project/elements/sandbox-bwrap/non-executable-shell.bst

---

	1	+kind: manual
	2	+
	3	+depends:
	4	+ - sandbox-bwrap/break-shell.bst
	5	+
	6	+config:
	7	+ build-commands:
	8	+ - |
	9	+ exit 42

tests/integration/sandbox-bwrap.py

---

1	1	import os
2	2	import pytest
3	3
	4	+from buildstream._exceptions import ErrorDomain
	5	+
4	6	from tests.testutils import cli_integration as cli
5	7	from tests.testutils.integration import assert_contains
6		-from tests.testutils.site import HAVE_BWRAP
	8	+from tests.testutils.site import HAVE_BWRAP, HAVE_BWRAP_JSON_STATUS
7	9
8	10
9	11	pytestmark = pytest.mark.integration
...	...	@@ -29,3 +31,32 @@ def test_sandbox_bwrap_cleanup_build(cli, tmpdir, datafiles):
29	31	# Here, BuildStream should not attempt any rmdir etc.
30	32	result = cli.run(project=project, args=['build', element_name])
31	33	assert result.exit_code == 0
	34	+
	35	+
	36	+@pytest.mark.skipif(not HAVE_BWRAP, reason='Only available with bubblewrap')
	37	+@pytest.mark.skipif(not HAVE_BWRAP_JSON_STATUS, reason='Only available with bubblewrap supporting --json-status-fd')
	38	+@pytest.mark.datafiles(DATA_DIR)
	39	+def test_sandbox_bwrap_distinguish_setup_error(cli, tmpdir, datafiles):
	40	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	41	+ element_name = 'sandbox-bwrap/non-executable-shell.bst'
	42	+
	43	+ result = cli.run(project=project, args=['build', element_name])
	44	+ result.assert_task_error(error_domain=ErrorDomain.SANDBOX, error_reason="bwrap-sandbox-fail")
	45	+
	46	+
	47	+@pytest.mark.integration
	48	+@pytest.mark.skipif(not HAVE_BWRAP, reason='Only available with bubblewrap')
	49	+@pytest.mark.datafiles(DATA_DIR)
	50	+def test_sandbox_bwrap_return_subprocess(cli, tmpdir, datafiles):
	51	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	52	+ element_name = 'sandbox-bwrap/command-exit-42.bst'
	53	+
	54	+ cli.configure({
	55	+ "logging": {
	56	+ "message-format": "%{element}|%{message}",
	57	+ },
	58	+ })
	59	+
	60	+ result = cli.run(project=project, args=['build', element_name])
	61	+ result.assert_task_error(error_domain=ErrorDomain.ELEMENT, error_reason=None)
	62	+ assert "sandbox-bwrap/command-exit-42.bst|Command 'exit 42' failed with exitcode 42" in result.stderr

tests/testutils/site.py

---

...	...	@@ -4,7 +4,7 @@
4	4	import os
5	5	import sys
6	6
7		-from buildstream import utils, ProgramNotFoundError
	7	+from buildstream import _site, utils, ProgramNotFoundError
8	8
9	9	try:
10	10	utils.get_host_tool('bzr')
...	...	@@ -33,8 +33,10 @@ except (ImportError, ValueError):
33	33	try:
34	34	utils.get_host_tool('bwrap')
35	35	HAVE_BWRAP = True
	36	+ HAVE_BWRAP_JSON_STATUS = _site.get_bwrap_version() >= (0, 3, 2)
36	37	except ProgramNotFoundError:
37	38	HAVE_BWRAP = False
	39	+ HAVE_BWRAP_JSON_STATUS = False
38	40
39	41	try:
40	42	utils.get_host_tool('lzip')

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.