launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04017
[Merge] lp:~jml/launchpad/buildd-cleanups into lp:launchpad
Jonathan Lange has proposed merging lp:~jml/launchpad/buildd-cleanups into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jml/launchpad/buildd-cleanups/+merge/65378
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.
--
https://code.launchpad.net/~jml/launchpad/buildd-cleanups/+merge/65378
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/buildd-cleanups into lp:launchpad.
=== modified file 'daemons/buildd-slave.tac'
--- daemons/buildd-slave.tac 2010-12-22 00:07:36 +0000
+++ daemons/buildd-slave.tac 2011-06-21 17:09:29 +0000
@@ -1,6 +1,13 @@
-# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+# XXX: JonathanLange 2011-06-21 bug=800295: The only modules in the Launchpad
+# tree that this is permitted to depend on are canonical.buildd and
+# canonical.launchpad.daemons.readyservice, since canonical.buildd is deployed
+# by copying lib/canonical/buildd,
+# lib/canonical/launchpad/daemons/readyservice.py and daemons/buildd-slave.tac
+# only.
+
# Buildd Slave implementation
# XXX: dsilvers: 2005/01/21: Currently everything logged in the slave gets
# passed through to the twistd log too. this could get dangerous/big
=== modified file 'lib/canonical/launchpad/daemons/readyservice.py'
--- lib/canonical/launchpad/daemons/readyservice.py 2010-10-20 18:43:29 +0000
+++ lib/canonical/launchpad/daemons/readyservice.py 2011-06-21 17:09:29 +0000
@@ -1,6 +1,12 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+# XXX: JonathanLange 2011-06-21 bug=800295: The only modules in the Launchpad
+# tree that this is permitted to depend on are canonical.buildd, since
+# canonical.buildd is deployed by copying lib/canonical/buildd,
+# lib/canonical/launchpad/daemons/readyservice.py and daemons/buildd-slave.tac
+# only.
+
"""Add logging for when twistd services start up.
Used externally to launchpad (by launchpad-buildd) - must not import
=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
--- lib/canonical/launchpad/daemons/tachandler.py 2010-10-26 15:47:24 +0000
+++ lib/canonical/launchpad/daemons/tachandler.py 2011-06-21 17:09:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test harness for TAC (Twisted Application Configuration) files."""
@@ -8,107 +8,24 @@
__all__ = [
'TacTestSetup',
'TacException',
- 'kill_by_pidfile',
- 'remove_if_exists',
- 'two_stage_kill',
]
-import errno
import os
-from signal import (
- SIGKILL,
- SIGTERM,
- )
import subprocess
import sys
import time
import warnings
from fixtures import Fixture
-from twisted.application import service
-from twisted.python import log
from canonical.launchpad.daemons import readyservice
-
-
-def _kill_may_race(pid, signal_number):
- """Kill a pid accepting that it may not exist."""
- try:
- os.kill(pid, signal_number)
- except OSError, e:
- if e.errno in (errno.ESRCH, errno.ECHILD):
- # Process has already been killed.
- return
- # Some other issue (e.g. different user owns it)
- raise
-
-
-def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
- """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
-
- :param pid: The pid of the process to kill.
- :param poll_interval: The polling interval used to check if the
- process is still around.
- :param num_polls: The number of polls to do before doing a SIGKILL.
- """
- # Kill the process.
- _kill_may_race(pid, SIGTERM)
-
- # Poll until the process has ended.
- for i in range(num_polls):
- try:
- # Reap the child process and get its return value. If it's not
- # gone yet, continue.
- new_pid, result = os.waitpid(pid, os.WNOHANG)
- if new_pid:
- return result
- time.sleep(poll_interval)
- except OSError, e:
- if e.errno in (errno.ESRCH, errno.ECHILD):
- # Raised if the process is gone by the time we try to get the
- # return value.
- return
-
- # The process is still around, so terminate it violently.
- _kill_may_race(pid, SIGKILL)
-
-
-def get_pid_from_file(pidfile_path):
- """Retrieve the PID from the given file, if it exists, None otherwise."""
- if not os.path.exists(pidfile_path):
- return None
- # Get the pid.
- pid = open(pidfile_path, 'r').read().split()[0]
- try:
- pid = int(pid)
- except ValueError:
- # pidfile contains rubbish
- return None
- return pid
-
-
-def kill_by_pidfile(pidfile_path, poll_interval=0.1, num_polls=50):
- """Kill a process identified by the pid stored in a file.
-
- The pid file is removed from disk.
- """
- try:
- pid = get_pid_from_file(pidfile_path)
- if pid is None:
- return
- two_stage_kill(pid, poll_interval, num_polls)
- finally:
- remove_if_exists(pidfile_path)
-
-
-def remove_if_exists(path):
- """Remove the given file if it exists."""
- try:
- os.remove(path)
- except OSError, e:
- if e.errno != errno.ENOENT:
- raise
+from lp.services.osutils import (
+ get_pid_from_file,
+ kill_by_pidfile,
+ remove_if_exists,
+ two_stage_kill,
+ )
twistd_script = os.path.abspath(os.path.join(
=== modified file 'lib/canonical/librarian/testing/server.py'
--- lib/canonical/librarian/testing/server.py 2011-02-17 13:18:43 +0000
+++ lib/canonical/librarian/testing/server.py 2011-06-21 17:09:29 +0000
@@ -23,11 +23,11 @@
import canonical
from canonical.config import config
from canonical.launchpad.daemons.tachandler import (
- get_pid_from_file,
TacException,
TacTestSetup,
)
from canonical.librarian.storage import _relFileLocation
+from lp.services.osutils import get_pid_from_file
class LibrarianServerFixture(TacTestSetup):
=== modified file 'lib/lp/services/osutils.py'
--- lib/lp/services/osutils.py 2011-02-25 17:14:44 +0000
+++ lib/lp/services/osutils.py 2011-06-21 17:09:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Utilities for doing the sort of thing the os module does."""
@@ -19,13 +19,12 @@
import errno
import os.path
import shutil
+from signal import (
+ SIGKILL,
+ SIGTERM,
+ )
import socket
-
-from canonical.launchpad.daemons.tachandler import (
- kill_by_pidfile,
- remove_if_exists,
- two_stage_kill,
- )
+import time
def remove_tree(path):
@@ -124,3 +123,82 @@
if e.errno == errno.ENOENT:
os.makedirs(os.path.dirname(filename), mode=dirmode)
return open(filename, mode)
+
+
+def _kill_may_race(pid, signal_number):
+ """Kill a pid accepting that it may not exist."""
+ try:
+ os.kill(pid, signal_number)
+ except OSError, e:
+ if e.errno in (errno.ESRCH, errno.ECHILD):
+ # Process has already been killed.
+ return
+ # Some other issue (e.g. different user owns it)
+ raise
+
+
+def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
+ """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
+
+ :param pid: The pid of the process to kill.
+ :param poll_interval: The polling interval used to check if the
+ process is still around.
+ :param num_polls: The number of polls to do before doing a SIGKILL.
+ """
+ # Kill the process.
+ _kill_may_race(pid, SIGTERM)
+
+ # Poll until the process has ended.
+ for i in range(num_polls):
+ try:
+ # Reap the child process and get its return value. If it's not
+ # gone yet, continue.
+ new_pid, result = os.waitpid(pid, os.WNOHANG)
+ if new_pid:
+ return result
+ time.sleep(poll_interval)
+ except OSError, e:
+ if e.errno in (errno.ESRCH, errno.ECHILD):
+ # Raised if the process is gone by the time we try to get the
+ # return value.
+ return
+
+ # The process is still around, so terminate it violently.
+ _kill_may_race(pid, SIGKILL)
+
+
+def get_pid_from_file(pidfile_path):
+ """Retrieve the PID from the given file, if it exists, None otherwise."""
+ if not os.path.exists(pidfile_path):
+ return None
+ # Get the pid.
+ pid = open(pidfile_path, 'r').read().split()[0]
+ try:
+ pid = int(pid)
+ except ValueError:
+ # pidfile contains rubbish
+ return None
+ return pid
+
+
+def kill_by_pidfile(pidfile_path, poll_interval=0.1, num_polls=50):
+ """Kill a process identified by the pid stored in a file.
+
+ The pid file is removed from disk.
+ """
+ try:
+ pid = get_pid_from_file(pidfile_path)
+ if pid is None:
+ return
+ two_stage_kill(pid, poll_interval, num_polls)
+ finally:
+ remove_if_exists(pidfile_path)
+
+
+def remove_if_exists(path):
+ """Remove the given file if it exists."""
+ try:
+ os.remove(path)
+ except OSError, e:
+ if e.errno != errno.ENOENT:
+ raise
