Merge lp:~evarlast/juju-quickstart/upload-tools-constraints into lp:juju-quickstart
- upload-tools-constraints
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Francesco Banconi | Approve | ||
Review via email: mp+227229@code.launchpad.net |
Commit message
Description of the change
support upload-tools and upload-series
support pass through of --upload-tools --upload-series and --constraints from quickstart to bootstrap
Francesco Banconi (frankban) wrote : | # |
- 91. By Jay R. Wren
-
addressing review concerns
Francesco Banconi (frankban) wrote : | # |
LGTM with minor changes, thank you!
- 92. By Jay R. Wren
-
semicolons and periods in comments
Jay R. Wren (evarlast) wrote : | # |
Reviewers: mp+227229_
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:/
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+93, -10 lines):
A [revision details]
M quickstart/app.py
M quickstart/
M quickstart/
M quickstart/
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:/
Preview Diff
1 | === modified file 'quickstart/app.py' | |||
2 | --- quickstart/app.py 2014-07-04 07:49:03 +0000 | |||
3 | +++ quickstart/app.py 2014-08-21 13:36:56 +0000 | |||
4 | @@ -163,7 +163,8 @@ | |||
5 | 163 | raise ProgramExit(bytes(err)) | 163 | raise ProgramExit(bytes(err)) |
6 | 164 | 164 | ||
7 | 165 | 165 | ||
9 | 166 | def bootstrap(env_name, juju_command, requires_sudo=False, debug=False): | 166 | def bootstrap(env_name, juju_command, requires_sudo=False, debug=False, |
10 | 167 | upload_tools=False, upload_series=None, constraints=None): | ||
11 | 167 | """Bootstrap the Juju environment with the given name. | 168 | """Bootstrap the Juju environment with the given name. |
12 | 168 | 169 | ||
13 | 169 | Do not bootstrap the environment if already bootstrapped. | 170 | Do not bootstrap the environment if already bootstrapped. |
14 | @@ -188,6 +189,14 @@ | |||
15 | 188 | cmd.insert(0, 'sudo') | 189 | cmd.insert(0, 'sudo') |
16 | 189 | if debug: | 190 | if debug: |
17 | 190 | cmd.append('--debug') | 191 | cmd.append('--debug') |
18 | 192 | if upload_tools: | ||
19 | 193 | cmd.append('--upload-tools') | ||
20 | 194 | if upload_series is not None: | ||
21 | 195 | cmd.append('--upload-series') | ||
22 | 196 | cmd.append(upload_series) | ||
23 | 197 | if constraints is not None: | ||
24 | 198 | cmd.append('--constraints') | ||
25 | 199 | cmd.append(constraints) | ||
26 | 191 | retcode, _, error = utils.call(*cmd) | 200 | retcode, _, error = utils.call(*cmd) |
27 | 192 | if retcode: | 201 | if retcode: |
28 | 193 | # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are | 202 | # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are |
29 | 194 | 203 | ||
30 | === modified file 'quickstart/manage.py' | |||
31 | --- quickstart/manage.py 2014-06-13 15:19:46 +0000 | |||
32 | +++ quickstart/manage.py 2014-08-21 13:36:56 +0000 | |||
33 | @@ -353,8 +353,12 @@ | |||
34 | 353 | - env_name: the name of the Juju environment to use; | 353 | - env_name: the name of the Juju environment to use; |
35 | 354 | - env_type: the provider type of the selected Juju environment; | 354 | - env_type: the provider type of the selected Juju environment; |
36 | 355 | - interactive: whether to start the interactive session; | 355 | - interactive: whether to start the interactive session; |
39 | 356 | - open_browser: whether the GUI browser must be opened. | 356 | - open_browser: whether the GUI browser must be opened; |
40 | 357 | - platform: The host platform. | 357 | - platform: The host platform; |
41 | 358 | - upload_tools: whether to upload local version of tools; | ||
42 | 359 | - upload_series: the comma-separated series list for which tools will | ||
43 | 360 | be uploaded, or None if not set; | ||
44 | 361 | - constraints: the environment constrains or None if not set. | ||
45 | 358 | 362 | ||
46 | 359 | The following attributes will also be included in the namespace if a bundle | 363 | The following attributes will also be included in the namespace if a bundle |
47 | 360 | deployment is requested: | 364 | deployment is requested: |
48 | @@ -450,6 +454,20 @@ | |||
49 | 450 | parser.add_argument( | 454 | parser.add_argument( |
50 | 451 | '--description', action=_DescriptionAction, default=argparse.SUPPRESS, | 455 | '--description', action=_DescriptionAction, default=argparse.SUPPRESS, |
51 | 452 | nargs=0, help="Show program's description and exit") | 456 | nargs=0, help="Show program's description and exit") |
52 | 457 | # These options are passed to juju bootstrap. | ||
53 | 458 | parser.add_argument( | ||
54 | 459 | '--upload-tools', action='store_true', | ||
55 | 460 | help='upload local version of tools before bootstrapping') | ||
56 | 461 | parser.add_argument( | ||
57 | 462 | '--upload-series', | ||
58 | 463 | help='upload tools for supplied comma-separated series list') | ||
59 | 464 | parser.add_argument( | ||
60 | 465 | '--constraints', | ||
61 | 466 | help='If constraints are specified in the bootstrap command,\n' | ||
62 | 467 | 'they will apply to the machine provisioned for the juju state\n' | ||
63 | 468 | 'server. They will also be set as default constraints on the\n' | ||
64 | 469 | 'environment for all future machines, exactly as if the\n' | ||
65 | 470 | 'constraints were set with juju set-constraints.\n') | ||
66 | 453 | # Parse the provided arguments. | 471 | # Parse the provided arguments. |
67 | 454 | options = parser.parse_args() | 472 | options = parser.parse_args() |
68 | 455 | 473 | ||
69 | @@ -500,7 +518,10 @@ | |||
70 | 500 | 518 | ||
71 | 501 | already_bootstrapped, bootstrap_node_series = app.bootstrap( | 519 | already_bootstrapped, bootstrap_node_series = app.bootstrap( |
72 | 502 | options.env_name, juju_command, | 520 | options.env_name, juju_command, |
74 | 503 | requires_sudo=requires_sudo, debug=options.debug) | 521 | requires_sudo=requires_sudo, debug=options.debug, |
75 | 522 | upload_tools=options.upload_tools, | ||
76 | 523 | upload_series=options.upload_series, | ||
77 | 524 | constraints=options.constraints) | ||
78 | 504 | 525 | ||
79 | 505 | # Retrieve the admin-secret for the current environment. | 526 | # Retrieve the admin-secret for the current environment. |
80 | 506 | try: | 527 | try: |
81 | 507 | 528 | ||
82 | === modified file 'quickstart/tests/test_app.py' | |||
83 | --- quickstart/tests/test_app.py 2014-07-04 13:09:11 +0000 | |||
84 | +++ quickstart/tests/test_app.py 2014-08-21 13:36:56 +0000 | |||
85 | @@ -510,6 +510,42 @@ | |||
86 | 510 | '--debug'), | 510 | '--debug'), |
87 | 511 | ] + self.make_status_calls(1)) | 511 | ] + self.make_status_calls(1)) |
88 | 512 | 512 | ||
89 | 513 | def test_success_upload_tools(self, mock_print): | ||
90 | 514 | # The environment is bootstrapped with local tools. | ||
91 | 515 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: | ||
92 | 516 | already_bootstrapped, series = app.bootstrap( | ||
93 | 517 | self.env_name, self.juju_command, upload_tools=True) | ||
94 | 518 | self.assertFalse(already_bootstrapped) | ||
95 | 519 | mock_call.assert_has_calls([ | ||
96 | 520 | mock.call( | ||
97 | 521 | self.juju_command, 'bootstrap', '-e', self.env_name, | ||
98 | 522 | '--upload-tools'), | ||
99 | 523 | ] + self.make_status_calls(1)) | ||
100 | 524 | |||
101 | 525 | def test_success_upload_series(self, mock_print): | ||
102 | 526 | # The environment is bootstrapped with tools for specific series. | ||
103 | 527 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: | ||
104 | 528 | already_bootstrapped, series = app.bootstrap( | ||
105 | 529 | self.env_name, self.juju_command, upload_series='hoary') | ||
106 | 530 | self.assertFalse(already_bootstrapped) | ||
107 | 531 | mock_call.assert_has_calls([ | ||
108 | 532 | mock.call( | ||
109 | 533 | self.juju_command, 'bootstrap', '-e', self.env_name, | ||
110 | 534 | '--upload-series', 'hoary'), | ||
111 | 535 | ] + self.make_status_calls(1)) | ||
112 | 536 | |||
113 | 537 | def test_success_constraints(self, mock_print): | ||
114 | 538 | # The environment is bootstrapped with given constraints. | ||
115 | 539 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: | ||
116 | 540 | already_bootstrapped, series = app.bootstrap( | ||
117 | 541 | self.env_name, self.juju_command, constraints='mem=7G') | ||
118 | 542 | self.assertFalse(already_bootstrapped) | ||
119 | 543 | mock_call.assert_has_calls([ | ||
120 | 544 | mock.call( | ||
121 | 545 | self.juju_command, 'bootstrap', '-e', self.env_name, | ||
122 | 546 | '--constraints', 'mem=7G'), | ||
123 | 547 | ] + self.make_status_calls(1)) | ||
124 | 548 | |||
125 | 513 | def test_already_bootstrapped(self, mock_print): | 549 | def test_already_bootstrapped(self, mock_print): |
126 | 514 | # The function succeeds and returns True if the environment is already | 550 | # The function succeeds and returns True if the environment is already |
127 | 515 | # bootstrapped. | 551 | # bootstrapped. |
128 | 516 | 552 | ||
129 | === modified file 'quickstart/tests/test_manage.py' | |||
130 | --- quickstart/tests/test_manage.py 2014-06-13 15:19:46 +0000 | |||
131 | +++ quickstart/tests/test_manage.py 2014-08-21 13:36:56 +0000 | |||
132 | @@ -660,9 +660,10 @@ | |||
133 | 660 | with mock.patch('sys.argv', ['juju-quickstart'] + args): | 660 | with mock.patch('sys.argv', ['juju-quickstart'] + args): |
134 | 661 | with mock.patch('sys.exit') as mock_exit: | 661 | with mock.patch('sys.exit') as mock_exit: |
135 | 662 | with self.patch_get_default_env_name(env_name): | 662 | with self.patch_get_default_env_name(env_name): |
137 | 663 | manage.setup() | 663 | options = manage.setup() |
138 | 664 | if exit_called: | 664 | if exit_called: |
139 | 665 | mock_exit.assert_called_once_with(0) | 665 | mock_exit.assert_called_once_with(0) |
140 | 666 | return options | ||
141 | 666 | 667 | ||
142 | 667 | def test_help(self): | 668 | def test_help(self): |
143 | 668 | # The program help message is properly formatted. | 669 | # The program help message is properly formatted. |
144 | @@ -735,7 +736,12 @@ | |||
145 | 735 | # Logging is properly set up at the debug level. | 736 | # Logging is properly set up at the debug level. |
146 | 736 | logger = logging.getLogger() | 737 | logger = logging.getLogger() |
147 | 737 | self.call_setup(['--debug'], 'ec2', exit_called=False) | 738 | self.call_setup(['--debug'], 'ec2', exit_called=False) |
149 | 738 | self.assertEqual(logging.DEBUG, logger.level) | 739 | self.assertTrue(logging.DEBUG, logger.level) |
150 | 740 | |||
151 | 741 | def test_upload_tools(self): | ||
152 | 742 | # Upload tools is properly set up. | ||
153 | 743 | options = self.call_setup(['--upload-tools'], exit_called=False) | ||
154 | 744 | self.assertTrue(options.upload_tools) | ||
155 | 739 | 745 | ||
156 | 740 | 746 | ||
157 | 741 | @mock.patch('webbrowser.open') | 747 | @mock.patch('webbrowser.open') |
158 | @@ -797,7 +803,9 @@ | |||
159 | 797 | mock_app.ensure_ssh_keys.assert_called() | 803 | mock_app.ensure_ssh_keys.assert_called() |
160 | 798 | mock_app.bootstrap.assert_called_once_with( | 804 | mock_app.bootstrap.assert_called_once_with( |
161 | 799 | options.env_name, self.juju_command, requires_sudo=False, | 805 | options.env_name, self.juju_command, requires_sudo=False, |
163 | 800 | debug=options.debug) | 806 | debug=options.debug, upload_tools=options.upload_tools, |
164 | 807 | upload_series=options.upload_series, | ||
165 | 808 | constraints=options.constraints) | ||
166 | 801 | mock_app.get_api_url.assert_called_once_with( | 809 | mock_app.get_api_url.assert_called_once_with( |
167 | 802 | options.env_name, self.juju_command) | 810 | options.env_name, self.juju_command) |
168 | 803 | mock_app.connect.assert_has_calls([ | 811 | mock_app.connect.assert_has_calls([ |
169 | @@ -878,7 +886,9 @@ | |||
170 | 878 | manage.run(options) | 886 | manage.run(options) |
171 | 879 | mock_app.bootstrap.assert_called_once_with( | 887 | mock_app.bootstrap.assert_called_once_with( |
172 | 880 | options.env_name, self.juju_command, requires_sudo=False, | 888 | options.env_name, self.juju_command, requires_sudo=False, |
174 | 881 | debug=options.debug) | 889 | debug=options.debug, upload_tools=options.upload_tools, |
175 | 890 | upload_series=options.upload_series, | ||
176 | 891 | constraints=options.constraints) | ||
177 | 882 | mock_app.bootstrap.reset_mock() | 892 | mock_app.bootstrap.reset_mock() |
178 | 883 | 893 | ||
179 | 884 | def test_local_provider_requiring_sudo(self, mock_app, mock_open): | 894 | def test_local_provider_requiring_sudo(self, mock_app, mock_open): |
180 | @@ -902,7 +912,9 @@ | |||
181 | 902 | manage.run(options) | 912 | manage.run(options) |
182 | 903 | mock_app.bootstrap.assert_called_once_with( | 913 | mock_app.bootstrap.assert_called_once_with( |
183 | 904 | options.env_name, self.juju_command, requires_sudo=True, | 914 | options.env_name, self.juju_command, requires_sudo=True, |
185 | 905 | debug=options.debug) | 915 | debug=options.debug, upload_tools=options.upload_tools, |
186 | 916 | upload_series=options.upload_series, | ||
187 | 917 | constraints=options.constraints) | ||
188 | 906 | mock_app.bootstrap.reset_mock() | 918 | mock_app.bootstrap.reset_mock() |
189 | 907 | 919 | ||
190 | 908 | def test_no_local_no_sudo(self, mock_app, mock_open): | 920 | def test_no_local_no_sudo(self, mock_app, mock_open): |
191 | @@ -921,7 +933,10 @@ | |||
192 | 921 | manage.run(options) | 933 | manage.run(options) |
193 | 922 | mock_app.bootstrap.assert_called_once_with( | 934 | mock_app.bootstrap.assert_called_once_with( |
194 | 923 | options.env_name, self.juju_command, | 935 | options.env_name, self.juju_command, |
196 | 924 | requires_sudo=False, debug=options.debug) | 936 | requires_sudo=False, |
197 | 937 | debug=options.debug, upload_tools=options.upload_tools, | ||
198 | 938 | upload_series=options.upload_series, | ||
199 | 939 | constraints=options.constraints) | ||
200 | 925 | 940 | ||
201 | 926 | def test_no_browser(self, mock_app, mock_open): | 941 | def test_no_browser(self, mock_app, mock_open): |
202 | 927 | # It is possible to avoid opening the GUI in the browser. | 942 | # It is possible to avoid opening the GUI in the browser. |
Nice branch Jay, it looks good with some changes I described below.
Thank you!