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: | db28bb9e14e9af53c98ab41669bf2703f21ea488 |
Proposed branch: | ~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm:master |
Merge into: | ~livepatch-charmers/charm-canonical-livepatch:master |
Diff against target: |
228 lines (+26/-89) 3 files modified
Makefile (+4/-0) actions/actions.py (+1/-5) reactive/canonical_livepatch.py (+21/-84) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Stuart Bishop (community) | Approve | ||
Review via email:
|
Commit message
Reformat to 120 character lines
Description of the change
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tom Haddon (mthaddon) wrote : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/Makefile b/Makefile | |||
2 | 0 | new file mode 100644 | 0 | new file mode 100644 |
3 | index 0000000..07b57c9 | |||
4 | --- /dev/null | |||
5 | +++ b/Makefile | |||
6 | @@ -0,0 +1,4 @@ | |||
7 | 1 | #!/usr/bin/make | ||
8 | 2 | |||
9 | 3 | lint: | ||
10 | 4 | @flake8 --max-line-length=120 | ||
11 | diff --git a/actions/actions.py b/actions/actions.py | |||
12 | index e9bb530..d618758 100755 | |||
13 | --- a/actions/actions.py | |||
14 | +++ b/actions/actions.py | |||
15 | @@ -19,11 +19,7 @@ def activate(): | |||
16 | 19 | except CalledProcessError as e: | 19 | except CalledProcessError as e: |
17 | 20 | hookenv.log('Unable to deactivate: {}'.format(str(e))) | 20 | hookenv.log('Unable to deactivate: {}'.format(str(e))) |
18 | 21 | # but let's soldier on... | 21 | # but let's soldier on... |
24 | 22 | cmd = [ | 22 | cmd = ['/snap/bin/canonical-livepatch', 'enable', '{}'.format(livepatch_key.strip())] |
20 | 23 | '/snap/bin/canonical-livepatch', | ||
21 | 24 | 'enable', | ||
22 | 25 | '{}'.format(livepatch_key.strip()) | ||
23 | 26 | ] | ||
25 | 27 | try: | 23 | try: |
26 | 28 | check_output(cmd, universal_newlines=True) | 24 | check_output(cmd, universal_newlines=True) |
27 | 29 | except CalledProcessError as e: | 25 | except CalledProcessError as e: |
28 | diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py | |||
29 | index 8b39eab..e176c45 100644 | |||
30 | --- a/reactive/canonical_livepatch.py | |||
31 | +++ b/reactive/canonical_livepatch.py | |||
32 | @@ -27,32 +27,21 @@ def file_to_units(local_path, unit_path): | |||
33 | 27 | 27 | ||
34 | 28 | 28 | ||
35 | 29 | def wait_for_path(file_path, timeout=30): | 29 | def wait_for_path(file_path, timeout=30): |
40 | 30 | hookenv.log('Waiting for up to {}s for {}'.format( | 30 | hookenv.log('Waiting for up to {}s for {}'.format(timeout, file_path)) |
37 | 31 | timeout, | ||
38 | 32 | file_path) | ||
39 | 33 | ) | ||
41 | 34 | seconds_waited = 0 | 31 | seconds_waited = 0 |
42 | 35 | interval = 1 | 32 | interval = 1 |
43 | 36 | while not (path.exists(file_path)): | 33 | while not (path.exists(file_path)): |
48 | 37 | hookenv.log('{} does not exist - waiting {}s'.format( | 34 | hookenv.log('{} does not exist - waiting {}s'.format(file_path, interval)) |
45 | 38 | file_path, | ||
46 | 39 | interval) | ||
47 | 40 | ) | ||
49 | 41 | sleep(interval) | 35 | sleep(interval) |
50 | 42 | seconds_waited += interval | 36 | seconds_waited += interval |
51 | 43 | if seconds_waited >= timeout: | 37 | if seconds_waited >= timeout: |
56 | 44 | hookenv.log( | 38 | hookenv.log('Giving up waiting for {}'.format(file_path), hookenv.ERROR) |
53 | 45 | 'Giving up waiting for {}'.format(file_path), | ||
54 | 46 | hookenv.ERROR | ||
55 | 47 | ) | ||
57 | 48 | return | 39 | return |
58 | 49 | return | 40 | return |
59 | 50 | 41 | ||
60 | 51 | 42 | ||
61 | 52 | def wait_for_livepatch(): | 43 | def wait_for_livepatch(): |
65 | 53 | wait_for_path( | 44 | wait_for_path('/var/snap/canonical-livepatch/current/livepatchd-priv.sock') |
63 | 54 | '/var/snap/canonical-livepatch/current/livepatchd-priv.sock' | ||
64 | 55 | ) | ||
66 | 56 | 45 | ||
67 | 57 | 46 | ||
68 | 58 | def unit_update(status=None, message=None): | 47 | def unit_update(status=None, message=None): |
69 | @@ -60,10 +49,7 @@ def unit_update(status=None, message=None): | |||
70 | 60 | hookenv.status_set(status, message) | 49 | hookenv.status_set(status, message) |
71 | 61 | else: | 50 | else: |
72 | 62 | patch_details = get_patch_details() | 51 | patch_details = get_patch_details() |
77 | 63 | hookenv.status_set( | 52 | hookenv.status_set('active', 'Running kernel {}, patchState: {}'.format(*patch_details)) |
74 | 64 | 'active', | ||
75 | 65 | 'Running kernel {}, patchState: {}'.format(*patch_details) | ||
76 | 66 | ) | ||
78 | 67 | 53 | ||
79 | 68 | 54 | ||
80 | 69 | def get_patch_details(): | 55 | def get_patch_details(): |
81 | @@ -90,10 +76,7 @@ def get_patch_details(): | |||
82 | 90 | kernel = k['kernel'] | 76 | kernel = k['kernel'] |
83 | 91 | patch_state = k['livepatch']['patchState'] | 77 | patch_state = k['livepatch']['patchState'] |
84 | 92 | except Exception: | 78 | except Exception: |
89 | 93 | hookenv.log( | 79 | hookenv.log('Unable to find patching details in status yaml', hookenv.ERROR) |
86 | 94 | 'Unable to find patching details in status yaml', | ||
87 | 95 | hookenv.ERROR | ||
88 | 96 | ) | ||
90 | 97 | return kernel, patch_state | 80 | return kernel, patch_state |
91 | 98 | 81 | ||
92 | 99 | return kernel, patch_state | 82 | return kernel, patch_state |
93 | @@ -105,10 +88,7 @@ def get_yaml_if_exists(path_to_yaml): | |||
94 | 105 | try: | 88 | try: |
95 | 106 | conf_yaml = load(stream) | 89 | conf_yaml = load(stream) |
96 | 107 | except Exception: | 90 | except Exception: |
101 | 108 | hookenv.log( | 91 | hookenv.log('Unable to load YAML from {}'.format(path_to_yaml), hookenv.ERROR) |
98 | 109 | 'Unable to load YAML from {}'.format(path_to_yaml), | ||
99 | 110 | hookenv.ERROR | ||
100 | 111 | ) | ||
102 | 112 | else: | 92 | else: |
103 | 113 | conf_yaml = {} | 93 | conf_yaml = {} |
104 | 114 | 94 | ||
105 | @@ -149,11 +129,7 @@ def configure_proxies(http_proxy=None, https_proxy=None, no_proxy=None): | |||
106 | 149 | 129 | ||
107 | 150 | def restart_livepatch(): | 130 | def restart_livepatch(): |
108 | 151 | # do a clean stop of the service first, 'restart' seems fragile right now | 131 | # do a clean stop of the service first, 'restart' seems fragile right now |
114 | 152 | cmd = [ | 132 | cmd = ['/bin/systemctl', 'stop', 'snap.canonical-livepatch.canonical-livepatchd.service'] |
110 | 153 | '/bin/systemctl', | ||
111 | 154 | 'stop', | ||
112 | 155 | 'snap.canonical-livepatch.canonical-livepatchd.service', | ||
113 | 156 | ] | ||
115 | 157 | hookenv.log('Stopping canonical-livepatch service') | 133 | hookenv.log('Stopping canonical-livepatch service') |
116 | 158 | try: | 134 | try: |
117 | 159 | check_call(cmd, universal_newlines=True) | 135 | check_call(cmd, universal_newlines=True) |
118 | @@ -161,19 +137,12 @@ def restart_livepatch(): | |||
119 | 161 | hookenv.log('Failed to stop the service', hookenv.ERROR) | 137 | hookenv.log('Failed to stop the service', hookenv.ERROR) |
120 | 162 | 138 | ||
121 | 163 | # and now try to start it again, it may fail the first time! | 139 | # and now try to start it again, it may fail the first time! |
127 | 164 | cmd = [ | 140 | cmd = ['/bin/systemctl', 'start', 'snap.canonical-livepatch.canonical-livepatchd.service'] |
123 | 165 | '/bin/systemctl', | ||
124 | 166 | 'start', | ||
125 | 167 | 'snap.canonical-livepatch.canonical-livepatchd.service', | ||
126 | 168 | ] | ||
128 | 169 | hookenv.log('Starting canonical-livepatch service') | 141 | hookenv.log('Starting canonical-livepatch service') |
129 | 170 | try: | 142 | try: |
130 | 171 | check_call(cmd, universal_newlines=True) | 143 | check_call(cmd, universal_newlines=True) |
131 | 172 | except CalledProcessError: | 144 | except CalledProcessError: |
136 | 173 | hookenv.log( | 145 | hookenv.log('Failed to start the service - waiting 5s and then trying again', hookenv.ERROR) |
133 | 174 | 'Failed to start the service - waiting 5s and then trying again', | ||
134 | 175 | hookenv.ERROR | ||
135 | 176 | ) | ||
137 | 177 | sleep(5) | 146 | sleep(5) |
138 | 178 | check_call(cmd, universal_newlines=True) | 147 | check_call(cmd, universal_newlines=True) |
139 | 179 | 148 | ||
140 | @@ -186,66 +155,37 @@ def activate_livepatch(): | |||
141 | 186 | if livepatch_key: | 155 | if livepatch_key: |
142 | 187 | # disable prior to enabling to work around LP#1628823 | 156 | # disable prior to enabling to work around LP#1628823 |
143 | 188 | cmd = ['/snap/bin/canonical-livepatch', 'disable'] | 157 | cmd = ['/snap/bin/canonical-livepatch', 'disable'] |
148 | 189 | hookenv.log( | 158 | hookenv.log('Disabling canonical-livepatch ahead of key change to work around LP#1628823') |
145 | 190 | 'Disabling canonical-livepatch ahead of key change ' | ||
146 | 191 | 'to work around LP#1628823' | ||
147 | 192 | ) | ||
149 | 193 | try: | 159 | try: |
150 | 194 | check_output(cmd, universal_newlines=True) | 160 | check_output(cmd, universal_newlines=True) |
151 | 195 | except CalledProcessError as e: | 161 | except CalledProcessError as e: |
162 | 196 | hookenv.log( | 162 | hookenv.log('Unable to deactivate: {}'.format(str(e)), hookenv.ERROR) |
163 | 197 | 'Unable to deactivate: {}'.format(str(e)), | 163 | |
164 | 198 | hookenv.ERROR | 164 | cmd = ['/snap/bin/canonical-livepatch', 'enable', '{}'.format(livepatch_key.strip())] |
155 | 199 | ) | ||
156 | 200 | |||
157 | 201 | cmd = [ | ||
158 | 202 | '/snap/bin/canonical-livepatch', | ||
159 | 203 | 'enable', | ||
160 | 204 | '{}'.format(livepatch_key.strip()) | ||
161 | 205 | ] | ||
165 | 206 | hookenv.log('Activating canonical-livepatch with your key') | 165 | hookenv.log('Activating canonical-livepatch with your key') |
166 | 207 | try: | 166 | try: |
167 | 208 | check_output(cmd, universal_newlines=True) | 167 | check_output(cmd, universal_newlines=True) |
168 | 209 | except CalledProcessError as e: | 168 | except CalledProcessError as e: |
173 | 210 | hookenv.log( | 169 | hookenv.log('Unable to activate: {}'.format(str(e)), hookenv.ERROR) |
170 | 211 | 'Unable to activate: {}'.format(str(e)), | ||
171 | 212 | hookenv.ERROR | ||
172 | 213 | ) | ||
174 | 214 | remove_state('canonical-livepatch.active') | 170 | remove_state('canonical-livepatch.active') |
175 | 215 | unit_update('blocked', 'Activation failed') | 171 | unit_update('blocked', 'Activation failed') |
176 | 216 | else: | 172 | else: |
177 | 217 | set_state('canonical-livepatch.active') | 173 | set_state('canonical-livepatch.active') |
178 | 218 | unit_update() | 174 | unit_update() |
179 | 219 | else: | 175 | else: |
184 | 220 | hookenv.log( | 176 | hookenv.log('Unable to activate canonical-livepatch as no key has been set', hookenv.ERROR) |
181 | 221 | 'Unable to activate canonical-livepatch as no key has been set', | ||
182 | 222 | hookenv.ERROR | ||
183 | 223 | ) | ||
185 | 224 | remove_state('canonical-livepatch.active') | 177 | remove_state('canonical-livepatch.active') |
190 | 225 | unit_update( | 178 | unit_update('blocked', 'Service disabled, please set livepatch_key to activate') |
187 | 226 | 'blocked', | ||
188 | 227 | 'Service disabled, please set livepatch_key to activate' | ||
189 | 228 | ) | ||
191 | 229 | 179 | ||
192 | 230 | 180 | ||
193 | 231 | @when_not('snap.installed.canonical-livepatch') | 181 | @when_not('snap.installed.canonical-livepatch') |
194 | 232 | def livepatch_supported(): | 182 | def livepatch_supported(): |
195 | 233 | arch = uname()[4] | 183 | arch = uname()[4] |
196 | 234 | opts = layer.options('snap') | 184 | opts = layer.options('snap') |
201 | 235 | supported_archs = opts['canonical-livepatch'].pop( | 185 | supported_archs = opts['canonical-livepatch'].pop('supported-architectures', None) |
198 | 236 | 'supported-architectures', | ||
199 | 237 | None | ||
200 | 238 | ) | ||
202 | 239 | if supported_archs and arch not in supported_archs: | 186 | if supported_archs and arch not in supported_archs: |
212 | 240 | hookenv.log( | 187 | hookenv.log('Livepatch does not currently support {} architecture'.format(arch)) |
213 | 241 | 'Livepatch does not currently support {} architecture'.format( | 188 | unit_update('blocked', 'Architecture {} is not supported by livepatch'.format(arch)) |
205 | 242 | arch | ||
206 | 243 | ) | ||
207 | 244 | ) | ||
208 | 245 | unit_update( | ||
209 | 246 | 'blocked', | ||
210 | 247 | 'Architecture {} is not supported by livepatch'.format(arch) | ||
211 | 248 | ) | ||
214 | 249 | if is_container(): | 189 | if is_container(): |
215 | 250 | hookenv.log('OS container detected, livepatch is not needed') | 190 | hookenv.log('OS container detected, livepatch is not needed') |
216 | 251 | unit_update('blocked', 'Livepatch is not needed in OS containers') | 191 | unit_update('blocked', 'Livepatch is not needed in OS containers') |
217 | @@ -270,10 +210,7 @@ def canonical_livepatch_connect(): | |||
218 | 270 | # Make sure the service is ready for us | 210 | # Make sure the service is ready for us |
219 | 271 | wait_for_livepatch() | 211 | wait_for_livepatch() |
220 | 272 | set_state('canonical-livepatch.connected') | 212 | set_state('canonical-livepatch.connected') |
225 | 273 | unit_update( | 213 | unit_update('blocked', 'Service disabled, please set livepatch_key to activate') |
222 | 274 | 'blocked', | ||
223 | 275 | 'Service disabled, please set livepatch_key to activate' | ||
224 | 276 | ) | ||
226 | 277 | 214 | ||
227 | 278 | 215 | ||
228 | 279 | @when('canonical-livepatch.connected') | 216 | @when('canonical-livepatch.connected') |
This looks fine, but I think it'd be worth encoding the 120 character line limit with a "make lint"