Merge branch 'samthursfield/handle-target-disk-full' of git://git.baserock.org/baserock/morph

A couple of nasties are fixed in here. Previously when the disk was
full, Morph logged a backtrace into morph.log and then tried to unmount
the disk image, but as there were still open file handles inside it
the unmount would fail and the user would end up with a backtrace for
a failed unmount and a stuck loopback device that they would need to
fix manually.

5 files changed, 101 insertions, 25 deletions

diff --git a/morphlib/bins.py b/morphlib/bins.py
index 71483172..622aa165 100644
--- a/morphlib/bins.py
+++ b/morphlib/bins.py
@@ -26,9 +26,24 @@ import os
 import re
 import errno
 import stat
+import shutil
 import tarfile
 
 
+# Work around http://bugs.python.org/issue16477
+def safe_makefile(self, tarinfo, targetpath):
+    '''Create a file, closing correctly in case of exception'''
+
+    source = self.extractfile(tarinfo)
+    try:
+        with open(targetpath, "wb") as target:
+            shutil.copyfileobj(source, target)
+    finally:
+        source.close()
+
+tarfile.TarFile.makefile = safe_makefile
+
+
 def create_chunk(rootdir, f, regexps, dump_memory_profile=None):
     '''Create a chunk from the contents of a directory.
 
@@ -187,11 +202,12 @@ def unpack_binary_from_file(f, dirname):  # pragma: no cover
     tf.makedev = monkey_patcher(tf.makedev)
     tf.makelink = monkey_patcher(tf.makelink)
 
-    tf.extractall(path=dirname)
-    tf.close
+    try:
+        tf.extractall(path=dirname)
+    finally:
+        tf.close()
 
 
 def unpack_binary(filename, dirname):
-    f = open(filename, "rb")
-    unpack_binary_from_file(f, dirname)
-    f.close()
+    with open(filename, "rb") as f:
+        unpack_binary_from_file(f, dirname)
diff --git a/morphlib/builder2.py b/morphlib/builder2.py
index 0de0ebff..674cbb17 100644
--- a/morphlib/builder2.py
+++ b/morphlib/builder2.py
@@ -15,6 +15,7 @@
 
 
 import datetime
+import errno
 import json
 import logging
 import os
@@ -488,6 +489,24 @@ class SystemKindBuilder(BuilderBase):  # pragma: no cover
 
     '''
 
+    def unpack_one_stratum(self, stratum_artifact, target):
+        '''Unpack a single stratum into a target directory'''
+
+        cache = self.local_artifact_cache
+        with cache.get(stratum_artifact) as stratum_file:
+            artifact_list = json.load(stratum_file)
+            for chunk in (ArtifactCacheReference(a) for a in artifact_list):
+                self.app.status(msg='Unpacking chunk %(basename)s',
+                                basename=chunk.basename(), chatty=True)
+                with cache.get(chunk) as chunk_file:
+                    morphlib.bins.unpack_binary_from_file(chunk_file, target)
+
+        target_metadata = os.path.join(
+                target, 'baserock', '%s.meta' % stratum_artifact.name)
+        with cache.get_artifact_metadata(stratum_artifact, 'meta') as meta_src:
+            with morphlib.savefile.SaveFile(target_metadata, 'w') as meta_dst:
+                shutil.copyfileobj(meta_src, meta_dst)
+
     def unpack_strata(self, path):
         '''Unpack strata into a directory.'''
 
@@ -523,22 +542,7 @@ class SystemKindBuilder(BuilderBase):  # pragma: no cover
 
             # unpack it from the local artifact cache
             for stratum_artifact in self.artifact.dependencies:
-                f = self.local_artifact_cache.get(stratum_artifact)
-                for chunk in (ArtifactCacheReference(a) for a in json.load(f)):
-                    self.app.status(msg='Unpacking chunk %(basename)s',
-                                    basename=chunk.basename(), chatty=True)
-                    chunk_handle = self.local_artifact_cache.get(chunk)
-                    morphlib.bins.unpack_binary_from_file(chunk_handle, path)
-                    chunk_handle.close()
-                f.close()
-                meta = self.local_artifact_cache.get_artifact_metadata(
-                    stratum_artifact, 'meta')
-                dst = morphlib.savefile.SaveFile(
-                    os.path.join(path, 'baserock',
-                                 '%s.meta' % stratum_artifact.name), 'w')
-                shutil.copyfileobj(meta, dst)
-                dst.close()
-                meta.close()
+                self.unpack_one_stratum(stratum_artifact, path)
 
             ldconfig(self.app.runcmd, path)
 
@@ -706,15 +710,20 @@ class DiskImageBuilder(SystemKindBuilder):  # pragma: no cover
                                              mount_point)
                     self._install_bootloader(mount_point)
                     self.copy_kernel_into_artifact_cache(factory_path)
-                    self._unmount(mount_point)
                 except BaseException, e:
                     logging.error(traceback.format_exc())
                     self.app.status(msg='Error while building system',
                                     error=True)
                     self._unmount(mount_point)
                     self._undo_device_mapping(image_name)
-                    raise
+                    if type(e) is IOError and e.errno==errno.ENOSPC:
+                        raise cliapp.AppException(
+                            'Ran out of space on %s disk image. Please '
+                            'increase the system\'s disk-size.' % rootfs_name)
+                    else:
+                        raise
 
+                self._unmount(mount_point)
                 self._undo_device_mapping(image_name)
 
                 self.app.status(msg='Compressing disk image',
@@ -819,5 +828,3 @@ class DiskImageBuilder(SystemKindBuilder):  # pragma: no cover
                         filename=image_name, chatty=True)
         with self.build_watch('undo-device-mapper'):
             morphlib.fsutils.undo_device_mapping(self.app.runcmd, image_name)
-
-
diff --git a/tests.as-root/target-disk-too-small.exit b/tests.as-root/target-disk-too-small.exit
new file mode 100644
index 00000000..d00491fd
--- /dev/null
+++ b/tests.as-root/target-disk-too-small.exit
@@ -0,0 +1 @@
+1
diff --git a/tests.as-root/target-disk-too-small.script b/tests.as-root/target-disk-too-small.script
new file mode 100755
index 00000000..92a73a58
--- /dev/null
+++ b/tests.as-root/target-disk-too-small.script
@@ -0,0 +1,51 @@
+#!/bin/bash
+#
+# Copyright (C) 2011, 2012  Codethink Limited
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+
+## Handle target disk image being too small for its contents.
+
+set -eu
+
+. "$SRCDIR/tests.as-root/lib"
+
+# Shrink linux-system to the minimum btrfs will allow.
+cd "$DATADIR/morphs"
+sed -e 's/"system-kind": "syslinux-disk"/"system-kind": "disk"/' \
+    -e 's/"disk-size": "1G"/"disk-size": "512M"/' \
+    -i linux-system.morph
+git add linux-system.morph
+git commit -q -m "Make linux-system as small as possible"
+
+# Grow hello-chunk to be absurdly large.
+cd "$DATADIR/chunk-repo"
+git checkout -q farrokh
+cat <<'EOF' > hello.morph
+{
+    "name": "hello",
+    "kind": "chunk",
+    "build-system": "dummy",
+    "install-commands": [
+        "dd if=/dev/zero of=\"$DESTDIR\"/huge-file seek=1048580 count=0"
+    ]
+}
+EOF
+git add hello.morph
+git commit -q -m "Make hello be very big"
+
+# Ignore stdout - Morph logs a timestamped error
+"$SRCDIR/scripts/test-morph" build-morphology test:morphs master linux-system \
+    > /dev/null
diff --git a/tests.as-root/target-disk-too-small.stderr b/tests.as-root/target-disk-too-small.stderr
new file mode 100644
index 00000000..487c72e2
--- /dev/null
+++ b/tests.as-root/target-disk-too-small.stderr
@@ -0,0 +1 @@
+ERROR: Ran out of space on linux-system-rootfs disk image. Please increase the system's disk-size.