Merge lp:~ltrager/maas/combined_result_refresh into lp:~maas-committers/maas/trunk

[Lee Trager]

Send combined commissioning results in addtion to stdout and stderr when refreshing.

[Andres Rodriguez (andreserl)]

lgtm! it mostly seems moving stuff around.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~ltrager/maas/combined_result_refresh into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Get:4 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Fetched 306 kB in 0s (612 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1).
python-net...

=== modified file 'src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py'
--- src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py	2017-03-02 21:53:33 +0000
+++ src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py	2017-03-03 03:26:15 +0000
@@ -4,7 +4,6 @@
 from io import BytesIO
 import json
 import os
-import selectors
 from subprocess import (
     PIPE,
     Popen,
@@ -17,11 +16,6 @@
 )
 import time
 
-# See fcntl(2), re. F_SETPIPE_SZ. By requesting this many bytes from a pipe on
-# each read we can be sure that we are always draining its buffer completely.
-with open("/proc/sys/fs/pipe-max-size") as _pms:
-    PIPE_MAX_SIZE = int(_pms.read())
-
 
 try:
     from maas_api_helper import (
@@ -30,6 +24,7 @@
         read_config,
         signal,
         SignalException,
+        capture_script_output,
     )
 except ImportError:
     # For running unit tests.
@@ -39,6 +34,7 @@
         read_config,
         signal,
         SignalException,
+        capture_script_output,
     )
 
 
@@ -66,51 +62,6 @@
         tar.extractall(scripts_dir)
 
 
-def capture_script_output(proc, combined_path, stdout_path, stderr_path):
-    """Capture stdout and stderr from `proc`.
-
-    Standard output is written to a file named by `stdout_path`, and standard
-    error is written to a file named by `stderr_path`. Both are also written
-    to a file named by `combined_path`.
-
-    If the given subprocess forks additional processes, and these write to the
-    same stdout and stderr, their output will be captured only as long as
-    `proc` is running.
-
-    :return: The exit code of `proc`.
-    """
-    with open(stdout_path, 'wb') as out, open(stderr_path, 'wb') as err:
-        with open(combined_path, 'wb') as combined:
-            with selectors.DefaultSelector() as selector:
-                selector.register(proc.stdout, selectors.EVENT_READ, out)
-                selector.register(proc.stderr, selectors.EVENT_READ, err)
-                while selector.get_map() and proc.poll() is None:
-                    # Select with a short timeout so that we don't tight loop.
-                    _select_script_output(selector, combined, 0.1)
-                else:
-                    # Process has finished or has closed stdout and stderr.
-                    # Process anything still sitting in the latter's buffers.
-                    _select_script_output(selector, combined, 0.0)
-
-    # Always wait for the process to finish.
-    return proc.wait()
-
-
-def _select_script_output(selector, combined, timeout):
-    """Helper for `capture_script_output`."""
-    for key, event in selector.select(timeout):
-        if event & selectors.EVENT_READ:
-            # Read from the _raw_ file. Ordinarily Python blocks until a
-            # read(n) returns n bytes or the stream reaches end-of-file,
-            # but here we only want to get what's there without blocking.
-            chunk = key.fileobj.raw.read(PIPE_MAX_SIZE)
-            if len(chunk) == 0:  # EOF
-                selector.unregister(key.fileobj)
-            else:
-                key.data.write(chunk)
-                combined.write(chunk)
-
-
 def run_scripts(url, creds, scripts_dir, out_dir, scripts):
     """Run and report results for the given scripts."""
     fail_count = 0

=== modified file 'src/metadataserver/user_data/templates/snippets/tests/test_maas_run_remote_scripts.py'
--- src/metadataserver/user_data/templates/snippets/tests/test_maas_run_remote_scripts.py	2017-03-02 21:53:33 +0000
+++ src/metadataserver/user_data/templates/snippets/tests/test_maas_run_remote_scripts.py	2017-03-03 03:26:15 +0000
@@ -8,17 +8,10 @@
 from io import BytesIO
 import json
 import os
-from pathlib import Path
 import random
-from subprocess import (
-    PIPE,
-    Popen,
-)
 import tarfile
-import time
 from unittest.mock import ANY
 
-from lxml import etree
 from maastesting.factory import factory
 from maastesting.fixtures import TempDirectory
 from maastesting.matchers import (
@@ -29,16 +22,10 @@
 from maastesting.testcase import MAASTestCase
 from snippets import maas_run_remote_scripts
 from snippets.maas_run_remote_scripts import (
-    capture_script_output,
     download_and_extract_tar,
     run_scripts,
     run_scripts_from_metadata,
 )
-from testtools.matchers import (
-    Equals,
-    MatchesAny,
-    MatchesListwise,
-)
 
 
 class TestMaasRunRemoteScripts(MAASTestCase):
@@ -302,105 +289,3 @@
             mock_signal,
             MockAnyCall(
                 None, None, 'FAILED', '1 scripts failed to run'))
-
-
-class TestCaptureScriptOutput(MAASTestCase):
-
-    # Iterate multiple times to shake out spurious failures.
-    scenarios = [
-        ("iteration %d" % iteration, {})
-        for iteration in range(1, 21)
-    ]
-
-    def capture(self, proc):
-        scripts_dir = Path(self.useFixture(TempDirectory()).path)
-        combined_path = scripts_dir.joinpath("combined")
-        stdout_path = scripts_dir.joinpath("stdout")
-        stderr_path = scripts_dir.joinpath("stderr")
-
-        returncode = capture_script_output(
-            proc, str(combined_path), str(stdout_path), str(stderr_path))
-
-        return (
-            returncode,
-            stdout_path.read_text(),
-            stderr_path.read_text(),
-            combined_path.read_text(),
-        )
-
-    def test__captures_script_output(self):
-        proc = Popen(
-            'echo "stdout"; echo "stderr" 1>&2', stdout=PIPE, stderr=PIPE,
-            shell=True)
-        self.assertThat(
-            self.capture(proc), MatchesListwise((
-                Equals(0), Equals("stdout\n"), Equals("stderr\n"),
-                # The writes to stdout and stderr occur so close in time that
-                # they may be received in any order.
-                MatchesAny(
-                    Equals("stdout\nstderr\n"),
-                    Equals("stderr\nstdout\n"),
-                ),
-            )))
-
-    def test__does_not_wait_for_forked_process(self):
-        start_time = time.time()
-        proc = Popen('sleep 6 &', stdout=PIPE, stderr=PIPE, shell=True)
-        self.assertThat(
-            self.capture(proc), MatchesListwise((
-                Equals(0), Equals(""), Equals(""), Equals(""),
-            )))
-        # A forked process should continue running after capture_script_output
-        # returns. capture_script_output should not block on the forked call.
-        self.assertLess(time.time() - start_time, 3)
-
-    def test__captures_output_from_completed_process(self):
-        # Write to both stdout and stderr.
-        proc = Popen(
-            'echo -n foo >&1 && echo -n bar >&2',
-            stdout=PIPE, stderr=PIPE, shell=True)
-        # Wait for it to finish before capturing.
-        self.assertEquals(0, proc.wait())
-        # Capturing now still gets foo and bar.
-        self.assertThat(
-            self.capture(proc), MatchesListwise((
-                Equals(0), Equals("foo"), Equals("bar"),
-                # The writes to stdout and stderr occur so close in time that
-                # they may be received in any order.
-                MatchesAny(Equals("foobar"), Equals("barfoo")),
-            )))
-
-    def test__captures_stderr_after_stdout_closes(self):
-        # Write to stdout, close stdout, then write to stderr.
-        proc = Popen(
-            'echo -n foo >&1 && exec 1>&- && echo -n bar >&2',
-            stdout=PIPE, stderr=PIPE, shell=True)
-        # Capturing gets the bar even after stdout is closed.
-        self.assertThat(
-            self.capture(proc), MatchesListwise((
-                Equals(0), Equals("foo"), Equals("bar"),
-                # The writes to stdout and stderr occur so close in time that
-                # they may be received in any order.
-                MatchesAny(Equals("foobar"), Equals("barfoo")),
-            )))
-
-    def test__captures_stdout_after_stderr_closes(self):
-        # Write to stderr, close stderr, then write to stdout.
-        proc = Popen(
-            'echo -n bar >&2 && exec 2>&- && echo -n foo >&1',
-            stdout=PIPE, stderr=PIPE, shell=True)
-        # Capturing gets the foo even after stderr is closed.
-        self.assertThat(
-            self.capture(proc), MatchesListwise((
-                Equals(0), Equals("foo"), Equals("bar"),
-                # The writes to stdout and stderr occur so close in time that
-                # they may be received in any order.
-                MatchesAny(Equals("foobar"), Equals("barfoo")),
-            )))
-
-    def test__captures_all_output(self):
-        proc = Popen(("lshw", "-xml"), stdout=PIPE, stderr=PIPE)
-        returncode, stdout, stderr, combined = self.capture(proc)
-        self.assertThat(returncode, Equals(0), stderr)
-        # This is a complete XML document; we've captured all output.
-        self.assertThat(etree.fromstring(stdout).tag, Equals("list"))

=== modified file 'src/provisioningserver/refresh/__init__.py'
--- src/provisioningserver/refresh/__init__.py	2017-01-28 00:51:47 +0000
+++ src/provisioningserver/refresh/__init__.py	2017-03-03 03:26:15 +0000
@@ -5,11 +5,15 @@
 import os
 import socket
 import stat
-import subprocess
+from subprocess import (
+    PIPE,
+    Popen,
+)
 import tempfile
 
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.refresh.maas_api_helper import (
+    capture_script_output,
     MD_VERSION,
     signal,
     SignalException,
@@ -123,7 +127,10 @@
     total_scripts = len(scripts)
     current_script = 1
     failed_scripts = []
-    for script_name, builtin_script in scripts.items():
+    out_dir = os.path.join(tmpdir, 'out')
+    os.makedirs(out_dir)
+    for script_name in sorted(scripts.keys()):
+        builtin_script = scripts[script_name]
         signal_wrapper(
             url, creds, 'WORKING', 'Starting %s [%d/%d]' %
             (script_name, current_script, total_scripts))
@@ -135,17 +142,48 @@
         st = os.stat(script_path)
         os.chmod(script_path, st.st_mode | stat.S_IEXEC)
 
-        # Execute script and store stdout/stderr
-        proc = subprocess.Popen(
-            script_path, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        stdout, stderr = proc.communicate()
+        # If we pipe the output of the subprocess and the subprocess also
+        # creates a subprocess we end up dead locking. Spawn a shell process
+        # and capture the output to the filesystem to avoid that and help with
+        # debugging.
+        combined_path = os.path.join(out_dir, script_name)
+        stdout_name = '%s.out' % script_name
+        stdout_path = os.path.join(out_dir, stdout_name)
+        stderr_name = '%s.err' % script_name
+        stderr_path = os.path.join(out_dir, stderr_name)
+
+        try:
+            proc = Popen(script_path, stdout=PIPE, stderr=PIPE)
+        except OSError as e:
+            if isinstance(e.errno, int) and e.errno != 0:
+                exit_status = e.errno
+            else:
+                # 2 is the return code bash gives when it can't execute.
+                exit_status = 2
+            result = str(e).encode()
+            if result == b'':
+                result = b'Unable to execute script'
+            files = {
+                script_name: result,
+                stderr_name: result,
+            }
+        else:
+            capture_script_output(
+                proc, combined_path, stdout_path, stderr_path)
+
+            exit_status = proc.returncode
+            files = {
+                script_name: open(combined_path, 'rb').read(),
+                stdout_name: open(stdout_path, 'rb').read(),
+                stderr_name: open(stderr_path, 'rb').read(),
+            }
+
         signal_wrapper(
             url, creds,
             "WORKING", "Finished %s [%d/%d]: %d" %
-            (script_name, current_script, total_scripts, proc.returncode),
-            files={script_name: stdout, "%s.err" % script_name: stderr},
-            exit_status=proc.returncode)
-        if proc.returncode != 0:
+            (script_name, current_script, total_scripts, exit_status),
+            files=files, exit_status=exit_status)
+        if exit_status != 0:
             failed_scripts.append(script_name)
         current_script += 1
     return failed_scripts

=== modified file 'src/provisioningserver/refresh/maas_api_helper.py'
--- src/provisioningserver/refresh/maas_api_helper.py	2017-01-28 00:51:47 +0000
+++ src/provisioningserver/refresh/maas_api_helper.py	2017-03-03 03:26:15 +0000
@@ -14,6 +14,7 @@
 import json
 import mimetypes
 import random
+import selectors
 import socket
 import string
 import sys
@@ -29,6 +30,12 @@
 MD_VERSION = '2012-03-01'
 
 
+# See fcntl(2), re. F_SETPIPE_SZ. By requesting this many bytes from a pipe on
+# each read we can be sure that we are always draining its buffer completely.
+with open("/proc/sys/fs/pipe-max-size") as _pms:
+    PIPE_MAX_SIZE = int(_pms.read())
+
+
 def oauth_headers(url, consumer_key, token_key, token_secret, consumer_secret,
                   clockskew=0):
     """Build OAuth headers using given credentials."""
@@ -257,3 +264,48 @@
         raise SignalException(str(exc))
     except Exception as exc:
         raise SignalException("Unexpected error [%s]" % exc)
+
+
+def capture_script_output(proc, combined_path, stdout_path, stderr_path):
+    """Capture stdout and stderr from `proc`.
+
+    Standard output is written to a file named by `stdout_path`, and standard
+    error is written to a file named by `stderr_path`. Both are also written
+    to a file named by `combined_path`.
+
+    If the given subprocess forks additional processes, and these write to the
+    same stdout and stderr, their output will be captured only as long as
+    `proc` is running.
+
+    :return: The exit code of `proc`.
+    """
+    with open(stdout_path, 'wb') as out, open(stderr_path, 'wb') as err:
+        with open(combined_path, 'wb') as combined:
+            with selectors.DefaultSelector() as selector:
+                selector.register(proc.stdout, selectors.EVENT_READ, out)
+                selector.register(proc.stderr, selectors.EVENT_READ, err)
+                while selector.get_map() and proc.poll() is None:
+                    # Select with a short timeout so that we don't tight loop.
+                    _select_script_output(selector, combined, 0.1)
+                else:
+                    # Process has finished or has closed stdout and stderr.
+                    # Process anything still sitting in the latter's buffers.
+                    _select_script_output(selector, combined, 0.0)
+
+    # Always wait for the process to finish.
+    return proc.wait()
+
+
+def _select_script_output(selector, combined, timeout):
+    """Helper for `capture_script_output`."""
+    for key, event in selector.select(timeout):
+        if event & selectors.EVENT_READ:
+            # Read from the _raw_ file. Ordinarily Python blocks until a
+            # read(n) returns n bytes or the stream reaches end-of-file,
+            # but here we only want to get what's there without blocking.
+            chunk = key.fileobj.raw.read(PIPE_MAX_SIZE)
+            if len(chunk) == 0:  # EOF
+                selector.unregister(key.fileobj)
+            else:
+                key.data.write(chunk)
+                combined.write(chunk)

=== modified file 'src/provisioningserver/refresh/node_info_scripts.py'
--- src/provisioningserver/refresh/node_info_scripts.py	2017-02-27 14:19:44 +0000
+++ src/provisioningserver/refresh/node_info_scripts.py	2017-03-03 03:26:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Builtin node info scripts."""
@@ -353,6 +353,7 @@
     import json
     import os
     import shlex
+    import sys
     from subprocess import check_output
 
     # XXX: Set LC_* and LANG environment variables to C.UTF-8 explicitly.
@@ -366,6 +367,30 @@
                     return os.path.join(dev_disk_byid, link)
         return None
 
+    running_dir = os.path.dirname(__file__)
+    # Results are stored differently when being run as part of node
+    # commissioning vs controller refresh.
+    virtuality_result_paths = [
+        os.path.join(running_dir, '..', '..', 'out', '00-maas-02-virtuality'),
+        os.path.join(running_dir, 'out', '00-maas-02-virtuality'),
+    ]
+    # This script doesn't work in containers as they don't have any block
+    # device. If the virtuality script detected its in one don't report
+    # anything.
+    for virtuality_result_path in virtuality_result_paths:
+        if not os.path.exists(virtuality_result_path):
+            continue
+        virtuality_result = open(virtuality_result_path, 'r').read().strip()
+        # Names from man SYSTEMD-DETECT-VIRT(1)
+        if virtuality_result in {
+                'openvz', 'lxc', 'lxc-libvirt', 'systemd-nspawn', 'docker',
+                'rkt'}:
+            print(
+                'Unable to detect block devices while running in container!',
+                file=sys.stderr)
+            print('[]')
+            return
+
     # Grab the block devices from lsblk. Excludes RAM devices
     # (default for lsblk), floppy disks, and loopback devices.
     blockdevs = []

=== modified file 'src/provisioningserver/refresh/tests/test_maas_api_helper.py'
--- src/provisioningserver/refresh/tests/test_maas_api_helper.py	2017-01-28 00:51:47 +0000
+++ src/provisioningserver/refresh/tests/test_maas_api_helper.py	2017-03-03 03:26:15 +0000
@@ -9,12 +9,19 @@
 from email.utils import formatdate
 from io import StringIO
 import json
+from pathlib import Path
 import random
 import re
+from subprocess import (
+    PIPE,
+    Popen,
+)
 import time
 import urllib
 
+from lxml import etree
 from maastesting.factory import factory
+from maastesting.fixtures import TempDirectory
 from maastesting.matchers import (
     GreaterThanOrEqual,
     LessThanOrEqual,
@@ -28,7 +35,9 @@
     AfterPreprocessing,
     Equals,
     MatchesAll,
+    MatchesAny,
     MatchesDict,
+    MatchesListwise,
 )
 
 
@@ -481,3 +490,105 @@
         self.assertRaises(
             maas_api_helper.SignalException,
             maas_api_helper.signal, None, None, status)
+
+
+class TestCaptureScriptOutput(MAASTestCase):
+
+    # Iterate multiple times to shake out spurious failures.
+    scenarios = [
+        ("iteration %d" % iteration, {})
+        for iteration in range(1, 21)
+    ]
+
+    def capture(self, proc):
+        scripts_dir = Path(self.useFixture(TempDirectory()).path)
+        combined_path = scripts_dir.joinpath("combined")
+        stdout_path = scripts_dir.joinpath("stdout")
+        stderr_path = scripts_dir.joinpath("stderr")
+
+        returncode = maas_api_helper.capture_script_output(
+            proc, str(combined_path), str(stdout_path), str(stderr_path))
+
+        return (
+            returncode,
+            stdout_path.read_text(),
+            stderr_path.read_text(),
+            combined_path.read_text(),
+        )
+
+    def test__captures_script_output(self):
+        proc = Popen(
+            'echo "stdout"; echo "stderr" 1>&2', stdout=PIPE, stderr=PIPE,
+            shell=True)
+        self.assertThat(
+            self.capture(proc), MatchesListwise((
+                Equals(0), Equals("stdout\n"), Equals("stderr\n"),
+                # The writes to stdout and stderr occur so close in time that
+                # they may be received in any order.
+                MatchesAny(
+                    Equals("stdout\nstderr\n"),
+                    Equals("stderr\nstdout\n"),
+                ),
+            )))
+
+    def test__does_not_wait_for_forked_process(self):
+        start_time = time.time()
+        proc = Popen('sleep 6 &', stdout=PIPE, stderr=PIPE, shell=True)
+        self.assertThat(
+            self.capture(proc), MatchesListwise((
+                Equals(0), Equals(""), Equals(""), Equals(""),
+            )))
+        # A forked process should continue running after capture_script_output
+        # returns. capture_script_output should not block on the forked call.
+        self.assertLess(time.time() - start_time, 3)
+
+    def test__captures_output_from_completed_process(self):
+        # Write to both stdout and stderr.
+        proc = Popen(
+            'echo -n foo >&1 && echo -n bar >&2',
+            stdout=PIPE, stderr=PIPE, shell=True)
+        # Wait for it to finish before capturing.
+        self.assertEquals(0, proc.wait())
+        # Capturing now still gets foo and bar.
+        self.assertThat(
+            self.capture(proc), MatchesListwise((
+                Equals(0), Equals("foo"), Equals("bar"),
+                # The writes to stdout and stderr occur so close in time that
+                # they may be received in any order.
+                MatchesAny(Equals("foobar"), Equals("barfoo")),
+            )))
+
+    def test__captures_stderr_after_stdout_closes(self):
+        # Write to stdout, close stdout, then write to stderr.
+        proc = Popen(
+            'echo -n foo >&1 && exec 1>&- && echo -n bar >&2',
+            stdout=PIPE, stderr=PIPE, shell=True)
+        # Capturing gets the bar even after stdout is closed.
+        self.assertThat(
+            self.capture(proc), MatchesListwise((
+                Equals(0), Equals("foo"), Equals("bar"),
+                # The writes to stdout and stderr occur so close in time that
+                # they may be received in any order.
+                MatchesAny(Equals("foobar"), Equals("barfoo")),
+            )))
+
+    def test__captures_stdout_after_stderr_closes(self):
+        # Write to stderr, close stderr, then write to stdout.
+        proc = Popen(
+            'echo -n bar >&2 && exec 2>&- && echo -n foo >&1',
+            stdout=PIPE, stderr=PIPE, shell=True)
+        # Capturing gets the foo even after stderr is closed.
+        self.assertThat(
+            self.capture(proc), MatchesListwise((
+                Equals(0), Equals("foo"), Equals("bar"),
+                # The writes to stdout and stderr occur so close in time that
+                # they may be received in any order.
+                MatchesAny(Equals("foobar"), Equals("barfoo")),
+            )))
+
+    def test__captures_all_output(self):
+        proc = Popen(("lshw", "-xml"), stdout=PIPE, stderr=PIPE)
+        returncode, stdout, stderr, combined = self.capture(proc)
+        self.assertThat(returncode, Equals(0), stderr)
+        # This is a complete XML document; we've captured all output.
+        self.assertThat(etree.fromstring(stdout).tag, Equals("list"))

=== modified file 'src/provisioningserver/refresh/tests/test_node_info_scripts.py'
--- src/provisioningserver/refresh/tests/test_node_info_scripts.py	2017-02-17 14:23:04 +0000
+++ src/provisioningserver/refresh/tests/test_node_info_scripts.py	2017-03-03 03:26:15 +0000
@@ -5,7 +5,6 @@
 
 __all__ = []
 
-from functools import partial
 from inspect import getsource
 from io import StringIO
 import json
@@ -18,11 +17,13 @@
     check_output,
     STDOUT,
 )
+import sys
 from textwrap import dedent
 import time
 from unittest.mock import call
 
 from maastesting.factory import factory
+from maastesting.fixtures import TempDirectory
 from maastesting.matchers import (
     MockCalledOnceWith,
     MockCallsMatch,
@@ -115,6 +116,8 @@
     source = dedent(getsource(function))
     modcode = compile(source, "isolated.py", "exec")
     namespace = {} if namespace is None else namespace
+    if '__file__' not in namespace:
+        namespace['__file__'] = __file__
     exec(modcode, namespace)
     return namespace[function.__name__]
 
@@ -373,9 +376,18 @@
         return output.encode("ascii")
 
     def call_gather_physical_block_devices(
-            self, dev_disk_byid='/dev/disk/by-id/'):
+            self, dev_disk_byid='/dev/disk/by-id/', file_path=None):
         output = StringIO()
-        namespace = {"print": partial(print, file=output)}
+
+        def neutered_print(*args, **kwargs):
+            file = kwargs.pop('file', None)
+            if file is not None and file == sys.stderr:
+                return
+            return print(*args, **kwargs, file=output)
+
+        namespace = {"print": neutered_print}
+        if file_path is not None:
+            namespace['__file__'] = file_path
         gather_physical_block_devices = isolate_function(
             node_info_module.gather_physical_block_devices, namespace)
         gather_physical_block_devices(dev_disk_byid=dev_disk_byid)
@@ -885,6 +897,17 @@
             ]
         self.assertEqual(names, device_names)
 
+    def test__quietly_exits_in_container(self):
+        script_dir = self.useFixture(TempDirectory()).path
+        script_path = os.path.join(script_dir, '00-maas-07-block-devices')
+        virtuality_result_path = os.path.join(script_dir, 'out')
+        os.makedirs(virtuality_result_path)
+        virtuality_result_path = os.path.join(
+            virtuality_result_path, '00-maas-02-virtuality')
+        open(virtuality_result_path, 'w').write('lxc\n')
+        self.assertItemsEqual(
+            [], self.call_gather_physical_block_devices(file_path=script_path))
+
 
 class TestVirtualityScript(MAASTestCase):
     """Tests for `VIRTUALITY_SCRIPT`."""

=== modified file 'src/provisioningserver/refresh/tests/test_refresh.py'
--- src/provisioningserver/refresh/tests/test_refresh.py	2017-01-28 00:51:47 +0000
+++ src/provisioningserver/refresh/tests/test_refresh.py	2017-03-03 03:26:15 +0000
@@ -236,10 +236,104 @@
             exit_status=0,
             files={
                 script_name: b'test script\n',
+                '%s.out' % script_name: b'test script\n',
                 '%s.err' % script_name: b'',
             },
         ))
 
+    def test_refresh_signals_failure_on_unexecutable_script(self):
+        signal = self.patch(refresh, 'signal')
+        script_name = factory.make_name('script_name')
+        self.patch_scripts_failure(script_name)
+        mock_popen = self.patch(refresh, 'Popen')
+        mock_popen.side_effect = OSError(8, 'Exec format error')
+
+        system_id = factory.make_name('system_id')
+        consumer_key = factory.make_name('consumer_key')
+        token_key = factory.make_name('token_key')
+        token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
+
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/%s/" % (url, MD_VERSION),
+            {
+                'consumer_secret': '',
+                'consumer_key': consumer_key,
+                'token_key': token_key,
+                'token_secret': token_secret,
+            },
+            'WORKING',
+            'Finished %s [1/1]: 8' % script_name,
+            exit_status=8,
+            files={
+                script_name: b'[Errno 8] Exec format error',
+                '%s.err' % script_name: b'[Errno 8] Exec format error',
+            },
+        ))
+
+    def test_refresh_signals_failure_on_unexecutable_script_no_errno(self):
+        signal = self.patch(refresh, 'signal')
+        script_name = factory.make_name('script_name')
+        self.patch_scripts_failure(script_name)
+        mock_popen = self.patch(refresh, 'Popen')
+        mock_popen.side_effect = OSError()
+
+        system_id = factory.make_name('system_id')
+        consumer_key = factory.make_name('consumer_key')
+        token_key = factory.make_name('token_key')
+        token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
+
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/%s/" % (url, MD_VERSION),
+            {
+                'consumer_secret': '',
+                'consumer_key': consumer_key,
+                'token_key': token_key,
+                'token_secret': token_secret,
+            },
+            'WORKING',
+            'Finished %s [1/1]: 2' % script_name,
+            exit_status=2,
+            files={
+                script_name: b'Unable to execute script',
+                '%s.err' % script_name: b'Unable to execute script',
+            },
+        ))
+
+    def test_refresh_signals_failure_on_unexecutable_script_baderrno(self):
+        signal = self.patch(refresh, 'signal')
+        script_name = factory.make_name('script_name')
+        self.patch_scripts_failure(script_name)
+        mock_popen = self.patch(refresh, 'Popen')
+        mock_popen.side_effect = OSError(0, 'Exec format error')
+
+        system_id = factory.make_name('system_id')
+        consumer_key = factory.make_name('consumer_key')
+        token_key = factory.make_name('token_key')
+        token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
+
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/%s/" % (url, MD_VERSION),
+            {
+                'consumer_secret': '',
+                'consumer_key': consumer_key,
+                'token_key': token_key,
+                'token_secret': token_secret,
+            },
+            'WORKING',
+            'Finished %s [1/1]: 2' % script_name,
+            exit_status=2,
+            files={
+                script_name: b'[Errno 0] Exec format error',
+                '%s.err' % script_name: b'[Errno 0] Exec format error',
+            },
+        ))
+
     def test_refresh_signals_finished(self):
         signal = self.patch(refresh, 'signal')
         script_name = factory.make_name('script_name')