Merge lp:~astara-drivers/charms/trusty/glance/api-ready-rebase into lp:~openstack-charmers-archive/charms/trusty/glance/next

Proposed by Adam Gandelman
Status: Needs review
Proposed branch: lp:~astara-drivers/charms/trusty/glance/api-ready-rebase
Merge into: lp:~openstack-charmers-archive/charms/trusty/glance/next
Diff against target: 229 lines (+82/-5)
4 files modified
hooks/glance_relations.py (+17/-0)
hooks/glance_utils.py (+5/-0)
unit_tests/test_glance_relations.py (+43/-3)
unit_tests/test_glance_utils.py (+17/-2)
To merge this branch: bzr merge lp:~astara-drivers/charms/trusty/glance/api-ready-rebase
Reviewer Review Type Date Requested Status
Adam Gandelman (community) Needs Resubmitting
James Page Needs Fixing
Review via email: mp+284958@code.launchpad.net
To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #121 glance-next for gandelman-a mp284958
    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/15002364/
Build: http://10.245.162.36:8080/job/charm_unit_test/121/

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

charm_lint_check #137 glance-next for gandelman-a mp284958
    LINT OK: passed

Build: http://10.245.162.36:8080/job/charm_lint_check/137/

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

charm_amulet_test #13 glance-next for gandelman-a mp284958
    AMULET OK: passed

Build: http://10.245.162.36:8080/job/charm_amulet_test/13/

Revision history for this message
James Page (james-page) wrote :

Hi Adam

I'm seeing:

==============================
Failed 1 tests - output below:
==============================

unit_tests.test_glance_relations.GlanceRelationTests.test_keystone_changed_with_object_store_relation
-----------------------------------------------------------------------------------------------------

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):
      File "/home/jamespage/src/charms/landing/glance/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py", line 1305, in patched
        return func(*args, **keywargs)
      File "/home/jamespage/src/charms/landing/glance/unit_tests/test_glance_relations.py", line 538, in test_keystone_changed_with_object_store_relation
        relations.keystone_changed()
      File "/home/jamespage/src/charms/landing/glance/hooks/charmhelpers/core/host.py", line 435, in wrapped_f
        f(*args, **kwargs)
      File "/home/jamespage/src/charms/landing/glance/hooks/glance_relations.py", line 347, in keystone_changed
        image_service_joined(rid)
      File "/home/jamespage/src/charms/landing/glance/hooks/glance_relations.py", line 227, in image_service_joined
        "{}:9292".format(canonical_url(CONFIGS, INTERNAL))
      File "/home/jamespage/src/charms/landing/glance/hooks/charmhelpers/contrib/openstack/ip.py", line 64, in canonical_url
        address = resolve_address(endpoint_type)
      File "/home/jamespage/src/charms/landing/glance/hooks/charmhelpers/contrib/openstack/ip.py", line 117, in resolve_address
        resolved_address = _get_address_override(endpoint_type)
      File "/home/jamespage/src/charms/landing/glance/hooks/charmhelpers/contrib/openstack/ip.py", line 99, in _get_address_override
        addr_override = config(override_key)
      File "/home/jamespage/src/charms/landing/glance/charmhelpers/core/hookenv.py", line 74, in wrapper
        res = func(*args, **kwargs)
      File "/home/jamespage/src/charms/landing/glance/charmhelpers/core/hookenv.py", line 340, in config
        subprocess.check_output(config_cmd_line).decode('UTF-8'))
      File "/usr/lib/python2.7/subprocess.py", line 566, in check_output
        process = Popen(stdout=PIPE, *popenargs, **kwargs)
      File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
        errread, errwrite)
      File "/usr/lib/python2.7/subprocess.py", line 1335, in _execute_child
        raise child_exception
    OSError: [Errno 2] No such file or directory

Looks like canonical_url will need mocking out as well for this test.

review: Needs Fixing
Revision history for this message
James Page (james-page) wrote :

Please set back to needs review once you've got that final test failure sorted out - other than that looks great to me...

Revision history for this message
Adam Gandelman (gandelman-a) wrote :

Thanks, james. Branch updated

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

charm_lint_check #1555 glance-next for gandelman-a mp284958
    LINT OK: passed

Build: http://10.245.162.36:8080/job/charm_lint_check/1555/

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

charm_unit_test #1303 glance-next for gandelman-a mp284958
    UNIT OK: passed

Build: http://10.245.162.36:8080/job/charm_unit_test/1303/

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

charm_amulet_test #550 glance-next for gandelman-a mp284958
    AMULET OK: passed

Build: http://10.245.162.36:8080/job/charm_amulet_test/550/

Unmerged revisions

168. By Adam Gandelman

Fix unit test error

Make sure image_service_joined is mocked.

Also add a missing mock that causes test_glance_utils to fail when run locally.

167. By Adam Gandelman

Remove unrelated change

166. By Adam Gandelman

Advertise API readiness

Adds a 'glance-api-ready' key to the image service relation that lets
remote services know the glance API is ready to be consumed, based on
complete contexts of required interfaces.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/glance_relations.py'
2--- hooks/glance_relations.py 2016-01-14 11:23:11 +0000
3+++ hooks/glance_relations.py 2016-02-23 18:52:15 +0000
4@@ -26,7 +26,9 @@
5 setup_ipv6,
6 swift_temp_url_key,
7 assess_status,
8+ is_api_ready,
9 )
10+
11 from charmhelpers.core.hookenv import (
12 config,
13 Hooks,
14@@ -43,6 +45,7 @@
15 UnregisteredHookError,
16 status_set,
17 )
18+
19 from charmhelpers.core.host import (
20 restart_on_change,
21 service_reload,
22@@ -184,6 +187,9 @@
23 juju_log('allowed_units either not presented, or local unit '
24 'not in acl list: %s' % allowed_units)
25
26+ for rid in relation_ids('image-service'):
27+ image_service_joined(rid)
28+
29
30 @hooks.hook('pgsql-db-relation-changed')
31 @restart_on_change(restart_map())
32@@ -210,6 +216,9 @@
33 juju_log('Cluster leader, performing db sync')
34 migrate_database()
35
36+ for rid in relation_ids('image-service'):
37+ image_service_joined(rid)
38+
39
40 @hooks.hook('image-service-relation-joined')
41 def image_service_joined(relation_id=None):
42@@ -218,6 +227,11 @@
43 "{}:9292".format(canonical_url(CONFIGS, INTERNAL))
44 }
45
46+ if is_api_ready(CONFIGS):
47+ relation_data['glance-api-ready'] = 'yes'
48+ else:
49+ relation_data['glance-api-ready'] = 'no'
50+
51 juju_log("%s: image-service_joined: To peer glance-api-server=%s" %
52 (CHARM, relation_data['glance-api-server']))
53
54@@ -329,6 +343,9 @@
55 # possibly configure HTTPS for API and registry
56 configure_https()
57
58+ for rid in relation_ids('image-service'):
59+ image_service_joined(rid)
60+
61
62 @hooks.hook('config-changed')
63 @restart_on_change(restart_map(), stopstart=True)
64
65=== modified file 'hooks/glance_utils.py'
66--- hooks/glance_utils.py 2016-01-27 12:41:15 +0000
67+++ hooks/glance_utils.py 2016-02-23 18:52:15 +0000
68@@ -63,6 +63,7 @@
69 configure_installation_source,
70 os_release,
71 set_os_workload_status,
72+ incomplete_relation_data,
73 )
74
75 from charmhelpers.core.templating import render
76@@ -537,3 +538,7 @@
77 # set the status according to the current state of the contexts
78 set_os_workload_status(
79 configs, REQUIRED_INTERFACES, charm_func=check_optional_relations)
80+
81+
82+def is_api_ready(configs):
83+ return (not incomplete_relation_data(configs, REQUIRED_INTERFACES))
84
85=== modified file 'unit_tests/test_glance_relations.py'
86--- unit_tests/test_glance_relations.py 2015-09-24 23:14:15 +0000
87+++ unit_tests/test_glance_relations.py 2016-02-23 18:52:15 +0000
88@@ -240,6 +240,14 @@
89 configs.write.call_args_list)
90 self.assertFalse(self.migrate_database.called)
91
92+ @patch.object(relations, 'image_service_joined')
93+ @patch.object(relations, 'CONFIGS')
94+ def test_db_changed_image_service_joined(self, configs, imgsj):
95+ rids = ['nova-cloud-controller:1', 'nova-compute:1']
96+ self.relation_ids.return_value = rids
97+ self._shared_db_test(configs, 'glance/2', None)
98+ [self.assertIn(call(r), imgsj.call_args_list) for r in rids]
99+
100 @patch.object(relations, 'CONFIGS')
101 def test_postgresql_db_changed_no_essex(self, configs):
102 self._postgresql_db_test(configs)
103@@ -276,6 +284,14 @@
104 )
105 self.migrate_database.assert_called_with()
106
107+ @patch.object(relations, 'image_service_joined')
108+ @patch.object(relations, 'CONFIGS')
109+ def test_postgresql_db_changed_image_service_joined(self, configs, imgsj):
110+ rids = ['nova-cloud-controller:1', 'nova-compute:1']
111+ self.relation_ids.return_value = rids
112+ self._postgresql_db_test(configs)
113+ [self.assertIn(call(r), imgsj.call_args_list) for r in rids]
114+
115 @patch.object(relations, 'CONFIGS')
116 def test_db_changed_with_essex_setting_version_control(self, configs):
117 self.os_release.return_value = "essex"
118@@ -307,11 +323,14 @@
119 )
120 self.migrate_database.assert_called_with()
121
122+ @patch.object(relations, 'is_api_ready')
123 @patch.object(relations, 'canonical_url')
124- def test_image_service_joined_leader(self, _canonical_url):
125+ def test_image_service_joined_leader(self, _canonical_url, _api_ready):
126+ _api_ready.return_value = True
127 _canonical_url.return_value = 'http://glancehost'
128 relations.image_service_joined()
129 args = {
130+ 'glance-api-ready': 'yes',
131 'glance-api-server': 'http://glancehost:9292',
132 'relation_id': None
133 }
134@@ -322,6 +341,7 @@
135 _canonical_url.return_value = 'http://glancehost'
136 relations.image_service_joined(relation_id='image-service:1')
137 args = {
138+ 'glance-api-ready': 'no',
139 'glance-api-server': 'http://glancehost:9292',
140 'relation_id': 'image-service:1',
141 }
142@@ -502,6 +522,7 @@
143 configs.write.call_args_list)
144 self.assertTrue(configure_https.called)
145
146+ @patch.object(relations, 'image_service_joined')
147 @patch.object(relations, 'configure_https')
148 @patch.object(relations, 'object_store_joined')
149 @patch.object(relations, 'CONFIGS')
150@@ -509,12 +530,16 @@
151 def test_keystone_changed_with_object_store_relation(self, git_requested,
152 configs,
153 object_store_joined,
154- configure_https):
155+ configure_https,
156+ image_service_joined):
157 git_requested.return_value = False
158 configs.complete_contexts = MagicMock()
159 configs.complete_contexts.return_value = ['identity-service']
160 configs.write = MagicMock()
161- self.relation_ids.return_value = ['object-store:0']
162+ self.relation_ids.side_effect = [
163+ ['object-store:0'],
164+ ['image-service:0'],
165+ ]
166 relations.keystone_changed()
167 self.assertEquals([call('/etc/glance/glance-api.conf'),
168 call('/etc/glance/glance-registry.conf'),
169@@ -523,6 +548,21 @@
170 configs.write.call_args_list)
171 object_store_joined.assert_called_with()
172 self.assertTrue(configure_https.called)
173+ image_service_joined.assert_called_with('image-service:0')
174+
175+ @patch.object(relations, 'configure_https')
176+ @patch.object(relations, 'object_store_joined')
177+ @patch.object(relations, 'image_service_joined')
178+ @patch.object(relations, 'CONFIGS')
179+ def test_keystone_changed_image_service_joined(self, configs, imgsj, osj,
180+ https):
181+ configs.complete_contexts = MagicMock()
182+ configs.complete_contexts.return_value = ['identity-service']
183+
184+ rids = ['nova-cloud-controller:1', 'nova-compute:1']
185+ self.relation_ids.return_value = rids
186+ relations.keystone_changed()
187+ [self.assertIn(call(r), imgsj.call_args_list) for r in rids]
188
189 @patch.object(relations, 'configure_https')
190 @patch.object(relations, 'git_install_requested')
191
192=== modified file 'unit_tests/test_glance_utils.py'
193--- unit_tests/test_glance_utils.py 2016-01-13 14:31:40 +0000
194+++ unit_tests/test_glance_utils.py 2016-02-23 18:52:15 +0000
195@@ -240,13 +240,14 @@
196 @patch.object(utils, 'git_pip_venv_dir')
197 @patch('os.path.join')
198 @patch('os.path.exists')
199+ @patch('os.remove')
200 @patch('os.symlink')
201 @patch('shutil.copytree')
202 @patch('shutil.rmtree')
203 @patch('subprocess.check_call')
204 def test_git_post_install(self, check_call, rmtree, copytree, symlink,
205- exists, join, venv, render, service_restart,
206- git_src_dir):
207+ remove, exists, join, venv, render,
208+ service_restart, git_src_dir):
209 projects_yaml = openstack_origin_git
210 join.return_value = 'joined-string'
211 venv.return_value = '/mnt/openstack-git/venv'
212@@ -327,3 +328,17 @@
213 "TEST CONFIG",
214 utils.REQUIRED_INTERFACES,
215 charm_func=utils.check_optional_relations)
216+
217+ def _test_is_api_ready(self, tgt):
218+ fake_config = MagicMock()
219+ with patch.object(utils, 'incomplete_relation_data') as ird:
220+ ird.return_value = (not tgt)
221+ self.assertEqual(utils.is_api_ready(fake_config), tgt)
222+ ird.assert_called_with(
223+ fake_config, utils.REQUIRED_INTERFACES)
224+
225+ def test_is_api_ready_true(self):
226+ self._test_is_api_ready(True)
227+
228+ def test_is_api_ready_false(self):
229+ self._test_is_api_ready(False)

Subscribers

People subscribed via source and target branches