Launchpad itself

Merge lp:~jml/launchpad/buildd-cleanups into lp:launchpad

  1. buildd-cleanups
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Merged at revision: 13278
Proposed branch: lp:~jml/launchpad/buildd-cleanups
Merge into: lp:launchpad
Diff against target: 289 lines (+108/-100)
5 files modified
daemons/buildd-slave.tac (+8/-1)
lib/canonical/launchpad/daemons/readyservice.py (+7/-1)
lib/canonical/launchpad/daemons/tachandler.py (+7/-90)
lib/canonical/librarian/testing/server.py (+1/-1)
lib/lp/services/osutils.py (+85/-7)
To merge this branch: bzr merge lp:~jml/launchpad/buildd-cleanups
Reviewer Review Type Date Requested Status
Данило Шеган (community) code Approve
Review via email: mp+65378@code.launchpad.net

Commit message

[r=danilo][no-qa] Document buildd deployment gotchas. Move generic code out of tachandler into services.

Description of the change

Since bug 663828 was fixed, a whole bunch of stuff in tachandler doesn't really need to be there. Specifically, there are quite a few general purpose functions that rightly belong in lp.services.osutils. This branch moves them.

The underlying issue – buildd-slave's unusual deployment process – hasn't really been addressed, so I've commented the other files in the Launchpad tree that have stricter dependency requirements and filed bug 800295 to track it.

Sadly, the moved methods didn't appear to add tests. Since this is a drive-by cleanup, I'm not bovvered.

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'daemons/buildd-slave.tac'
2--- daemons/buildd-slave.tac 2010-12-22 00:07:36 +0000
3+++ daemons/buildd-slave.tac 2011-06-21 17:09:43 +0000
4@@ -1,6 +1,13 @@
5-# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9+# XXX: JonathanLange 2011-06-21 bug=800295: The only modules in the Launchpad
10+# tree that this is permitted to depend on are canonical.buildd and
11+# canonical.launchpad.daemons.readyservice, since canonical.buildd is deployed
12+# by copying lib/canonical/buildd,
13+# lib/canonical/launchpad/daemons/readyservice.py and daemons/buildd-slave.tac
14+# only.
15+
16 # Buildd Slave implementation
17 # XXX: dsilvers: 2005/01/21: Currently everything logged in the slave gets
18 # passed through to the twistd log too. this could get dangerous/big
19
20=== modified file 'lib/canonical/launchpad/daemons/readyservice.py'
21--- lib/canonical/launchpad/daemons/readyservice.py 2010-10-20 18:43:29 +0000
22+++ lib/canonical/launchpad/daemons/readyservice.py 2011-06-21 17:09:43 +0000
23@@ -1,6 +1,12 @@
24-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
25+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
26 # GNU Affero General Public License version 3 (see the file LICENSE).
27
28+# XXX: JonathanLange 2011-06-21 bug=800295: The only modules in the Launchpad
29+# tree that this is permitted to depend on are canonical.buildd, since
30+# canonical.buildd is deployed by copying lib/canonical/buildd,
31+# lib/canonical/launchpad/daemons/readyservice.py and daemons/buildd-slave.tac
32+# only.
33+
34 """Add logging for when twistd services start up.
35
36 Used externally to launchpad (by launchpad-buildd) - must not import
37
38=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
39--- lib/canonical/launchpad/daemons/tachandler.py 2010-10-26 15:47:24 +0000
40+++ lib/canonical/launchpad/daemons/tachandler.py 2011-06-21 17:09:43 +0000
41@@ -1,4 +1,4 @@
42-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
43+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
44 # GNU Affero General Public License version 3 (see the file LICENSE).
45
46 """Test harness for TAC (Twisted Application Configuration) files."""
47@@ -8,107 +8,24 @@
48 __all__ = [
49 'TacTestSetup',
50 'TacException',
51- 'kill_by_pidfile',
52- 'remove_if_exists',
53- 'two_stage_kill',
54 ]
55
56
57-import errno
58 import os
59-from signal import (
60- SIGKILL,
61- SIGTERM,
62- )
63 import subprocess
64 import sys
65 import time
66 import warnings
67
68 from fixtures import Fixture
69-from twisted.application import service
70-from twisted.python import log
71
72 from canonical.launchpad.daemons import readyservice
73-
74-
75-def _kill_may_race(pid, signal_number):
76- """Kill a pid accepting that it may not exist."""
77- try:
78- os.kill(pid, signal_number)
79- except OSError, e:
80- if e.errno in (errno.ESRCH, errno.ECHILD):
81- # Process has already been killed.
82- return
83- # Some other issue (e.g. different user owns it)
84- raise
85-
86-
87-def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
88- """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
89-
90- :param pid: The pid of the process to kill.
91- :param poll_interval: The polling interval used to check if the
92- process is still around.
93- :param num_polls: The number of polls to do before doing a SIGKILL.
94- """
95- # Kill the process.
96- _kill_may_race(pid, SIGTERM)
97-
98- # Poll until the process has ended.
99- for i in range(num_polls):
100- try:
101- # Reap the child process and get its return value. If it's not
102- # gone yet, continue.
103- new_pid, result = os.waitpid(pid, os.WNOHANG)
104- if new_pid:
105- return result
106- time.sleep(poll_interval)
107- except OSError, e:
108- if e.errno in (errno.ESRCH, errno.ECHILD):
109- # Raised if the process is gone by the time we try to get the
110- # return value.
111- return
112-
113- # The process is still around, so terminate it violently.
114- _kill_may_race(pid, SIGKILL)
115-
116-
117-def get_pid_from_file(pidfile_path):
118- """Retrieve the PID from the given file, if it exists, None otherwise."""
119- if not os.path.exists(pidfile_path):
120- return None
121- # Get the pid.
122- pid = open(pidfile_path, 'r').read().split()[0]
123- try:
124- pid = int(pid)
125- except ValueError:
126- # pidfile contains rubbish
127- return None
128- return pid
129-
130-
131-def kill_by_pidfile(pidfile_path, poll_interval=0.1, num_polls=50):
132- """Kill a process identified by the pid stored in a file.
133-
134- The pid file is removed from disk.
135- """
136- try:
137- pid = get_pid_from_file(pidfile_path)
138- if pid is None:
139- return
140- two_stage_kill(pid, poll_interval, num_polls)
141- finally:
142- remove_if_exists(pidfile_path)
143-
144-
145-def remove_if_exists(path):
146- """Remove the given file if it exists."""
147- try:
148- os.remove(path)
149- except OSError, e:
150- if e.errno != errno.ENOENT:
151- raise
152+from lp.services.osutils import (
153+ get_pid_from_file,
154+ kill_by_pidfile,
155+ remove_if_exists,
156+ two_stage_kill,
157+ )
158
159
160 twistd_script = os.path.abspath(os.path.join(
161
162=== modified file 'lib/canonical/librarian/testing/server.py'
163--- lib/canonical/librarian/testing/server.py 2011-02-17 13:18:43 +0000
164+++ lib/canonical/librarian/testing/server.py 2011-06-21 17:09:43 +0000
165@@ -23,11 +23,11 @@
166 import canonical
167 from canonical.config import config
168 from canonical.launchpad.daemons.tachandler import (
169- get_pid_from_file,
170 TacException,
171 TacTestSetup,
172 )
173 from canonical.librarian.storage import _relFileLocation
174+from lp.services.osutils import get_pid_from_file
175
176
177 class LibrarianServerFixture(TacTestSetup):
178
179=== modified file 'lib/lp/services/osutils.py'
180--- lib/lp/services/osutils.py 2011-02-25 17:14:44 +0000
181+++ lib/lp/services/osutils.py 2011-06-21 17:09:43 +0000
182@@ -1,4 +1,4 @@
183-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
184+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
185 # GNU Affero General Public License version 3 (see the file LICENSE).
186
187 """Utilities for doing the sort of thing the os module does."""
188@@ -19,13 +19,12 @@
189 import errno
190 import os.path
191 import shutil
192+from signal import (
193+ SIGKILL,
194+ SIGTERM,
195+ )
196 import socket
197-
198-from canonical.launchpad.daemons.tachandler import (
199- kill_by_pidfile,
200- remove_if_exists,
201- two_stage_kill,
202- )
203+import time
204
205
206 def remove_tree(path):
207@@ -124,3 +123,82 @@
208 if e.errno == errno.ENOENT:
209 os.makedirs(os.path.dirname(filename), mode=dirmode)
210 return open(filename, mode)
211+
212+
213+def _kill_may_race(pid, signal_number):
214+ """Kill a pid accepting that it may not exist."""
215+ try:
216+ os.kill(pid, signal_number)
217+ except OSError, e:
218+ if e.errno in (errno.ESRCH, errno.ECHILD):
219+ # Process has already been killed.
220+ return
221+ # Some other issue (e.g. different user owns it)
222+ raise
223+
224+
225+def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
226+ """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
227+
228+ :param pid: The pid of the process to kill.
229+ :param poll_interval: The polling interval used to check if the
230+ process is still around.
231+ :param num_polls: The number of polls to do before doing a SIGKILL.
232+ """
233+ # Kill the process.
234+ _kill_may_race(pid, SIGTERM)
235+
236+ # Poll until the process has ended.
237+ for i in range(num_polls):
238+ try:
239+ # Reap the child process and get its return value. If it's not
240+ # gone yet, continue.
241+ new_pid, result = os.waitpid(pid, os.WNOHANG)
242+ if new_pid:
243+ return result
244+ time.sleep(poll_interval)
245+ except OSError, e:
246+ if e.errno in (errno.ESRCH, errno.ECHILD):
247+ # Raised if the process is gone by the time we try to get the
248+ # return value.
249+ return
250+
251+ # The process is still around, so terminate it violently.
252+ _kill_may_race(pid, SIGKILL)
253+
254+
255+def get_pid_from_file(pidfile_path):
256+ """Retrieve the PID from the given file, if it exists, None otherwise."""
257+ if not os.path.exists(pidfile_path):
258+ return None
259+ # Get the pid.
260+ pid = open(pidfile_path, 'r').read().split()[0]
261+ try:
262+ pid = int(pid)
263+ except ValueError:
264+ # pidfile contains rubbish
265+ return None
266+ return pid
267+
268+
269+def kill_by_pidfile(pidfile_path, poll_interval=0.1, num_polls=50):
270+ """Kill a process identified by the pid stored in a file.
271+
272+ The pid file is removed from disk.
273+ """
274+ try:
275+ pid = get_pid_from_file(pidfile_path)
276+ if pid is None:
277+ return
278+ two_stage_kill(pid, poll_interval, num_polls)
279+ finally:
280+ remove_if_exists(pidfile_path)
281+
282+
283+def remove_if_exists(path):
284+ """Remove the given file if it exists."""
285+ try:
286+ os.remove(path)
287+ except OSError, e:
288+ if e.errno != errno.ENOENT:
289+ raise