Merge lp:~frankban/charms/precise/juju-gui/new-requirements into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

Update requirements and improve their handling.

Updated the juju-deployer and jujucleint
dependencies to the latest versions, which
include our recent fixes. This way we avoid
using our own forks of the projects.

Updated the code that installs the builtin
server dependencies: now a pip requirement
file is used, and the test requirement file
includes the former. The overall dependency
infrastructure should now be less confusing.

Also added documentation about how to update
the builtin server requirements.

Removed the no longer required --upload-tools
from the functional tests runner.

The deployer functional tests now also use
bundles including numunits > 1 and constraints.

QA:
- Bootstrap a Juju environment.
- Deploy and expose the GUI (make deploy).
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
  the services are vertically aligned), wordpress has
  customized constraints, mysql customized options and
  two units.
- No try to deploy the same bundle again, you
  will see a "services already there" kind of error.

https://codereview.appspot.com/26130043/

[Francesco Banconi (frankban)]

Reviewers: mp+195100_code.launchpad.net,

Message:
Please take a look.

Description:
Update requirements and improve their handling.

Updated the juju-deployer and jujucleint
dependencies to the latest versions, which
include our recent fixes. This way we avoid
using our own forks of the projects.

Updated the code that installs the builtin
server dependencies: now a pip requirement
file is used, and the test requirement file
includes the former. The overall dependency
infrastructure should now be less confusing.

Also added documentation about how to update
the builtin server requirements.

Removed the no longer required --upload-tools
from the functional tests runner.

The deployer functional tests now also use
bundles including numunits > 1 and constraints.

QA:
- Bootstrap a Juju environment.
- Deploy and expose the GUI (make deploy).
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
   the services are vertically aligned), wordpress has
   customized constraints, mysql customized options and
   two units.
- No try to deploy the same bundle again, you
   will see a "services already there" kind of error.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/new-requirements/+merge/195100

(do not edit description out of merge proposal)

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

Affected files (+85, -30 lines):
   M HACKING.md
   M Makefile
   A [revision details]
   D deps/juju-deployer-0.2.8.tar.gz
   A deps/juju-deployer-0.2.9.tar.gz
   D deps/jujuclient-0.13.tar.gz
   A deps/jujuclient-0.15.tar.gz
   M hooks/utils.py
   M revision
   A server-requirements.pip
   M test-requirements.pip
   M tests/00-setup
   M tests/example.py
   M tests/test_utils.py

[Gary Poster (gary)]

LGTM with trivial comment request. Doing QA now.

https://codereview.appspot.com/26130043/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/26130043/diff/1/hooks/utils.py#newcode550
hooks/utils.py:550: '--find-links', 'file:///{}'.format(deps), '-r',
requirements
yay, nice

https://codereview.appspot.com/26130043/diff/1/server-requirements.pip
File server-requirements.pip (right):

https://codereview.appspot.com/26130043/diff/1/server-requirements.pip#newcode19
server-requirements.pip:19: # Note: the order of the following
dependencies is important!
Would be nice to indicate why.

https://codereview.appspot.com/26130043/diff/1/test-requirements.pip
File test-requirements.pip (right):

https://codereview.appspot.com/26130043/diff/1/test-requirements.pip#newcode32
test-requirements.pip:32: -r server-requirements.pip
cool. I didn't know about this feature.

https://codereview.appspot.com/26130043/diff/1/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/26130043/diff/1/tests/test_utils.py#newcode1117
tests/test_utils.py:1117: mock.call(
wow. cool that you can do this.

https://codereview.appspot.com/26130043/

[Gary Poster (gary)]

QA good. Thanks!

https://codereview.appspot.com/26130043/

[lp:~frankban/charms/precise/juju-gui/new-requirements]

139. By Francesco Banconi on 2013-11-13

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

Update requirements and improve their handling.

Updated the juju-deployer and jujucleint
dependencies to the latest versions, which
include our recent fixes. This way we avoid
using our own forks of the projects.

Updated the code that installs the builtin
server dependencies: now a pip requirement
file is used, and the test requirement file
includes the former. The overall dependency
infrastructure should now be less confusing.

Also added documentation about how to update
the builtin server requirements.

Removed the no longer required --upload-tools
from the functional tests runner.

The deployer functional tests now also use
bundles including numunits > 1 and constraints.

QA:
- Bootstrap a Juju environment.
- Deploy and expose the GUI (make deploy).
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
   the services are vertically aligned), wordpress has
   customized constraints, mysql customized options and
   two units.
- No try to deploy the same bundle again, you
   will see a "services already there" kind of error.

R=gary.poster
CC=
https://codereview.appspot.com/26130043

https://codereview.appspot.com/26130043/diff/1/server-requirements.pip
File server-requirements.pip (right):

https://codereview.appspot.com/26130043/diff/1/server-requirements.pip#newcode19
server-requirements.pip:19: # Note: the order of the following
dependencies is important!
On 2013/11/13 17:15:47, gary.poster wrote:
> Would be nice to indicate why.

Done.

https://codereview.appspot.com/26130043/

=== modified file 'HACKING.md'
--- HACKING.md	2013-10-08 15:30:04 +0000
+++ HACKING.md	2013-11-13 17:46:04 +0000
@@ -175,3 +175,13 @@
 the local charm.
 Note that at least one release must be always present in the repository,
 otherwise the deployment process will fail.
+
+## Upgrading the builtin server dependencies ##
+
+The builtin server dependencies are stored in the `deps` directory located in
+the branch source. This way, when the charm is deployed, the builtin server can
+be set up without downloading dependencies from the network (pypi) and so the
+charm deployment succeeds also behind a firewall.
+To upgrade the dependencies, add a tarball to the `deps` directory, remove the
+old dependency if required, and update the `server-requirements.pip` file.
+At this point, running `make` should also update the virtualenv used for tests.

=== modified file 'Makefile'
--- Makefile	2013-11-12 16:38:27 +0000
+++ Makefile	2013-11-13 17:46:04 +0000
@@ -14,13 +14,16 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)" --upload-tools
+JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
 VENV = ./tests/.venv
 SYSDEPS = build-essential bzr libapt-pkg-dev python-pip python-virtualenv xvfb \
 		libpython-dev
 
 all: setup
 
+# The virtualenv is created by the 00-setup script and not here. This way we
+# support the juju-test plugin, which calls the executable files in
+# alphabetical order.
 setup:
 	@./tests/00-setup
 

=== removed file 'deps/juju-deployer-0.2.8.tar.gz'
Binary files deps/juju-deployer-0.2.8.tar.gz	2013-11-07 11:45:47 +0000 and deps/juju-deployer-0.2.8.tar.gz	1970-01-01 00:00:00 +0000 differ
=== added file 'deps/juju-deployer-0.2.9.tar.gz'
Binary files deps/juju-deployer-0.2.9.tar.gz	1970-01-01 00:00:00 +0000 and deps/juju-deployer-0.2.9.tar.gz	2013-11-13 17:46:04 +0000 differ
=== removed file 'deps/jujuclient-0.13.tar.gz'
Binary files deps/jujuclient-0.13.tar.gz	2013-11-07 17:27:20 +0000 and deps/jujuclient-0.13.tar.gz	1970-01-01 00:00:00 +0000 differ
=== added file 'deps/jujuclient-0.15.tar.gz'
Binary files deps/jujuclient-0.15.tar.gz	1970-01-01 00:00:00 +0000 and deps/jujuclient-0.15.tar.gz	2013-11-13 17:46:04 +0000 differ
=== modified file 'hooks/utils.py'
--- hooks/utils.py	2013-11-07 17:27:20 +0000
+++ hooks/utils.py	2013-11-13 17:46:04 +0000
@@ -114,19 +114,6 @@
 JUJU_AGENT_DIR = os.path.join(BASE_DIR, 'juju')
 JUJU_GUI_DIR = os.path.join(BASE_DIR, 'juju-gui')
 RELEASES_DIR = os.path.join(CURRENT_DIR, 'releases')
-# Builtin server dependencies. The order of these requirements is important.
-SERVER_DEPENDENCIES = (
-    'futures-2.1.4.tar.gz',
-    'tornado-3.1.1.tar.gz',
-    'websocket-client-0.12.0.tar.gz',
-    # XXX frankban 2013-11-07: we are currently using a customized jujuclient
-    # version built from this branch:
-    # lp:~frankban/python-jujuclient/pickable-enverror.
-    'jujuclient-0.13.tar.gz',
-    # XXX frankban 2013-11-07: we are currently using a customized deployer
-    # version built from this branch: lp:~frankban/juju-deployer/guienv-fixes.
-    'juju-deployer-0.2.8.tar.gz',
-)
 SERVER_DIR = os.path.join(CURRENT_DIR, 'server')
 
 APACHE_CFG_DIR = os.path.join(os.path.sep, 'etc', 'apache2')
@@ -553,10 +540,15 @@
 def install_builtin_server():
     """Install the builtin server code."""
     log('Installing the builtin server dependencies.')
-    for dependency_name in SERVER_DEPENDENCIES:
-        dependency = os.path.join(CURRENT_DIR, 'deps', dependency_name)
-        with su('root'):
-            cmd_log(run('pip', 'install', dependency))
+    deps = os.path.join(CURRENT_DIR, 'deps')
+    requirements = os.path.join(CURRENT_DIR, 'server-requirements.pip')
+    # Install the builtin server dependencies avoiding to download requirements
+    # from the network.
+    with su('root'):
+        cmd_log(run(
+            'pip', 'install', '--no-index', '--no-dependencies',
+            '--find-links', 'file:///{}'.format(deps), '-r', requirements
+        ))
     log('Installing the builtin server.')
     setup_cmd = os.path.join(SERVER_DIR, 'setup.py')
     with su('root'):

=== modified file 'revision'
--- revision	2013-11-12 16:39:09 +0000
+++ revision	2013-11-13 17:46:04 +0000
@@ -1,1 +1,1 @@
-96
+97

=== added file 'server-requirements.pip'
--- server-requirements.pip	1970-01-01 00:00:00 +0000
+++ server-requirements.pip	2013-11-13 17:46:04 +0000
@@ -0,0 +1,26 @@
+# Juju GUI server requirements.
+
+# This file is part of the Juju GUI, which lets users view and manage Juju
+# environments within a graphical interface (https://launchpad.net/juju-gui).
+# Copyright (C) 2012-2013 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Note: the order of the following dependencies is important! The last ones
+# depends on the previous.
+
+futures==2.1.4
+tornado==3.1.1
+websocket-client==0.12.0
+jujuclient==0.15
+juju-deployer==0.2.9

=== renamed file 'tests/requirements.pip' => 'test-requirements.pip'
--- tests/requirements.pip	2013-11-07 17:27:20 +0000
+++ test-requirements.pip	2013-11-13 17:46:04 +0000
@@ -24,18 +24,12 @@
 
 # GUI server -> juju deployer.
 bzr==2.6.0
-jujuclient==0.13
-
-# Charm tests + juju deployer.
-websocket-client==0.12.0
 
 # Charm hooks + GUI server.
 PyYAML==3.10
 
 # GUI server.
-futures==2.1.4
-juju-deployer==0.2.8
-tornado==3.1.1
+-r server-requirements.pip
 
 # Charm hooks.
 launchpadlib==1.10.2
@@ -43,6 +37,8 @@
 Tempita==0.5.1
 
 # Charm tests + GUI server tests.
+# Note that websocket-client, required by charm integration tests,
+# is already installed by server-requirements.
 flake8==2.0
 mock==1.0.1
 

=== modified file 'tests/00-setup'
--- tests/00-setup	2013-11-06 18:48:54 +0000
+++ tests/00-setup	2013-11-13 17:46:04 +0000
@@ -19,16 +19,17 @@
 TESTDIR=`dirname $0`
 VENV="$TESTDIR/.venv"
 ACTIVATE="$VENV/bin/activate"
-REQUIREMENTS="$TESTDIR/requirements.pip"
+TEST_REQUIREMENTS="test-requirements.pip"
+SERVER_REQUIREMENTS="server-requirements.pip"
 
 
 createvenv() {
     # Create a virtualenv if it does not exist, or it is older than requirements.
-    if [ ! -f "$ACTIVATE" -o "$REQUIREMENTS" -nt "$ACTIVATE" ]; then
+    if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
         virtualenv --distribute $VENV
         . $VENV/bin/activate && \
             yes w | env BZR_PLUGIN_PATH='-user' \
-            pip install --use-mirrors -r $REQUIREMENTS --find-links deps
+            pip install --use-mirrors -r $TEST_REQUIREMENTS --find-links deps
         touch $VENV/bin/activate
     fi
 }

=== modified file 'tests/example.py'
--- tests/example.py	2013-09-24 16:45:46 +0000
+++ tests/example.py	2013-11-13 17:46:04 +0000
@@ -29,12 +29,13 @@
         engine: nginx
         tuning: single
         "wp-content": ""
+      constraints: "cpu-cores=4,mem=4000"
       annotations:
         "gui-x": 313
         "gui-y": 51
     mysql:
       charm: "cs:precise/mysql-26"
-      num_units: 1
+      num_units: 2
       options:
         "binlog-format": MIXED
         "block-size": "5"

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2013-11-07 11:17:54 +0000
+++ tests/test_utils.py	2013-11-13 17:46:04 +0000
@@ -34,7 +34,6 @@
     API_PORT,
     JUJU_GUI_DIR,
     JUJU_PEM,
-    SERVER_DEPENDENCIES,
     WEB_PORT,
     _get_by_attr,
     cmd_log,
@@ -978,9 +977,9 @@
 
     def test_install_builtin_server(self):
         install_builtin_server()
-        # The function executes one "pip install" call for each dependency, and
-        # a final "python setup.py" call for the GUI server itself.
-        self.assertEqual(self.run_call_count, len(SERVER_DEPENDENCIES) + 1)
+        # Two run calls are executed: one for the dependencies, one for the
+        # server itself.
+        self.assertEqual(2, self.run_call_count)
 
     def test_write_builtin_server_startup(self):
         write_builtin_server_startup(
@@ -1100,6 +1099,32 @@
 
 
 @mock.patch('utils.run')
[e-mail]('utils.log')
[e-mail]('utils.cmd_log', mock.Mock())
[e-mail]('utils.su', mock.MagicMock())
+class TestInstallBuiltinServer(unittest.TestCase):
+
+    def test_call(self, mock_log, mock_run):
+        # The builtin server its correctly installed.
+        install_builtin_server()
+        charm_dir = os.path.abspath(
+            os.path.join(os.path.dirname(__file__), '..'))
+        mock_log.assert_has_calls([
+            mock.call('Installing the builtin server dependencies.'),
+            mock.call('Installing the builtin server.'),
+        ])
+        mock_run.assert_has_calls([
+            mock.call(
+                'pip', 'install', '--no-index', '--no-dependencies',
+                '--find-links', 'file:///{}/deps'.format(charm_dir),
+                '-r', os.path.join(charm_dir, 'server-requirements.pip')),
+            mock.call(
+                '/usr/bin/python',
+                os.path.join(charm_dir, 'server', 'setup.py'), 'install')
+        ])
+
+
[e-mail]('utils.run')
 @mock.patch('utils.cmd_log', mock.Mock())
 @mock.patch('utils.log', mock.Mock())
 @mock.patch('utils.su', mock.MagicMock())