Merge lp:~javier.collado/utah/retry-timeout into lp:utah
- retry-timeout
- Merge into dev
Status: | Merged |
---|---|
Merged at revision: | 833 |
Proposed branch: | lp:~javier.collado/utah/retry-timeout |
Merge into: | lp:utah |
Diff against target: |
264 lines (+34/-55) 9 files modified
utah/client/runner.py (+0/-3) utah/config.py (+4/-1) utah/exceptions.py (+1/-4) utah/provisioning/baremetal/bamboofeeder.py (+2/-1) utah/provisioning/baremetal/cobbler.py (+2/-2) utah/provisioning/provisioning.py (+3/-17) utah/provisioning/ssh.py (+3/-15) utah/retry.py (+19/-9) utah/url.py (+0/-3) |
To merge this branch: | bzr merge lp:~javier.collado/utah/retry-timeout |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Max Brustkern (community) | Approve | ||
Review via email: mp+153198@code.launchpad.net |
Commit message
Description of the change
This branch moves the different time.sleep calls in methods like `pingcheck`
and `sshcheck` to the `retry` function that is already wrapping them.
Before this change `retry` called the callable passed as argument as soon as a
retriable exception was caught and was the responsibiliy of the callable to
call `time.sleep` if needed. With this change, the `retry` takes care of that
and uses a default value from the configuration file so that callables only
need to take care of raising an exception when they fail.
Max Brustkern (nuclearbob) wrote : | # |
Max Brustkern (nuclearbob) wrote : | # |
The actual changes look sound. I think at some point when we overhaul config, we can address the circular import problem.
Andy Doan (doanac) wrote : | # |
these are real merge conflicts. minus those, i like the changes.
- 832. By Javier Collado
-
Merged changes to fix battery behavior with regard to logging
Source branch: lp:~javier.collado/utah/battery-measurements-logging
Javier Collado (javier.collado) wrote : | # |
I've rebased the changes and the merge conflicts are not fixed.
I've run a test and everything worked fine, but I believe the code wasn't
really exercised because rsyslog implementation does a good work in monitoring
the reboot after the installation so the first ping command doesn't fail in VMs
as it used to do in the past.
Anyway, since I tested already before the rebasing, I don't expect any
problem.
Max Brustkern (nuclearbob) wrote : | # |
Looks reasonable to me, if you're already tested.
Preview Diff
1 | === modified file 'utah/client/runner.py' | |||
2 | --- utah/client/runner.py 2013-03-11 09:17:54 +0000 | |||
3 | +++ utah/client/runner.py 2013-03-14 17:40:27 +0000 | |||
4 | @@ -31,7 +31,6 @@ | |||
5 | 31 | import stat | 31 | import stat |
6 | 32 | import urllib | 32 | import urllib |
7 | 33 | import jsonschema | 33 | import jsonschema |
8 | 34 | import time | ||
9 | 35 | 34 | ||
10 | 36 | from utah.client.common import ( | 35 | from utah.client.common import ( |
11 | 37 | MASTER_RUNLIST, | 36 | MASTER_RUNLIST, |
12 | @@ -472,8 +471,6 @@ | |||
13 | 472 | if (isinstance(vcs_handler, BzrHandler) and | 471 | if (isinstance(vcs_handler, BzrHandler) and |
14 | 473 | result['returncode'] == 3 and | 472 | result['returncode'] == 3 and |
15 | 474 | 'Unable to handle http code 502' in result['stderr']): | 473 | 'Unable to handle http code 502' in result['stderr']): |
16 | 475 | # Separate every retry attempt with a timeout of 3 seconds | ||
17 | 476 | time.sleep(3) | ||
18 | 477 | raise UTAHException('Launchpad temporary error detected', | 474 | raise UTAHException('Launchpad temporary error detected', |
19 | 478 | retry=True) | 475 | retry=True) |
20 | 479 | return result | 476 | return result |
21 | 480 | 477 | ||
22 | === modified file 'utah/config.py' | |||
23 | --- utah/config.py 2013-03-12 16:10:49 +0000 | |||
24 | +++ utah/config.py 2013-03-14 17:40:27 +0000 | |||
25 | @@ -186,7 +186,10 @@ | |||
26 | 186 | 'pattern': '.*NetworkManager', | 186 | 'pattern': '.*NetworkManager', |
27 | 187 | 'timeout': 180, | 187 | 'timeout': 180, |
28 | 188 | } | 188 | } |
30 | 189 | ] | 189 | ], |
31 | 190 | |||
32 | 191 | # default timeout between retry attempts | ||
33 | 192 | retry_timeout=3, | ||
34 | 190 | ) | 193 | ) |
35 | 191 | 194 | ||
36 | 192 | # These depend on the local user/path, and need to be filtered out | 195 | # These depend on the local user/path, and need to be filtered out |
37 | 193 | 196 | ||
38 | === modified file 'utah/exceptions.py' | |||
39 | --- utah/exceptions.py 2012-12-03 14:02:18 +0000 | |||
40 | +++ utah/exceptions.py 2013-03-14 17:40:27 +0000 | |||
41 | @@ -24,8 +24,5 @@ | |||
42 | 24 | Support retry argument, but default to False. | 24 | Support retry argument, but default to False. |
43 | 25 | """ | 25 | """ |
44 | 26 | def __init__(self, *args, **kw): | 26 | def __init__(self, *args, **kw): |
49 | 27 | try: | 27 | self.retry = kw.pop('retry', False) |
46 | 28 | self.retry = kw.pop('retry') | ||
47 | 29 | except KeyError: | ||
48 | 30 | self.retry = False | ||
50 | 31 | super(UTAHException, self).__init__(*args, **kw) | 28 | super(UTAHException, self).__init__(*args, **kw) |
51 | 32 | 29 | ||
52 | === modified file 'utah/provisioning/baremetal/bamboofeeder.py' | |||
53 | --- utah/provisioning/baremetal/bamboofeeder.py 2013-03-05 21:01:23 +0000 | |||
54 | +++ utah/provisioning/baremetal/bamboofeeder.py 2013-03-14 17:40:27 +0000 | |||
55 | @@ -275,7 +275,8 @@ | |||
56 | 275 | 275 | ||
57 | 276 | self.rsyslog.wait_for_install(config.install_steps) | 276 | self.rsyslog.wait_for_install(config.install_steps) |
58 | 277 | self.rsyslog.wait_for_booted(config.boot_steps) | 277 | self.rsyslog.wait_for_booted(config.boot_steps) |
60 | 278 | retry(self.sshcheck, config.checktimeout, logmethod=self.logger.info) | 278 | retry(self.sshcheck, logmethod=self.logger.info, |
61 | 279 | retry_timeout=config.checktimeout) | ||
62 | 279 | 280 | ||
63 | 280 | self.provisioned = True | 281 | self.provisioned = True |
64 | 281 | self.active = True | 282 | self.active = True |
65 | 282 | 283 | ||
66 | === modified file 'utah/provisioning/baremetal/cobbler.py' | |||
67 | --- utah/provisioning/baremetal/cobbler.py 2013-03-05 21:01:23 +0000 | |||
68 | +++ utah/provisioning/baremetal/cobbler.py 2013-03-14 17:40:27 +0000 | |||
69 | @@ -21,7 +21,6 @@ | |||
70 | 21 | import pipes | 21 | import pipes |
71 | 22 | import subprocess | 22 | import subprocess |
72 | 23 | import tempfile | 23 | import tempfile |
73 | 24 | import time | ||
74 | 25 | 24 | ||
75 | 26 | from utah import config | 25 | from utah import config |
76 | 27 | from utah.provisioning.baremetal.exceptions import UTAHBMProvisioningException | 26 | from utah.provisioning.baremetal.exceptions import UTAHBMProvisioningException |
77 | @@ -198,7 +197,8 @@ | |||
78 | 198 | if self.installtype == 'desktop': | 197 | if self.installtype == 'desktop': |
79 | 199 | self._removenfs() | 198 | self._removenfs() |
80 | 200 | 199 | ||
82 | 201 | retry(self.sshcheck, config.checktimeout, logmethod=self.logger.info) | 200 | retry(self.sshcheck, logmethod=self.logger.info, |
83 | 201 | retry_timeout=config.checktimeout) | ||
84 | 202 | 202 | ||
85 | 203 | self.provisioned = True | 203 | self.provisioned = True |
86 | 204 | self.active = True | 204 | self.active = True |
87 | 205 | 205 | ||
88 | === modified file 'utah/provisioning/provisioning.py' | |||
89 | --- utah/provisioning/provisioning.py 2013-03-05 21:03:09 +0000 | |||
90 | +++ utah/provisioning/provisioning.py 2013-03-14 17:40:27 +0000 | |||
91 | @@ -28,7 +28,6 @@ | |||
92 | 28 | import shutil | 28 | import shutil |
93 | 29 | import subprocess | 29 | import subprocess |
94 | 30 | import sys | 30 | import sys |
95 | 31 | import time | ||
96 | 32 | import urllib | 31 | import urllib |
97 | 33 | import uuid | 32 | import uuid |
98 | 34 | 33 | ||
99 | @@ -310,16 +309,9 @@ | |||
100 | 310 | if not self.active: | 309 | if not self.active: |
101 | 311 | self._start() | 310 | self._start() |
102 | 312 | 311 | ||
104 | 313 | def pingcheck(self, timeout=config.checktimeout): | 312 | def pingcheck(self): |
105 | 314 | """Check network connectivity using ping. | 313 | """Check network connectivity using ping. |
106 | 315 | 314 | ||
107 | 316 | :param timeout: Amount of time in seconds to sleep after a failure | ||
108 | 317 | :type timeout: int | ||
109 | 318 | :raises: UTAHProvisioningException | ||
110 | 319 | |||
111 | 320 | If there's a network connectivity failure, then sleep ``timeout`` | ||
112 | 321 | seconds and raise a retriable exception. | ||
113 | 322 | |||
114 | 323 | .. seealso:: :func:`utah.retry.retry`, :meth:`pingpoll` | 315 | .. seealso:: :func:`utah.retry.retry`, :meth:`pingpoll` |
115 | 324 | 316 | ||
116 | 325 | """ | 317 | """ |
117 | @@ -328,12 +320,6 @@ | |||
118 | 328 | self._runargs(['ping', '-c1', '-w5', self.name])['returncode'] | 320 | self._runargs(['ping', '-c1', '-w5', self.name])['returncode'] |
119 | 329 | if returncode != 0: | 321 | if returncode != 0: |
120 | 330 | err = 'Ping returned {0}'.format(returncode) | 322 | err = 'Ping returned {0}'.format(returncode) |
121 | 331 | |||
122 | 332 | if timeout > 0: | ||
123 | 333 | self.logger.info('Sleeping {timeout} seconds' | ||
124 | 334 | .format(timeout=timeout)) | ||
125 | 335 | time.sleep(timeout) | ||
126 | 336 | |||
127 | 337 | raise UTAHProvisioningException(err, retry=True) | 323 | raise UTAHProvisioningException(err, retry=True) |
128 | 338 | 324 | ||
129 | 339 | def pingpoll(self, | 325 | def pingpoll(self, |
130 | @@ -345,8 +331,8 @@ | |||
131 | 345 | timeout = self.boottimeout | 331 | timeout = self.boottimeout |
132 | 346 | if logmethod is None: | 332 | if logmethod is None: |
133 | 347 | logmethod = self.logger.debug | 333 | logmethod = self.logger.debug |
136 | 348 | utah.timeout.timeout(timeout, retry, self.pingcheck, checktimeout, | 334 | utah.timeout.timeout(timeout, retry, self.pingcheck, |
137 | 349 | logmethod=logmethod) | 335 | logmethod=logmethod, retry_timeout=checktimeout) |
138 | 350 | 336 | ||
139 | 351 | def getutahdeb(self, deb): | 337 | def getutahdeb(self, deb): |
140 | 352 | """ | 338 | """ |
141 | 353 | 339 | ||
142 | === modified file 'utah/provisioning/ssh.py' | |||
143 | --- utah/provisioning/ssh.py 2013-03-07 21:55:56 +0000 | |||
144 | +++ utah/provisioning/ssh.py 2013-03-14 17:40:27 +0000 | |||
145 | @@ -238,16 +238,9 @@ | |||
146 | 238 | 238 | ||
147 | 239 | super(SSHMixin, self).destroy(*args, **kw) | 239 | super(SSHMixin, self).destroy(*args, **kw) |
148 | 240 | 240 | ||
150 | 241 | def sshcheck(self, timeout=config.checktimeout): | 241 | def sshcheck(self): |
151 | 242 | """Check if the machine is available via ssh. | 242 | """Check if the machine is available via ssh. |
152 | 243 | 243 | ||
153 | 244 | :param timeout: Amount of time in seconds to sleep after a failure | ||
154 | 245 | :type timeout: int | ||
155 | 246 | :raises: UTAHProvisioningException | ||
156 | 247 | |||
157 | 248 | If there's a network connectivity failure, then sleep ``timeout`` | ||
158 | 249 | seconds and raise a retriable exception. | ||
159 | 250 | |||
160 | 251 | .. seealso:: :func:`utah.retry.retry`, :meth:`sshpoll` | 244 | .. seealso:: :func:`utah.retry.retry`, :meth:`sshpoll` |
161 | 252 | 245 | ||
162 | 253 | """ | 246 | """ |
163 | @@ -257,11 +250,6 @@ | |||
164 | 257 | username=config.user, | 250 | username=config.user, |
165 | 258 | key_filename=config.sshprivatekey) | 251 | key_filename=config.sshprivatekey) |
166 | 259 | except socket.error as err: | 252 | except socket.error as err: |
167 | 260 | if timeout > 0: | ||
168 | 261 | self.ssh_logger.info('Sleeping {timeout} seconds' | ||
169 | 262 | .format(timeout=timeout)) | ||
170 | 263 | time.sleep(timeout) | ||
171 | 264 | |||
172 | 265 | raise UTAHProvisioningException(str(err), retry=True) | 253 | raise UTAHProvisioningException(str(err), retry=True) |
173 | 266 | 254 | ||
174 | 267 | def sshpoll(self, timeout=None, | 255 | def sshpoll(self, timeout=None, |
175 | @@ -273,8 +261,8 @@ | |||
176 | 273 | timeout = self.boottimeout | 261 | timeout = self.boottimeout |
177 | 274 | if logmethod is None: | 262 | if logmethod is None: |
178 | 275 | logmethod = self.ssh_logger.debug | 263 | logmethod = self.ssh_logger.debug |
181 | 276 | utah.timeout.timeout(timeout, retry, self.sshcheck, checktimeout, | 264 | utah.timeout.timeout(timeout, retry, self.sshcheck, |
182 | 277 | logmethod=logmethod) | 265 | logmethod=logmethod, retry_timeout=checktimeout) |
183 | 278 | 266 | ||
184 | 279 | def activecheck(self): | 267 | def activecheck(self): |
185 | 280 | """ | 268 | """ |
186 | 281 | 269 | ||
187 | === modified file 'utah/retry.py' | |||
188 | --- utah/retry.py 2013-02-06 10:53:54 +0000 | |||
189 | +++ utah/retry.py 2013-03-14 17:40:27 +0000 | |||
190 | @@ -17,6 +17,7 @@ | |||
191 | 17 | Provide a retry loop that exits on success or an unretryable exception. | 17 | Provide a retry loop that exits on success or an unretryable exception. |
192 | 18 | """ | 18 | """ |
193 | 19 | import sys | 19 | import sys |
194 | 20 | import time | ||
195 | 20 | 21 | ||
196 | 21 | from utah.exceptions import UTAHException | 22 | from utah.exceptions import UTAHException |
197 | 22 | from utah.commandstr import commandstr | 23 | from utah.commandstr import commandstr |
198 | @@ -36,18 +37,24 @@ | |||
199 | 36 | Keyword arguments to be passed to the callable. | 37 | Keyword arguments to be passed to the callable. |
200 | 37 | 38 | ||
201 | 38 | .. note:: | 39 | .. note:: |
205 | 39 | If a keyword argument named ``logmethod`` is found, it will be | 40 | There are a few keywords that are consumed by ``retry`` and not |
206 | 40 | extracted (i.e. it won't be passed to the callable) and used to log | 41 | passed to the callable: |
207 | 41 | every retry attempt (using ``sys.stderr`` by default). | 42 | * ``logmethod``: Preferred log method for every retry attempt |
208 | 43 | (``sys.stderr`` by default) | ||
209 | 44 | * ``retry_timeout``: Timeout in seconds between each retry | ||
210 | 45 | attempt | ||
211 | 46 | |||
212 | 42 | :returns: The value returned by the callable. | 47 | :returns: The value returned by the callable. |
213 | 43 | 48 | ||
214 | 44 | .. seealso:: :func:`utah.timeout.timeout` | 49 | .. seealso:: :func:`utah.timeout.timeout` |
215 | 45 | 50 | ||
216 | 46 | """ | 51 | """ |
221 | 47 | try: | 52 | # The following import is in the function body to avoid a cycle: |
222 | 48 | logmethod = kw.pop('logmethod') | 53 | # utah.config -> utah.url -> utah.retry -> utah.config |
223 | 49 | except KeyError: | 54 | from utah import config |
224 | 50 | logmethod = sys.stderr.write | 55 | |
225 | 56 | logmethod = kw.pop('logmethod', sys.stderr.write) | ||
226 | 57 | retry_timeout = kw.pop('retry_timeout', config.retry_timeout) | ||
227 | 51 | retval = False | 58 | retval = False |
228 | 52 | while True: | 59 | while True: |
229 | 53 | try: | 60 | try: |
230 | @@ -57,8 +64,11 @@ | |||
231 | 57 | try: | 64 | try: |
232 | 58 | if err.retry: | 65 | if err.retry: |
233 | 59 | if logmethod is not None: | 66 | if logmethod is not None: |
236 | 60 | logmethod('Caught ' + str(err) + ', retrying ' | 67 | logmethod('Caught {}, retrying {} in {} seconds' |
237 | 61 | + commandstr(command, *args, **kw)) | 68 | .format(err, |
238 | 69 | commandstr(command, *args, **kw), | ||
239 | 70 | retry_timeout)) | ||
240 | 71 | time.sleep(retry_timeout) | ||
241 | 62 | else: | 72 | else: |
242 | 63 | raise AttributeError | 73 | raise AttributeError |
243 | 64 | except AttributeError: | 74 | except AttributeError: |
244 | 65 | 75 | ||
245 | === modified file 'utah/url.py' | |||
246 | --- utah/url.py 2013-02-06 10:07:52 +0000 | |||
247 | +++ utah/url.py 2013-03-14 17:40:27 +0000 | |||
248 | @@ -24,7 +24,6 @@ | |||
249 | 24 | import urllib2 | 24 | import urllib2 |
250 | 25 | import tempfile | 25 | import tempfile |
251 | 26 | import logging | 26 | import logging |
252 | 27 | import time | ||
253 | 28 | from urlparse import urlparse | 27 | from urlparse import urlparse |
254 | 29 | from argparse import ArgumentTypeError | 28 | from argparse import ArgumentTypeError |
255 | 30 | 29 | ||
256 | @@ -226,8 +225,6 @@ | |||
257 | 226 | try: | 225 | try: |
258 | 227 | cmd.run(tmp_dir, url) | 226 | cmd.run(tmp_dir, url) |
259 | 228 | except bzrlib.errors.InvalidHttpResponse as exception: | 227 | except bzrlib.errors.InvalidHttpResponse as exception: |
260 | 229 | # Separate every retry attempt with a timeout of 3 seconds | ||
261 | 230 | time.sleep(3) | ||
262 | 231 | raise UTAHException(exception.path, exception.msg, retry=True) | 228 | raise UTAHException(exception.path, exception.msg, retry=True) |
263 | 232 | 229 | ||
264 | 233 | try: | 230 | try: |
I see some merge conflicts. It seems weird that this and the rsyslog branch have been getting them lately. I wonder if there's been a change in launchpad that's causing this.