Refactor giftwrap builders

Previously the two builders had operated as separate build procesess.
What this meant is that when one builder would change, the other would
also need to change in order to support build parity.

This change moves all of the build logic into the base Builder abstract
class. Each sub Builder then implements the necessary primitives to
support the build steps outlined by the base class.

giftwrap/builder.py

@@ -1,54 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2014, Craig Tracey <craigtracey@gmail.com>
-# All Rights Reserved.
-#
-# Licensed under the Apache License, Version 2.0 (the "License"); you may
-# not use this file except in compliance with the License. You may obtain
-# a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-# License for the specific language governing permissions and limitations
-
-import logging
-
-LOG = logging.getLogger(__name__)
-
-
-class Builder(object):
-
-    def __init__(self, spec):
-        self._spec = spec
-        self.settings = spec.settings
-
-    def _validate_settings(self):
-        raise NotImplementedError()
-
-    def _build(self):
-        raise NotImplementedError()
-
-    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
-
-
-def create_builder(spec):
-    if spec.settings.build_type == 'package':
-        return PackageBuilder(spec)
-    elif spec.settings.build_type == 'docker':
-        return DockerBuilder(spec)
-    raise Exception("Unknown build_type: '%s'", spec.settings.build_type)

giftwrap/builders/__init__.py

@@ -0,0 +1,157 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2014, Craig Tracey <craigtracey@gmail.com>
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+
+import logging
+import os
+
+from giftwrap.gerrit import GerritReview
+
+from abc import abstractmethod, ABCMeta
+
+LOG = logging.getLogger(__name__)
+
+
+class Builder(object):
+    __metaclass__ = ABCMeta
+
+    def __init__(self, spec):
+        self._temp_dir = None
+        self._temp_src_dir = None
+        self._spec = spec
+
+    def _get_venv_pip_path(self, venv_path):
+        return os.path.join(venv_path, 'bin/pip')
+
+    def _get_gerrit_dependencies(self, repo, project):
+        try:
+            review = GerritReview(repo.head.change_id, project.git_path)
+            return review.build_pip_dependencies(string=True)
+        except Exception as e:
+            LOG.warning("Could not install gerrit dependencies!!! "
+                        "Error was: %s", e)
+            return ""
+
+    def _build_project(self, project):
+        self._prepare_project_build(project)
+        self._make_dir(project.install_path)
+
+        # clone the source
+        src_clone_dir = os.path.join(self._temp_src_dir, project.name)
+        repo = self._clone_project(project.giturl, project.name,
+                                   project.gitref, project.gitdepth,
+                                   src_clone_dir)
+
+        # create and build the virtualenv
+        self._create_virtualenv(project.venv_command, project.install_path)
+        dependencies = ""
+        if project.pip_dependencies:
+            dependencies = " ".join(project.pip_dependencies)
+        if self._spec.settings.gerrit_dependencies:
+            dependencies = "%s %s" % (dependencies,
+                                      self._get_gerrit_dependencies(repo,
+                                                                    project))
+        if len(dependencies):
+            self._install_pip_dependencies(project.install_path, dependencies)
+
+        if self._spec.settings.include_config:
+            self._copy_sample_config(src_clone_dir, project)
+
+        self._install_project(project.install_path, src_clone_dir)
+
+        # finish up
+        self._finalize_project_build(project)
+
+    def build(self):
+        spec = self._spec
+
+        self._prepare_build()
+
+        # Create a temporary directory for the source code
+        self._temp_dir = self._make_temp_dir()
+        self._temp_src_dir = os.path.join(self._temp_dir, 'src')
+        LOG.debug("Temporary working directory: %s", self._temp_dir)
+
+        for project in spec.projects:
+            self._build_project(project)
+
+        self._finalize_build()
+
+    def cleanup(self):
+        self._cleanup_build()
+
+    @abstractmethod
+    def _execute(self, command, cwd=None, exit=0):
+        return
+
+    @abstractmethod
+    def _make_temp_dir(self, prefix='giftwrap'):
+        return
+
+    @abstractmethod
+    def _make_dir(self, path, mode=0777):
+        return
+
+    @abstractmethod
+    def _prepare_build(self):
+        return
+
+    @abstractmethod
+    def _prepare_project_build(self, project):
+        return
+
+    @abstractmethod
+    def _clone_project(self, project):
+        return
+
+    @abstractmethod
+    def _create_virtualenv(self, venv_command, path):
+        return
+
+    @abstractmethod
+    def _install_pip_dependencies(self, venv_path, dependencies):
+        return
+
+    @abstractmethod
+    def _copy_sample_config(self, src_clone_dir, project):
+        return
+
+    @abstractmethod
+    def _install_project(self, venv_path, src_clone_dir):
+        return
+
+    @abstractmethod
+    def _finalize_project_build(self, project):
+        return
+
+    @abstractmethod
+    def _finalize_build(self):
+        return
+
+    @abstractmethod
+    def _cleanup_build(self):
+        return
+
+
+from giftwrap.builders.package_builder import PackageBuilder  # noqa
+from giftwrap.builders.docker_builder import DockerBuilder  # noqa
+
+
+class BuilderFactory:
+
+    @staticmethod
+    def create_builder(builder_type, build_spec):
+        targetclass = "%sBuilder" % builder_type.capitalize()
+        return globals()[targetclass](build_spec)

giftwrap/builders/docker_builder.py

@@ -22,7 +22,7 @@ import os
 import re
 import tempfile
 
-from giftwrap.builder import Builder
+from giftwrap.builders import Builder
 
 LOG = logging.getLogger(__name__)
 
@@ -41,7 +41,6 @@ APT_REQUIRED_PACKAGES = [
     'libssl-dev',
     'python-dev',
     'libmysqlclient-dev',
-    'python-virtualenv',
     'python-pip',
     'build-essential'
 ]
@@ -51,61 +50,71 @@ DEFAULT_SRC_PATH = '/opt/openstack'
 class DockerBuilder(Builder):
 
     def __init__(self, spec):
-        self.template = DEFAULT_TEMPLATE_FILE
         self.base_image = 'ubuntu:12.04'
         self.maintainer = 'maintainer@example.com'
         self.envvars = {'DEBIAN_FRONTEND': 'noninteractive'}
-        self._paths = []
+        self._commands = []
         super(DockerBuilder, self).__init__(spec)
 
-    def _validate_settings(self):
-        pass
+    def _execute(self, command, cwd=None, exit=0):
+        if cwd:
+            self._commands.append("cd %s" % (cwd))
+        self._commands.append(command)
+        if cwd:
+            self._commands.append("cd -")
 
-    def _cleanup(self):
-        pass
+    def _make_temp_dir(self, prefix='giftwrap'):
+        return "/tmp/giftwrap"
+        self._commands.append("mktemp -d -t %s.XXXXXXXXXX" % prefix)
 
-    def _get_prep_commands(self):
-        commands = []
-        commands.append('apt-get update && apt-get install -y %s' %
-                        ' '.join(APT_REQUIRED_PACKAGES))
-        return commands
+    def _make_dir(self, path, mode=0777):
+        self._commands.append("mkdir -p -m %o %s" % (mode, path))
 
-    def _get_build_commands(self, src_path):
-        commands = []
-        commands.append('mkdir -p %s' % src_path)
+    def _prepare_project_build(self, project):
+        return
 
-        for project in self._spec.projects:
-            if project.system_dependencies:
-                commands.append('apt-get update && apt-get install -y %s' %
-                                ' '.join(project.system_dependencies))
+    def _clone_project(self, giturl, name, gitref, depth, path):
+        cmd = "git clone %s -b %s --depth=%d %s" % (giturl, gitref,
+                                                    depth, path)
+        self._commands.append(cmd)
 
-            project_src_path = os.path.join(src_path, project.name)
-            commands.append('git clone --depth 1 %s -b %s %s' %
-                            (project.giturl, project.gitref, project_src_path))
-            commands.append('COMMIT=`git rev-parse HEAD` && echo "%s $COMMIT" '
-                            '> %s/gitinfo' % (project.giturl,
-                                              project.install_path))
-            commands.append('mkdir -p %s' %
-                            os.path.dirname(project.install_path))
-            commands.append('virtualenv --system-site-packages %s' %
-                            project.install_path)
+    def _create_virtualenv(self, venv_command, path):
+        self._execute(venv_command, path)
 
-            project_bin_path = os.path.join(project.install_path, 'bin')
-            self._paths.append(project_bin_path)
-            venv_pip_path = os.path.join(project_bin_path, 'pip')
+    def _install_pip_dependencies(self, venv_path, dependencies):
+        pip_path = self._get_venv_pip_path(venv_path)
+        self._execute("%s install %s" % (pip_path, dependencies))
 
-            if project.pip_dependencies:
-                commands.append("%s install %s" % (venv_pip_path,
-                                ' '.join(project.pip_dependencies)))
-            commands.append("%s install %s" % (venv_pip_path,
-                                               project_src_path))
+    def _copy_sample_config(self, src_clone_dir, project):
+        src_config = os.path.join(src_clone_dir, 'etc')
+        dest_config = os.path.join(project.install_path, 'etc')
 
-        return commands
+        self._commands.append("if [ -d %s ]; then cp -R %s %s; fi" % (
+            src_config, src_config, dest_config))
 
-    def _get_cleanup_commands(self, src_path):
-        commands = []
-        commands.append('rm -rf %s' % src_path)
-        return commands
+    def _install_project(self, venv_path, src_clone_dir):
+        pip_path = self._get_venv_pip_path(venv_path)
+        self._execute("%s install %s" % (pip_path, src_clone_dir))
+
+    def _finalize_project_build(self, project):
+        self._commands.append("rm -rf %s" % self._temp_dir)
+        for command in self._commands:
+            print command
+
+    def _finalize_build(self):
+        template_vars = {
+            'commands': self._commands
+        }
+        print self._render_dockerfile(template_vars)
+        self._build_image()
+
+    def _cleanup_build(self):
+        return
+
+    def _prepare_build(self):
+        self._commands.append('apt-get update && apt-get install -y %s' %
+                              ' '.join(APT_REQUIRED_PACKAGES))
+        self._commands.append("pip install -U pip virtualenv")
 
     def _set_path(self):
         path = ":".join(self._paths)
@@ -116,16 +125,14 @@ class DockerBuilder(Builder):
         template_vars.update(extra_vars)
         template_loader = jinja2.FileSystemLoader(searchpath='/')
         template_env = jinja2.Environment(loader=template_loader)
-        template = template_env.get_template(self.template)
+        template = template_env.get_template(DEFAULT_TEMPLATE_FILE)
         return template.render(template_vars)
 
-    def _build(self):
-        src_path = DEFAULT_SRC_PATH
-        commands = self._get_prep_commands()
-        commands += self._get_build_commands(src_path)
-        commands += self._get_cleanup_commands(src_path)
-        self._set_path()
-        dockerfile_contents = self._render_dockerfile(locals())
+    def _build_image(self):
+        template_vars = {
+            'commands': self._commands
+        }
+        dockerfile_contents = self._render_dockerfile(template_vars)
 
         tempdir = tempfile.mkdtemp()
         dockerfile = os.path.join(tempdir, 'Dockerfile')

giftwrap/builders/package_builder.py

@@ -20,8 +20,7 @@ import os
 import shutil
 import tempfile
 
-from giftwrap.builder import Builder
-from giftwrap.gerrit import GerritReview
+from giftwrap.builders import Builder
 from giftwrap.openstack_git_repo import OpenstackGitRepo
 from giftwrap.package import Package
 from giftwrap.util import execute
@@ -32,95 +31,73 @@ LOG = logging.getLogger(__name__)
 class PackageBuilder(Builder):
 
     def __init__(self, spec):
-        self._tempdir = None
+        self._temp_dir = None
         super(PackageBuilder, self).__init__(spec)
 
-    def _validate_settings(self):
-        pass
+    def _execute(self, command, cwd=None, exit=0):
+        return execute(command, cwd, exit)
 
-    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), install_path)
-        except Exception as e:
-            LOG.warning("Could not install gerrit dependencies!!! "
-                        "Error was: %s", e)
+    def _make_temp_dir(self, prefix='giftwrap'):
+        return tempfile.mkdtemp(prefix)
 
-    def _build(self):
-        spec = self._spec
+    def _make_dir(self, path, mode=0777):
+        os.makedirs(path, mode)
 
-        self._tempdir = tempfile.mkdtemp(prefix='giftwrap')
-        src_path = os.path.join(self._tempdir, 'src')
-        LOG.debug("Temporary working directory: %s", self._tempdir)
+    def _prepare_build(self):
+        return
 
-        for project in spec.projects:
-            LOG.info("Beginning to build '%s'", project.name)
+    def _prepare_project_build(self, project):
+        install_path = project.install_path
 
-            install_path = project.install_path
-            LOG.debug("Installing '%s' to '%s'", project.name, install_path)
+        LOG.info("Beginning to build '%s'", project.name)
+        if os.path.exists(install_path):
+            if self._spec.settings.force_overwrite:
+                LOG.info("force_overwrite is set, so removing "
+                         "existing path '%s'" % install_path)
+                shutil.rmtree(install_path)
+            else:
+                raise Exception("Install path '%s' already exists" %
+                                install_path)
 
-            # if anything is in our way, see if we can get rid of it
-            if os.path.exists(install_path):
-                if spec.settings.force_overwrite:
-                    LOG.info("force_overwrite is set, so removing "
-                             "existing path '%s'" % install_path)
-                    shutil.rmtree(install_path)
-                else:
-                    raise Exception("Install path '%s' already exists" %
-                                    install_path)
-            os.makedirs(install_path)
+    def _clone_project(self, giturl, name, gitref, depth, path):
+        LOG.info("Fetching source code for '%s'", name)
+        repo = OpenstackGitRepo(giturl, name, gitref, depth)
+        repo.clone(path)
+        return repo
 
-            # clone the project's source to a temporary directory
-            project_src_path = os.path.join(src_path, project.name)
-            os.makedirs(project_src_path)
+    def _create_virtualenv(self, venv_command, path):
+        self._execute(venv_command, path)
 
-            LOG.info("Fetching source code for '%s'", project.name)
-            repo = OpenstackGitRepo(project.giturl, project.name,
-                                    project.gitref,
-                                    depth=project.gitdepth)
-            repo.clone(project_src_path)
+    def _install_pip_dependencies(self, venv_path, dependencies):
+        pip_path = self._get_venv_pip_path(venv_path)
+        self._execute("%s install %s" % (pip_path, dependencies))
 
-            # tell package users where this came from
-            gitinfo_file = os.path.join(install_path, 'gitinfo')
-            with open(gitinfo_file, 'w') as fh:
-                fh.write("%s %s" % (project.giturl, repo.head.hexsha))
+    def _copy_sample_config(self, src_clone_dir, project):
+        src_config = os.path.join(src_clone_dir, 'etc')
+        dest_config = os.path.join(project.install_path, 'etc')
 
-            # start building the virtualenv for the project
-            LOG.info("Creating the virtualenv for '%s'", project.name)
-            execute(project.venv_command, install_path)
+        if not os.path.exists(src_config):
+            LOG.warning("Project configuration does not seem to exist "
+                        "in source repo '%s'. Skipping.", project.name)
+        else:
+            LOG.debug("Copying config from '%s' to '%s'", src_config,
+                      dest_config)
+        distutils.dir_util.copy_tree(src_config, dest_config)
 
-            # install into the virtualenv
-            LOG.info("Installing '%s' to the virtualenv", project.name)
-            venv_pip_path = os.path.join(install_path, 'bin/pip')
+    def _install_project(self, venv_path, src_clone_dir):
+        pip_path = self._get_venv_pip_path(venv_path)
+        self._execute("%s install %s" % (pip_path, src_clone_dir))
 
-            deps = " ".join(project.pip_dependencies)
-            execute("%s install %s" % (venv_pip_path, deps))
+    def _finalize_project_build(self, project):
+        # build the package
+        pkg = Package(project.package_name, project.version,
+                      project.install_path, self._spec.settings.output_dir,
+                      self._spec.settings.force_overwrite,
+                      project.system_dependencies)
+        pkg.build()
 
-            if spec.settings.include_config:
-                src_config = os.path.join(project_src_path, 'etc')
-                dest_config = os.path.join(install_path, 'etc')
-                if not os.path.exists(src_config):
-                    LOG.warning("Project configuration does not seem to exist "
-                                "in source repo '%s'. Skipping.", project.name)
-                else:
-                    LOG.debug("Copying config from '%s' to '%s'", src_config,
-                              dest_config)
-                    distutils.dir_util.copy_tree(src_config, dest_config)
+    def _finalize_build(self):
+        return
 
-            if spec.settings.gerrit_dependencies:
-                self._install_gerrit_dependencies(repo, project, install_path)
-
-            execute("%s install %s" % (venv_pip_path, project_src_path))
-
-            # now build the package
-            pkg = Package(project.package_name, project.version,
-                          install_path, spec.settings.output_dir,
-                          spec.settings.force_overwrite,
-                          project.system_dependencies)
-            pkg.build()
-
-    def _cleanup(self):
-        shutil.rmtree(self._tempdir)
+    def _cleanup_build(self):
+        shutil.rmtree(self._temp_dir)

giftwrap/gerrit.py

@@ -50,7 +50,8 @@ class GerritReview(object):
                     freeze_found = True
                 continue
             elif re.match('[\w\-]+==.+', line) and not line.startswith('-e'):
-                dependencies.append(line)
+                dependency = line.split('#')[0].strip()  # remove any comments
+                dependencies.append(dependency)
 
         short_name = self.project.split('/')[1]
         dependencies = filter(lambda x: not x.startswith(short_name + "=="),

giftwrap/openstack_project.py

@@ -23,7 +23,7 @@ DEFAULT_GITURL = {
     'openstack': 'https://git.openstack.org/openstack/',
     'stackforge': 'https://github.com/stackforge/'
 }
-DEFAULT_VENV_COMMAND = "virtualenv ."
+DEFAULT_VENV_COMMAND = "virtualenv --no-wheel ."
 DEFAULT_INSTALL_COMMAND = "./bin/pip install %s"  # noqa
 
 TEMPLATE_VARS = ('name', 'version', 'gitref', 'stackforge')
@@ -32,7 +32,7 @@ TEMPLATE_VARS = ('name', 'version', 'gitref', 'stackforge')
 class OpenstackProject(object):
 
     def __init__(self, settings, name, version=None, gitref=None, giturl=None,
-                 gitdepth=None, venv_command=None, install_command=None,
+                 gitdepth=1, venv_command=None, install_command=None,
                  install_path=None, package_name=None, stackforge=False,
                  system_dependencies=[], pip_dependencies=[]):
         self._settings = settings

giftwrap/shell.py

@@ -19,8 +19,7 @@ import logging
 import signal
 import sys
 
-import giftwrap.builder
-
+from giftwrap.builders import BuilderFactory
 from giftwrap.build_spec import BuildSpec
 from giftwrap.color import ColorStreamHandler
 
@@ -48,7 +47,7 @@ def build(args):
             manifest = fh.read()
 
         buildspec = BuildSpec(manifest, args.version, args.type)
-        builder = giftwrap.builder.create_builder(buildspec)
+        builder = BuilderFactory.create_builder(args.type, buildspec)
 
         def _signal_handler(*args):
             LOG.info("Process interrrupted. Cleaning up.")
@@ -79,7 +78,8 @@ def main():
                                          description='build giftwrap packages')
     build_subcmd.add_argument('-m', '--manifest', required=True)
     build_subcmd.add_argument('-v', '--version')
-    build_subcmd.add_argument('-t', '--type', choices=('docker', 'package'))
+    build_subcmd.add_argument('-t', '--type', choices=('docker', 'package'),
+                              required=True)
     build_subcmd.set_defaults(func=build)
 
     args = parser.parse_args()

giftwrap/templates/Dockerfile.jinja2

@@ -7,6 +7,4 @@ MAINTAINER {{ maintainer }}
 ENV {{ k }} {{ v }}
 {% endfor -%}
 
-{% for command in commands -%}
-RUN {{ command }}
-{% endfor %}
+RUN {% for command in commands[:-1] -%}{{ command|safe }} && {% endfor -%} {{ commands[-1]|safe }}