Merge lp:~evarlast/juju-quickstart/upload-tools-constraints into lp:juju-quickstart

[Jay R. Wren]

support upload-tools and upload-series

support pass through of --upload-tools --upload-series and --constraints from quickstart to bootstrap

https://bugs.launchpad.net/juju-quickstart/+bug/1274584

https://codereview.appspot.com/132760043/

[Francesco Banconi (frankban)]

Nice branch Jay, it looks good with some changes I described below.
Thank you!

[lp:~evarlast/juju-quickstart/upload-tools-constraints]

91. By Jay R. Wren on 2014-08-01

addressing review concerns

[Francesco Banconi (frankban)]

LGTM with minor changes, thank you!

review: Approve

[lp:~evarlast/juju-quickstart/upload-tools-constraints]

92. By Jay R. Wren on 2014-08-21

semicolons and periods in comments

[Jay R. Wren (evarlast)]

Reviewers: mp+227229_code.launchpad.net,

Message:
Please take a look.

Description:
support upload-tools and upload-series

support pass through of --upload-tools --upload-series and --constraints
from quickstart to bootstrap

https://bugs.launchpad.net/juju-quickstart/+bug/1274584

https://code.launchpad.net/~evarlast/juju-quickstart/upload-tools-constraints/+merge/227229

(do not edit description out of merge proposal)

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

Affected files (+93, -10 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py

[Jay R. Wren (evarlast)]

*** Submitted:

support upload-tools and upload-series

support pass through of --upload-tools --upload-series and --constraints
from quickstart to bootstrap

https://bugs.launchpad.net/juju-quickstart/+bug/1274584

R=
CC=
https://codereview.appspot.com/132760043

https://codereview.appspot.com/132760043/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2014-07-04 07:49:03 +0000
+++ quickstart/app.py	2014-08-21 13:36:56 +0000
@@ -163,7 +163,8 @@
         raise ProgramExit(bytes(err))
 
 
-def bootstrap(env_name, juju_command, requires_sudo=False, debug=False):
+def bootstrap(env_name, juju_command, requires_sudo=False, debug=False,
+              upload_tools=False, upload_series=None, constraints=None):
     """Bootstrap the Juju environment with the given name.
 
     Do not bootstrap the environment if already bootstrapped.
@@ -188,6 +189,14 @@
         cmd.insert(0, 'sudo')
     if debug:
         cmd.append('--debug')
+    if upload_tools:
+        cmd.append('--upload-tools')
+    if upload_series is not None:
+        cmd.append('--upload-series')
+        cmd.append(upload_series)
+    if constraints is not None:
+        cmd.append('--constraints')
+        cmd.append(constraints)
     retcode, _, error = utils.call(*cmd)
     if retcode:
         # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-06-13 15:19:46 +0000
+++ quickstart/manage.py	2014-08-21 13:36:56 +0000
@@ -353,8 +353,12 @@
         - env_name: the name of the Juju environment to use;
         - env_type: the provider type of the selected Juju environment;
         - interactive: whether to start the interactive session;
-        - open_browser: whether the GUI browser must be opened.
-        - platform: The host platform.
+        - open_browser: whether the GUI browser must be opened;
+        - platform: The host platform;
+        - upload_tools: whether to upload local version of tools;
+        - upload_series: the comma-separated series list for which tools will
+          be uploaded, or None if not set;
+        - constraints: the environment constrains or None if not set.
 
     The following attributes will also be included in the namespace if a bundle
     deployment is requested:
@@ -450,6 +454,20 @@
     parser.add_argument(
         '--description', action=_DescriptionAction, default=argparse.SUPPRESS,
         nargs=0, help="Show program's description and exit")
+    # These options are passed to juju bootstrap.
+    parser.add_argument(
+        '--upload-tools', action='store_true',
+        help='upload local version of tools before bootstrapping')
+    parser.add_argument(
+        '--upload-series',
+        help='upload tools for supplied comma-separated series list')
+    parser.add_argument(
+        '--constraints',
+        help='If constraints are specified in the bootstrap command,\n'
+             'they will apply to the machine provisioned for the juju state\n'
+             'server.  They will also be set as default constraints on the\n'
+             'environment for all future machines, exactly as if the\n'
+             'constraints were set with juju set-constraints.\n')
     # Parse the provided arguments.
     options = parser.parse_args()
 
@@ -500,7 +518,10 @@
 
     already_bootstrapped, bootstrap_node_series = app.bootstrap(
         options.env_name, juju_command,
-        requires_sudo=requires_sudo, debug=options.debug)
+        requires_sudo=requires_sudo, debug=options.debug,
+        upload_tools=options.upload_tools,
+        upload_series=options.upload_series,
+        constraints=options.constraints)
 
     # Retrieve the admin-secret for the current environment.
     try:

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2014-07-04 13:09:11 +0000
+++ quickstart/tests/test_app.py	2014-08-21 13:36:56 +0000
@@ -510,6 +510,42 @@
                 '--debug'),
         ] + self.make_status_calls(1))
 
+    def test_success_upload_tools(self, mock_print):
+        # The environment is bootstrapped with local tools.
+        with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
+            already_bootstrapped, series = app.bootstrap(
+                self.env_name, self.juju_command, upload_tools=True)
+        self.assertFalse(already_bootstrapped)
+        mock_call.assert_has_calls([
+            mock.call(
+                self.juju_command, 'bootstrap', '-e', self.env_name,
+                '--upload-tools'),
+        ] + self.make_status_calls(1))
+
+    def test_success_upload_series(self, mock_print):
+        # The environment is bootstrapped with tools for specific series.
+        with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
+            already_bootstrapped, series = app.bootstrap(
+                self.env_name, self.juju_command, upload_series='hoary')
+        self.assertFalse(already_bootstrapped)
+        mock_call.assert_has_calls([
+            mock.call(
+                self.juju_command, 'bootstrap', '-e', self.env_name,
+                '--upload-series', 'hoary'),
+        ] + self.make_status_calls(1))
+
+    def test_success_constraints(self, mock_print):
+        # The environment is bootstrapped with given constraints.
+        with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
+            already_bootstrapped, series = app.bootstrap(
+                self.env_name, self.juju_command, constraints='mem=7G')
+        self.assertFalse(already_bootstrapped)
+        mock_call.assert_has_calls([
+            mock.call(
+                self.juju_command, 'bootstrap', '-e', self.env_name,
+                '--constraints', 'mem=7G'),
+        ] + self.make_status_calls(1))
+
     def test_already_bootstrapped(self, mock_print):
         # The function succeeds and returns True if the environment is already
         # bootstrapped.

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-06-13 15:19:46 +0000
+++ quickstart/tests/test_manage.py	2014-08-21 13:36:56 +0000
@@ -660,9 +660,10 @@
         with mock.patch('sys.argv', ['juju-quickstart'] + args):
             with mock.patch('sys.exit') as mock_exit:
                 with self.patch_get_default_env_name(env_name):
-                    manage.setup()
+                    options = manage.setup()
         if exit_called:
             mock_exit.assert_called_once_with(0)
+        return options
 
     def test_help(self):
         # The program help message is properly formatted.
@@ -735,7 +736,12 @@
         # Logging is properly set up at the debug level.
         logger = logging.getLogger()
         self.call_setup(['--debug'], 'ec2', exit_called=False)
-        self.assertEqual(logging.DEBUG, logger.level)
+        self.assertTrue(logging.DEBUG, logger.level)
+
+    def test_upload_tools(self):
+        # Upload tools is properly set up.
+        options = self.call_setup(['--upload-tools'], exit_called=False)
+        self.assertTrue(options.upload_tools)
 
 
 @mock.patch('webbrowser.open')
@@ -797,7 +803,9 @@
         mock_app.ensure_ssh_keys.assert_called()
         mock_app.bootstrap.assert_called_once_with(
             options.env_name, self.juju_command, requires_sudo=False,
-            debug=options.debug)
+            debug=options.debug, upload_tools=options.upload_tools,
+            upload_series=options.upload_series,
+            constraints=options.constraints)
         mock_app.get_api_url.assert_called_once_with(
             options.env_name, self.juju_command)
         mock_app.connect.assert_has_calls([
@@ -878,7 +886,9 @@
                 manage.run(options)
             mock_app.bootstrap.assert_called_once_with(
                 options.env_name, self.juju_command, requires_sudo=False,
-                debug=options.debug)
+                debug=options.debug, upload_tools=options.upload_tools,
+                upload_series=options.upload_series,
+                constraints=options.constraints)
             mock_app.bootstrap.reset_mock()
 
     def test_local_provider_requiring_sudo(self, mock_app, mock_open):
@@ -902,7 +912,9 @@
                 manage.run(options)
             mock_app.bootstrap.assert_called_once_with(
                 options.env_name, self.juju_command, requires_sudo=True,
-                debug=options.debug)
+                debug=options.debug, upload_tools=options.upload_tools,
+                upload_series=options.upload_series,
+                constraints=options.constraints)
             mock_app.bootstrap.reset_mock()
 
     def test_no_local_no_sudo(self, mock_app, mock_open):
@@ -921,7 +933,10 @@
             manage.run(options)
         mock_app.bootstrap.assert_called_once_with(
             options.env_name, self.juju_command,
-            requires_sudo=False, debug=options.debug)
+            requires_sudo=False,
+            debug=options.debug, upload_tools=options.upload_tools,
+            upload_series=options.upload_series,
+            constraints=options.constraints)
 
     def test_no_browser(self, mock_app, mock_open):
         # It is possible to avoid opening the GUI in the browser.