summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTiago Gomes <tiago.gomes@codethink.co.uk>2015-08-03 10:03:29 +0000
committerBaserock Gerrit <gerrit@baserock.org>2015-09-01 08:25:24 +0000
commit0b50b9ceca5fc604550537297e786d876d28508a (patch)
treecdc599c2e293ff8269e612c8b265648c9ac505d1
parent41cbcdf6b5c0f823ac5589faaf6da3651ad69dab (diff)
downloadmorph-0b50b9ceca5fc604550537297e786d876d28508a.tar.gz
Stop moving staging areas of failed builds
Stop moving staging areas of failed builds from the 'staging' directory to the 'failed' directory. Moving staging areas make it very difficult to debug build failures on the build essential chunks, as the paths set on the configure scripts and some environment variables (e.g. STAGE2_SYSROOT, DESTDIR) will be invalid after moving the staging area. This change will also make it easier to create scripts that chroot n environment similiar to the one where the build failure occurred. To make it still possible to safely do a build an run `morph gc` in parallel, we use flock(2) to control access to the staging area directory. Also, move the `test_supports_non_isolated_mode` test into a different class, as it requires a different SetUp() routine (the staging area is contructed with different parameters). Change-Id: I06c3c435ad05c12afabc0adc2a9d4f8a284ccc02
-rw-r--r--morphlib/app.py1
-rw-r--r--morphlib/buildcommand.py6
-rw-r--r--morphlib/builder.py2
-rw-r--r--morphlib/plugins/gc_plugin.py28
-rw-r--r--morphlib/stagingarea.py42
-rw-r--r--morphlib/stagingarea_tests.py17
6 files changed, 64 insertions, 32 deletions
diff --git a/morphlib/app.py b/morphlib/app.py
index 9f9ab01a..8fd52a46 100644
--- a/morphlib/app.py
+++ b/morphlib/app.py
@@ -284,7 +284,6 @@ class Morph(cliapp.Application):
tmpdir = self.settings['tempdir']
for required_dir in (os.path.join(tmpdir, 'chunks'),
os.path.join(tmpdir, 'staging'),
- os.path.join(tmpdir, 'failed'),
os.path.join(tmpdir, 'deployments'),
self.settings['cachedir']):
if not os.path.exists(required_dir):
diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py
index 46c3104f..3ace34bd 100644
--- a/morphlib/buildcommand.py
+++ b/morphlib/buildcommand.py
@@ -366,11 +366,7 @@ class BuildCommand(object):
use_chroot,
extra_env=extra_env,
extra_path=extra_path)
- try:
- self.install_dependencies(staging_area, deps, source)
- except BaseException:
- staging_area.abort()
- raise
+ self.install_dependencies(staging_area, deps, source)
else:
staging_area = self.create_staging_area(build_env, False)
diff --git a/morphlib/builder.py b/morphlib/builder.py
index 39af4146..038fd5ba 100644
--- a/morphlib/builder.py
+++ b/morphlib/builder.py
@@ -303,8 +303,6 @@ class ChunkBuilder(BuilderBase):
os.rename(temppath, logpath)
else:
logging.error("Couldn't find build log at %s", temppath)
-
- self.staging_area.abort()
raise
self.staging_area.chroot_close()
diff --git a/morphlib/plugins/gc_plugin.py b/morphlib/plugins/gc_plugin.py
index 71522b04..79c65e5d 100644
--- a/morphlib/plugins/gc_plugin.py
+++ b/morphlib/plugins/gc_plugin.py
@@ -17,6 +17,7 @@ import logging
import os
import shutil
import time
+import fcntl
import fs.osfs
import cliapp
@@ -82,7 +83,32 @@ class GCPlugin(cliapp.Plugin):
# assumes that they exist in various places.
self.app.status(msg='Cleaning up temp dir %(temp_path)s',
temp_path=temp_path, chatty=True)
- for subdir in ('deployments', 'failed', 'chunks'):
+
+ self.app.status(msg='Removing temp subdirectory: staging')
+ staging_dir = os.path.join(temp_path, 'staging')
+ subdirs = (f for f in os.listdir(staging_dir)
+ if os.path.isdir(os.path.join(staging_dir, f)))
+ for subdir in subdirs:
+ fd = None
+ prefix_dir = os.path.join(staging_dir, subdir)
+ try:
+ fd = os.open(prefix_dir, os.O_RDONLY)
+ fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
+ chroot_script = os.path.join('%s.sh' % prefix_dir)
+ if os.path.exists(chroot_script):
+ os.remove(chroot_script)
+ log_file = os.path.join('%s.log' % prefix_dir)
+ if os.path.exists(log_file):
+ os.remove(log_file)
+ if os.path.exists(prefix_dir):
+ shutil.rmtree(prefix_dir)
+ except IOError:
+ pass
+ finally:
+ if fd is not None:
+ os.close(fd)
+
+ for subdir in ('deployments', 'chunks'):
if morphlib.util.get_bytes_free_in_path(temp_path) >= min_space:
self.app.status(msg='Not Removing subdirectory '
'%(subdir)s, enough space already cleared',
diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py
index 76bb3a18..c8c77229 100644
--- a/morphlib/stagingarea.py
+++ b/morphlib/stagingarea.py
@@ -20,6 +20,7 @@ import stat
import cliapp
from urlparse import urlparse
import tempfile
+import fcntl
import morphlib
@@ -58,6 +59,25 @@ class StagingArea(object):
path = full_path + os.environ['PATH'].split(':')
self.env['PATH'] = ':'.join(path)
+
+ # Keep trying until we have created a directory with an
+ # exclusive lock on it, as if the user runs `morph gc` in
+ # parallel the staging area directory could have been removed
+ # or have its exclusive lock associated with the `morph gc`
+ # process
+ while True:
+ try:
+ fd = os.open(dirname, os.O_RDONLY)
+ fcntl.flock(fd, fcntl.LOCK_EX)
+ if os.path.exists(dirname):
+ self.staging_area_fd = fd
+ break
+ else:
+ os.close(fd) # pragma: no cover
+ except OSError: # pragma: no cover
+ if not os.path.exists(dirname):
+ os.makedirs(dirname)
+
# Wrapper to be overridden by unit tests.
def _mkdir(self, dirname): # pragma: no cover
os.makedirs(dirname)
@@ -170,9 +190,6 @@ class StagingArea(object):
# the other renamed its tempdir here first.
os.rename(savedir, unpacked_artifact)
- if not os.path.exists(self.dirname):
- self._mkdir(self.dirname)
-
self.hardlink_all_files(unpacked_artifact, self.dirname)
def remove(self):
@@ -185,6 +202,7 @@ class StagingArea(object):
'''
shutil.rmtree(self.dirname)
+ os.close(self.staging_area_fd)
to_mount_in_staging = (
('dev/shm', 'tmpfs', 'none'),
@@ -303,21 +321,5 @@ class StagingArea(object):
msg = morphlib.util.error_message_for_containerised_commandline(
argv, err, container_config)
raise cliapp.AppException(
- 'In staging area %s: %s' % (self._failed_location(), msg))
-
- def _failed_location(self): # pragma: no cover
- '''Path this staging area will be moved to if an error occurs.'''
- return os.path.join(self._app.settings['tempdir'], 'failed',
- os.path.basename(self.dirname))
-
- def abort(self): # pragma: no cover
- '''Handle what to do with a staging area in the case of failure.
- This may either remove it or save it for later inspection.
- '''
- # TODO: when we add the option to throw away failed builds,
- # hook it up here
-
- dest_dir = self._failed_location()
- os.rename(self.dirname, dest_dir)
- self.dirname = dest_dir
+ 'In staging area %s: %s' % (self.dirname, msg))
diff --git a/morphlib/stagingarea_tests.py b/morphlib/stagingarea_tests.py
index 97d78236..f08a3989 100644
--- a/morphlib/stagingarea_tests.py
+++ b/morphlib/stagingarea_tests.py
@@ -135,8 +135,19 @@ class StagingAreaTests(unittest.TestCase):
self.sa.remove()
self.assertFalse(os.path.exists(self.staging))
- def test_supports_non_isolated_mode(self):
- sa = morphlib.stagingarea.StagingArea(
+
+class StagingAreaNonIsolatedTests(unittest.TestCase):
+
+ def setUp(self):
+ self.tempdir = tempfile.mkdtemp()
+ self.staging = os.path.join(self.tempdir, 'staging')
+ self.build_env = FakeBuildEnvironment()
+ self.sa = morphlib.stagingarea.StagingArea(
object(), self.staging, self.build_env, use_chroot=False)
+
+ def tearDown(self):
+ shutil.rmtree(self.tempdir)
+
+ def test_supports_non_isolated_mode(self):
filename = os.path.join(self.staging, 'foobar')
- self.assertEqual(sa.relative(filename), filename)
+ self.assertEqual(self.sa.relative(filename), filename)