Merge lp:~frankban/charms/trusty/juju-gui/config-version into lp:~juju-gui/charms/trusty/juju-gui/trunk

[Francesco Banconi]

Add the Juju version to the GUI config file.

The version can be specified using a charm option.
If not specified, the Juju version is retrieved
dynamically by calling jujud --version.

Initial implementation by Huw.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- run `make deploy`;
- wait for the unit to be ready;
- run `juju ssh juju-gui/0 cat /var/lib/juju-gui/juju-gui/build-prod/juju-ui/assets/config.js`
  and check the Juju version is included in the config file;
- run `juju set juju-gui juju-core-version=42.47`;
- check the version is changed to the one specified;
- run `juju unset juju-gui juju-core-version`;
- check the correct version is back in the config file;
- destroy the environment.

Done, thank you!

https://codereview.appspot.com/188890043/

[Francesco Banconi (frankban)]

Reviewers: mp+244273_code.launchpad.net,

Message:
Please take a look.

Description:
Add the Juju version to the GUI config file.

The version can be specified using a charm option.
If not specified, the Juju version is retrieved
dynamically by calling jujud --version.

Initial implementation by Huw.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- run `make deploy`;
- wait for the unit to be ready;
- run `juju ssh juju-gui/0 cat
/var/lib/juju-gui/juju-gui/build-prod/juju-ui/assets/config.js`
   and check the Juju version is included in the config file;
- run `juju set juju-gui juju-core-version=42.47`;
- check the version is changed to the one specified;
- run `juju unset juju-gui juju-core-version`;
- check the correct version is back in the config file;
- destroy the environment.

Done, thank you!

https://code.launchpad.net/~frankban/charms/trusty/juju-gui/config-version/+merge/244273

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/188890043/

Affected files (+50, -2 lines):
   A [revision details]
   M config.yaml
   M config/config.js.template
   M hooks/backend.py
   M hooks/utils.py
   M tests/test_backends.py
   M tests/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision:
<email address hidden>

Index: config.yaml
=== modified file 'config.yaml'
--- config.yaml 2014-11-14 11:35:36 +0000
+++ config.yaml 2014-12-10 10:19:42 +0000
@@ -193,3 +193,9 @@
        fonts will let the GUI load much faster and render properly.
      type: boolean
      default: false
+ juju-core-version:
+ description: |
+ The version of Juju tools used for the charm deployment. If left
empty,
+ the Juju version is dynamically retrieved.
+ type: string
+ default:

Index: config/config.js.template
=== modified file 'config/config.js.template'
--- config/config.js.template 2014-02-04 20:07:31 +0000
+++ config/config.js.template 2014-12-10 03:16:32 +0000
@@ -25,5 +25,6 @@
    sandbox: {{sandbox}},
    GA_key: {{ga_key}},
    login_help: {{login_help}},
- showGetJujuButton: {{show_get_juju_button}}
+ showGetJujuButton: {{show_get_juju_button}},
+ jujuCoreVersion: {{juju_core_version}}
  };

Index: hooks/backend.py
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2014-11-14 09:14:54 +0000
+++ hooks/backend.py 2014-12-10 10:47:53 +0000
@@ -131,6 +131,7 @@
              build_dir, secure=config['secure'], sandbox=config['sandbox'],
              cached_fonts=config['cached-fonts'], ga_key=config['ga-key'],
              show_get_juju_button=config['show-get-juju-button'],
+ juju_core_version=config.get('juju-core-version'),
              password=config.get('password'))
          # Set up TCP ports.
          previous_port = backend.prev_config.get('port')

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2014-11-14 13:14:33 +0000
+++ hooks/utils.py 2014-12-10 10:4...

[Richard Harding (rharding)]

:+1: will QA

https://codereview.appspot.com/188890043/

[Richard Harding (rharding)]

On 2014/12/10 13:10:40, rharding wrote:
> :+1: will QA

QA OK thank you! LGTM

https://codereview.appspot.com/188890043/

[Francesco Banconi (frankban)]

Thanks Rick for the review and QA!

https://codereview.appspot.com/188890043/

[Francesco Banconi (frankban)]

*** Submitted:

Add the Juju version to the GUI config file.

The version can be specified using a charm option.
If not specified, the Juju version is retrieved
dynamically by calling jujud --version.

Initial implementation by Huw.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- run `make deploy`;
- wait for the unit to be ready;
- run `juju ssh juju-gui/0 cat
/var/lib/juju-gui/juju-gui/build-prod/juju-ui/assets/config.js`
   and check the Juju version is included in the config file;
- run `juju set juju-gui juju-core-version=42.47`;
- check the version is changed to the one specified;
- run `juju unset juju-gui juju-core-version`;
- check the correct version is back in the config file;
- destroy the environment.

Done, thank you!

R=rharding
CC=
https://codereview.appspot.com/188890043

https://codereview.appspot.com/188890043/

=== modified file 'config.yaml'
--- config.yaml	2014-11-14 11:35:36 +0000
+++ config.yaml	2014-12-10 11:46:31 +0000
@@ -193,3 +193,9 @@
       fonts will let the GUI load much faster and render properly.
     type: boolean
     default: false
+  juju-core-version:
+    description: |
+      The version of Juju tools used for the charm deployment. If left empty,
+      the Juju version is dynamically retrieved.
+    type: string
+    default:

=== modified file 'config/config.js.template'
--- config/config.js.template	2014-02-04 20:07:31 +0000
+++ config/config.js.template	2014-12-10 11:46:31 +0000
@@ -25,5 +25,6 @@
   sandbox: {{sandbox}},
   GA_key: {{ga_key}},
   login_help: {{login_help}},
-  showGetJujuButton: {{show_get_juju_button}}
+  showGetJujuButton: {{show_get_juju_button}},
+  jujuCoreVersion: {{juju_core_version}}
 };

=== modified file 'hooks/backend.py'
--- hooks/backend.py	2014-11-14 09:14:54 +0000
+++ hooks/backend.py	2014-12-10 11:46:31 +0000
@@ -131,6 +131,7 @@
             build_dir, secure=config['secure'], sandbox=config['sandbox'],
             cached_fonts=config['cached-fonts'], ga_key=config['ga-key'],
             show_get_juju_button=config['show-get-juju-button'],
+            juju_core_version=config.get('juju-core-version'),
             password=config.get('password'))
         # Set up TCP ports.
         previous_port = backend.prev_config.get('port')

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2014-11-14 13:14:33 +0000
+++ hooks/utils.py	2014-12-10 11:46:31 +0000
@@ -348,7 +348,7 @@
         console_enabled, login_help, readonly, charmworld_url,
         build_dir, secure=True, sandbox=False, cached_fonts=False,
         show_get_juju_button=False, config_js_path=None, ga_key='',
-        password=None):
+        juju_core_version=None, password=None):
     """Generate the GUI configuration file."""
     log('Generating the Juju GUI configuration file.')
     user = 'user-admin'
@@ -393,6 +393,9 @@
                 'using juju-quickstart '
                 '(https://launchpad.net/juju-quickstart) can automate logging '
                 'in, as well as other parts of installing and starting Juju.')
+    if not juju_core_version:
+        log('Retrieving Juju version.')
+        juju_core_version = run('jujud', '--version').strip()
 
     context = {
         'cached_fonts': json.dumps(cached_fonts),
@@ -409,6 +412,7 @@
         'charmworld_url': json.dumps(charmworld_url),
         'ga_key': json.dumps(ga_key),
         'show_get_juju_button': json.dumps(show_get_juju_button),
+        'juju_core_version': json.dumps(juju_core_version),
     }
     if config_js_path is None:
         config_js_path = os.path.join(

=== modified file 'tests/test_backends.py'
--- tests/test_backends.py	2014-11-14 11:26:41 +0000
+++ tests/test_backends.py	2014-12-10 11:46:31 +0000
@@ -147,6 +147,7 @@
             'secure': True,
             'serve-tests': False,
             'show-get-juju-button': False,
+            'juju-core-version': '1.21',
             'ssl-cert-path': self.ssl_cert_path,
         }
         if options is not None:
@@ -200,6 +201,7 @@
             mocks.compute_build_dir(), secure=config['secure'],
             sandbox=config['sandbox'], cached_fonts=config['cached-fonts'],
             ga_key=config['ga-key'],
+            juju_core_version=config['juju-core-version'],
             show_get_juju_button=config['show-get-juju-button'], password=None)
 
     def test_base_dir_created(self):

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2014-11-14 13:14:33 +0000
+++ tests/test_utils.py	2014-12-10 11:46:31 +0000
@@ -778,6 +778,9 @@
 
 
 class TestStartImprovAgentGui(unittest.TestCase):
+    # XXX frankban 2014-12-10: change this test case so that functions being
+    # tested are better separated. Also avoid manually patching helper
+    # functions and use the mock library instead.
 
     def setUp(self):
         self.service_names = []
@@ -801,6 +804,7 @@
 
         def run(*args):
             self.run_call_count += 1
+            return ''
 
         @contextmanager
         def su(user):
@@ -1051,6 +1055,34 @@
         js_conf = self.files['config']
         self.assertIn('cachedFonts: true', js_conf)
 
+    def test_write_gui_config_with_provided_version(self):
+        # The Juju version is included in the GUI config file when provided as
+        # an option.
+        write_gui_config(
+            False, 'This is login help.', False, False, self.charmworld_url,
+            self.build_dir, sandbox=True, juju_core_version='1.20',
+            config_js_path='config')
+        self.assertIn('jujuCoreVersion: "1.20"', self.files['config'])
+
+    def test_write_gui_config_with_version_from_jujud(self):
+        # If not provided as an option, the Juju version is dynamically
+        # retrieved and included in the GUI config file.
+        with mock.patch('utils.run', return_value='1.42.47\n'):
+            write_gui_config(
+                False, None, True, True, self.charmworld_url, self.build_dir,
+                config_js_path='config')
+        self.assertIn('jujuCoreVersion: "1.42.47"', self.files['config'])
+
+    def test_write_gui_config_with_provided_empty_version(self):
+        # If the provided Juju version is empty, the Juju version is still
+        # dynamically retrieved and included in the GUI config file.
+        with mock.patch('utils.run', return_value='1.20.13-trusty-amd64\n'):
+            write_gui_config(
+                False, None, True, True, self.charmworld_url, self.build_dir,
+                config_js_path='config', juju_core_version='')
+        self.assertIn(
+            'jujuCoreVersion: "1.20.13-trusty-amd64"', self.files['config'])
+
 
 class TestPortInRange(unittest.TestCase):