Merge lp:~ddellav/charms/trusty/cinder/upgrade-action into lp:~openstack-charmers-archive/charms/trusty/cinder/next
- Trusty Tahr (14.04)
- upgrade-action
- Merge into next
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corey Bryant (community) | Approve | ||
Review via email: mp+269247@code.launchpad.net |
Commit message
Description of the change
Added openstack-upgrade action to cinder charm as well as associated required configs and unit tests.
uosci-testing-bot (uosci-testing-bot) wrote : | # |
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #8091 cinder-next for ddellav mp269247
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6038 cinder-next for ddellav mp269247
AMULET OK: passed
Build: http://
Corey Bryant (corey.bryant) wrote : | # |
It looks you may not have bzr added some files?
Also can you update the action-
Corey Bryant (corey.bryant) : | # |
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #8935 cinder-next for ddellav mp269247
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #8257 cinder-next for ddellav mp269247
UNIT OK: passed
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/
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6081 cinder-next for ddellav mp269247
AMULET OK: passed
Build: http://
Corey Bryant (corey.bryant) wrote : | # |
Some more comments inline
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #9096 cinder-next for ddellav mp269247
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #8405 cinder-next for ddellav mp269247
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6154 cinder-next for ddellav mp269247
AMULET OK: passed
Build: http://
- 122. By David Della Vecchia
-
Adding tests for relation_ids, relation_set. Fixing comment formatting. Hiding juju_log output in tests.
- 123. By David Della Vecchia
-
merging trunk
uosci-testing-bot (uosci-testing-bot) wrote : | # |
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
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://
Build: http://
- 124. By David Della Vecchia
-
Proper merge of trunk.
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #9163 cinder-next for ddellav mp269247
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #8470 cinder-next for ddellav mp269247
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6174 cinder-next for ddellav mp269247
AMULET OK: passed
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6177 cinder-next for ddellav mp269247
AMULET OK: passed
Build: http://
Corey Bryant (corey.bryant) : | # |
Preview Diff
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 | 1 | git-reinstall: | 1 | git-reinstall: |
6 | 2 | description: Reinstall cinder from the openstack-origin-git repositories. | 2 | description: Reinstall cinder from the openstack-origin-git repositories. |
7 | 3 | openstack-upgrade: | ||
8 | 4 | description: Perform openstack upgrades. Config option action-managed-upgrade must be set to True. | ||
9 | 3 | 5 | ||
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 | 1 | #!/usr/bin/python | ||
17 | 2 | import sys | ||
18 | 3 | import traceback | ||
19 | 4 | import uuid | ||
20 | 5 | |||
21 | 6 | sys.path.append('hooks/') | ||
22 | 7 | |||
23 | 8 | from charmhelpers.core.hookenv import ( | ||
24 | 9 | action_set, | ||
25 | 10 | action_fail, | ||
26 | 11 | config, | ||
27 | 12 | relation_ids, | ||
28 | 13 | relation_set | ||
29 | 14 | ) | ||
30 | 15 | |||
31 | 16 | from cinder_hooks import config_changed | ||
32 | 17 | |||
33 | 18 | from charmhelpers.contrib.openstack.utils import ( | ||
34 | 19 | juju_log, | ||
35 | 20 | git_install_requested, | ||
36 | 21 | openstack_upgrade_available | ||
37 | 22 | ) | ||
38 | 23 | |||
39 | 24 | from cinder_utils import ( | ||
40 | 25 | do_openstack_upgrade, | ||
41 | 26 | register_configs | ||
42 | 27 | ) | ||
43 | 28 | |||
44 | 29 | |||
45 | 30 | CONFIGS = register_configs() | ||
46 | 31 | |||
47 | 32 | |||
48 | 33 | def openstack_upgrade(): | ||
49 | 34 | """Upgrade packages to config-set Openstack version. | ||
50 | 35 | |||
51 | 36 | If the charm was installed from source we cannot upgrade it. | ||
52 | 37 | For backwards compatibility a config flag must be set for this | ||
53 | 38 | code to run, otherwise a full service level upgrade will fire | ||
54 | 39 | on config-changed.""" | ||
55 | 40 | |||
56 | 41 | if git_install_requested(): | ||
57 | 42 | action_set({'outcome': 'installed from source, skipped upgrade.'}) | ||
58 | 43 | else: | ||
59 | 44 | if openstack_upgrade_available('cinder-common'): | ||
60 | 45 | if config('action-managed-upgrade'): | ||
61 | 46 | juju_log('Upgrading OpenStack release') | ||
62 | 47 | |||
63 | 48 | try: | ||
64 | 49 | do_openstack_upgrade(configs=CONFIGS) | ||
65 | 50 | |||
66 | 51 | # NOTE(jamespage) tell any storage-backends we just | ||
67 | 52 | # upgraded | ||
68 | 53 | for rid in relation_ids('storage-backend'): | ||
69 | 54 | relation_set(relation_id=rid, | ||
70 | 55 | upgrade_nonce=uuid.uuid4()) | ||
71 | 56 | |||
72 | 57 | action_set({'outcome': 'success, upgrade completed.'}) | ||
73 | 58 | except: | ||
74 | 59 | action_set({'outcome': 'upgrade failed, see traceback.'}) | ||
75 | 60 | action_set({'traceback': traceback.format_exc()}) | ||
76 | 61 | action_fail('do_openstack_upgrade resulted in an ' | ||
77 | 62 | 'unexpected error') | ||
78 | 63 | |||
79 | 64 | config_changed() | ||
80 | 65 | else: | ||
81 | 66 | action_set({'outcome': 'action-managed-upgrade config is ' | ||
82 | 67 | 'False, skipped upgrade.'}) | ||
83 | 68 | else: | ||
84 | 69 | action_set({'outcome': 'no upgrade available.'}) | ||
85 | 70 | |||
86 | 71 | if __name__ == '__main__': | ||
87 | 72 | openstack_upgrade() | ||
88 | 0 | 73 | ||
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 | 282 | description: | | 282 | description: | |
94 | 283 | A comma-separated list of nagios servicegroups. | 283 | A comma-separated list of nagios servicegroups. |
95 | 284 | If left empty, the nagios_context will be used as the servicegroup | 284 | If left empty, the nagios_context will be used as the servicegroup |
97 | 285 | 285 | action-managed-upgrade: | |
98 | 286 | type: boolean | ||
99 | 287 | default: False | ||
100 | 288 | description: | | ||
101 | 289 | If True enables openstack upgrades for this charm via juju actions. | ||
102 | 290 | You will still need to set openstack-origin to the new repository but | ||
103 | 291 | instead of an upgrade running automatically across all units, it will | ||
104 | 292 | wait for you to execute the openstack-upgrade action for this charm on | ||
105 | 293 | each unit. If False it will revert to existing behavior of upgrading | ||
106 | 294 | all units on config change. | ||
107 | 286 | 295 | ||
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 | 141 | if git_install_requested(): | 141 | if git_install_requested(): |
113 | 142 | if config_value_changed('openstack-origin-git'): | 142 | if config_value_changed('openstack-origin-git'): |
114 | 143 | git_install(config('openstack-origin-git')) | 143 | git_install(config('openstack-origin-git')) |
116 | 144 | else: | 144 | elif not config('action-managed-upgrade'): |
117 | 145 | if openstack_upgrade_available('cinder-common'): | 145 | if openstack_upgrade_available('cinder-common'): |
118 | 146 | do_openstack_upgrade(configs=CONFIGS) | 146 | do_openstack_upgrade(configs=CONFIGS) |
119 | 147 | # NOTE(jamespage) tell any storage-backends we just upgraded | 147 | # NOTE(jamespage) tell any storage-backends we just upgraded |
120 | 148 | 148 | ||
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 | 1 | from mock import patch | ||
126 | 2 | import os | ||
127 | 3 | |||
128 | 4 | os.environ['JUJU_UNIT_NAME'] = 'cinder' | ||
129 | 5 | |||
130 | 6 | with patch('cinder_utils.register_configs') as register_configs: | ||
131 | 7 | import openstack_upgrade | ||
132 | 8 | |||
133 | 9 | from test_utils import ( | ||
134 | 10 | CharmTestCase | ||
135 | 11 | ) | ||
136 | 12 | |||
137 | 13 | TO_PATCH = [ | ||
138 | 14 | 'config', | ||
139 | 15 | 'juju_log', | ||
140 | 16 | 'relation_set', | ||
141 | 17 | 'relation_ids', | ||
142 | 18 | 'uuid' | ||
143 | 19 | ] | ||
144 | 20 | |||
145 | 21 | |||
146 | 22 | class TestCinderUpgradeActions(CharmTestCase): | ||
147 | 23 | |||
148 | 24 | def setUp(self): | ||
149 | 25 | super(TestCinderUpgradeActions, self).setUp(openstack_upgrade, | ||
150 | 26 | TO_PATCH) | ||
151 | 27 | self.config.side_effect = self.test_config.get | ||
152 | 28 | |||
153 | 29 | @patch.object(openstack_upgrade, 'action_set') | ||
154 | 30 | @patch.object(openstack_upgrade, 'action_fail') | ||
155 | 31 | @patch.object(openstack_upgrade, 'do_openstack_upgrade') | ||
156 | 32 | @patch.object(openstack_upgrade, 'openstack_upgrade_available') | ||
157 | 33 | @patch.object(openstack_upgrade, 'config_changed') | ||
158 | 34 | @patch('charmhelpers.contrib.openstack.utils.config') | ||
159 | 35 | def test_openstack_upgrade(self, _config, config_changed, | ||
160 | 36 | openstack_upgrade_available, | ||
161 | 37 | do_openstack_upgrade, action_fail, | ||
162 | 38 | action_set): | ||
163 | 39 | _config.return_value = None | ||
164 | 40 | openstack_upgrade_available.return_value = True | ||
165 | 41 | self.relation_ids.return_value = ['relid1'] | ||
166 | 42 | self.uuid.uuid4.return_value = 12345 | ||
167 | 43 | |||
168 | 44 | self.test_config.set('action-managed-upgrade', True) | ||
169 | 45 | |||
170 | 46 | openstack_upgrade.openstack_upgrade() | ||
171 | 47 | |||
172 | 48 | self.assertTrue(do_openstack_upgrade.called) | ||
173 | 49 | self.assertTrue(config_changed.called) | ||
174 | 50 | self.assertTrue(self.relation_ids.called) | ||
175 | 51 | self.relation_set.assert_called_with(relation_id='relid1', | ||
176 | 52 | upgrade_nonce=12345) | ||
177 | 53 | self.assertFalse(action_fail.called) | ||
178 | 54 | |||
179 | 55 | @patch.object(openstack_upgrade, 'action_set') | ||
180 | 56 | @patch.object(openstack_upgrade, 'do_openstack_upgrade') | ||
181 | 57 | @patch.object(openstack_upgrade, 'openstack_upgrade_available') | ||
182 | 58 | @patch.object(openstack_upgrade, 'config_changed') | ||
183 | 59 | @patch('charmhelpers.contrib.openstack.utils.config') | ||
184 | 60 | def test_openstack_upgrade_not_configured(self, _config, config_changed, | ||
185 | 61 | openstack_upgrade_available, | ||
186 | 62 | do_openstack_upgrade, | ||
187 | 63 | action_set): | ||
188 | 64 | _config.return_value = None | ||
189 | 65 | openstack_upgrade_available.return_value = True | ||
190 | 66 | |||
191 | 67 | openstack_upgrade.openstack_upgrade() | ||
192 | 68 | |||
193 | 69 | msg = ('action-managed-upgrade config is False, skipped upgrade.') | ||
194 | 70 | |||
195 | 71 | action_set.assert_called_with({'outcome': msg}) | ||
196 | 72 | self.assertFalse(do_openstack_upgrade.called) | ||
197 | 73 | |||
198 | 74 | @patch.object(openstack_upgrade, 'action_set') | ||
199 | 75 | @patch.object(openstack_upgrade, 'do_openstack_upgrade') | ||
200 | 76 | @patch.object(openstack_upgrade, 'openstack_upgrade_available') | ||
201 | 77 | @patch.object(openstack_upgrade, 'config_changed') | ||
202 | 78 | @patch('charmhelpers.contrib.openstack.utils.config') | ||
203 | 79 | def test_openstack_upgrade_git_install(self, _config, config_changed, | ||
204 | 80 | openstack_upgrade_available, | ||
205 | 81 | do_openstack_upgrade, | ||
206 | 82 | action_set): | ||
207 | 83 | |||
208 | 84 | self.test_config.set('action-managed-upgrade', True) | ||
209 | 85 | self.test_config.set('openstack-origin-git', True) | ||
210 | 86 | |||
211 | 87 | openstack_upgrade.openstack_upgrade() | ||
212 | 88 | |||
213 | 89 | msg = ('installed from source, skipped upgrade.') | ||
214 | 90 | action_set.assert_called_with({'outcome': msg}) | ||
215 | 91 | self.assertFalse(do_openstack_upgrade.called) | ||
216 | 92 | |||
217 | 93 | @patch.object(openstack_upgrade, 'action_set') | ||
218 | 94 | @patch.object(openstack_upgrade, 'action_fail') | ||
219 | 95 | @patch.object(openstack_upgrade, 'do_openstack_upgrade') | ||
220 | 96 | @patch.object(openstack_upgrade, 'openstack_upgrade_available') | ||
221 | 97 | @patch.object(openstack_upgrade, 'config_changed') | ||
222 | 98 | @patch('traceback.format_exc') | ||
223 | 99 | @patch('charmhelpers.contrib.openstack.utils.config') | ||
224 | 100 | def test_openstack_upgrade_exception(self, _config, format_exc, | ||
225 | 101 | config_changed, | ||
226 | 102 | openstack_upgrade_available, | ||
227 | 103 | do_openstack_upgrade, | ||
228 | 104 | action_fail, action_set): | ||
229 | 105 | _config.return_value = None | ||
230 | 106 | self.test_config.set('action-managed-upgrade', True) | ||
231 | 107 | openstack_upgrade_available.return_value = True | ||
232 | 108 | |||
233 | 109 | e = OSError('something bad happened') | ||
234 | 110 | do_openstack_upgrade.side_effect = e | ||
235 | 111 | traceback = ( | ||
236 | 112 | "Traceback (most recent call last):\n" | ||
237 | 113 | " File \"actions/openstack_upgrade.py\", line 37, in openstack_upgrade\n" # noqa | ||
238 | 114 | " openstack_upgrade(config(\'openstack-origin-git\'))\n" | ||
239 | 115 | " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 964, in __call__\n" # noqa | ||
240 | 116 | " return _mock_self._mock_call(*args, **kwargs)\n" | ||
241 | 117 | " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 1019, in _mock_call\n" # noqa | ||
242 | 118 | " raise effect\n" | ||
243 | 119 | "OSError: something bad happened\n") | ||
244 | 120 | format_exc.return_value = traceback | ||
245 | 121 | |||
246 | 122 | openstack_upgrade.openstack_upgrade() | ||
247 | 123 | |||
248 | 124 | msg = 'do_openstack_upgrade resulted in an unexpected error' | ||
249 | 125 | action_fail.assert_called_with(msg) | ||
250 | 126 | action_set.assert_called_with({'traceback': traceback}) | ||
251 | 0 | 127 | ||
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 | 254 | False, False, False) | 254 | False, False, False) |
257 | 255 | self.service_restart.assert_called_with('cinder-volume') | 255 | self.service_restart.assert_called_with('cinder-volume') |
258 | 256 | 256 | ||
259 | 257 | @patch.object(hooks, 'git_install_requested') | ||
260 | 258 | @patch.object(hooks, 'config_value_changed') | ||
261 | 259 | def test_config_changed_with_openstack_upgrade_action(self, | ||
262 | 260 | config_value_changed, | ||
263 | 261 | git_requested): | ||
264 | 262 | git_requested.return_value = False | ||
265 | 263 | self.openstack_upgrade_available.return_value = True | ||
266 | 264 | self.test_config.set('action-managed-upgrade', True) | ||
267 | 265 | |||
268 | 266 | hooks.hooks.execute(['hooks/config-changed']) | ||
269 | 267 | |||
270 | 268 | self.assertFalse(self.do_openstack_upgrade.called) | ||
271 | 269 | |||
272 | 257 | def test_db_changed(self): | 270 | def test_db_changed(self): |
273 | 258 | 'It writes out cinder.conf on db changed' | 271 | 'It writes out cinder.conf on db changed' |
274 | 259 | self.relation_get.return_value = 'cinder/0 cinder/1' | 272 | self.relation_get.return_value = 'cinder/0 cinder/1' |
charm_lint_check #8760 cinder-next for ddellav mp269247
LINT OK: passed
Build: http:// 10.245. 162.77: 8080/job/ charm_lint_ check/8760/