From c9dbeadae38502b2622dbaaeadca68d7f67ece8b Mon Sep 17 00:00:00 2001 From: Craig Tracey Date: Thu, 20 Nov 2014 15:48:23 -0500 Subject: [PATCH] Utilize a temporary build path Previously giftwrap was building packages directly in the destination path. This often required sudo access to drop directories and files in system paths. In addition to just adding the code to build in a tempdir, add the supporting code as well: cleanup, signal handling, etc. --- giftwrap/builder.py | 8 ++++- giftwrap/builders/docker_builder.py | 10 +++--- giftwrap/builders/package_builder.py | 49 ++++++++++++++-------------- giftwrap/package.py | 12 ++++--- giftwrap/settings.py | 5 ++- giftwrap/shell.py | 14 ++++++++ giftwrap/util.py | 6 ++++ 7 files changed, 66 insertions(+), 38 deletions(-) diff --git a/giftwrap/builder.py b/giftwrap/builder.py index c690ad1..d9403cf 100644 --- a/giftwrap/builder.py +++ b/giftwrap/builder.py @@ -25,16 +25,22 @@ class Builder(object): self._spec = spec self.settings = spec.settings + def _validate_settings(self): + raise NotImplementedError() + def _build(self): raise NotImplementedError() - def _validate_settings(self): + def _cleanup(self): raise NotImplementedError() def build(self): self._validate_settings() self._build() + def cleanup(self): + self._cleanup() + from giftwrap.builders.package_builder import PackageBuilder from giftwrap.builders.docker_builder import DockerBuilder diff --git a/giftwrap/builders/docker_builder.py b/giftwrap/builders/docker_builder.py index 13ec6a9..eaf0f12 100644 --- a/giftwrap/builders/docker_builder.py +++ b/giftwrap/builders/docker_builder.py @@ -46,6 +46,7 @@ APT_REQUIRED_PACKAGES = [ 'python-pip', 'build-essential' ] +DEFAULT_SRC_PATH = '/opt/openstack' class DockerBuilder(Builder): @@ -59,8 +60,10 @@ class DockerBuilder(Builder): super(DockerBuilder, self).__init__(spec) def _validate_settings(self): - if not self.settings.all_in_one: - LOG.warn("The Docker builder does not support all-in-one") + pass + + def _cleanup(self): + pass def _get_prep_commands(self): commands = [] @@ -69,7 +72,6 @@ class DockerBuilder(Builder): return commands def _get_build_commands(self, src_path): - commands = [] commands.append('mkdir -p %s' % src_path) @@ -119,7 +121,7 @@ class DockerBuilder(Builder): return template.render(template_vars) def _build(self): - src_path = '/tmp/build' + src_path = DEFAULT_SRC_PATH commands = self._get_prep_commands() commands += self._get_build_commands(src_path) commands += self._get_cleanup_commands(src_path) diff --git a/giftwrap/builders/package_builder.py b/giftwrap/builders/package_builder.py index 9f21351..91aadcb 100644 --- a/giftwrap/builders/package_builder.py +++ b/giftwrap/builders/package_builder.py @@ -22,7 +22,7 @@ import tempfile from giftwrap.gerrit import GerritReview from giftwrap.openstack_git_repo import OpenstackGitRepo from giftwrap.package import Package -from giftwrap.util import execute +from giftwrap.util import execute, relative_pathify LOG = logging.getLogger(__name__) @@ -33,20 +33,19 @@ from giftwrap.builder import Builder class PackageBuilder(Builder): def __init__(self, spec): - self._all_in_one = False + self._tempdir = None super(PackageBuilder, self).__init__(spec) def _validate_settings(self): pass - def _install_gerrit_dependencies(self, repo, project): + def _install_gerrit_dependencies(self, repo, project, install_path): try: review = GerritReview(repo.head.change_id, project.git_path) LOG.info("Installing '%s' pip dependencies to the virtualenv", project.name) execute(project.install_command % - review.build_pip_dependencies(string=True), - project.install_path) + review.build_pip_dependencies(string=True), install_path) except Exception as e: LOG.warning("Could not install gerrit dependencies!!! " "Error was: %s", e) @@ -54,26 +53,22 @@ class PackageBuilder(Builder): def _build(self): spec = self._spec - tempdir = tempfile.mkdtemp(prefix='giftwrap') - src_dir = os.path.join(tempdir, 'src') - LOG.debug("Temporary working directory: %s", tempdir) + self._tempdir = tempfile.mkdtemp(prefix='giftwrap') + src_path = os.path.join(self._tempdir, 'src') + build_path = os.path.join(self._tempdir, 'build') + os.makedirs(build_path) + LOG.debug("Temporary working directory: %s", self._tempdir) for project in spec.projects: LOG.info("Beginning to build '%s'", project.name) - # if anything is in our way, see if we can get rid of it - if os.path.exists(project.install_path): - if spec.settings.force_overwrite: - LOG.info("force_overwrite is set, so removing " - "existing path '%s'" % project.install_path) - shutil.rmtree(project.install_path) - else: - raise Exception("Install path '%s' already exists" % - project.install_path) - os.makedirs(project.install_path) + install_path = os.path.join(build_path, + relative_pathify(project.install_path)) + LOG.debug("Installing '%s' to '%s'", project.name, install_path) + os.makedirs(install_path) # clone the project's source to a temporary directory - project_src_path = os.path.join(src_dir, project.name) + project_src_path = os.path.join(src_path, project.name) os.makedirs(project_src_path) LOG.info("Fetching source code for '%s'", project.name) @@ -82,24 +77,28 @@ class PackageBuilder(Builder): # start building the virtualenv for the project LOG.info("Creating the virtualenv for '%s'", project.name) - execute(project.venv_command, project.install_path) + execute(project.venv_command, install_path) # install into the virtualenv LOG.info("Installing '%s' to the virtualenv", project.name) - venv_python_path = os.path.join(project.install_path, 'bin/python') - venv_pip_path = os.path.join(project.install_path, 'bin/pip') + venv_python_path = os.path.join(install_path, 'bin/python') + venv_pip_path = os.path.join(install_path, 'bin/pip') deps = " ".join(project.pip_dependencies) execute("%s install %s" % (venv_pip_path, deps)) if spec.settings.gerrit_dependencies: - self._install_gerrit_dependencies(repo, project) + self._install_gerrit_dependencies(repo, project, install_path) execute("%s setup.py install" % venv_python_path, project_src_path) execute("%s install pbr" % venv_pip_path) # now build the package - pkg = Package(project.package_name, project.version, - project.install_path, spec.settings.force_overwrite, + pkg = Package(project.package_name, project.version, build_path, + relative_pathify(project.install_path), + spec.settings.force_overwrite, project.system_dependencies) pkg.build() + + def _cleanup(self): + shutil.rmtree(self._tempdir) diff --git a/giftwrap/package.py b/giftwrap/package.py index 19a19f0..79209ce 100644 --- a/giftwrap/package.py +++ b/giftwrap/package.py @@ -26,11 +26,12 @@ SUPPORTED_DISTROS = { class Package(object): - def __init__(self, name, version, path, overwrite=False, - dependencies=None): + def __init__(self, name, version, build_path, install_path, + overwrite=False, dependencies=None): self.name = name self.version = version - self.path = path + self.build_path = build_path + self.install_path = install_path self.overwrite = overwrite self.dependencies = dependencies @@ -49,5 +50,6 @@ class Package(object): deps = '-d %s' % (' -d '.join(self.dependencies)) # not wrapping in a try block - handled by caller - execute("fpm %s -s dir -t %s -n %s -v %s %s %s" % - (overwrite, target, self.name, self.version, deps, self.path)) + execute("fpm %s -s dir -t %s -n %s -v %s %s %s" % (overwrite, target, + self.name, self.version, deps, self.install_path), + self.build_path) diff --git a/giftwrap/settings.py b/giftwrap/settings.py index 18e21c3..87ec3fe 100644 --- a/giftwrap/settings.py +++ b/giftwrap/settings.py @@ -26,15 +26,14 @@ class Settings(object): def __init__(self, build_type=DEFAULT_BUILD_TYPE, package_name_format=None, version=None, - base_path=None, all_in_one=False, - gerrit_dependencies=True, force_overwrite=False): + base_path=None, gerrit_dependencies=True, + force_overwrite=False): if not version: raise Exception("'version' is a required settings") self.build_type = build_type self._package_name_format = package_name_format self.version = version self._base_path = base_path - self.all_in_one = all_in_one self.gerrit_dependencies = gerrit_dependencies self.force_overwrite = force_overwrite diff --git a/giftwrap/shell.py b/giftwrap/shell.py index 1b711ed..1ebe44e 100644 --- a/giftwrap/shell.py +++ b/giftwrap/shell.py @@ -16,6 +16,7 @@ import argparse import logging +import signal import sys import giftwrap.builder @@ -38,6 +39,8 @@ def _setup_logger(level=logging.INFO): def build(args): """ the entry point for all build subcommand tasks """ + builder = None + fail = False try: manifest = None @@ -46,9 +49,20 @@ def build(args): buildspec = BuildSpec(manifest, args.version) builder = giftwrap.builder.create_builder(buildspec) + + def _signal_handler(*args): + LOG.info("Process interrrupted. Cleaning up.") + builder.cleanup() + sys.exit() + signal.signal(signal.SIGINT, _signal_handler) + builder.build() except Exception as e: LOG.exception("Oops something went wrong: %s", e) + fail = True + + builder.cleanup() + if fail: sys.exit(-1) diff --git a/giftwrap/util.py b/giftwrap/util.py index 0fcd490..7a4376e 100644 --- a/giftwrap/util.py +++ b/giftwrap/util.py @@ -60,3 +60,9 @@ def execute(command, cwd=None, exit=0): (command, exitcode, out, err)) return out + + +def relative_pathify(path): + if path.startswith('/'): + return path[1:] + return path