cf-charmers team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

Benjamin Saller has proposed merging lp:~bcsaller/charms/trusty/cloudfoundry/cmd-refactor into lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.

Requested reviews:
  Cloud Foundry Charmers (cf-charmers)

For more details, see:
https://code.launchpad.net/~bcsaller/charms/trusty/cloudfoundry/cmd-refactor/+merge/245153

Refactor Command/Commander. Uniform response object, native bool cmp for exit status. 
Create a JUJU_REPO in tmp with the current dir linked in for cloudfoundry charm deploy
  to work around oddness with links
-- 
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~bcsaller/charms/trusty/cloudfoundry/cmd-refactor into lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.

=== modified file 'cfdeploy'
--- cfdeploy	2014-12-03 23:14:23 +0000
+++ cfdeploy	2014-12-18 19:48:29 +0000
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 # modeline vim:set syntax=python
 import argparse
+import atexit
 import logging
 import os
 import subprocess
@@ -29,8 +30,9 @@
 
 
 def prepare_runtime():
+    in_venv = os.environ.get('VIRTUAL_ENV', None)
     activate = "bin/activate_this.py"
-    if not verify_dep('virtualenv'):
+    if not verify_dep('virtualenv') and not in_venv:
         print "(sudo) installing python-virtualenv package."
         try:
             subprocess.check_output(
@@ -39,12 +41,13 @@
         except subprocess.CalledProcessError as e:
             print e.output
             raise
-    import virtualenv
-    venv = os.path.join(os.getcwd(), '.venv')
-    if not os.path.exists(venv):
-        virtualenv.create_environment(venv, site_packages=False)
-    venv_launcher = os.path.join(venv, activate)
-    execfile(venv_launcher, dict(__file__=venv_launcher))
+    if not in_venv:
+        import virtualenv
+        venv = os.path.join(os.getcwd(), '.venv')
+        if not os.path.exists(venv):
+            virtualenv.create_environment(venv, site_packages=False)
+        venv_launcher = os.path.join(venv, activate)
+        execfile(venv_launcher, dict(__file__=venv_launcher))
     install_python_deps()
 
 prepare_runtime()
@@ -69,6 +72,12 @@
 
 from progress.bar import Bar
 
+@atexit.register
+def reset_terminal():
+    sh.stty('sane')
+    sh.tput('rs1')
+    sh.reset()
+
 
 def setup():
     parser = argparse.ArgumentParser()
@@ -143,6 +152,7 @@
         root_logger = logging.getLogger()
         root_logger.handlers = [logging.FileHandler('cfdeploy.log', 'w')]
         root_logger.setLevel(logging.DEBUG)
+        sh.set_log(root_logger)
     install_deps()
 
     bar = ProgressBar('Deploying CloudFoundry', max=10)

=== modified file 'cloudfoundry/utils.py'
--- cloudfoundry/utils.py	2014-12-09 22:56:46 +0000
+++ cloudfoundry/utils.py	2014-12-18 19:48:29 +0000
@@ -4,9 +4,9 @@
 import logging
 import os
 import re
+import shutil
 import socket
 import subprocess
-import sys
 import tempfile
 import time
 from contextlib import contextmanager
@@ -17,7 +17,7 @@
 
 from charmhelpers import fetch
 
-
+log = logging.getLogger('utils')
 ip_pat = re.compile('^(\d{1,3}\.){3}\d{1,3}$')
 _client = None  # cached client connection
 
@@ -168,52 +168,115 @@
         return conf
 
 
-def command(*base_args, **kwargs):
-    check = kwargs.pop('check', False)
-    throw = kwargs.pop('throw', True)
-
-    def callable_command(*args, **kws):
-        kwargs.update(kws)
-        all_args = base_args + args
-        logging.debug(' '.join(all_args))
+class ProcessResult(object):
+    def __init__(self, command, exit_code, stdout, stderr):
+        self.command = command
+        self.exit_code = exit_code
+        self.stdout = stdout
+        self.stderr = stderr
+
+    def __repr__(self):
+        return '<ProcessResult "%s":%s>' % (self.command, self.exit_code)
+
+    @property
+    def cmd(self):
+        return ' '.join(self.command)
+
+    @property
+    def output(self):
+        result = ''
+        if self.stdout:
+            result += self.stdout
+        if self.stderr:
+            result += self.stderr
+        return result.strip()
+
+    @property
+    def json(self):
+        if self.stdout:
+            return json.loads(self.stdout)
+        return None
+
+    def __eq__(self, other):
+        return self.exit_code == other
+
+    def __bool__(self):
+        return self.exit_code == 0
+
+    def throw_on_error(self):
+        if not bool(self):
+            raise subprocess.CalledProcessError(
+                self.exit_code, self.command, output=self.output)
+
+
+class Process(object):
+    def __init__(self, command=None, throw=False, log=log, **kwargs):
+        if isinstance(command, str):
+            command = (command, )
+        self.command = command
+        self._throw_on_error = False
+        self.log = log
+        self._kw = kwargs
+
+    def throw_on_error(self, throw=True):
+        self._throw_on_error = throw
+        return self
+
+    def __call__(self, *args, **kw):
+        kwargs = dict(stdout=subprocess.PIPE,
+                      stderr=subprocess.STDOUT)
+        if self._kw:
+            kwargs.update(self._kw)
+        kwargs.update(kw)
+        if self.command:
+            all_args = self.command + args
+        else:
+            all_args = args
         if 'env' not in kwargs:
             kwargs['env'] = os.environ
-        logging.debug("invoke: %s", all_args)
+        p = subprocess.Popen(all_args, **kwargs)
+        stdout, stderr = p.communicate()
+        self.log.debug(stdout)
+        stdout = stdout.strip()
+        if stderr is not None:
+            stderr = stderr.strip()
+            self.log.debug(stderr)
+        exit_code = p.poll()
+        result = ProcessResult(all_args, exit_code, stdout, stderr)
+        self.log.debug("process: %s (%d)", result.cmd, result.exit_code)
+        if self._throw_on_error and not bool(self):
+            result.throw_on_error()
+        return result
 
-        p = subprocess.Popen(all_args,
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.STDOUT,
-                             **kwargs)
-        output, _ = p.communicate()
-        ret_code = p.poll()
-        logging.debug('output: %s', output)
-        if check is True:
-            if ret_code != 0 and throw:
-                raise subprocess.CalledProcessError(ret_code, all_args, output=output)
-            return ret_code
-        else:
-            return output.strip()
-    return callable_command
+command = Process
 
 
 class Commander(object):
+    def __init__(self, log=log):
+        self.log = log
+
+    def set_log(self, logger):
+        self.log = logger
+
     def __getattr__(self, key):
-        return command(key)
+        return command((key,), log=self.log)
 
     def check(self, *args, **kwargs):
-        return command(*args, check=True, **kwargs)
+        kwargs.update({'log': self.log})
+        return command(args, **kwargs).throw_on_error()
 
-    def __call__(self, cmd, **kwargs):
-        subprocess.check_output(cmd, shell=True, **kwargs)
+    def __call__(self, *args, **kwargs):
+        kwargs.update({'log': self.log})
+        return command(args, shell=True, **kwargs)
 
 
 sh = Commander()
 dig = command('dig', '+short')
-api_endpoints = sh.check('juju', 'api-endpoints', throw=False)
+api_endpoints = sh('juju', 'api-endpoints')
 
 
 def current_env():
-    return sh.juju('switch')
+    return sh.juju('switch').output
 
 
 def get_jenv():
@@ -285,9 +348,9 @@
 
 
 def juju_state_server():
-    if api_endpoints() != 0:
+    if not api_endpoints():
         return False
-    endpoints = json.loads(sh.juju('api-endpoints', '--format=json'))
+    endpoints = sh.juju('api-endpoints', '--format=json').json
     for ep in endpoints:
         host, port = ep.split(':', 1)
         result = socket_open(host, int(port))
@@ -374,9 +437,8 @@
 
 
 def bootstrap():
-    if not os.path.exists(get_jenv()) or api_endpoints() != 0:
-        juju = sh.check('juju', throw=False)
-        return juju('bootstrap') != 0
+    if not os.path.exists(get_jenv()) or not api_endpoints():
+        return bool(sh.juju('bootstrap', '--upload-tools'))
     return True
 
 
@@ -386,22 +448,16 @@
     return data['password']
 
 
-def get_repo_path():
-    candidates = [os.getcwd(), os.environ.get('JUJU_REPOSITORY')]
-    for repo_path in candidates:
-        if not repo_path:
-            continue
-        if 'trusty' in repo_path:
-            repo_path, _ = os.path.split(repo_path)
-            if repo_path.endswith('trusty'):
-                repo_path = os.path.dirname(repo_path)
-        if os.path.exists(os.path.join(repo_path, 'trusty', 'cloudfoundry')):
-            return repo_path
-    sys.stderr.write(
-        'Unable to determine valid repository path.\n'
-        'Please set JUJU_REPOSITORY and/or ensure that directory containing\n'
-        'the charm includes the series.\n')
-    sys.exit(1)
+@contextmanager
+def cf_repo():
+    """Context manager to ensure we are deploying the cloudfoundry charm
+    containing this instance of cfdeploy."""
+    repo = tempfile.mkdtemp()
+    repo_path = os.path.join(repo, "trusty")
+    os.mkdir(repo_path)
+    os.symlink(os.path.abspath(os.getcwd()), os.path.join(repo_path, "cloudfoundry"))
+    yield repo
+    shutil.rmtree(repo)
 
 
 def deploy(**config):
@@ -421,20 +477,18 @@
     with open(fn, 'w') as fp:
         yaml.dump({'cloudfoundry': config}, fp)
 
-    repo_path = get_repo_path()
-
-    args = ['deploy', '--config=%s' % fn,
-            '--repository=%s' % repo_path]
-    if constraints:
-        args.append('--constraints=%s' % constraints)
-    args.append('local:trusty/cloudfoundry')
-    juju = sh.check('juju', throw=False)
-    if juju(*args) != 0:
-        return False
-    time.sleep(5)
-    if juju('expose', 'cloudfoundry') != 0:
-        return False
-    os.unlink(fn)
+    with cf_repo() as repo_path:
+        args = ['deploy', '--config=%s' % fn,
+                '--repository=%s' % repo_path]
+        if constraints:
+            args.append('--constraints=%s' % constraints)
+        args.append('local:trusty/cloudfoundry')
+        if not sh.juju(*args):
+            return False
+        time.sleep(5)
+        if not sh.juju('expose', 'cloudfoundry'):
+            return False
+        os.unlink(fn)
     return True