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

Proposed by David Della Vecchia
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 (community) Approve
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.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
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?

Revision history for this message
Corey Bryant (corey.bryant) :
review: Needs Fixing
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
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
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Some more comments inline

review: Needs Fixing
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

123. By David Della Vecchia

merging trunk

Revision history for this message
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://paste.ubuntu.com/12246226/
Build: http://10.245.162.77:8080/job/charm_lint_check/9159/

Revision history for this message
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://paste.ubuntu.com/12246228/
Build: http://10.245.162.77:8080/job/charm_unit_test/8465/

Revision history for this message
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://paste.ubuntu.com/12246272/
Build: http://10.245.162.77:8080/job/charm_lint_check/9161/

Revision history for this message
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://paste.ubuntu.com/12246275/
Build: http://10.245.162.77:8080/job/charm_unit_test/8467/

124. By David Della Vecchia

Proper merge of trunk.

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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

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

Revision history for this message
Corey Bryant (corey.bryant) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'actions.yaml'
--- actions.yaml 2015-03-21 02:18:45 +0000
+++ actions.yaml 2015-09-01 16:22:19 +0000
@@ -1,2 +1,4 @@
1git-reinstall:1git-reinstall:
2 description: Reinstall cinder from the openstack-origin-git repositories.2 description: Reinstall cinder from the openstack-origin-git repositories.
3openstack-upgrade:
4 description: Perform openstack upgrades. Config option action-managed-upgrade must be set to True.
35
=== added symlink 'actions/openstack-upgrade'
=== target is u'openstack_upgrade.py'
=== added file 'actions/openstack_upgrade.py'
--- actions/openstack_upgrade.py 1970-01-01 00:00:00 +0000
+++ actions/openstack_upgrade.py 2015-09-01 16:22:19 +0000
@@ -0,0 +1,72 @@
1#!/usr/bin/python
2import sys
3import traceback
4import uuid
5
6sys.path.append('hooks/')
7
8from charmhelpers.core.hookenv import (
9 action_set,
10 action_fail,
11 config,
12 relation_ids,
13 relation_set
14)
15
16from cinder_hooks import config_changed
17
18from charmhelpers.contrib.openstack.utils import (
19 juju_log,
20 git_install_requested,
21 openstack_upgrade_available
22)
23
24from cinder_utils import (
25 do_openstack_upgrade,
26 register_configs
27)
28
29
30CONFIGS = register_configs()
31
32
33def openstack_upgrade():
34 """Upgrade packages to config-set Openstack version.
35
36 If the charm was installed from source we cannot upgrade it.
37 For backwards compatibility a config flag must be set for this
38 code to run, otherwise a full service level upgrade will fire
39 on config-changed."""
40
41 if git_install_requested():
42 action_set({'outcome': 'installed from source, skipped upgrade.'})
43 else:
44 if openstack_upgrade_available('cinder-common'):
45 if config('action-managed-upgrade'):
46 juju_log('Upgrading OpenStack release')
47
48 try:
49 do_openstack_upgrade(configs=CONFIGS)
50
51 # NOTE(jamespage) tell any storage-backends we just
52 # upgraded
53 for rid in relation_ids('storage-backend'):
54 relation_set(relation_id=rid,
55 upgrade_nonce=uuid.uuid4())
56
57 action_set({'outcome': 'success, upgrade completed.'})
58 except:
59 action_set({'outcome': 'upgrade failed, see traceback.'})
60 action_set({'traceback': traceback.format_exc()})
61 action_fail('do_openstack_upgrade resulted in an '
62 'unexpected error')
63
64 config_changed()
65 else:
66 action_set({'outcome': 'action-managed-upgrade config is '
67 'False, skipped upgrade.'})
68 else:
69 action_set({'outcome': 'no upgrade available.'})
70
71if __name__ == '__main__':
72 openstack_upgrade()
073
=== modified file 'config.yaml'
--- config.yaml 2015-08-03 20:18:39 +0000
+++ config.yaml 2015-09-01 16:22:19 +0000
@@ -282,4 +282,13 @@
282 description: |282 description: |
283 A comma-separated list of nagios servicegroups.283 A comma-separated list of nagios servicegroups.
284 If left empty, the nagios_context will be used as the servicegroup284 If left empty, the nagios_context will be used as the servicegroup
285285 action-managed-upgrade:
286 type: boolean
287 default: False
288 description: |
289 If True enables openstack upgrades for this charm via juju actions.
290 You will still need to set openstack-origin to the new repository but
291 instead of an upgrade running automatically across all units, it will
292 wait for you to execute the openstack-upgrade action for this charm on
293 each unit. If False it will revert to existing behavior of upgrading
294 all units on config change.
286295
=== modified file 'hooks/cinder_hooks.py'
--- hooks/cinder_hooks.py 2015-08-31 20:26:18 +0000
+++ hooks/cinder_hooks.py 2015-09-01 16:22:19 +0000
@@ -141,7 +141,7 @@
141 if git_install_requested():141 if git_install_requested():
142 if config_value_changed('openstack-origin-git'):142 if config_value_changed('openstack-origin-git'):
143 git_install(config('openstack-origin-git'))143 git_install(config('openstack-origin-git'))
144 else:144 elif not config('action-managed-upgrade'):
145 if openstack_upgrade_available('cinder-common'):145 if openstack_upgrade_available('cinder-common'):
146 do_openstack_upgrade(configs=CONFIGS)146 do_openstack_upgrade(configs=CONFIGS)
147 # NOTE(jamespage) tell any storage-backends we just upgraded147 # NOTE(jamespage) tell any storage-backends we just upgraded
148148
=== added file 'unit_tests/test_actions_openstack_upgrade.py'
--- unit_tests/test_actions_openstack_upgrade.py 1970-01-01 00:00:00 +0000
+++ unit_tests/test_actions_openstack_upgrade.py 2015-09-01 16:22:19 +0000
@@ -0,0 +1,126 @@
1from mock import patch
2import os
3
4os.environ['JUJU_UNIT_NAME'] = 'cinder'
5
6with patch('cinder_utils.register_configs') as register_configs:
7 import openstack_upgrade
8
9from test_utils import (
10 CharmTestCase
11)
12
13TO_PATCH = [
14 'config',
15 'juju_log',
16 'relation_set',
17 'relation_ids',
18 'uuid'
19]
20
21
22class TestCinderUpgradeActions(CharmTestCase):
23
24 def setUp(self):
25 super(TestCinderUpgradeActions, self).setUp(openstack_upgrade,
26 TO_PATCH)
27 self.config.side_effect = self.test_config.get
28
29 @patch.object(openstack_upgrade, 'action_set')
30 @patch.object(openstack_upgrade, 'action_fail')
31 @patch.object(openstack_upgrade, 'do_openstack_upgrade')
32 @patch.object(openstack_upgrade, 'openstack_upgrade_available')
33 @patch.object(openstack_upgrade, 'config_changed')
34 @patch('charmhelpers.contrib.openstack.utils.config')
35 def test_openstack_upgrade(self, _config, config_changed,
36 openstack_upgrade_available,
37 do_openstack_upgrade, action_fail,
38 action_set):
39 _config.return_value = None
40 openstack_upgrade_available.return_value = True
41 self.relation_ids.return_value = ['relid1']
42 self.uuid.uuid4.return_value = 12345
43
44 self.test_config.set('action-managed-upgrade', True)
45
46 openstack_upgrade.openstack_upgrade()
47
48 self.assertTrue(do_openstack_upgrade.called)
49 self.assertTrue(config_changed.called)
50 self.assertTrue(self.relation_ids.called)
51 self.relation_set.assert_called_with(relation_id='relid1',
52 upgrade_nonce=12345)
53 self.assertFalse(action_fail.called)
54
55 @patch.object(openstack_upgrade, 'action_set')
56 @patch.object(openstack_upgrade, 'do_openstack_upgrade')
57 @patch.object(openstack_upgrade, 'openstack_upgrade_available')
58 @patch.object(openstack_upgrade, 'config_changed')
59 @patch('charmhelpers.contrib.openstack.utils.config')
60 def test_openstack_upgrade_not_configured(self, _config, config_changed,
61 openstack_upgrade_available,
62 do_openstack_upgrade,
63 action_set):
64 _config.return_value = None
65 openstack_upgrade_available.return_value = True
66
67 openstack_upgrade.openstack_upgrade()
68
69 msg = ('action-managed-upgrade config is False, skipped upgrade.')
70
71 action_set.assert_called_with({'outcome': msg})
72 self.assertFalse(do_openstack_upgrade.called)
73
74 @patch.object(openstack_upgrade, 'action_set')
75 @patch.object(openstack_upgrade, 'do_openstack_upgrade')
76 @patch.object(openstack_upgrade, 'openstack_upgrade_available')
77 @patch.object(openstack_upgrade, 'config_changed')
78 @patch('charmhelpers.contrib.openstack.utils.config')
79 def test_openstack_upgrade_git_install(self, _config, config_changed,
80 openstack_upgrade_available,
81 do_openstack_upgrade,
82 action_set):
83
84 self.test_config.set('action-managed-upgrade', True)
85 self.test_config.set('openstack-origin-git', True)
86
87 openstack_upgrade.openstack_upgrade()
88
89 msg = ('installed from source, skipped upgrade.')
90 action_set.assert_called_with({'outcome': msg})
91 self.assertFalse(do_openstack_upgrade.called)
92
93 @patch.object(openstack_upgrade, 'action_set')
94 @patch.object(openstack_upgrade, 'action_fail')
95 @patch.object(openstack_upgrade, 'do_openstack_upgrade')
96 @patch.object(openstack_upgrade, 'openstack_upgrade_available')
97 @patch.object(openstack_upgrade, 'config_changed')
98 @patch('traceback.format_exc')
99 @patch('charmhelpers.contrib.openstack.utils.config')
100 def test_openstack_upgrade_exception(self, _config, format_exc,
101 config_changed,
102 openstack_upgrade_available,
103 do_openstack_upgrade,
104 action_fail, action_set):
105 _config.return_value = None
106 self.test_config.set('action-managed-upgrade', True)
107 openstack_upgrade_available.return_value = True
108
109 e = OSError('something bad happened')
110 do_openstack_upgrade.side_effect = e
111 traceback = (
112 "Traceback (most recent call last):\n"
113 " File \"actions/openstack_upgrade.py\", line 37, in openstack_upgrade\n" # noqa
114 " openstack_upgrade(config(\'openstack-origin-git\'))\n"
115 " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 964, in __call__\n" # noqa
116 " return _mock_self._mock_call(*args, **kwargs)\n"
117 " File \"/usr/lib/python2.7/dist-packages/mock.py\", line 1019, in _mock_call\n" # noqa
118 " raise effect\n"
119 "OSError: something bad happened\n")
120 format_exc.return_value = traceback
121
122 openstack_upgrade.openstack_upgrade()
123
124 msg = 'do_openstack_upgrade resulted in an unexpected error'
125 action_fail.assert_called_with(msg)
126 action_set.assert_called_with({'traceback': traceback})
0127
=== modified file 'unit_tests/test_cinder_hooks.py'
--- unit_tests/test_cinder_hooks.py 2015-07-30 07:50:05 +0000
+++ unit_tests/test_cinder_hooks.py 2015-09-01 16:22:19 +0000
@@ -254,6 +254,19 @@
254 False, False, False)254 False, False, False)
255 self.service_restart.assert_called_with('cinder-volume')255 self.service_restart.assert_called_with('cinder-volume')
256256
257 @patch.object(hooks, 'git_install_requested')
258 @patch.object(hooks, 'config_value_changed')
259 def test_config_changed_with_openstack_upgrade_action(self,
260 config_value_changed,
261 git_requested):
262 git_requested.return_value = False
263 self.openstack_upgrade_available.return_value = True
264 self.test_config.set('action-managed-upgrade', True)
265
266 hooks.hooks.execute(['hooks/config-changed'])
267
268 self.assertFalse(self.do_openstack_upgrade.called)
269
257 def test_db_changed(self):270 def test_db_changed(self):
258 'It writes out cinder.conf on db changed'271 'It writes out cinder.conf on db changed'
259 self.relation_get.return_value = 'cinder/0 cinder/1'272 self.relation_get.return_value = 'cinder/0 cinder/1'

Subscribers

People subscribed via source and target branches