cf-charmers team mailing list archive

[Cory Johns <cory.johns@xxxxxxxxxxxxx>]

Review: Needs Fixing

Typo in python-virtualenv package, noted inline, below.

Diff comments:

> === modified file 'README.rst'
> --- README.rst	2014-11-06 23:37:55 +0000
> +++ README.rst	2014-11-13 00:05:21 +0000
> @@ -24,11 +24,6 @@
>  We provide a set of helper scripts (in bash) which can be used to assist in the deployment and
>  management of CF.
>  
> -You will need the cf command line client. This can easily be installed using the following:
> -
> -  wget http://go-cli.s3-website-us-east-1.amazonaws.com/releases/latest/cf-cli_amd64.deb
> -  dpkg -i cf-cli_amd64.deb
> -
>  
>  Deployment
>  ----------
> @@ -51,7 +46,10 @@
>  will block until the deployment is fully up and running. It also will create a
>  CF "admin" user with the given password, leaving you logged in and ready to push
>  a CF app. Note that while this can take quite some time, the whole process is
> -automated.
> +automated. 
> +
> +If you don't have the cf command line client the latest one will be installed
> +from a .deb package to your local system as part of cfdeploy.
>  
>  Once the deploy completes, you can push apps. A sample application we recommend
>  to become familiar with the system is GitHub High Score website. This can be
> 
> === modified file 'cfdeploy'
> --- cfdeploy	2014-11-07 00:06:24 +0000
> +++ cfdeploy	2014-11-13 00:05:21 +0000
> @@ -2,9 +2,48 @@
>  # modeline vim:set syntax=python
>  import argparse
>  import logging
> -import webbrowser
> +import os
> +import subprocess
> +import sys
> +from contextlib import contextmanager
>  from functools import partial
>  
> +
> +def verify_dep(name):
> +    try:
> +        __import__(name)
> +        return True
> +    except ImportError:
> +        pass
> +    return False
> +
> +
> +def install_python_deps():
> +    print "Setting up virutalenv."
> +    subprocess.check_output(['pip', 'install', 'wheel'])
> +    for dep in ['progress', 'requests', 'jujuclient']:
> +        subprocess.check_output(['pip', 'install',
> +                                 '--use-wheel', '--find-links',
> +                                 os.path.abspath('wheelhouse'),
> +                                 dep])
> +
> +
> +def prepare_runtime():
> +    activate = "bin/activate_this.py"
> +    if not verify_dep('virtualenv'):
> +        print "(sudo) installing python-virtualenv package."
> +        subprocess.check_call(
> +            ['sudo', 'apt-get', 'install', '-yq', 'python-virutalenv'])

Typo (swapped "t" and "u" in "python-virtualenv").  I do that all the time.

> +    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()
> +
>  from cloudfoundry.releases import RELEASES
>  from cloudfoundry.utils import (bootstrap,
>                                  cf_service,
> @@ -15,9 +54,13 @@
>                                  login,
>                                  reconciler_endpoint,
>                                  webadmin_endpoint,
> +                                sh,
>                                  socket_open,
>                                  until,
> -                                wait_for)
> +                                which
> +                                )
> +
> +from progress.bar import Bar
>  
>  
>  def setup():
> @@ -35,31 +78,91 @@
>      return options
>  
>  
> +# This is done because the default webbrowser invocation will output
> +# on stderr and muck with the progress bar. By hiding the output
> +# with devnull it proceeds as expected.
> +show = sh.check('xdg-open')
> +
> +
> +@contextmanager
> +def devnull():
> +    _save = sys.stderr
> +    fp = open('/dev/null', 'w')
> +    sys.stderr = fp
> +    yield
> +    sys.stderr = _save
> +
> +
>  def show_reconciler():
> -    uri = "http://%s:8888/"; % reconciler_endpoint()
> -    webbrowser.open_new_tab(uri)
> +    with devnull():
> +        uri = "http://%s:8888/"; % reconciler_endpoint()
> +        show(uri)
>  
>  
>  def show_webadmin():
> -    uri = "http://%s:8070/"; % webadmin_endpoint()
> -    webbrowser.open_new_tab(uri)
> +    with devnull():
> +        uri = "http://%s:8070/"; % webadmin_endpoint()
> +        show(uri)
> +
> +
> +class ProgressBar(Bar):
> +    message = "Deploying CloudFoundry"
> +    fill = "."
> +    suffix = "%(percent).1f%% %(elapsed_td)s"
> +
> +    def next(self, i=1, message=None):
> +        if message:
> +            self.message = message
> +        super(ProgressBar, self).next(i)
> +
> +
> +def install_deps():
> +    if not which('cf'):
> +        from platform import machine
> +        if machine() != "x86_64":
> +            print "Unable to install CF CLI for your architecture. "
> +            "Deploy will not work as expected."
> +            return
> +        sh.wget('http://go-cli.s3-website-us-east-1.amazonaws.com/'
> +                'releases/latest/cf-cli_amd64.deb')
> +        print "Installing CF CLI (this requires sudo access)"
> +        sh.sudo('dpkg', '-i', 'cf-cli_amd64.deb')
>  
>  
>  def main():
>      options = setup()
>      logging.basicConfig(level=options.log_level)
> +    install_deps()
> +
> +    bar = ProgressBar('Deploying CloudFoundry', max=10)
> +    bar.start()
> +    bar.next(message='Bootstrapping')
>      bootstrap()
> -    until(juju_state_server)
> +    until(juju_state_server, bar=bar, message="Waiting for State Server")
> +    bar.next(message='Deploying Orchestrator')
>      deploy(constraints=options.constraints,
>             generate_dependents=options.generate,
>             admin_password=options.admin_password)
> -    until(lambda: socket_open(reconciler_endpoint(), 8888))
> +    until(lambda: socket_open(reconciler_endpoint(), 8888),
> +          bar=bar, message="Waiting on Reconciler")
> +    bar.next(message='Showing Reconciler')
>      show_reconciler()
> +
>      # Wait forever, its in the reconciler's hands now.
> -    wait_for(0, 30, cf_service, endpoint, partial(login, options.admin_password))
> -    until(lambda: socket_open(webadmin_endpoint(), 8070))
> +    until(cf_service, bar=bar, message="Waiting for Orchestrator Charm")
> +    until(endpoint, bar=bar, message='Waiting for CloudFoundry API endpoint')
> +    until(partial(login, options.admin_password),
> +          bar=bar, message='Waiting to login to CloudFoundry (long)')
> +    until(lambda: socket_open(webadmin_endpoint(), 8070),
> +          bar=bar, message="Waiting on webadmin.")
> +    bar.next(message="Opening Admin Console")
>      show_webadmin()
> -    print "You should now be logged into a running CF deployment"
> +    bar.finish()
> +
> +    print "You should now be logged into a running CF deployment."
> +    if which('cf'):
> +        print "The 'cf' command line client is installed and available."
> +
>  
>  if __name__ == "__main__":
>      main()
> 
> === modified file 'cloudfoundry/utils.py'
> --- cloudfoundry/utils.py	2014-11-07 00:06:24 +0000
> +++ cloudfoundry/utils.py	2014-11-13 00:05:21 +0000
> @@ -168,6 +168,7 @@
>  
>  def command(*base_args, **kwargs):
>      check = kwargs.pop('check', False)
> +    throw = kwargs.pop('throw', True)
>  
>      def callable_command(*args, **kws):
>          kwargs.update(kws)
> @@ -176,10 +177,20 @@
>          if 'env' not in kwargs:
>              kwargs['env'] = os.environ
>          logging.debug("invoke: %s", all_args)
> +
> +        p = subprocess.Popen(all_args,
> +                             stdout=subprocess.PIPE,
> +                             stderr=subprocess.STDOUT,
> +                             **kwargs)
> +        output, _ = p.communicate()
> +        ret_code = p.poll()
> +        logging.debug('result: %s', output)
>          if check is True:
> -            return subprocess.check_call(all_args, **kwargs)
> +            if ret_code != 0 and throw:
> +                raise subprocess.CalledProcessError(ret_code, all_args, output=output)
> +            return ret_code
>          else:
> -            return subprocess.check_output(all_args, **kwargs).strip()
> +            return output.strip()
>      return callable_command
>  
>  
> @@ -196,7 +207,7 @@
>  
>  sh = Commander()
>  dig = command('dig', '+short')
> -api_endpoints = sh.check('juju', 'api-endpoints')
> +api_endpoints = sh.check('juju', 'api-endpoints', throw=False)
>  
>  
>  def current_env():
> @@ -210,7 +221,7 @@
>                          'environments/%s.jenv' % cenv)
>  
>  
> -def wait_for(timeout, interval, *callbacks):
> +def wait_for(timeout, interval, *callbacks, **kwargs):
>      """
>      Repeatedly try callbacks until all return True
>  
> @@ -223,25 +234,35 @@
>      hardware fails, or the heat death of the universe.
>      """
>      start = time.time()
> +    if timeout:
> +        end = start + timeout
> +    else:
> +        end = 0
> +
> +    bar = kwargs.get('bar', None)
> +    message = kwargs.get('message', None)
> +    once = 1
>      while True:
>          passes = True
> -        for callback in callbacks:
> -            result = callback()
> -            passes = passes & bool(result)
> -            if passes is False:
> -                break
> -        if passes is True:
> -            break
> -        current = time.time()
> -        if timeout != 0 and (
> -            current - start >= timeout or
> -                (current - start) + interval > timeout):
> +        if end > 0 and time.time() > end:
>              raise OSError("Timeout exceeded in wait_for")
> -        time.sleep(interval)
> -
> -
> -def until(*callbacks):
> -    return wait_for(0, 20, *callbacks)
> +        if bar:
> +            bar.next(once, message=message)
> +            if once == 1:
> +                once = 0
> +        if int(time.time()) % interval == 0:
> +            for callback in callbacks:
> +                result = callback()
> +                passes = passes & bool(result)
> +                if passes is False:
> +                    break
> +            if passes is True:
> +                break
> +        time.sleep(1)
> +
> +
> +def until(*callbacks, **kwargs):
> +    return wait_for(0, 20, *callbacks, **kwargs)
>  
>  
>  def juju_state_server():
> @@ -249,7 +270,6 @@
>          return False
>      endpoints = json.loads(sh.juju('api-endpoints', '--format=json'))
>      for ep in endpoints:
> -        print "Attempt to connect to juju state server:", ep
>          host, port = ep.split(':', 1)
>          result = socket_open(host, int(port))
>          if result is True:
> @@ -266,7 +286,6 @@
>              # .jenv with the state server
>              sh.juju('status')
>              data = yaml.load(open(get_jenv()))
> -            print "Waiting for state server(s)", data['state-servers']
>              return data.get('state-servers', []) != []
>          current = current_env()
>          wait_for(1000, 20, env_connection)
> @@ -387,7 +406,6 @@
>          if constraints:
>              args.append('--constraints=%s' % constraints)
>          args.append('local:trusty/cloudfoundry')
> -        print "Deploying:", ' '.join(args)
>          sh.juju(*args)
>          time.sleep(5)
>          sh.juju('expose', 'cloudfoundry')
> 
> === modified file 'reconciler/app.py'
> --- reconciler/app.py	2014-11-03 16:13:57 +0000
> +++ reconciler/app.py	2014-11-13 00:05:21 +0000
> @@ -49,7 +49,6 @@
>  import tornado.web
>  
>  from tornado.options import define, options
> -from cloudfoundry import config
>  from cloudfoundry import utils
>  from cloudfoundry.path import path
>  from reconciler import strategy
> @@ -57,6 +56,8 @@
>  from reconciler.ui.app import DashboardIO
>  from reconciler.ui.app import static_resources
>  
> +import config
> +
>  application = None
>  env_name = None
>  server = None
> @@ -283,7 +284,6 @@
>      loop = tornado.ioloop.IOLoop.instance()
>      # call poll_health ASAP, to populate the initial health data, and
>      # also schedule poll_health for every 60s to refresh health data
> -
>      loop.add_callback(poll_health)
>      tornado.ioloop.PeriodicCallback(poll_health, 60000, io_loop=loop).start()
>      tornado.ioloop.PeriodicCallback(poll_current_state, 60000,
> @@ -294,6 +294,5 @@
>                                          io_loop=loop).start()
>      loop.start()
>  
> -
>  if __name__ == "__main__":
>      main()
> 
> === renamed file 'cloudfoundry/config.py' => 'reconciler/config.py'
> === modified file 'reconciler/strategy.py'
> --- reconciler/strategy.py	2014-11-05 23:40:05 +0000
> +++ reconciler/strategy.py	2014-11-13 00:05:21 +0000
> @@ -7,7 +7,7 @@
>  from tornado import gen
>  
>  from reconciler import tactics
> -from cloudfoundry.config import (PENDING, COMPLETE, FAILED, RUNNING)
> +from reconciler.config import (PENDING, COMPLETE, FAILED, RUNNING)
>  from cloudfoundry import utils
>  from cloudfoundry import tsort
>  
> @@ -160,7 +160,7 @@
>              elif expose_intent is False and exposed is True:
>                  result.append(tactics.UnexposeTactic(service=service))
>  
> -        logging.debug("Build New %s", result)
> +        logging.debug("Build Strategy %s", result)
>          return result
>  
>      def build_relations(self):
> 
> === modified file 'reconciler/tactics.py'
> --- reconciler/tactics.py	2014-11-05 23:40:05 +0000
> +++ reconciler/tactics.py	2014-11-13 00:05:21 +0000
> @@ -3,7 +3,7 @@
>  import os
>  import shutil
>  
> -from cloudfoundry.config import (
> +from reconciler.config import (
>      PENDING, COMPLETE, RUNNING, FAILED, STATES
>      )
>  
> @@ -169,7 +169,7 @@
>  class UnexposeTactic(Tactic):
>      name = "Unexpose Service"
>  
> -    def run(self, env, **kwargs):
> +    def _run(self, env, **kwargs):
>          s = kwargs['service']
>          env.unexpose(s['service_name'])
>  
> 
> === modified file 'tests/test_strategy.py'
> --- tests/test_strategy.py	2014-10-30 18:34:30 +0000
> +++ tests/test_strategy.py	2014-11-13 00:05:21 +0000
> @@ -2,7 +2,7 @@
>  import unittest
>  import mock
>  
> -from cloudfoundry import config
> +from reconciler import config
>  from reconciler import tactics
>  from reconciler import strategy
>  
> 
> === modified file 'tests/test_utils.py'
> --- tests/test_utils.py	2014-10-27 19:48:56 +0000
> +++ tests/test_utils.py	2014-11-13 00:05:21 +0000
> @@ -7,10 +7,10 @@
>  
>  
>  class TestUtils(unittest.TestCase):
> -    @mock.patch('subprocess.check_output')
> +    @mock.patch('cloudfoundry.utils.sh.juju')
>      def test_current_env(self, check_output):
>          utils.current_env()
> -        check_output.assert_called_once_with(('juju', 'switch'), env=mock.ANY)
> +        check_output.assert_called_once_with('switch')
>  
>      def test_flatten_relations(self):
>          r = utils.flatten_relations([
> 
> === modified file 'tox.ini'
> --- tox.ini	2014-09-30 21:15:05 +0000
> +++ tox.ini	2014-11-13 00:05:21 +0000
> @@ -30,3 +30,4 @@
>      clint
>      path.py
>      subparse
> +    progress
> 
> === added file 'wheelhouse/progress-1.2-py2-none-any.whl'
> Binary files wheelhouse/progress-1.2-py2-none-any.whl	1970-01-01 00:00:00 +0000 and wheelhouse/progress-1.2-py2-none-any.whl	2014-11-13 00:05:21 +0000 differ


-- 
https://code.launchpad.net/~bcsaller/charms/trusty/cloudfoundry/progressbar/+merge/241618
Your team Cloud Foundry Charmers is subscribed to branch lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.