cf-charmers team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

Review: Approve

LGTM +1

Thanks, some minors inline

Diff comments:

> === modified file 'README.rst'
> --- README.rst	2014-10-16 15:01:08 +0000
> +++ README.rst	2014-11-03 22:00:38 +0000
> @@ -61,6 +61,27 @@
>      cf push
>  
>  
> +Admin UI
> +--------
> +
> +By default (unless the `admin_ui` option is set to false), the charm for the
> +Administration UI project from cloudfoundry-incubator will be deployed
> +alongside Cloud Foundry.  For security reasons, it will not be exposed
> +by default.  To expose the service and open it in a web browser, you can
> +use the following:
> +
> +    juju expose webadmin
> +    . helpers.sh
> +    cfadminui  # opens browser to Admin UI URL

cfdeploy should already be in trunk (though the most recent version is on the reconciler branch). I would like to see cfdeploy pull this during its process and print it post login. This will be the recommended path (rather than helpers) moving forward. We might also want to include information about how to reset/change it though I expect that needs additional testing.

> +
> +You can then use the CF admin user credentials to log in.
> +
> +Note that, currently, once the Admin UI is deployed, you must manually remove
> +it if you change the value of the `admin_ui` option, but that changing the
> +option is required to prevent the Admin UI from potentially being re-deployed
> +after being manually removed.
> +
> +
>  Development
>  -----------
>  
> 
> === modified file 'charmgen/generator.py'
> --- charmgen/generator.py	2014-10-03 15:47:17 +0000
> +++ charmgen/generator.py	2014-11-03 22:00:38 +0000
> @@ -22,13 +22,15 @@
>  class CharmGenerator(object):
>      author = "CloudFoundry Charm Generator <cs:~cf-charmers/cloudfoundry>"
>  
> -    def __init__(self, releases, service_registry, from_charmstore=True, cs_namespace='~cf-charmers'):
> +    def __init__(self, releases, service_registry, from_charmstore=True,
> +                 cs_namespace='~cf-charmers', admin_ui=True):
>          self.__releases = releases
>          self.release = None
>          self.release_version = None
>          self.service_registry = service_registry
>          self.from_charmstore = from_charmstore
>          self.cs_namespace = cs_namespace
> +        self.admin_ui = admin_ui
>  
>      def select_release(self, version):
>          if isinstance(version, basestring):
> @@ -266,6 +268,19 @@
>              rel_data.setdefault(lhs, set()).add(rhs)
>          for k, v in rel_data.items():
>              relations.append((k, tuple(v)))
> +
> +        if self.admin_ui:
> +            namespace = '{}/'.format(self.cs_namespace) if self.cs_namespace else ''
> +            services['webadmin'] = {
> +                'charm': 'cs:{}trusty/{}'.format(namespace, 'cf-webadmin'),
> +            }
> +            relations.append(('webadmin:db', ('mysql:db',)))
> +            relations.append(('webadmin:uaa', ('uaa:uaa',)))
> +            relations.append(('webadmin:uaadb', ('uaa:uaadb',)))
> +            relations.append(('webadmin:ccdb', ('cc:ccdb',)))
> +            relations.append(('webadmin:nats', ('nats:nats',)))
> +            #relations.append(('webadmin:orchestrator', ('cloudfoundry:orchestrator',)))
> +
>          if placement:
>              self._do_placement(services, placement)
>          return result
> @@ -311,6 +326,7 @@
>      from cloudfoundry.services import SERVICES
>      parser = argparse.ArgumentParser()
>      parser.add_argument('release', type=int)
> +    parser.add_argument('-a', '--admin-ui', dest="admin_ui", action="store_true")

What do you think about making this the default (unless we are doing PCF) and adding a placement line to co-lo this with other services, maybe haproxy?

>      parser.add_argument('-d', '--directory', dest="directory")
>      parser.add_argument('-f', '--force', action="store_true")
>      parser.add_argument('-p', '--placement', default="sparse")
> @@ -332,7 +348,8 @@
>              raise SystemExit("Release already generated: {}".format(
>                  options.directory))
>  
> -    g = CharmGenerator(RELEASES, SERVICES, from_charmstore=options.charmstore)
> +    g = CharmGenerator(RELEASES, SERVICES, from_charmstore=options.charmstore,
> +                       admin_ui=options.admin_ui)
>      g.select_release(options.release)
>      placement = g.load_placement_policy(options.placement, options.placements_file)
>      g.generate(options.directory, placement)
> 
> === modified file 'config.yaml'
> --- config.yaml	2014-09-30 21:15:05 +0000
> +++ config.yaml	2014-11-03 22:00:38 +0000
> @@ -62,3 +62,10 @@
>              components on demand, instead of installing from the charm store.
>              This option is intended only for testing and development purposes,
>              and is not recommended for normal usage.
> +    admin_ui:
> +        type: boolean
> +        default: true
> +        description: >
> +            Deploy and configure the Administration UI from the Cloud Foundry
> +            Incubator.  The Admin UI service will be called "webadmin" and
> +            must be manually exposed.  See the README for more info.
> 
> === modified file 'helpers.sh'
> --- helpers.sh	2014-10-06 06:48:12 +0000
> +++ helpers.sh	2014-11-03 22:00:38 +0000
> @@ -1,3 +1,13 @@
> +function cfadminurl() {
> +    echo "http://$(juju status webadmin | grep public-address | awk '{print $NF}'):8070"
> +}
> +
> +
> +function cfadminui() {
> +    python -m webbrowser -t "$(cfadminurl)"
> +}
> +
> +
>  function cflogin() {
>      ENDPOINT=`juju status haproxy/0 |grep public-address|cut -f 2 -d : `
>      IP=`dig +short $ENDPOINT`
> 
> === modified file 'hooks/common.py'
> --- hooks/common.py	2014-10-14 07:05:52 +0000
> +++ hooks/common.py	2014-11-03 22:00:38 +0000
> @@ -58,9 +58,11 @@
>      build_dir = path(hookenv.charm_dir()) / 'build' / version
>      from_charmstore = not config['generate_dependents']
>      cs_namespace = config['charmstore_namespace']
> +    admin_ui = config['admin_ui']
>      if os.path.exists(build_dir):
>          shutil.rmtree(build_dir)
> -    generator = CharmGenerator(RELEASES, SERVICES, from_charmstore, cs_namespace)
> +    generator = CharmGenerator(RELEASES, SERVICES,
> +                               from_charmstore, cs_namespace, admin_ui)
>      generator.select_release(version)
>      placement = generator.load_placement_policy(config['placement'])
>      generator.generate(build_dir, placement)
> 


-- 
https://code.launchpad.net/~johnsca/charms/trusty/cloudfoundry/webadmin-bundle/+merge/240509
Your team Cloud Foundry Charmers is subscribed to branch lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.