Merge lp:~james-page/charm-helpers/is-relation-made into lp:charm-helpers
- is-relation-made
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | 92 |
Proposed branch: | lp:~james-page/charm-helpers/is-relation-made |
Merge into: | lp:charm-helpers |
Diff against target: |
260 lines (+109/-18) 7 files modified
charmhelpers/cli/host.py (+1/-0) charmhelpers/contrib/ssl/__init__.py (+7/-8) charmhelpers/core/hookenv.py (+20/-0) charmhelpers/fetch/bzrurl.py (+1/-1) tests/contrib/ssl/test_ssl.py (+1/-5) tests/core/test_hookenv.py (+79/-0) tests/fetch/test_bzrurl.py (+0/-4) |
To merge this branch: | bzr merge lp:~james-page/charm-helpers/is-relation-made |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Page | Needs Resubmitting | ||
Adam Gandelman (community) | Needs Fixing | ||
Review via email:
|
Commit message
Description of the change
Add is_relation_made helper for deep testing of relation state.
For example, the ceph charm only passes an auth key to clients once
its bootstrapped; until then the client can do anything:
if is_relation_
configure_
else:
don_t()
Unit tests as well.
Thanks for looking.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
James Page (james-page) wrote : | # |
- 82. By James Page
-
Rebase against parent branch
- 83. By James Page
-
Rebase on parent
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Adam Gandelman (gandelman-a) wrote : | # |
This looks good, but any chance we can allow is_relation_made() to also take a list of keys? That way we could use the same for relations that require a bit more data, ie:
keys = ['password, 'db-host']
if is_relation_
configure_db()
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Adam Gandelman (gandelman-a) wrote : | # |
Also, I think the current behavior of relation_get() will cause this to evalutate as True even if the relation setting has not been set:
if relation_get(key, rid=r_id, unit=unit):
return True
Might be a good idea to also check whatever relation_get() returns != ''
- 84. By James Page
-
Add support for multiple key checks in is_relation_made
- 85. By James Page
-
Rework a bit for lists
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
James Page (james-page) wrote : | # |
relation_get for a missing key should always return None so I think that is OK.
I updated to support lists as well as single strings as keys.
Preview Diff
1 | === modified file 'charmhelpers/cli/host.py' | |||
2 | --- charmhelpers/cli/host.py 2013-06-20 15:03:24 +0000 | |||
3 | +++ charmhelpers/cli/host.py 2013-10-21 22:54:42 +0000 | |||
4 | @@ -7,6 +7,7 @@ | |||
5 | 7 | "List mounts" | 7 | "List mounts" |
6 | 8 | return host.mounts() | 8 | return host.mounts() |
7 | 9 | 9 | ||
8 | 10 | |||
9 | 10 | @cmdline.subcommand_builder('service', description="Control system services") | 11 | @cmdline.subcommand_builder('service', description="Control system services") |
10 | 11 | def service(subparser): | 12 | def service(subparser): |
11 | 12 | subparser.add_argument("action", help="The action to perform (start, stop, etc...)") | 13 | subparser.add_argument("action", help="The action to perform (start, stop, etc...)") |
12 | 13 | 14 | ||
13 | === modified file 'charmhelpers/contrib/ssl/__init__.py' | |||
14 | --- charmhelpers/contrib/ssl/__init__.py 2013-08-28 09:12:05 +0000 | |||
15 | +++ charmhelpers/contrib/ssl/__init__.py 2013-10-21 22:54:42 +0000 | |||
16 | @@ -34,7 +34,7 @@ | |||
17 | 34 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", | 34 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", |
18 | 35 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", | 35 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", |
19 | 36 | "-keyout", keyfile, | 36 | "-keyout", keyfile, |
21 | 37 | "-out", certfile, "-config", config] | 37 | "-out", certfile, "-config", config] |
22 | 38 | elif subject: | 38 | elif subject: |
23 | 39 | ssl_subject = "" | 39 | ssl_subject = "" |
24 | 40 | if "country" in subject: | 40 | if "country" in subject: |
25 | @@ -50,8 +50,8 @@ | |||
26 | 50 | if "cn" in subject: | 50 | if "cn" in subject: |
27 | 51 | ssl_subject = ssl_subject + "/CN={}".format(subject["cn"]) | 51 | ssl_subject = ssl_subject + "/CN={}".format(subject["cn"]) |
28 | 52 | else: | 52 | else: |
31 | 53 | hookenv.log("When using \"subject\" argument you must " \ | 53 | hookenv.log("When using \"subject\" argument you must " |
32 | 54 | "provide \"cn\" field at very least") | 54 | "provide \"cn\" field at very least") |
33 | 55 | return False | 55 | return False |
34 | 56 | if "email" in subject: | 56 | if "email" in subject: |
35 | 57 | ssl_subject = ssl_subject + "/emailAddress={}".format(subject["email"]) | 57 | ssl_subject = ssl_subject + "/emailAddress={}".format(subject["email"]) |
36 | @@ -59,16 +59,16 @@ | |||
37 | 59 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", | 59 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", |
38 | 60 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", | 60 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", |
39 | 61 | "-keyout", keyfile, | 61 | "-keyout", keyfile, |
41 | 62 | "-out", certfile, "-subj", ssl_subject] | 62 | "-out", certfile, "-subj", ssl_subject] |
42 | 63 | elif cn: | 63 | elif cn: |
43 | 64 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", | 64 | cmd = ["/usr/bin/openssl", "req", "-new", "-newkey", |
44 | 65 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", | 65 | "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509", |
45 | 66 | "-keyout", keyfile, | 66 | "-keyout", keyfile, |
47 | 67 | "-out", certfile, "-subj", "/CN={}".format(cn)] | 67 | "-out", certfile, "-subj", "/CN={}".format(cn)] |
48 | 68 | 68 | ||
49 | 69 | if not cmd: | 69 | if not cmd: |
52 | 70 | hookenv.log("No config, subject or cn provided," \ | 70 | hookenv.log("No config, subject or cn provided," |
53 | 71 | "unable to generate self signed SSL certificates") | 71 | "unable to generate self signed SSL certificates") |
54 | 72 | return False | 72 | return False |
55 | 73 | try: | 73 | try: |
56 | 74 | subprocess.check_call(cmd) | 74 | subprocess.check_call(cmd) |
57 | @@ -76,4 +76,3 @@ | |||
58 | 76 | except Exception as e: | 76 | except Exception as e: |
59 | 77 | print "Execution of openssl command failed:\n{}".format(e) | 77 | print "Execution of openssl command failed:\n{}".format(e) |
60 | 78 | return False | 78 | return False |
61 | 79 | |||
62 | 80 | 79 | ||
63 | === modified file 'charmhelpers/core/hookenv.py' | |||
64 | --- charmhelpers/core/hookenv.py 2013-10-21 08:34:03 +0000 | |||
65 | +++ charmhelpers/core/hookenv.py 2013-10-21 22:54:42 +0000 | |||
66 | @@ -285,6 +285,26 @@ | |||
67 | 285 | return rels | 285 | return rels |
68 | 286 | 286 | ||
69 | 287 | 287 | ||
70 | 288 | @cached | ||
71 | 289 | def is_relation_made(relation, keys='private-address'): | ||
72 | 290 | ''' | ||
73 | 291 | Determine whether a relation is established by checking for | ||
74 | 292 | presence of key(s). If a list of keys is provided, they | ||
75 | 293 | must all be present for the relation to be identified as made | ||
76 | 294 | ''' | ||
77 | 295 | if isinstance(keys, str): | ||
78 | 296 | keys = [keys] | ||
79 | 297 | for r_id in relation_ids(relation): | ||
80 | 298 | for unit in related_units(r_id): | ||
81 | 299 | context = {} | ||
82 | 300 | for k in keys: | ||
83 | 301 | context[k] = relation_get(k, rid=r_id, | ||
84 | 302 | unit=unit) | ||
85 | 303 | if None not in context.values(): | ||
86 | 304 | return True | ||
87 | 305 | return False | ||
88 | 306 | |||
89 | 307 | |||
90 | 288 | def open_port(port, protocol="TCP"): | 308 | def open_port(port, protocol="TCP"): |
91 | 289 | """Open a service network port""" | 309 | """Open a service network port""" |
92 | 290 | _args = ['open-port'] | 310 | _args = ['open-port'] |
93 | 291 | 311 | ||
94 | === modified file 'charmhelpers/fetch/bzrurl.py' | |||
95 | --- charmhelpers/fetch/bzrurl.py 2013-08-22 10:19:51 +0000 | |||
96 | +++ charmhelpers/fetch/bzrurl.py 2013-10-21 22:54:42 +0000 | |||
97 | @@ -12,6 +12,7 @@ | |||
98 | 12 | apt_install("python-bzrlib") | 12 | apt_install("python-bzrlib") |
99 | 13 | from bzrlib.branch import Branch | 13 | from bzrlib.branch import Branch |
100 | 14 | 14 | ||
101 | 15 | |||
102 | 15 | class BzrUrlFetchHandler(BaseFetchHandler): | 16 | class BzrUrlFetchHandler(BaseFetchHandler): |
103 | 16 | """Handler for bazaar branches via generic and lp URLs""" | 17 | """Handler for bazaar branches via generic and lp URLs""" |
104 | 17 | def can_handle(self, source): | 18 | def can_handle(self, source): |
105 | @@ -46,4 +47,3 @@ | |||
106 | 46 | except OSError as e: | 47 | except OSError as e: |
107 | 47 | raise UnhandledSource(e.strerror) | 48 | raise UnhandledSource(e.strerror) |
108 | 48 | return dest_dir | 49 | return dest_dir |
109 | 49 | |||
110 | 50 | 50 | ||
111 | === modified file 'tests/contrib/ssl/test_ssl.py' | |||
112 | --- tests/contrib/ssl/test_ssl.py 2013-08-27 17:08:44 +0000 | |||
113 | +++ tests/contrib/ssl/test_ssl.py 2013-10-21 22:54:42 +0000 | |||
114 | @@ -1,6 +1,4 @@ | |||
118 | 1 | import subprocess | 1 | from mock import patch |
116 | 2 | |||
117 | 3 | from mock import patch, call, MagicMock | ||
119 | 4 | from testtools import TestCase | 2 | from testtools import TestCase |
120 | 5 | 3 | ||
121 | 6 | from charmhelpers.contrib import ssl | 4 | from charmhelpers.contrib import ssl |
122 | @@ -39,7 +37,6 @@ | |||
123 | 39 | result = ssl.generate_selfsigned("mykey.key", "mycert.crt", subject=subject) | 37 | result = ssl.generate_selfsigned("mykey.key", "mycert.crt", subject=subject) |
124 | 40 | self.assertFalse(result) | 38 | self.assertFalse(result) |
125 | 41 | 39 | ||
126 | 42 | |||
127 | 43 | @patch('subprocess.check_call') | 40 | @patch('subprocess.check_call') |
128 | 44 | def test_generate_selfsigned_file(self, mock_call): | 41 | def test_generate_selfsigned_file(self, mock_call): |
129 | 45 | ssl.generate_selfsigned("mykey.key", "mycert.crt", config="test.cnf") | 42 | ssl.generate_selfsigned("mykey.key", "mycert.crt", config="test.cnf") |
130 | @@ -49,7 +46,6 @@ | |||
131 | 49 | 'mykey.key', '-out', 'mycert.crt', | 46 | 'mykey.key', '-out', 'mycert.crt', |
132 | 50 | '-config', 'test.cnf']) | 47 | '-config', 'test.cnf']) |
133 | 51 | 48 | ||
134 | 52 | |||
135 | 53 | @patch('subprocess.check_call') | 49 | @patch('subprocess.check_call') |
136 | 54 | def test_generate_selfsigned_cn_key(self, mock_call): | 50 | def test_generate_selfsigned_cn_key(self, mock_call): |
137 | 55 | ssl.generate_selfsigned("mykey.key", "mycert.crt", keysize="2048", cn="mysite.example.com") | 51 | ssl.generate_selfsigned("mykey.key", "mycert.crt", keysize="2048", cn="mysite.example.com") |
138 | 56 | 52 | ||
139 | === modified file 'tests/core/test_hookenv.py' | |||
140 | --- tests/core/test_hookenv.py 2013-10-21 08:34:03 +0000 | |||
141 | +++ tests/core/test_hookenv.py 2013-10-21 22:54:42 +0000 | |||
142 | @@ -498,6 +498,85 @@ | |||
143 | 498 | }, | 498 | }, |
144 | 499 | }) | 499 | }) |
145 | 500 | 500 | ||
146 | 501 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
147 | 502 | @patch('charmhelpers.core.hookenv.related_units') | ||
148 | 503 | @patch('charmhelpers.core.hookenv.relation_get') | ||
149 | 504 | def test_is_relation_made(self, relation_get, related_units, | ||
150 | 505 | relation_ids): | ||
151 | 506 | relation_get.return_value = 'hostname' | ||
152 | 507 | related_units.return_value = ['test/1'] | ||
153 | 508 | relation_ids.return_value = ['test:0'] | ||
154 | 509 | self.assertTrue(hookenv.is_relation_made('test')) | ||
155 | 510 | relation_get.assert_called_with('private-address', | ||
156 | 511 | rid='test:0', unit='test/1') | ||
157 | 512 | |||
158 | 513 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
159 | 514 | @patch('charmhelpers.core.hookenv.related_units') | ||
160 | 515 | @patch('charmhelpers.core.hookenv.relation_get') | ||
161 | 516 | def test_is_relation_made_multi_unit(self, relation_get, related_units, | ||
162 | 517 | relation_ids): | ||
163 | 518 | relation_get.side_effect = [None, 'hostname'] | ||
164 | 519 | related_units.return_value = ['test/1', 'test/2'] | ||
165 | 520 | relation_ids.return_value = ['test:0'] | ||
166 | 521 | self.assertTrue(hookenv.is_relation_made('test')) | ||
167 | 522 | |||
168 | 523 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
169 | 524 | @patch('charmhelpers.core.hookenv.related_units') | ||
170 | 525 | @patch('charmhelpers.core.hookenv.relation_get') | ||
171 | 526 | def test_is_relation_made_different_key(self, | ||
172 | 527 | relation_get, related_units, | ||
173 | 528 | relation_ids): | ||
174 | 529 | relation_get.return_value = 'hostname' | ||
175 | 530 | related_units.return_value = ['test/1'] | ||
176 | 531 | relation_ids.return_value = ['test:0'] | ||
177 | 532 | self.assertTrue(hookenv.is_relation_made('test', keys='auth')) | ||
178 | 533 | relation_get.assert_called_with('auth', | ||
179 | 534 | rid='test:0', unit='test/1') | ||
180 | 535 | |||
181 | 536 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
182 | 537 | @patch('charmhelpers.core.hookenv.related_units') | ||
183 | 538 | @patch('charmhelpers.core.hookenv.relation_get') | ||
184 | 539 | def test_is_relation_made_multiple_keys(self, | ||
185 | 540 | relation_get, related_units, | ||
186 | 541 | relation_ids): | ||
187 | 542 | relation_get.side_effect = ['password', 'hostname'] | ||
188 | 543 | related_units.return_value = ['test/1'] | ||
189 | 544 | relation_ids.return_value = ['test:0'] | ||
190 | 545 | self.assertTrue(hookenv.is_relation_made('test', | ||
191 | 546 | keys=['auth', 'host'])) | ||
192 | 547 | relation_get.assert_has_calls( | ||
193 | 548 | [call('auth', rid='test:0', unit='test/1'), | ||
194 | 549 | call('host', rid='test:0', unit='test/1')] | ||
195 | 550 | ) | ||
196 | 551 | |||
197 | 552 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
198 | 553 | @patch('charmhelpers.core.hookenv.related_units') | ||
199 | 554 | @patch('charmhelpers.core.hookenv.relation_get') | ||
200 | 555 | def test_is_relation_made_not_made(self, | ||
201 | 556 | relation_get, related_units, | ||
202 | 557 | relation_ids): | ||
203 | 558 | relation_get.return_value = None | ||
204 | 559 | related_units.return_value = ['test/1'] | ||
205 | 560 | relation_ids.return_value = ['test:0'] | ||
206 | 561 | self.assertFalse(hookenv.is_relation_made('test')) | ||
207 | 562 | |||
208 | 563 | @patch('charmhelpers.core.hookenv.relation_ids') | ||
209 | 564 | @patch('charmhelpers.core.hookenv.related_units') | ||
210 | 565 | @patch('charmhelpers.core.hookenv.relation_get') | ||
211 | 566 | def test_is_relation_made_not_made_multiple_keys(self, | ||
212 | 567 | relation_get, | ||
213 | 568 | related_units, | ||
214 | 569 | relation_ids): | ||
215 | 570 | relation_get.side_effect = ['password', None] | ||
216 | 571 | related_units.return_value = ['test/1'] | ||
217 | 572 | relation_ids.return_value = ['test:0'] | ||
218 | 573 | self.assertFalse(hookenv.is_relation_made('test', | ||
219 | 574 | keys=['auth', 'host'])) | ||
220 | 575 | relation_get.assert_has_calls( | ||
221 | 576 | [call('auth', rid='test:0', unit='test/1'), | ||
222 | 577 | call('host', rid='test:0', unit='test/1')] | ||
223 | 578 | ) | ||
224 | 579 | |||
225 | 501 | @patch('charmhelpers.core.hookenv.config') | 580 | @patch('charmhelpers.core.hookenv.config') |
226 | 502 | @patch('charmhelpers.core.hookenv.relation_type') | 581 | @patch('charmhelpers.core.hookenv.relation_type') |
227 | 503 | @patch('charmhelpers.core.hookenv.local_unit') | 582 | @patch('charmhelpers.core.hookenv.local_unit') |
228 | 504 | 583 | ||
229 | === modified file 'tests/fetch/test_bzrurl.py' | |||
230 | --- tests/fetch/test_bzrurl.py 2013-08-21 11:45:37 +0000 | |||
231 | +++ tests/fetch/test_bzrurl.py 2013-10-21 22:54:42 +0000 | |||
232 | @@ -41,7 +41,6 @@ | |||
233 | 41 | ) | 41 | ) |
234 | 42 | self.fh = bzrurl.BzrUrlFetchHandler() | 42 | self.fh = bzrurl.BzrUrlFetchHandler() |
235 | 43 | 43 | ||
236 | 44 | |||
237 | 45 | def test_handles_bzr_urls(self): | 44 | def test_handles_bzr_urls(self): |
238 | 46 | for url in self.valid_urls: | 45 | for url in self.valid_urls: |
239 | 47 | result = self.fh.can_handle(url) | 46 | result = self.fh.can_handle(url) |
240 | @@ -50,7 +49,6 @@ | |||
241 | 50 | result = self.fh.can_handle(url) | 49 | result = self.fh.can_handle(url) |
242 | 51 | self.assertNotEqual(result, True, url) | 50 | self.assertNotEqual(result, True, url) |
243 | 52 | 51 | ||
244 | 53 | |||
245 | 54 | @patch('bzrlib.branch.Branch.open') | 52 | @patch('bzrlib.branch.Branch.open') |
246 | 55 | def test_branch(self, _open): | 53 | def test_branch(self, _open): |
247 | 56 | dest_path = "/destination/path" | 54 | dest_path = "/destination/path" |
248 | @@ -65,7 +63,6 @@ | |||
249 | 65 | with patch.dict('os.environ', {'CHARM_DIR': 'foo'}): | 63 | with patch.dict('os.environ', {'CHARM_DIR': 'foo'}): |
250 | 66 | self.assertRaises(UnhandledSource, self.fh.branch, url, dest_path) | 64 | self.assertRaises(UnhandledSource, self.fh.branch, url, dest_path) |
251 | 67 | 65 | ||
252 | 68 | |||
253 | 69 | @patch('charmhelpers.fetch.bzrurl.mkdir') | 66 | @patch('charmhelpers.fetch.bzrurl.mkdir') |
254 | 70 | def test_installs(self, _mkdir): | 67 | def test_installs(self, _mkdir): |
255 | 71 | self.fh.branch = MagicMock() | 68 | self.fh.branch = MagicMock() |
256 | @@ -77,4 +74,3 @@ | |||
257 | 77 | where = self.fh.install(url) | 74 | where = self.fh.install(url) |
258 | 78 | self.assertEqual(where, dest) | 75 | self.assertEqual(where, dest) |
259 | 79 | _mkdir.assert_called_with(where, perms=0755) | 76 | _mkdir.assert_called_with(where, perms=0755) |
260 | 80 |
I also tidied the majority of lint in the project;
make lint is a good check when reviewing merge proposals; don't think everyone has been doing that!