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

Proposed by Jay R. Wren
Status: Merged
Merged at revision: 91
Proposed branch: lp:~evarlast/juju-quickstart/upload-tools-constraints
Merge into: lp:juju-quickstart
Diff against target: 202 lines (+91/-10)
4 files modified
quickstart/app.py (+10/-1)
quickstart/manage.py (+24/-3)
quickstart/tests/test_app.py (+36/-0)
quickstart/tests/test_manage.py (+21/-6)
To merge this branch: bzr merge lp:~evarlast/juju-quickstart/upload-tools-constraints
Reviewer Review Type Date Requested Status
Francesco Banconi Approve
Review via email: mp+227229@code.launchpad.net

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

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

91. By Jay R. Wren

addressing review concerns

Revision history for this message
Francesco Banconi (frankban) wrote :

LGTM with minor changes, thank you!

review: Approve
92. By Jay R. Wren

semicolons and periods in comments

Revision history for this message
Jay R. Wren (evarlast) wrote :

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

Revision history for this message
Jay R. Wren (evarlast) wrote :

*** 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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
163 raise ProgramExit(bytes(err))163 raise ProgramExit(bytes(err))
164164
165165
166def bootstrap(env_name, juju_command, requires_sudo=False, debug=False):166def bootstrap(env_name, juju_command, requires_sudo=False, debug=False,
167 upload_tools=False, upload_series=None, constraints=None):
167 """Bootstrap the Juju environment with the given name.168 """Bootstrap the Juju environment with the given name.
168169
169 Do not bootstrap the environment if already bootstrapped.170 Do not bootstrap the environment if already bootstrapped.
@@ -188,6 +189,14 @@
188 cmd.insert(0, 'sudo')189 cmd.insert(0, 'sudo')
189 if debug:190 if debug:
190 cmd.append('--debug')191 cmd.append('--debug')
192 if upload_tools:
193 cmd.append('--upload-tools')
194 if upload_series is not None:
195 cmd.append('--upload-series')
196 cmd.append(upload_series)
197 if constraints is not None:
198 cmd.append('--constraints')
199 cmd.append(constraints)
191 retcode, _, error = utils.call(*cmd)200 retcode, _, error = utils.call(*cmd)
192 if retcode:201 if retcode:
193 # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are202 # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are
194203
=== 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 @@
353 - env_name: the name of the Juju environment to use;353 - env_name: the name of the Juju environment to use;
354 - env_type: the provider type of the selected Juju environment;354 - env_type: the provider type of the selected Juju environment;
355 - interactive: whether to start the interactive session;355 - interactive: whether to start the interactive session;
356 - open_browser: whether the GUI browser must be opened.356 - open_browser: whether the GUI browser must be opened;
357 - platform: The host platform.357 - platform: The host platform;
358 - upload_tools: whether to upload local version of tools;
359 - upload_series: the comma-separated series list for which tools will
360 be uploaded, or None if not set;
361 - constraints: the environment constrains or None if not set.
358362
359 The following attributes will also be included in the namespace if a bundle363 The following attributes will also be included in the namespace if a bundle
360 deployment is requested:364 deployment is requested:
@@ -450,6 +454,20 @@
450 parser.add_argument(454 parser.add_argument(
451 '--description', action=_DescriptionAction, default=argparse.SUPPRESS,455 '--description', action=_DescriptionAction, default=argparse.SUPPRESS,
452 nargs=0, help="Show program's description and exit")456 nargs=0, help="Show program's description and exit")
457 # These options are passed to juju bootstrap.
458 parser.add_argument(
459 '--upload-tools', action='store_true',
460 help='upload local version of tools before bootstrapping')
461 parser.add_argument(
462 '--upload-series',
463 help='upload tools for supplied comma-separated series list')
464 parser.add_argument(
465 '--constraints',
466 help='If constraints are specified in the bootstrap command,\n'
467 'they will apply to the machine provisioned for the juju state\n'
468 'server. They will also be set as default constraints on the\n'
469 'environment for all future machines, exactly as if the\n'
470 'constraints were set with juju set-constraints.\n')
453 # Parse the provided arguments.471 # Parse the provided arguments.
454 options = parser.parse_args()472 options = parser.parse_args()
455473
@@ -500,7 +518,10 @@
500518
501 already_bootstrapped, bootstrap_node_series = app.bootstrap(519 already_bootstrapped, bootstrap_node_series = app.bootstrap(
502 options.env_name, juju_command,520 options.env_name, juju_command,
503 requires_sudo=requires_sudo, debug=options.debug)521 requires_sudo=requires_sudo, debug=options.debug,
522 upload_tools=options.upload_tools,
523 upload_series=options.upload_series,
524 constraints=options.constraints)
504525
505 # Retrieve the admin-secret for the current environment.526 # Retrieve the admin-secret for the current environment.
506 try:527 try:
507528
=== 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 @@
510 '--debug'),510 '--debug'),
511 ] + self.make_status_calls(1))511 ] + self.make_status_calls(1))
512512
513 def test_success_upload_tools(self, mock_print):
514 # The environment is bootstrapped with local tools.
515 with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
516 already_bootstrapped, series = app.bootstrap(
517 self.env_name, self.juju_command, upload_tools=True)
518 self.assertFalse(already_bootstrapped)
519 mock_call.assert_has_calls([
520 mock.call(
521 self.juju_command, 'bootstrap', '-e', self.env_name,
522 '--upload-tools'),
523 ] + self.make_status_calls(1))
524
525 def test_success_upload_series(self, mock_print):
526 # The environment is bootstrapped with tools for specific series.
527 with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
528 already_bootstrapped, series = app.bootstrap(
529 self.env_name, self.juju_command, upload_series='hoary')
530 self.assertFalse(already_bootstrapped)
531 mock_call.assert_has_calls([
532 mock.call(
533 self.juju_command, 'bootstrap', '-e', self.env_name,
534 '--upload-series', 'hoary'),
535 ] + self.make_status_calls(1))
536
537 def test_success_constraints(self, mock_print):
538 # The environment is bootstrapped with given constraints.
539 with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
540 already_bootstrapped, series = app.bootstrap(
541 self.env_name, self.juju_command, constraints='mem=7G')
542 self.assertFalse(already_bootstrapped)
543 mock_call.assert_has_calls([
544 mock.call(
545 self.juju_command, 'bootstrap', '-e', self.env_name,
546 '--constraints', 'mem=7G'),
547 ] + self.make_status_calls(1))
548
513 def test_already_bootstrapped(self, mock_print):549 def test_already_bootstrapped(self, mock_print):
514 # The function succeeds and returns True if the environment is already550 # The function succeeds and returns True if the environment is already
515 # bootstrapped.551 # bootstrapped.
516552
=== 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 @@
660 with mock.patch('sys.argv', ['juju-quickstart'] + args):660 with mock.patch('sys.argv', ['juju-quickstart'] + args):
661 with mock.patch('sys.exit') as mock_exit:661 with mock.patch('sys.exit') as mock_exit:
662 with self.patch_get_default_env_name(env_name):662 with self.patch_get_default_env_name(env_name):
663 manage.setup()663 options = manage.setup()
664 if exit_called:664 if exit_called:
665 mock_exit.assert_called_once_with(0)665 mock_exit.assert_called_once_with(0)
666 return options
666667
667 def test_help(self):668 def test_help(self):
668 # The program help message is properly formatted.669 # The program help message is properly formatted.
@@ -735,7 +736,12 @@
735 # Logging is properly set up at the debug level.736 # Logging is properly set up at the debug level.
736 logger = logging.getLogger()737 logger = logging.getLogger()
737 self.call_setup(['--debug'], 'ec2', exit_called=False)738 self.call_setup(['--debug'], 'ec2', exit_called=False)
738 self.assertEqual(logging.DEBUG, logger.level)739 self.assertTrue(logging.DEBUG, logger.level)
740
741 def test_upload_tools(self):
742 # Upload tools is properly set up.
743 options = self.call_setup(['--upload-tools'], exit_called=False)
744 self.assertTrue(options.upload_tools)
739745
740746
741@mock.patch('webbrowser.open')747@mock.patch('webbrowser.open')
@@ -797,7 +803,9 @@
797 mock_app.ensure_ssh_keys.assert_called()803 mock_app.ensure_ssh_keys.assert_called()
798 mock_app.bootstrap.assert_called_once_with(804 mock_app.bootstrap.assert_called_once_with(
799 options.env_name, self.juju_command, requires_sudo=False,805 options.env_name, self.juju_command, requires_sudo=False,
800 debug=options.debug)806 debug=options.debug, upload_tools=options.upload_tools,
807 upload_series=options.upload_series,
808 constraints=options.constraints)
801 mock_app.get_api_url.assert_called_once_with(809 mock_app.get_api_url.assert_called_once_with(
802 options.env_name, self.juju_command)810 options.env_name, self.juju_command)
803 mock_app.connect.assert_has_calls([811 mock_app.connect.assert_has_calls([
@@ -878,7 +886,9 @@
878 manage.run(options)886 manage.run(options)
879 mock_app.bootstrap.assert_called_once_with(887 mock_app.bootstrap.assert_called_once_with(
880 options.env_name, self.juju_command, requires_sudo=False,888 options.env_name, self.juju_command, requires_sudo=False,
881 debug=options.debug)889 debug=options.debug, upload_tools=options.upload_tools,
890 upload_series=options.upload_series,
891 constraints=options.constraints)
882 mock_app.bootstrap.reset_mock()892 mock_app.bootstrap.reset_mock()
883893
884 def test_local_provider_requiring_sudo(self, mock_app, mock_open):894 def test_local_provider_requiring_sudo(self, mock_app, mock_open):
@@ -902,7 +912,9 @@
902 manage.run(options)912 manage.run(options)
903 mock_app.bootstrap.assert_called_once_with(913 mock_app.bootstrap.assert_called_once_with(
904 options.env_name, self.juju_command, requires_sudo=True,914 options.env_name, self.juju_command, requires_sudo=True,
905 debug=options.debug)915 debug=options.debug, upload_tools=options.upload_tools,
916 upload_series=options.upload_series,
917 constraints=options.constraints)
906 mock_app.bootstrap.reset_mock()918 mock_app.bootstrap.reset_mock()
907919
908 def test_no_local_no_sudo(self, mock_app, mock_open):920 def test_no_local_no_sudo(self, mock_app, mock_open):
@@ -921,7 +933,10 @@
921 manage.run(options)933 manage.run(options)
922 mock_app.bootstrap.assert_called_once_with(934 mock_app.bootstrap.assert_called_once_with(
923 options.env_name, self.juju_command,935 options.env_name, self.juju_command,
924 requires_sudo=False, debug=options.debug)936 requires_sudo=False,
937 debug=options.debug, upload_tools=options.upload_tools,
938 upload_series=options.upload_series,
939 constraints=options.constraints)
925940
926 def test_no_browser(self, mock_app, mock_open):941 def test_no_browser(self, mock_app, mock_open):
927 # It is possible to avoid opening the GUI in the browser.942 # It is possible to avoid opening the GUI in the browser.

Subscribers

People subscribed via source and target branches

to all changes: