Merge lp:~sidnei/juju-deployer/build-command into lp:~gandelman-a/juju-deployer/trunk

[Sidnei da Silva]

- Add minimal support for a 'build' step after fetching the charm source
- Resolve includes as relative to the config files provided on command line if they are relative paths
- General code cleanup to conform to pep8

- Add minimal support for a 'build' step after fetching the charm source
- Resolve includes as relative to the config files provided on command line if they are relative paths
- General code cleanup to conform to pep8

[lp:~sidnei/juju-deployer/build-command]

67. By Sidnei da Silva on 2013-03-04

- Merge from trunk, resolve conflicts

68. By Sidnei da Silva on 2013-03-04

- Fix bad merge

69. By Sidnei da Silva on 2013-03-04

- Also set non-include options.

=== modified file 'deployer.py'
--- deployer.py	2013-03-01 18:09:35 +0000
+++ deployer.py	2013-03-04 19:16:20 +0000
@@ -4,89 +4,96 @@
 import subprocess
 import time
 import os
-import sys
 import yaml
 import json
 import optparse
 import pprint
 import signal
 import __builtin__
+
+from os.path import dirname, abspath
 from pdb import *
-
 from utils import *
 
+
 def timed_out(signal, frame):
     log.error("Deployment timed out after %s sec.", opts.timeout)
     exit(1)
 
-start_time=time.time()
+start_time = time.time()
 
 parser = optparse.OptionParser()
 parser.add_option('-c', '--config',
-                  help='File containing deployment(s) json config. This option can be repeated, with later files overriding values in earlier ones.',
+                  help=('File containing deployment(s) json config. This '
+                        'option can be repeated, with later files overriding '
+                        'values in earlier ones.'),
                   dest='configs', action='append')
-parser.add_option('-d', '--debug', help='Enable debugging to stdout', dest="debug",
+parser.add_option('-d', '--debug', help='Enable debugging to stdout',
+                  dest="debug",
                   action="store_true", default=False)
 parser.add_option('-L', '--local-mods',
                   help='Allow deployment of locally-modified charms',
                   dest="no_local_mods", default=True, action='store_false')
-parser.add_option('-u', '--update-charms', help='Update existing charm branches',
+parser.add_option('-u', '--update-charms',
+                  help='Update existing charm branches',
                   dest="update_charms", default=False, action="store_true")
 parser.add_option('-l', '--ls', help='List available deployments',
-                  dest="list_deploys", action="store_true", default=False )
+                  dest="list_deploys", action="store_true", default=False)
 parser.add_option('-D', '--destroy-services',
                   help='Destroy all services (do not terminate machines)',
-                  dest="destroy_services", action="store_true", default="False")
+                  dest="destroy_services", action="store_true",
+                  default="False")
 parser.add_option('-S', '--scrub-zk', action='store_true', default=False,
-                   dest='scrub_zk',
-                   help='Remove charm nodes from ZK tree after service destroy.')
+                  dest='scrub_zk',
+                  help='Remove charm nodes from ZK after service destroy.')
 parser.add_option('-T', '--terminate-machines',
-                  help='Terminate all machines but the bootstrap node.  Destroy'\
-                       'any services that exist on each',
-                  dest="terminate_machines", action="store_true", default="False")
+                  help=('Terminate all machines but the bootstrap node.  '
+                        'Destroy any services that exist on each'),
+                  dest="terminate_machines", action="store_true",
+                  default="False")
 parser.add_option('-t', '--timeout',
                   help='Timeout (sec) for entire deployment (45min default)',
                   dest='timeout', action='store', type='int', default=2700)
 parser.add_option("-f", '--find-service', action="store", type="string",
-                  help='Return hostname of the first unit of a specific service.',
+                  help='Find hostname from first unit of a specific service.',
                   dest="find_service")
 parser.add_option("-m", '--max-concurrent', action="store", type="int",
-                  help="Maximum number of concurrent deployments to send to "\
-                       "provider. Default: no limit",
+                  help=("Maximum number of concurrent deployments to send "
+                        " to provider. Default: no limit"),
                   dest="max_concur_deploy", default=0)
 parser.add_option('-s', '--deploy-delay', action='store', type='float',
-                  help="Time in seconds to sleep between 'deploy' commands, "\
-                       "to allow machine provider to process requests. This "\
-                       "delay is also enforced between calls to"
-                       "terminate_machine",
+                  help=("Time in seconds to sleep between 'deploy' commands, "
+                        "to allow machine provider to process requests. This "
+                        "delay is also enforced between calls to"
+                        "terminate_machine"),
                   dest="deploy_delay", default=0)
 parser.add_option('-e', '--environment', action='store', dest='juju_env',
                   help='Deploy to a specific Juju environment.',
                   default=os.getenv('JUJU_ENV'))
 parser.add_option('-o', '--override', action='append', type='string',
-                  help='Override *all* config options of the same name '\
-                       'across all services.  Input as key=value.',
+                  help=('Override *all* config options of the same name '
+                        'across all services.  Input as key=value.'),
                   dest='overrides', default=None)
 parser.add_option('-w', '--relation-wait', action='store', dest='rel_wait',
                   default='60',
-                  help='Number of seconds to wait before checking for relation '\
-                       'errors after all relations have been added and '\
-                       'subordinates started. (default: 60)')
+                  help=('Number of seconds to wait before checking for '
+                        'relation errors after all relations have been added '
+                        'and subordinates started. (default: 60)'))
 (opts, args) = parser.parse_args()
 
 if not opts.configs:
     opts.configs = ['deployments.cfg']
-update_charms=opts.update_charms
+update_charms = opts.update_charms
 
 # temporarily abuse __builtin__ till this is setup properly
 __builtin__.juju_log = juju_log = open("juju.log", "w")
-__builtin__.juju_cmds=[]
+__builtin__.juju_cmds = []
 
 init_logging("debug.log", opts.debug)
 
-ORIGCWD=os.getcwd()
+ORIGCWD = os.getcwd()
 
-if opts.destroy_services == True or opts.terminate_machines == True:
+if opts.destroy_services is True or opts.terminate_machines is True:
     destroy_all(juju_status(opts.juju_env), opts.juju_env,
                 terminate_machines=opts.terminate_machines,
                 scrub_zk=opts.scrub_zk,
@@ -94,7 +101,7 @@
     exit(0)
 
 if opts.find_service is not None:
-    rc=find_service(juju_status(opts.juju_env), opts.find_service)
+    rc = find_service(juju_status(opts.juju_env), opts.find_service)
     exit(rc)
 
 # load the json configuration for possible deployments.
@@ -105,8 +112,10 @@
 
 debug_msg("Loading deployments from {}".format(", ".join(opts.configs)))
 cfg = {}
+include_dirs = []
 for config in opts.configs:
-    with open(config,'r') as f:
+    include_dirs.append(dirname(abspath(config)))
+    with open(config, 'r') as f:
         try:
             cfg = dict_merge(cfg, json.load(f))
         except ValueError as exc:
@@ -125,7 +134,7 @@
 
 deployment = args[0]
 SERIES, CHARMS, RELATIONS, overrides = load_deployment(cfg, deployment)
-series_store="%s/%s" % (ORIGCWD, SERIES)
+series_store = "%s/%s" % (ORIGCWD, SERIES)
 
 # series store ends up being the local juju charm repository
 if not os.path.exists(series_store):
@@ -139,16 +148,19 @@
     charm_path = "%s/%s" % (series_store, k)
     debug_msg("Charm '%s' - using charm path '%s'" % (k, charm_path))
     (branch, sep, revno) = CHARMS[k].get("branch", '').partition('@')
+    needs_build = update_charms
     if branch:
         debug_msg("Branch: {}, revision: {}".format(branch, revno))
     else:
         debug_msg("No remote branch specified")
+        needs_build = False
     if os.path.exists(charm_path):
         if opts.no_local_mods:
             with cd(charm_path):
                 # is there a better way to check for changes?
-                bzrstatus = subprocess.check_output(['bzr','st']).strip()
-                if bzrstatus not in ("","working tree is out of date, run 'bzr update'"):
+                bzrstatus = subprocess.check_output(['bzr', 'st']).strip()
+                if bzrstatus not in (
+                    "", "working tree is out of date, run 'bzr update'"):
                     log.error("Charm is locally modified: {}".format(
                         charm_path))
                     log.error("Aborting")
@@ -157,14 +169,15 @@
         if update_charms and branch:
             debug_msg("Updating charm branch '%s'" % k)
             code = subprocess.call(
-                    ["bzr", "pull", "-d", charm_path, '--remember', branch])
+                ["bzr", "pull", "-d", charm_path, '--remember', branch])
             if code != 0:
                 log.error("Could not update branch at {} from {}".format(
-                        charm_path, branch))
+                    charm_path, branch))
                 exit(code)
     elif branch:
         print "- Cloning %s from %s" % (k, branch)
         subprocess.call(["bzr", "branch", branch, charm_path])
+        needs_build = True
     if revno:
         cmd = ["bzr", "update", charm_path]
         revno != 'tip' and cmd.extend(['-r', revno])
@@ -172,6 +185,14 @@
         if code != 0:
             log.error("Unable to check out branch revision {}".format(revno))
             exit(code)
+    if CHARMS[k].get("build") is not None and needs_build:
+        cmd = CHARMS[k]["build"]
+        debug_msg("Running build command at {}...".format(charm_path))
+        with cd(charm_path):
+            code = subprocess.call(cmd)
+        if code != 0:
+            log.error("Failed to build charm {}".format(k))
+            exit(code)
     # load charms metadata
     if not os.path.isdir(charm_path):
         print "Branch for {} does not exist ({})".format(k, charm_path)
@@ -208,7 +229,7 @@
 # create a temporary deploy-time config yaml
 temp = tempfile.NamedTemporaryFile()
 deploy_config = temp.name
-CONFIG = generate_deployment_config(temp, CHARMS)
+CONFIG = generate_deployment_config(temp, CHARMS, include_dirs)
 log.debug("Using the following config:\n%s", pprint.pformat(CONFIG))
 
 # make sure we're bootstrapped
@@ -216,8 +237,8 @@
 if status == 1:
     log.error("Is juju bootstrapped?")
     exit(1)
-if status["machines"][0]["instance-state"] != "provisioned" and \
-    status["machines"][0]["agent-state"] != "running":
+if (status["machines"][0]["instance-state"] != "provisioned" and
+    status["machines"][0]["agent-state"] != "running"):
     log.error("Bootstrap node not running?")
     exit(1)
 
@@ -226,7 +247,7 @@
 signal.alarm(opts.timeout)
 
 # figure out what needs to be done
-to_deploy=[]
+to_deploy = []
 for c in CHARMS.keys():
     if c not in status["services"].keys():
         to_deploy.append(c)
@@ -241,12 +262,12 @@
 start_groups = []
 if to_deploy:
     # go through to_deploy in chunks of group_by
-    start_groups = [to_deploy[i:i+groups_of]
-                    for i  in range(0, len(to_deploy), groups_of)]
+    start_groups = [to_deploy[i:i + groups_of]
+                    for i in range(0, len(to_deploy), groups_of)]
     for group_num in range(0, len(start_groups)):
         for c in start_groups[group_num]:
-            print "- Deploying %s in group %d/%d" % \
-                (c, group_num+1, len(start_groups))
+            print ("- Deploying %s in group %d/%d" %
+                   (c, group_num + 1, len(start_groups)))
             cmd = "deploy"
             if "units" in CHARMS[c]:
                 cmd += " -n %s" % CHARMS[c]["units"]
@@ -254,16 +275,18 @@
                 cmd += " --constraints=%s" % CHARMS[c]["constraints"]
             if c in CONFIG.keys():
                 cmd += " --config=%s" % deploy_config
-            cmd += " --repository=%s local:%s %s" % (ORIGCWD,
-                CHARMS[c]["metadata"]["name"], c)
+            cmd += " --repository=%s local:%s %s" % (
+                ORIGCWD, CHARMS[c]["metadata"]["name"], c)
             if opts.juju_env:
                 cmd += " -e %s" % opts.juju_env
             juju_call(cmd)
             if opts.deploy_delay > 0:
-                debug_msg("Delaying %s sec. between deployment" % opts.deploy_delay)
+                debug_msg("Delaying %s sec. between deployment" %
+                          opts.deploy_delay)
                 time.sleep(opts.deploy_delay)
         wait_for_started(opts.debug, opts.juju_env, sleep=3.0,
-           msg="- Waiting for started: %s" % start_groups[group_num])
+                         msg="- Waiting for started: %s" %
+                         start_groups[group_num])
 
 if len(start_groups) != 0:
     status = juju_status(opts.juju_env)
@@ -280,13 +303,12 @@
                 cmd = "add-unit --num-units %d %s" % (needed_units, c)
                 juju_call(cmd)
         else:
-            debug_msg("Service '%s' does not require any more units added." % c)
+            debug_msg("Service '%s' does not need any more units added." % c)
 
 # poll juju status until all services report strated. fail on any error
 wait_for_started(opts.debug, opts.juju_env,
-  "- Waiting for all service units to reach 'started' state.")
+                 "- Waiting for all service units to reach 'started' state.")
 
-time.sleep(1)
 # add all relations, ordered by weight
 if RELATIONS:
     print "- Adding relations:"
@@ -313,7 +335,9 @@
 if not ensure_relations_up(juju_status(opts.juju_env)):
     exit(1)
 
-print "- Deployment complete in %d seconds.\n\n" % (int(time.time() - start_time))
+print ("- Deployment complete in %d seconds.\n\n" %
+       (int(time.time() - start_time)))
+
 print "- Juju command log:"
 for c in __builtin__.juju_cmds:
     print c

=== modified file 'utils.py'
--- utils.py	2013-02-21 02:05:38 +0000
+++ utils.py	2013-03-04 19:16:20 +0000
@@ -2,10 +2,11 @@
 import subprocess
 import yaml
 import __builtin__
-import pdb
 import sys
 import time
 import os
+
+from os.path import isabs, join, exists
 from contextlib import contextmanager
 from copy import deepcopy
 from base64 import b64encode
@@ -24,10 +25,12 @@
     finally:
         os.chdir(cwd)
 
+
 def relations_combine(onto, source):
     target = deepcopy(onto)
     for (key, value) in source.items():
-        if key in target and isinstance(target[key], dict) and isinstance(value, dict):
+        if (key in target and isinstance(target[key], dict) and
+            isinstance(value, dict)):
             target[key] = relations_combine(target[key], value)
         else:
             if isinstance(target[key], list) and isinstance(value, list):
@@ -36,10 +39,12 @@
                 target[key] = value
     return target
 
+
 def dict_merge(onto, source):
     target = deepcopy(onto)
     for (key, value) in source.items():
-        if key in target and isinstance(target[key], dict) and isinstance(value, dict):
+        if (key in target and isinstance(target[key], dict) and
+            isinstance(value, dict)):
             if key == 'relations':
                 target[key] = relations_combine(target[key], value)
             else:
@@ -65,7 +70,7 @@
 
 def ensure_started(status):
     """
-         ensure all service units in an environment are in the 'started' state
+    ensure all service units in an environment are in the 'started' state
     """
     started = True
     for s in status["services"]:
@@ -111,8 +116,8 @@
             if 'subordinates' in u:
                 sub = [(u['public-address'],
                         subunit,
-                        u['subordinates'][subunit]['agent-state']) \
-                        for subunit in u['subordinates']][0]
+                        u['subordinates'][subunit]['agent-state'])
+                       for subunit in u['subordinates']][0]
                 subs_status.append(sub)
 
     started = True
@@ -126,7 +131,8 @@
             subs.remove(sub)
 
         if 'error' in state:
-            log.error('Failed subordinate: %s @ %s, state: %s' % (unit, addr, state))
+            log.error('Failed subordinate: %s @ %s, state: %s' %
+                      (unit, addr, state))
             exit(1)
 
         if state != 'started':
@@ -137,11 +143,13 @@
 
     return started
 
+
 def wait_for_subordinates_started(debug, env):
     if not debug:
         sys.stdout.write('Waiting for subordinate units started')
         sys.stdout.flush()
-    while ensure_subordinates_started(juju_status(env)) == False:
+
+    while ensure_subordinates_started(juju_status(env)) is False:
         if not debug:
             sys.stdout.write('.')
             sys.stdout.flush()
@@ -149,6 +157,7 @@
     if not debug:
         sys.stdout.write('\n')
 
+
 def ensure_relations_up(status):
     """ ensure all relations are at least 'up' and error free """
     failed = []
@@ -158,7 +167,7 @@
         for u in status["services"][s]["units"]:
             if 'relation-errors' in status['services'][s]['units'][u]:
                 errors = status['services'][s]['units'][u]['relation-errors']
-                for k,v in errors.iteritems():
+                for k, v in errors.iteritems():
                     failed.append("%s:%s: %s" % (u, k, v))
     if failed:
         log.error("Failed relations: %s\n\t", "\n\t".join(failed))
@@ -166,38 +175,74 @@
 
     return True
 
-def generate_deployment_config(temp, charms):
+
+def resolve_include(fname, include_dirs):
+    if isabs(fname):
+        return fname
+
+    for path in include_dirs:
+        full_path = join(path, fname)
+        if exists(full_path):
+            return full_path
+
+    return None
+
+
+def generate_deployment_config(temp, charms, include_dirs):
     """ for a given deployment, generate a deploy-time temp. config yaml for
         services that have options specified in deployments.cfg
     """
+    # When resolving includes, use reverse order of the include_dirs to lookup
+    # relative includes.
+    include_dirs = include_dirs[::-1]
+
     config = {}
     for c in charms:
         cc = {}
-        if "options" in charms[c].keys():
-            if "config" not in charms[c].keys():
-                debug_msg("WARNING: Options passed to %s, but it has" \
-                     "no config.yaml" % (c))
+        charm_opts = charms[c].get("options")
+        charm_cfg = charms[c].get("config")
+        if charm_opts is None:
+            continue
+        if charm_cfg is None:
+            debug_msg("WARNING: Options passed to %s, but it has"
+                      "no config.yaml" % (c))
+            continue
+        for opt, val in charm_opts.items():
+            if not opt in charm_cfg.keys():
+                debug_msg("WARNING: Skipping unknown config "
+                          "option to %s: %s" % (c, opt))
+                continue
+
+            debug_msg("Adding option to '%s' to deploy config: %s"
+                      % (opt, c))
+            if (isinstance(val, basestring) and
+                (val.startswith("include-file://")
+                 or val.startswith("include-base64://"))):
+                include, fname = val.split("://", 1)
+                full_path = resolve_include(fname, include_dirs)
+                if full_path is None:
+                    debug_msg("WARNING: Skipping non-existing "
+                              "include file %s." % fname)
+                    continue
+                else:
+                    debug_msg("Using external file: %s for '%s'"
+                              % (full_path, opt))
+
+                with open(full_path, 'r') as f:
+                    cc[opt] = f.read()
+
+                if include.endswith("base64"):
+                    cc[opt] = b64encode(cc[opt])
             else:
-                for opt, val in charms[c]["options"].items():
-                    if opt in charms[c]["config"].keys():
-                        debug_msg("Adding option to '%s' to deploy config: %s" \
-                            % (opt, c))
-                        if isinstance(val, basestring) and val.startswith("include-file://"):
-                            with open(val[15:], 'r') as f:
-                                cc[opt] = f.read()
-                        elif isinstance(val, basestring) and val.startswith("include-base64://"):
-                            with open(val[17:], 'r') as f:
-                                cc[opt] = b64encode(f.read())
-                        else:
-                            cc[opt] = val
-                    else:
-                        debug_msg("WARNING: Skipping unknown config option to" \
-                            "%s: %s" % (c, opt))
-            if cc:
-                config[c] = cc
+                cc[opt] = val
+
+        if cc:
+            config[c] = cc
+
     yaml.safe_dump(config, temp)
     return config
 
+
 def juju_status(juju_env=None):
     """ load yaml output of 'juju status'
         retry failed 'status' calls, which sometimes seems to
@@ -209,8 +254,8 @@
         if juju_env:
             cmd = cmd + ['-e', juju_env]
         p = subprocess.Popen(cmd,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE)
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
         p.poll()
         (stdout, stderr) = p.communicate()
         return (stdout, stderr, p.returncode)
@@ -222,7 +267,7 @@
     while rc != 0:
         if attempt == max_retries:
             log.error("Could not get Juju status after %s attempts, "
-                      " giving up.")
+                      " giving up.", attempt)
             sys.exit(1)
         log.error("Call to 'juju status' failed!")
         log.error(stderr)
@@ -232,6 +277,7 @@
     y = yaml.load(stdout)
     return y
 
+
 def juju_call(cmd, retries=5, ignore_failure=False):
     juju_log = __builtin__.juju_log
 
@@ -254,14 +300,17 @@
             log.error("Call to '%s', ignoring." % (cmd))
             return
         if attempt == retries:
-            log.error('Call to "%s" failed %s times, giving up.' % (cmd, retries))
+            log.error('Call to "%s" failed %s times, giving up.' %
+                      (cmd, retries))
             sys.exit(1)
         attempt += 1
-        log.error('Call to "%s" failed, %s attempts left.' % (cmd, (retries - attempt)))
+        log.error('Call to "%s" failed, %s attempts left.' %
+                  (cmd, (retries - attempt)))
         time.sleep(3)
         rc = _call(cmd, attempt)
     return
 
+
 def find_service(status, service):
     if service == "bootstrap":
         if "machines" not in status.keys():
@@ -278,10 +327,11 @@
     if len(status["services"][service]["units"]) == 0:
         log.error("No units assigned to %s", service)
         return 1
-    first_unit=status["services"][service]["units"].keys()[0]
+    first_unit = status["services"][service]["units"].keys()[0]
     print status['services'][service]["units"][first_unit]["public-address"]
     return 0
 
+
 def destroy_all(status=None, env=None, terminate_machines=False,
                 scrub_zk=False, delay=None):
     """ destroy all services and optionally terminate all machines
@@ -299,7 +349,7 @@
         subordinates = []
         for s in status['services']:
             if ('subordinate' in status['services'][s] and
-                status['services'][s]['subordinate'] == True):
+                status['services'][s]['subordinate'] is True):
                 subordinates.append(s)
             else:
                 principles.append(s)
@@ -319,7 +369,7 @@
         debug_msg('Cleaning cached charms from ZK.')
         scrub_zookeeper(destroyed)
 
-    if len(status["machines"]) > 0 and terminate_machines == True:
+    if len(status["machines"]) > 0 and terminate_machines is True:
         print "- Destroying machines"
         machines = status["machines"].keys()
         for m in machines[1:]:
@@ -327,9 +377,9 @@
             juju_call("terminate-machine %s%s" % (m, env_arg))
             if delay:
                 debug_msg("Sleeping for %s between after 'terminate-machine'"
-                           % delay)
+                          % delay)
                 time.sleep(delay)
-    elif len(status["machines"]) == 0 and terminate_machines == True:
+    elif len(status["machines"]) == 0 and terminate_machines is True:
         debug_msg("No machines to destroy")
 
 
@@ -341,6 +391,7 @@
             out += " (%s)" % deployments[d]['series']
         print out
 
+
 def determine_interface(CHARMS, relation):
     """ finds the appropriate interface for a given relation
         relation = tuple (consumer, provider).  Can also return None if a
@@ -354,17 +405,21 @@
             return
         for r in consumer["requires"].keys():
             if r in provider["provides"].keys():
-                if consumer["requires"][r]["interface"] == provider["provides"][r]["interface"]:
-                    debug_msg("Found interface for relation '%s': %s" % (relation, r))
+                if (consumer["requires"][r]["interface"] ==
+                    provider["provides"][r]["interface"]):
+                    debug_msg("Found interface for relation '%s': %s"
+                              % (relation, r))
                     return r, r
-        # Also try to match by interface type if not found by matching interface name
+        # Also try to match by interface type if not found by matching
+        # interface name
         for cr in consumer["requires"].keys():
             for pr in  provider["provides"].keys():
-                if consumer["requires"][cr]["interface"] == provider["provides"][pr]["interface"]:
-                    debug_msg("Found interface for relation '%s': %s" % (relation, cr))
+                if (consumer["requires"][cr]["interface"] ==
+                    provider["provides"][pr]["interface"]):
+                    debug_msg("Found interface for relation '%s': %s"
+                              % (relation, cr))
                     return cr, pr
 
-
     consumer = CHARMS[relation[0]]["metadata"]
     provider = CHARMS[relation[1]]["metadata"]
 
@@ -389,6 +444,7 @@
         return False
     return charm["subordinate"]
 
+
 def ensure_interface(CHARMS, services, interface):
     """ given a specified interface, ensure they are at least
         listed in the metadata of two charms. """
@@ -396,17 +452,18 @@
     provider = CHARMS[services[1]]["metadata"]
     if interface not in consumer["requires"].keys():
         log.error("Interface %s not listed as a required interface of %s",
-            services[0])
+                  services[0])
         return False
     if interface not in provider["providers"].keys():
         log.error("Interface %s not listed as a provided interface of %s",
-            services[1])
+                  services[1])
         return False
     return True
 
+
 def relations_json_to_tuples(relations):
     """ convert deployment.cfg json relation configuration
-        to a list of tuples that describe a relation as (consumer, provider) 
+        to a list of tuples that describe a relation as (consumer, provider)
     """
     t = {}
     for consumer in relations.keys():
@@ -415,11 +472,12 @@
             t[relations[consumer]["weight"]].append((consumer, provider))
     return t
 
+
 def wait_for_started(debug, env, msg="Waiting for all units started", sleep=1):
     if not debug:
         sys.stdout.write(msg)
         sys.stdout.flush()
-    while ensure_started(juju_status(env)) == False:
+    while ensure_started(juju_status(env)) is False:
         if not debug:
             sys.stdout.write(".")
             sys.stdout.flush()
@@ -427,6 +485,7 @@
     if not debug:
         sys.stdout.write("\n")
 
+
 def scrub_zookeeper(deleted_services):
     """ remove charm nodes from ZK for destroyed services. useful
         when recycling a juju environment and not having to worry
@@ -440,7 +499,7 @@
     from twisted.internet.defer import inlineCallbacks
     from juju.environment.config import EnvironmentsConfig
     from juju.state.service import ServiceStateManager
-    import zookeeper;
+    import zookeeper
 
     env_config = EnvironmentsConfig()
     env_config.load_or_write_sample()
@@ -453,7 +512,6 @@
     def _clean_juju_state():
         zookeeper.set_debug_level(0)
         provider = environment.get_machine_provider()
-        storage = provider.get_file_storage()
 
         client = yield provider.connect()
         charms = yield client.get_children("/charms")
@@ -482,6 +540,7 @@
     reactor.callWhenRunning(_clean_juju_state)
     reactor.run()
 
+
 def load_deployment(config, deployment):
     if deployment not in config.keys():
         log.error("deployment %s not found.", deployment)
@@ -499,8 +558,8 @@
             try:
                 cur = config[parent]
             except KeyError:
-                log.error("Could not find parent deployment in config: %s" %\
-                            parent)
+                log.error("Could not find parent deployment in config: %s" %
+                          parent)
                 exit(1)
 
         base = cur
@@ -510,8 +569,8 @@
         deploy_config = reduce(dict_merge, configs)
 
     series = deploy_config.get('series')
-    charms = deploy_config.get('services',{})
-    overrides = deploy_config.get('overrides',{})
+    charms = deploy_config.get('services', {})
+    overrides = deploy_config.get('overrides', {})
     if 'relations' in deploy_config:
         relations = relations_json_to_tuples(deploy_config["relations"])
     else: