Merge ~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm:master into ~livepatch-charmers/charm-canonical-livepatch:master
- Git
- lp:~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm
- master
- Merge into master
Proposed by
Barry Price
Status: | Merged |
---|---|
Merged at revision: | d3c18df3d4a7d750a49a5cbaa8efd54597cc5cc0 |
Proposed branch: | ~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm:master |
Merge into: | ~livepatch-charmers/charm-canonical-livepatch:master |
Diff against target: |
309 lines (+97/-38) 3 files modified
files/check_canonical-livepatch.py (+8/-6) reactive/canonical_livepatch.py (+63/-23) tests/99-autogen (+26/-9) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Stuart Bishop (community) | Approve | ||
Review via email: mp+316306@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/files/check_canonical-livepatch.py b/files/check_canonical-livepatch.py |
2 | index 27c9f69..74c61e5 100644 |
3 | --- a/files/check_canonical-livepatch.py |
4 | +++ b/files/check_canonical-livepatch.py |
5 | @@ -7,17 +7,19 @@ import nagios_plugin |
6 | from subprocess import check_output |
7 | |
8 | |
9 | -################################################################################ |
10 | +############################################################################## |
11 | |
12 | def check_package_installed(): |
13 | cmd = ['snap', 'list', 'canonical-livepatch'] |
14 | try: |
15 | check_output(cmd, universal_newlines=True) |
16 | except: |
17 | - raise nagios_plugin.CriticalError("canonical-livepatch snap is not installed") |
18 | + raise nagios_plugin.CriticalError( |
19 | + "canonical-livepatch snap is not installed" |
20 | + ) |
21 | |
22 | |
23 | -################################################################################ |
24 | +############################################################################## |
25 | |
26 | def check_vmlinuz(): |
27 | vmlinuz = '/vmlinuz' |
28 | @@ -39,7 +41,7 @@ def check_vmlinuz(): |
29 | return kernel_version.strip() |
30 | |
31 | |
32 | -################################################################################ |
33 | +############################################################################## |
34 | |
35 | def check_status(): |
36 | livepatch_output_path = '/var/lib/nagios/canonical-livepatch-status.txt' |
37 | @@ -67,7 +69,7 @@ def check_status(): |
38 | raise nagios_plugin.WarnError(wrn) |
39 | |
40 | |
41 | -################################################################################ |
42 | +############################################################################## |
43 | |
44 | def main(): |
45 | nagios_plugin.try_check(check_package_installed) |
46 | @@ -75,7 +77,7 @@ def main(): |
47 | print("OK - canonical-livepatch seems to be installed and working") |
48 | |
49 | |
50 | -################################################################################ |
51 | +############################################################################## |
52 | |
53 | if __name__ == "__main__": |
54 | main() |
55 | diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py |
56 | index 14f75ec..e511669 100644 |
57 | --- a/reactive/canonical_livepatch.py |
58 | +++ b/reactive/canonical_livepatch.py |
59 | @@ -26,11 +26,17 @@ def file_to_units(local_path, unit_path): |
60 | |
61 | |
62 | def wait_for_path(file_path, timeout=30): |
63 | - hookenv.log('Waiting for up to {}s for {}'.format(timeout, file_path)) |
64 | + hookenv.log('Waiting for up to {}s for {}'.format( |
65 | + timeout, |
66 | + file_path) |
67 | + ) |
68 | seconds_waited = 0 |
69 | interval = 1 |
70 | while not (path.exists(file_path)): |
71 | - hookenv.log('{} does not exist - waiting {}s'.format(file_path, interval)) |
72 | + hookenv.log('{} does not exist - waiting {}s'.format( |
73 | + file_path, |
74 | + interval) |
75 | + ) |
76 | sleep(interval) |
77 | seconds_waited += interval |
78 | if seconds_waited >= timeout: |
79 | @@ -40,14 +46,18 @@ def wait_for_path(file_path, timeout=30): |
80 | |
81 | |
82 | def wait_for_livepatch(): |
83 | - wait_for_path('/var/snap/canonical-livepatch/current/livepatchd-priv.sock') |
84 | + wait_for_path( |
85 | + '/var/snap/canonical-livepatch/current/livepatchd-priv.sock' |
86 | + ) |
87 | |
88 | |
89 | def unit_update(status=None, message=None): |
90 | if status and message: |
91 | hookenv.status_set(status, message) |
92 | else: |
93 | - hookenv.status_set('active', 'Effective kernel {}'.format(get_equiv_kernel_version())) |
94 | + hookenv.status_set('active', 'Effective kernel {}'.format( |
95 | + get_equiv_kernel_version()) |
96 | + ) |
97 | |
98 | |
99 | def get_equiv_kernel_version(): |
100 | @@ -78,18 +88,25 @@ def get_equiv_kernel_version(): |
101 | return version_string |
102 | |
103 | |
104 | -def configure_proxies(http_proxy=None, https_proxy=None, no_proxy=None): |
105 | - config_path = '/var/snap/canonical-livepatch/common/config' |
106 | - |
107 | - if path.exists(config_path): |
108 | - with open(config_path, 'r') as stream: |
109 | +def get_yaml_if_exists(path_to_yaml): |
110 | + if path.exists(path_to_yaml): |
111 | + with open(path_to_yaml, 'r') as stream: |
112 | try: |
113 | conf_yaml = load(stream) |
114 | - except: |
115 | - hookenv.log('Unable to load YAML from {}'.format(config_path)) |
116 | + except Exception: |
117 | + hookenv.log( |
118 | + 'Unable to load YAML from {}'.format(path_to_yaml) |
119 | + ) |
120 | else: |
121 | conf_yaml = {} |
122 | |
123 | + return conf_yaml |
124 | + |
125 | + |
126 | +def configure_proxies(http_proxy=None, https_proxy=None, no_proxy=None): |
127 | + config_path = '/var/snap/canonical-livepatch/common/config' |
128 | + conf_yaml = get_yaml_if_exists(config_path) |
129 | + |
130 | if conf_yaml.get('http-proxy'): |
131 | if http_proxy is None: |
132 | del conf_yaml['http-proxy'] |
133 | @@ -120,7 +137,11 @@ def configure_proxies(http_proxy=None, https_proxy=None, no_proxy=None): |
134 | |
135 | def restart_livepatch(): |
136 | # do a clean stop of the service first, 'restart' seems fragile right now |
137 | - cmd = ['/usr/sbin/service', 'snap.canonical-livepatch.canonical-livepatchd', 'stop'] |
138 | + cmd = [ |
139 | + '/usr/sbin/service', |
140 | + 'snap.canonical-livepatch.canonical-livepatchd', |
141 | + 'stop' |
142 | + ] |
143 | hookenv.log('Stopping canonical-livepatch service') |
144 | try: |
145 | check_call(cmd, universal_newlines=True) |
146 | @@ -128,12 +149,18 @@ def restart_livepatch(): |
147 | hookenv.log('Failed to stop the service') |
148 | |
149 | # and now try to start it again, it may fail the first time! |
150 | - cmd = ['/usr/sbin/service', 'snap.canonical-livepatch.canonical-livepatchd', 'start'] |
151 | + cmd = [ |
152 | + '/usr/sbin/service', |
153 | + 'snap.canonical-livepatch.canonical-livepatchd', |
154 | + 'start' |
155 | + ] |
156 | hookenv.log('Starting canonical-livepatch service') |
157 | try: |
158 | check_call(cmd, universal_newlines=True) |
159 | except CalledProcessError: |
160 | - hookenv.log('Failed to start the service - waiting 5s and then trying again') |
161 | + hookenv.log( |
162 | + 'Failed to start the service - waiting 5s and then trying again' |
163 | + ) |
164 | sleep(5) |
165 | check_call(cmd, universal_newlines=True) |
166 | |
167 | @@ -158,13 +185,20 @@ def update_key(): |
168 | if livepatch_key: |
169 | # disable prior to enabling to work around LP#1628823 |
170 | cmd = ['/snap/bin/canonical-livepatch', 'disable'] |
171 | - hookenv.log('Disabling canonical-livepatch ahead of key change to work around LP#1628823') |
172 | + hookenv.log( |
173 | + 'Disabling canonical-livepatch ahead of key change ' |
174 | + + 'to work around LP#1628823' |
175 | + ) |
176 | try: |
177 | check_output(cmd, universal_newlines=True) |
178 | except CalledProcessError as e: |
179 | hookenv.log('Unable to deactivate: {}'.format(str(e))) |
180 | |
181 | - cmd = ['/snap/bin/canonical-livepatch', 'enable', '{}'.format(livepatch_key.strip())] |
182 | + cmd = [ |
183 | + '/snap/bin/canonical-livepatch', |
184 | + 'enable', |
185 | + '{}'.format(livepatch_key.strip()) |
186 | + ] |
187 | hookenv.log('Activating canonical-livepatch with your key') |
188 | try: |
189 | check_output(cmd, universal_newlines=True) |
190 | @@ -176,9 +210,14 @@ def update_key(): |
191 | set_state('canonical-livepatch.active') |
192 | unit_update() |
193 | else: |
194 | - hookenv.log('Unable to activate canonical-livepatch as no key has been set') |
195 | + hookenv.log( |
196 | + 'Unable to activate canonical-livepatch as no key has been set' |
197 | + ) |
198 | remove_state('canonical-livepatch.active') |
199 | - unit_update('blocked', 'Service disabled, please set livepatch_key to activate') |
200 | + unit_update( |
201 | + 'blocked', |
202 | + 'Service disabled, please set livepatch_key to activate' |
203 | + ) |
204 | |
205 | |
206 | @when('canonical-livepatch.connected') |
207 | @@ -195,7 +234,7 @@ def update_livepatch_proxy(): |
208 | # Make sure the service is ready for us |
209 | wait_for_livepatch() |
210 | |
211 | - # force a key update, registration may have been blocked before proxy was set |
212 | + # force a key update, we may have been blocked before proxy was set |
213 | update_key() |
214 | |
215 | |
216 | @@ -224,10 +263,11 @@ def init_nagios_checks(nagios): |
217 | ) |
218 | |
219 | # use charmhelpers to create the check |
220 | - nrpe_setup.add_check('check_canonical-livepatch', |
221 | - 'Verify canonical-livepatch is working', |
222 | - '/usr/lib/nagios/plugins/check_canonical-livepatch.py' |
223 | - ) |
224 | + nrpe_setup.add_check( |
225 | + 'check_canonical-livepatch', |
226 | + 'Verify canonical-livepatch is working', |
227 | + '/usr/lib/nagios/plugins/check_canonical-livepatch.py' |
228 | + ) |
229 | |
230 | nrpe_setup.write() |
231 | |
232 | diff --git a/tests/99-autogen b/tests/99-autogen |
233 | index c57bbf4..05824f0 100755 |
234 | --- a/tests/99-autogen |
235 | +++ b/tests/99-autogen |
236 | @@ -20,17 +20,29 @@ class TestDeployment(unittest.TestCase): |
237 | cls.deployment.add('nrpe') |
238 | |
239 | # relate subordinates to parent charm |
240 | - cls.deployment.relate('postgresql:juju-info', 'canonical-livepatch:juju-info') |
241 | - cls.deployment.relate('postgresql:nrpe-external-master', 'nrpe:nrpe-external-master') |
242 | + cls.deployment.relate( |
243 | + 'postgresql:juju-info', |
244 | + 'canonical-livepatch:juju-info' |
245 | + ) |
246 | + cls.deployment.relate( |
247 | + 'postgresql:nrpe-external-master', |
248 | + 'nrpe:nrpe-external-master' |
249 | + ) |
250 | |
251 | # relate livepatch to nrpe for its own nagios checks |
252 | - cls.deployment.relate('canonical-livepatch:nrpe-external-master', 'nrpe:nrpe-external-master') |
253 | + cls.deployment.relate( |
254 | + 'canonical-livepatch:nrpe-external-master', |
255 | + 'nrpe:nrpe-external-master' |
256 | + ) |
257 | |
258 | try: |
259 | cls.deployment.setup(timeout=3600) |
260 | cls.deployment.sentry.wait() |
261 | except amulet.helpers.TimeoutError: |
262 | - amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time") |
263 | + amulet.raise_status( |
264 | + amulet.SKIP, |
265 | + msg="Environment wasn't stood up in time" |
266 | + ) |
267 | except: |
268 | raise |
269 | |
270 | @@ -38,13 +50,15 @@ class TestDeployment(unittest.TestCase): |
271 | livepatch = self.deployment.sentry['canonical-livepatch'][0] |
272 | |
273 | # verify the snap was installed |
274 | - output, exit_code = livepatch.run('stat /snap/bin/canonical-livepatch') |
275 | + output, exit_code = livepatch.run( |
276 | + 'stat /snap/bin/canonical-livepatch' |
277 | + ) |
278 | self.assertEqual(exit_code, 0) |
279 | |
280 | def test_status(self): |
281 | livepatch = self.deployment.sentry['canonical-livepatch'][0] |
282 | |
283 | - # run a status check - we expect this to return code 1 due to no access key |
284 | + # run a status check - we expect this to return 1 due to no access key |
285 | output, exit_code = livepatch.run('sudo canonical-livepatch status') |
286 | self.assertEqual(exit_code, 1) |
287 | |
288 | @@ -75,8 +89,9 @@ class TestDeployment(unittest.TestCase): |
289 | |
290 | # confirm it showed up in the right place |
291 | output, exit_code = livepatch.run( |
292 | - 'grep {} /var/lib/nagios/export/service__*_check_canonical-livepatch.cfg'.format(test_context_name) |
293 | - ) |
294 | + 'grep {} /var/lib/nagios/export/' |
295 | + + 'service__*_check_canonical-livepatch' |
296 | + + '.cfg'.format(test_context_name)) |
297 | self.assertEqual(exit_code, 0) |
298 | |
299 | def test_nagios_servicegroup_change(self): |
300 | @@ -94,7 +109,9 @@ class TestDeployment(unittest.TestCase): |
301 | |
302 | # confirm it showed up in the right place |
303 | output, exit_code = livepatch.run( |
304 | - 'grep {} /var/lib/nagios/export/service__*_check_canonical-livepatch.cfg'.format(test_servicegroup_name) |
305 | + 'grep {} /var/lib/nagios/export/' |
306 | + + 'service__*_check_canonical-livepatch' |
307 | + + '.cfg'.format(test_servicegroup_name) |
308 | ) |
309 | self.assertEqual(exit_code, 0) |
310 |
Looks like a pile of trivial lint fixes to me.
+1