Merge lp:~ddellav/charms/trusty/cinder/upgrade-action into lp:~openstack-charmers-archive/charms/trusty/cinder/next

Proposed by David Della Vecchia on 2015-08-26
Status: Merged
Merged at revision: 120
Proposed branch: lp:~ddellav/charms/trusty/cinder/upgrade-action
Merge into: lp:~openstack-charmers-archive/charms/trusty/cinder/next
Diff against target: 274 lines (+224/-2)
6 files modified
actions.yaml (+2/-0)
actions/openstack_upgrade.py (+72/-0)
config.yaml (+10/-1)
hooks/cinder_hooks.py (+1/-1)
unit_tests/test_actions_openstack_upgrade.py (+126/-0)
unit_tests/test_cinder_hooks.py (+13/-0)
To merge this branch: bzr merge lp:~ddellav/charms/trusty/cinder/upgrade-action
Reviewer Review Type Date Requested Status
Corey Bryant 2015-08-26 Approve on 2015-09-01
Review via email: mp+269247@code.launchpad.net

Description of the change

Added openstack-upgrade action to cinder charm as well as associated required configs and unit tests.

To post a comment you must log in.

charm_lint_check #8760 cinder-next for ddellav mp269247
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8760/

charm_unit_test #8091 cinder-next for ddellav mp269247
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8091/

charm_amulet_test #6038 cinder-next for ddellav mp269247
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6038/

Corey Bryant (corey.bryant) wrote :

It looks you may not have bzr added some files?
Also can you update the action-managed-upgrade description to match the one in glance?

review: Needs Fixing

charm_lint_check #8935 cinder-next for ddellav mp269247
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8935/

charm_unit_test #8257 cinder-next for ddellav mp269247
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8257/

Corey Bryant (corey.bryant) wrote :

I have some inline comments below.

Also, can you add a unit test like what you added to unit_tests/test_glance_relations.py? Asserting that the old migration isn't called when new migration is configured.

review: Needs Fixing

charm_amulet_test #6081 cinder-next for ddellav mp269247
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6081/

Corey Bryant (corey.bryant) wrote :

Some more comments inline

review: Needs Fixing

charm_lint_check #9096 cinder-next for ddellav mp269247
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/9096/

charm_unit_test #8405 cinder-next for ddellav mp269247
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8405/

charm_amulet_test #6154 cinder-next for ddellav mp269247
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6154/

122. By David Della Vecchia on 2015-09-01

Adding tests for relation_ids, relation_set. Fixing comment formatting. Hiding juju_log output in tests.

123. By David Della Vecchia on 2015-09-01

merging trunk

charm_lint_check #9159 cinder-next for ddellav mp269247
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/12246226/
Build: http://10.245.162.77:8080/job/charm_lint_check/9159/

charm_unit_test #8465 cinder-next for ddellav mp269247
    UNIT FAIL: unit-test failed

UNIT Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full unit test output: http://paste.ubuntu.com/12246228/
Build: http://10.245.162.77:8080/job/charm_unit_test/8465/

charm_lint_check #9161 cinder-next for ddellav mp269247
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/12246272/
Build: http://10.245.162.77:8080/job/charm_lint_check/9161/

charm_unit_test #8467 cinder-next for ddellav mp269247
    UNIT FAIL: unit-test failed

UNIT Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full unit test output: http://paste.ubuntu.com/12246275/
Build: http://10.245.162.77:8080/job/charm_unit_test/8467/

124. By David Della Vecchia on 2015-09-01

Proper merge of trunk.

charm_lint_check #9163 cinder-next for ddellav mp269247
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/9163/

charm_unit_test #8470 cinder-next for ddellav mp269247
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8470/

charm_amulet_test #6174 cinder-next for ddellav mp269247
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6174/

charm_amulet_test #6177 cinder-next for ddellav mp269247
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6177/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'actions.yaml'
2--- actions.yaml 2015-03-21 02:18:45 +0000
3+++ actions.yaml 2015-09-01 16:22:19 +0000
4@@ -1,2 +1,4 @@
5 git-reinstall:
6 description: Reinstall cinder from the openstack-origin-git repositories.
7+openstack-upgrade:
8+ description: Perform openstack upgrades. Config option action-managed-upgrade must be set to True.
9
10=== added symlink 'actions/openstack-upgrade'
11=== target is u'openstack_upgrade.py'
12=== added file 'actions/openstack_upgrade.py'
13--- actions/openstack_upgrade.py 1970-01-01 00:00:00 +0000
14+++ actions/openstack_upgrade.py 2015-09-01 16:22:19 +0000
15@@ -0,0 +1,72 @@
16+#!/usr/bin/python
17+import sys
18+import traceback
19+import uuid
20+
21+sys.path.append('hooks/')
22+
23+from charmhelpers.core.hookenv import (
24+ action_set,
25+ action_fail,
26+ config,
27+ relation_ids,
28+ relation_set
29+)
30+
31+from cinder_hooks import config_changed
32+
33+from charmhelpers.contrib.openstack.utils import (
34+ juju_log,
35+ git_install_requested,
36+ openstack_upgrade_available
37+)
38+
39+from cinder_utils import (
40+ do_openstack_upgrade,
41+ register_configs
42+)
43+
44+
45+CONFIGS = register_configs()
46+
47+
48+def openstack_upgrade():
49+ """Upgrade packages to config-set Openstack version.
50+
51+ If the charm was installed from source we cannot upgrade it.
52+ For backwards compatibility a config flag must be set for this
53+ code to run, otherwise a full service level upgrade will fire
54+ on config-changed."""
55+
56+ if git_install_requested():
57+ action_set({'outcome': 'installed from source, skipped upgrade.'})
58+ else:
59+ if openstack_upgrade_available('cinder-common'):
60+ if config('action-managed-upgrade'):
61+ juju_log('Upgrading OpenStack release')
62+
63+ try:
64+ do_openstack_upgrade(configs=CONFIGS)
65+
66+ # NOTE(jamespage) tell any storage-backends we just
67+ # upgraded
68+ for rid in relation_ids('storage-backend'):
69+ relation_set(relation_id=rid,
70+ upgrade_nonce=uuid.uuid4())
71+
72+ action_set({'outcome': 'success, upgrade completed.'})
73+ except:
74+ action_set({'outcome': 'upgrade failed, see traceback.'})
75+ action_set({'traceback': traceback.format_exc()})
76+ action_fail('do_openstack_upgrade resulted in an '
77+ 'unexpected error')
78+
79+ config_changed()
80+ else:
81+ action_set({'outcome': 'action-managed-upgrade config is '
82+ 'False, skipped upgrade.'})
83+ else:
84+ action_set({'outcome': 'no upgrade available.'})
85+
86+if __name__ == '__main__':
87+ openstack_upgrade()
88
89=== modified file 'config.yaml'
90--- config.yaml 2015-08-03 20:18:39 +0000
91+++ config.yaml 2015-09-01 16:22:19 +0000
92@@ -282,4 +282,13 @@
93 description: |
94 A comma-separated list of nagios servicegroups.
95 If left empty, the nagios_context will be used as the servicegroup
96-
97+ action-managed-upgrade:
98+ type: boolean
99+ default: False
100+ description: |
101+ If True enables openstack upgrades for this charm via juju actions.
102+ You will still need to set openstack-origin to the new repository but
103+ instead of an upgrade running automatically across all units, it will
104+ wait for you to execute the openstack-upgrade action for this charm on
105+ each unit. If False it will revert to existing behavior of upgrading
106+ all units on config change.
107
108=== modified file 'hooks/cinder_hooks.py'
109--- hooks/cinder_hooks.py 2015-08-31 20:26:18 +0000
110+++ hooks/cinder_hooks.py 2015-09-01 16:22:19 +0000
111@@ -141,7 +141,7 @@
112 if git_install_requested():
113 if config_value_changed('openstack-origin-git'):
114 git_install(config('openstack-origin-git'))
115- else:
116+ elif not config('action-managed-upgrade'):
117 if openstack_upgrade_available('cinder-common'):
118 do_openstack_upgrade(configs=CONFIGS)
119 # NOTE(jamespage) tell any storage-backends we just upgraded
120
121=== added file 'unit_tests/test_actions_openstack_upgrade.py'
122--- unit_tests/test_actions_openstack_upgrade.py 1970-01-01 00:00:00 +0000
123+++ unit_tests/test_actions_openstack_upgrade.py 2015-09-01 16:22:19 +0000
124@@ -0,0 +1,126 @@
125+from mock import patch
126+import os
127+
128+os.environ['JUJU_UNIT_NAME'] = 'cinder'
129+
130+with patch('cinder_utils.register_configs') as register_configs:
131+ import openstack_upgrade
132+
133+from test_utils import (
134+ CharmTestCase
135+)
136+
137+TO_PATCH = [
138+ 'config',
139+ 'juju_log',
140+ 'relation_set',
141+ 'relation_ids',
142+ 'uuid'
143+]
144+
145+
146+class TestCinderUpgradeActions(CharmTestCase):
147+
148+ def setUp(self):
149+ super(TestCinderUpgradeActions, self).setUp(openstack_upgrade,
150+ TO_PATCH)
151+ self.config.side_effect = self.test_config.get
152+
153+ @patch.object(openstack_upgrade, 'action_set')
154+ @patch.object(openstack_upgrade, 'action_fail')
155+ @patch.object(openstack_upgrade, 'do_openstack_upgrade')
156+ @patch.object(openstack_upgrade, 'openstack_upgrade_available')
157+ @patch.object(openstack_upgrade, 'config_changed')
158+ @patch('charmhelpers.contrib.openstack.utils.config')
159+ def test_openstack_upgrade(self, _config, config_changed,
160+ openstack_upgrade_available,
161+ do_openstack_upgrade, action_fail,
162+ action_set):
163+ _config.return_value = None
164+ openstack_upgrade_available.return_value = True
165+ self.relation_ids.return_value = ['relid1']
166+ self.uuid.uuid4.return_value = 12345
167+
168+ self.test_config.set('action-managed-upgrade', True)
169+
170+ openstack_upgrade.openstack_upgrade()
171+
172+ self.assertTrue(do_openstack_upgrade.called)
173+ self.assertTrue(config_changed.called)
174+ self.assertTrue(self.relation_ids.called)
175+ self.relation_set.assert_called_with(relation_id='relid1',
176+ upgrade_nonce=12345)
177+ self.assertFalse(action_fail.called)
178+
179+ @patch.object(openstack_upgrade, 'action_set')
180+ @patch.object(openstack_upgrade, 'do_openstack_upgrade')
181+ @patch.object(openstack_upgrade, 'openstack_upgrade_available')
182+ @patch.object(openstack_upgrade, 'config_changed')
183+ @patch('charmhelpers.contrib.openstack.utils.config')
184+ def test_openstack_upgrade_not_configured(self, _config, config_changed,
185+ openstack_upgrade_available,
186+ do_openstack_upgrade,
187+ action_set):
188+ _config.return_value = None
189+ openstack_upgrade_available.return_value = True
190+
191+ openstack_upgrade.openstack_upgrade()
192+
193+ msg = ('action-managed-upgrade config is False, skipped upgrade.')
194+
195+ action_set.assert_called_with({'outcome': msg})
196+ self.assertFalse(do_openstack_upgrade.called)
197+
198+ @patch.object(openstack_upgrade, 'action_set')
199+ @patch.object(openstack_upgrade, 'do_openstack_upgrade')
200+ @patch.object(openstack_upgrade, 'openstack_upgrade_available')
201+ @patch.object(openstack_upgrade, 'config_changed')
202+ @patch('charmhelpers.contrib.openstack.utils.config')
203+ def test_openstack_upgrade_git_install(self, _config, config_changed,
204+ openstack_upgrade_available,
205+ do_openstack_upgrade,
206+ action_set):
207+
208+ self.test_config.set('action-managed-upgrade', True)
209+ self.test_config.set('openstack-origin-git', True)
210+
211+ openstack_upgrade.openstack_upgrade()
212+
213+ msg = ('installed from source, skipped upgrade.')
214+ action_set.assert_called_with({'outcome': msg})
215+ self.assertFalse(do_openstack_upgrade.called)
216+
217+ @patch.object(openstack_upgrade, 'action_set')
218+ @patch.object(openstack_upgrade, 'action_fail')
219+ @patch.object(openstack_upgrade, 'do_openstack_upgrade')
220+ @patch.object(openstack_upgrade, 'openstack_upgrade_available')
221+ @patch.object(openstack_upgrade, 'config_changed')
222+ @patch('traceback.format_exc')
223+ @patch('charmhelpers.contrib.openstack.utils.config')
224+ def test_openstack_upgrade_exception(self, _config, format_exc,
225+ config_changed,
226+ openstack_upgrade_available,
227+ do_openstack_upgrade,
228+ action_fail, action_set):
229+ _config.return_value = None
230+ self.test_config.set('action-managed-upgrade', True)
231+ openstack_upgrade_available.return_value = True
232+
233+ e = OSError('something bad happened')
234+ do_openstack_upgrade.side_effect = e
235+ traceback = (
236+ "Traceback (most recent call last):\n"
237+ " File \"actions/openstack_upgrade.py\", line 37, in openstack_upgrade\n" # noqa
238+ " openstack_upgrade(config(\'openstack-origin-git\'))\n"
239+ " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 964, in __call__\n" # noqa
240+ " return _mock_self._mock_call(*args, **kwargs)\n"
241+ " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 1019, in _mock_call\n" # noqa
242+ " raise effect\n"
243+ "OSError: something bad happened\n")
244+ format_exc.return_value = traceback
245+
246+ openstack_upgrade.openstack_upgrade()
247+
248+ msg = 'do_openstack_upgrade resulted in an unexpected error'
249+ action_fail.assert_called_with(msg)
250+ action_set.assert_called_with({'traceback': traceback})
251
252=== modified file 'unit_tests/test_cinder_hooks.py'
253--- unit_tests/test_cinder_hooks.py 2015-07-30 07:50:05 +0000
254+++ unit_tests/test_cinder_hooks.py 2015-09-01 16:22:19 +0000
255@@ -254,6 +254,19 @@
256 False, False, False)
257 self.service_restart.assert_called_with('cinder-volume')
258
259+ @patch.object(hooks, 'git_install_requested')
260+ @patch.object(hooks, 'config_value_changed')
261+ def test_config_changed_with_openstack_upgrade_action(self,
262+ config_value_changed,
263+ git_requested):
264+ git_requested.return_value = False
265+ self.openstack_upgrade_available.return_value = True
266+ self.test_config.set('action-managed-upgrade', True)
267+
268+ hooks.hooks.execute(['hooks/config-changed'])
269+
270+ self.assertFalse(self.do_openstack_upgrade.called)
271+
272 def test_db_changed(self):
273 'It writes out cinder.conf on db changed'
274 self.relation_get.return_value = 'cinder/0 cinder/1'

Subscribers

People subscribed via source and target branches