Merge lp:~frankban/juju-quickstart/distro-only-flag into lp:juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 61
Proposed branch: lp:~frankban/juju-quickstart/distro-only-flag
Merge into: lp:juju-quickstart
Diff against target: 172 lines (+66/-12)
4 files modified
quickstart/__init__.py (+1/-1)
quickstart/app.py (+13/-7)
quickstart/manage.py (+5/-1)
quickstart/tests/test_app.py (+47/-3)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/distro-only-flag
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+213021@code.launchpad.net

Description of the change

Add the distro-only flag.

This can be used (e.g. in trusty) to
prevent quickstart from installing
the Juju stable PPA.

Tests: `make check`.

No QA required, I already tested this on
a trusty VM.

https://codereview.appspot.com/81400043/

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

LGTM thanks for the quick update.

https://codereview.appspot.com/81400043/

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

*** Submitted:

Add the distro-only flag.

This can be used (e.g. in trusty) to
prevent quickstart from installing
the Juju stable PPA.

Tests: `make check`.

No QA required, I already tested this on
a trusty VM.

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

https://codereview.appspot.com/81400043/

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

Hi Rick, thanks for the review!

https://codereview.appspot.com/81400043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'quickstart/__init__.py'
2--- quickstart/__init__.py 2014-03-14 16:50:22 +0000
3+++ quickstart/__init__.py 2014-03-27 11:09:37 +0000
4@@ -45,7 +45,7 @@
5 Once Juju has been installed, the command can also be run as a juju plugin,
6 without the hyphen ("juju quickstart").
7 """
8-VERSION = (1, 2, 0)
9+VERSION = (1, 3, 0)
10
11
12 def get_version():
13
14=== modified file 'quickstart/app.py'
15--- quickstart/app.py 2014-03-11 12:47:44 +0000
16+++ quickstart/app.py 2014-03-27 11:09:37 +0000
17@@ -57,11 +57,16 @@
18 return b'juju-quickstart: error: {}'.format(self.message)
19
20
21-def ensure_dependencies():
22+def ensure_dependencies(distro_only):
23 """Ensure that Juju and LXC are installed.
24
25- If juju is not found in the PATH, then add the juju stable PPA and install
26- both juju and lxc.
27+ If the "juju" command is not found in the PATH, then install and setup
28+ Juju, including the packages required to bootstrap local environments.
29+ This is usually done by adding the Juju stable PPA and installing the
30+ juju-core and juju-local packages.
31+
32+ If distro_only is True, the above PPA is not added to the apt sources, and
33+ we assume Juju packages are already available in the distro repository.
34
35 Return the Juju version tuple when Juju is available.
36
37@@ -87,10 +92,11 @@
38 # All those packages are installed as juju-local dependencies.
39 required_packages.append('juju-local')
40 if required_packages:
41- try:
42- utils.add_apt_repository('ppa:juju/stable')
43- except OSError as err:
44- raise ProgramExit(bytes(err))
45+ if not distro_only:
46+ try:
47+ utils.add_apt_repository('ppa:juju/stable')
48+ except OSError as err:
49+ raise ProgramExit(bytes(err))
50 print('sudo privileges will be used for the installation of \n'
51 'the following packages: {}\n'
52 'this can take a while...'.format(', '.join(required_packages)))
53
54=== modified file 'quickstart/manage.py'
55--- quickstart/manage.py 2014-03-14 16:50:22 +0000
56+++ quickstart/manage.py 2014-03-27 11:09:37 +0000
57@@ -299,6 +299,7 @@
58 - bundle: the optional bundle (path or URL) to be deployed;
59 - charm_url: the Juju GUI charm URL or None if not specified;
60 - debug: whether debug mode is activated;
61+ - distro_only: install Juju only using the distribution packages;
62 - env_file: the absolute path of the Juju environments.yaml file;
63 - env_name: the name of the Juju environment to use;
64 - env_type: the provider type of the selected Juju environment;
65@@ -375,6 +376,9 @@
66 '--no-browser', action='store_false', dest='open_browser',
67 help='Avoid opening the browser to the GUI at the end of the\nprocess')
68 parser.add_argument(
69+ '--distro-only', action='store_true', dest='distro_only',
70+ help='Do not use external sources when installing and setting up Juju')
71+ parser.add_argument(
72 '--version', action='version', version='%(prog)s {}'.format(version))
73 parser.add_argument(
74 '--debug', action='store_true',
75@@ -408,7 +412,7 @@
76 options.bundle_name, len(options.bundle_services)))
77
78 logging.debug('ensuring juju and lxc are installed')
79- juju_version = app.ensure_dependencies()
80+ juju_version = app.ensure_dependencies(options.distro_only)
81
82 logging.debug('ensuring SSH keys are available')
83 app.ensure_ssh_keys()
84
85=== modified file 'quickstart/tests/test_app.py'
86--- quickstart/tests/test_app.py 2014-03-11 12:47:44 +0000
87+++ quickstart/tests/test_app.py 2014-03-27 11:09:37 +0000
88@@ -67,7 +67,7 @@
89 add_repository = '/usr/bin/add-apt-repository'
90 apt_get = '/usr/bin/apt-get'
91
92- def call_ensure_dependencies(self, call_effects):
93+ def call_ensure_dependencies(self, call_effects, distro_only=False):
94 """Execute the quickstart.app.ensure_dependencies call.
95
96 The call_effects argument is used to customize the values returned by
97@@ -76,7 +76,7 @@
98 Return the mock call object and the ensure_dependencies return value.
99 """
100 with self.patch_multiple_calls(call_effects) as mock_call:
101- juju_version = app.ensure_dependencies()
102+ juju_version = app.ensure_dependencies(distro_only)
103 return mock_call, juju_version
104
105 def test_success_install(self, mock_print):
106@@ -112,6 +112,28 @@
107 ])
108 self.assertEqual((1, 18, 0), juju_version)
109
110+ def test_distro_only_install(self, mock_print):
111+ # All the missing packages are installed from the distro repository.
112+ side_effects = (
113+ (127, '', 'no juju'), # Retrieve the Juju version.
114+ (0, 'install', ''), # Install missing packages.
115+ (0, '1.17.42', ''), # Retrieve the version again.
116+ )
117+ mock_call, juju_version = self.call_ensure_dependencies(
118+ side_effects, distro_only=True)
119+ self.assertEqual(len(side_effects), mock_call.call_count)
120+ mock_call.assert_has_calls([
121+ mock.call(settings.JUJU_CMD, 'version'),
122+ mock.call('sudo', self.apt_get, 'install', '-y',
123+ 'juju-core', 'juju-local'),
124+ mock.call(settings.JUJU_CMD, 'version'),
125+ ])
126+ mock_print.assert_called_once_with(
127+ 'sudo privileges will be used for the installation of \n'
128+ 'the following packages: juju-core, juju-local\n'
129+ 'this can take a while...')
130+ self.assertEqual((1, 17, 42), juju_version)
131+
132 def test_success_no_install(self, mock_print):
133 # There is no need to install packages/PPAs if everything is already
134 # set up.
135@@ -158,6 +180,28 @@
136 ])
137 self.assertEqual((1, 16, 42), juju_version)
138
139+ def test_distro_only_partial_install(self, mock_print):
140+ # One missing installation is correctly handled when using distro only
141+ # packages.
142+ side_effects = (
143+ (0, '1.16.42', ''), # Check the juju command.
144+ (127, '', 'no lxc'), # Check the lxc-ls command.
145+ (0, 'install', ''), # Install missing packages.
146+ )
147+ mock_call, juju_version = self.call_ensure_dependencies(
148+ side_effects, distro_only=True)
149+ self.assertEqual(len(side_effects), mock_call.call_count)
150+ mock_call.assert_has_calls([
151+ mock.call(settings.JUJU_CMD, 'version'),
152+ mock.call('/usr/bin/lxc-ls'),
153+ mock.call('sudo', self.apt_get, 'install', '-y', 'juju-local'),
154+ ])
155+ mock_print.assert_called_once_with(
156+ 'sudo privileges will be used for the installation of \n'
157+ 'the following packages: juju-local\n'
158+ 'this can take a while...')
159+ self.assertEqual((1, 16, 42), juju_version)
160+
161 def test_add_repository_failure(self, mock_print):
162 # A ProgramExit is raised if the PPA is not successfully installed.
163 side_effects = (
164@@ -696,7 +740,7 @@
165 # Set up the add_unit return value.
166 if unit_name is not None:
167 env.add_unit.return_value = {'Units': [unit_name]}
168- #Set up the get_status return value.
169+ # Set up the get_status return value.
170 status = []
171 if service_data is not None:
172 status.append(self.make_service_change(data=service_data))

Subscribers

People subscribed via source and target branches