launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21788
[Merge] lp:~cjwatson/launchpad-buildd/scan-for-processes-python into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/scan-for-processes-python into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/start-stop-python as a prerequisite.
Commit message:
Rewrite scan-for-processes in Python, allowing it to have unit tests.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/scan-for-processes-python/+merge/328573
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/scan-for-processes-python into lp:launchpad-buildd.
=== modified file 'bin/scan-for-processes'
--- bin/scan-for-processes 2017-07-25 22:11:19 +0000
+++ bin/scan-for-processes 2017-08-04 13:10:42 +0000
@@ -1,43 +1,32 @@
-#!/bin/bash
+#! /usr/bin/python -u
#
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-# Buildd Slave tool to scan a chroot in case any processes are underneath it
-
-## This script uses bashisms, must be run under bash
-
-# Expects build id as arg 1, makes build-id to contain the build
-
-# Needs SUDO to be set to a sudo instance for passwordless access
-
-SUDO=/usr/bin/sudo
-BUILDID="$1"
-REALHOME=$(cd $HOME && pwd -P)
-
-set -e
-
-exec 2>&1
-
-[ $(id -u) = "0" ] || exec $SUDO $0 "$REALHOME/build-$BUILDID/chroot-autobuild"
-
-echo "Scanning for processes to kill in build $BUILDID..."
-
-PREFIX="$BUILDID"
-FOUND=0
-
-for ROOT in /proc/*/root; do
- LINK=$(readlink $ROOT || true)
- if [ "x$LINK" != "x" ]; then
- if [ "x${LINK:0:${#PREFIX}}" = "x$PREFIX" ]; then
- # this process is in the chroot...
- PID=$(basename $(dirname "$ROOT"))
- kill -9 "$PID" || true
- FOUND=1
- fi
- fi
-done
-
-if [ "x$FOUND" = "x1" ]; then
- exec $0 $1
-fi
+"""Kill any processes running in a chroot."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+
+import os
+import sys
+
+from lpbuildd.target.kill_processes import KillProcesses
+
+
+def main():
+ # This whole script must run as root, since we want to iterate over
+ # other users' processes in Python.
+ if os.geteuid() != 0:
+ cmd = ["sudo"]
+ if "PYTHONPATH" in os.environ:
+ cmd.append("PYTHONPATH=%s" % os.environ["PYTHONPATH"])
+ cmd.append("--")
+ cmd.extend(sys.argv)
+ os.execv("/usr/bin/sudo", cmd)
+ return KillProcesses().run()
+
+
+if __name__ == "__main__":
+ sys.exit(main())
=== modified file 'debian/changelog'
--- debian/changelog 2017-08-04 13:10:42 +0000
+++ debian/changelog 2017-08-04 13:10:42 +0000
@@ -19,6 +19,7 @@
unit tests.
* Rewrite mount-chroot and umount-chroot in Python, allowing them to have
unit tests.
+ * Rewrite scan-for-processes in Python, allowing it to have unit tests.
-- Colin Watson <cjwatson@xxxxxxxxxx> Tue, 25 Jul 2017 23:07:58 +0100
=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/slave.py 2017-08-04 13:10:42 +0000
@@ -181,9 +181,8 @@
iterate = partial(self.iterateReap, state)
else:
iterate = lambda success: None
- self.runSubProcess(
- self._scanpath, ["scan-for-processes", self._buildid],
- iterate=iterate)
+ self.runTargetSubProcess(
+ self._scanpath, ["scan-for-processes"], iterate=iterate)
def doCleanup(self):
"""Remove the build tree etc."""
=== modified file 'lpbuildd/target/backend.py'
--- lpbuildd/target/backend.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/target/backend.py 2017-08-04 13:10:42 +0000
@@ -69,6 +69,14 @@
"""
raise NotImplementedError
+ def kill_processes(self):
+ """Kill any processes left running in the target.
+
+ This is allowed to do nothing if stopping the target will reliably
+ kill all processes running in it.
+ """
+ pass
+
def stop(self):
"""Stop the backend."""
raise NotImplementedError
=== modified file 'lpbuildd/target/chroot.py'
--- lpbuildd/target/chroot.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/target/chroot.py 2017-08-04 13:10:42 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
import os.path
+import signal
import stat
import subprocess
import time
@@ -81,6 +82,26 @@
["sudo", "install", "-o", "root", "-g", "root", "-m", "%o" % mode,
source_path, full_target_path])
+ def kill_processes(self):
+ """See `Backend`."""
+ prefix = os.path.realpath(self.chroot_path)
+ while True:
+ found = False
+ pids = [int(pid) for pid in os.listdir("/proc") if pid.isdigit()]
+ for pid in sorted(pids):
+ try:
+ link = os.readlink(os.path.join("/proc", str(pid), "root"))
+ except OSError:
+ continue
+ if link and (link == prefix or link.startswith(prefix + "/")):
+ try:
+ os.kill(pid, signal.SIGKILL)
+ except OSError:
+ pass
+ found = True
+ if not found:
+ break
+
def _get_chroot_mounts(self):
with open("/proc/mounts") as mounts_file:
for line in mounts_file:
=== added file 'lpbuildd/target/kill_processes.py'
--- lpbuildd/target/kill_processes.py 1970-01-01 00:00:00 +0000
+++ lpbuildd/target/kill_processes.py 2017-08-04 13:10:42 +0000
@@ -0,0 +1,23 @@
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import print_function
+
+__metaclass__ = type
+
+import logging
+
+from lpbuildd.target.operation import Operation
+
+
+logger = logging.getLogger(__name__)
+
+
+class KillProcesses(Operation):
+
+ description = "Kill any processes in the target."
+
+ def run(self):
+ logger.info(
+ "Scanning for processes to kill in build %s", self.args.build_id)
+ self.backend.kill_processes()
=== modified file 'lpbuildd/target/tests/test_chroot.py'
--- lpbuildd/target/tests/test_chroot.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/target/tests/test_chroot.py 2017-08-04 13:10:42 +0000
@@ -4,6 +4,7 @@
__metaclass__ = type
import os.path
+import signal
from textwrap import dedent
import time
@@ -17,10 +18,14 @@
FakeTime,
)
from testtools import TestCase
+from testtools.matchers import DirContains
from lpbuildd.target.backend import BackendException
from lpbuildd.target.chroot import Chroot
-from lpbuildd.target.tests.testfixtures import SudoUmount
+from lpbuildd.target.tests.testfixtures import (
+ KillFixture,
+ SudoUmount,
+ )
class TestChroot(TestCase):
@@ -114,6 +119,37 @@
expected_args,
[proc._args["args"] for proc in processes_fixture.procs])
+ def test_kill_processes(self):
+ self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
+ fs_fixture = self.useFixture(FakeFilesystem())
+ fs_fixture.add("/expected")
+ os.makedirs("/expected/home/build-1/chroot-autobuild")
+ fs_fixture.add("/proc")
+ os.mkdir("/proc")
+ os.mkdir("/proc/1")
+ os.symlink("/", "/proc/1/root")
+ os.mkdir("/proc/10")
+ os.symlink("/expected/home/build-1/chroot-autobuild", "/proc/10/root")
+ os.mkdir("/proc/11")
+ os.symlink("/expected/home/build-1/chroot-autobuild", "/proc/11/root")
+ os.mkdir("/proc/12")
+ os.symlink(
+ "/expected/home/build-1/chroot-autobuild/submount",
+ "/proc/12/root")
+ os.mkdir("/proc/13")
+ os.symlink(
+ "/expected/home/build-1/chroot-autobuildsomething",
+ "/proc/13/root")
+ with open("/proc/version", "w"):
+ pass
+ kill_fixture = self.useFixture(KillFixture(delays={10: 1}))
+ Chroot("1", "xenial", "amd64").kill_processes()
+
+ self.assertEqual(
+ [(pid, signal.SIGKILL) for pid in (11, 12, 10)],
+ kill_fixture.kills)
+ self.assertThat("/proc", DirContains(["1", "13", "version"]))
+
def _make_initial_proc_mounts(self):
fs_fixture = self.useFixture(FakeFilesystem())
fs_fixture.add("/proc")
=== added file 'lpbuildd/target/tests/test_kill_processes.py'
--- lpbuildd/target/tests/test_kill_processes.py 1970-01-01 00:00:00 +0000
+++ lpbuildd/target/tests/test_kill_processes.py 2017-08-04 13:10:42 +0000
@@ -0,0 +1,34 @@
+# Copyright 2017 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import os.path
+import signal
+
+from fixtures import EnvironmentVariable
+from systemfixtures import FakeFilesystem
+from testtools import TestCase
+from testtools.matchers import DirContains
+
+from lpbuildd.target.kill_processes import KillProcesses
+from lpbuildd.target.tests.testfixtures import KillFixture
+
+
+class TestKillProcesses(TestCase):
+
+ def test_succeeds(self):
+ self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
+ fs_fixture = self.useFixture(FakeFilesystem())
+ fs_fixture.add("/expected")
+ os.makedirs("/expected/home/build-1/chroot-autobuild")
+ fs_fixture.add("/proc")
+ os.mkdir("/proc")
+ os.mkdir("/proc/10")
+ os.symlink("/expected/home/build-1/chroot-autobuild", "/proc/10/root")
+ kill_fixture = self.useFixture(KillFixture())
+ args = ["--backend=chroot", "--series=xenial", "--arch=amd64", "1"]
+ KillProcesses(args=args).run()
+
+ self.assertEqual([(10, signal.SIGKILL)], kill_fixture.kills)
+ self.assertThat("/proc", DirContains([]))
=== modified file 'lpbuildd/target/tests/testfixtures.py'
--- lpbuildd/target/tests/testfixtures.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/target/tests/testfixtures.py 2017-08-04 13:10:42 +0000
@@ -4,6 +4,9 @@
__metaclass__ = type
import argparse
+import shutil
+
+from fixtures import MonkeyPatch
class SudoUmount:
@@ -36,3 +39,32 @@
for mount in mounts:
mounts_file.write(mount)
return {}
+
+
+class Kill:
+ """A substitute for `os.kill` that may fail sometimes.
+
+ This must run with a fake `/proc` (e.g. using
+ `systemfixtures.FakeFilesystem`).
+ """
+
+ def __init__(self, delays=None):
+ self.delays = delays or {}
+ self.kills = []
+
+ def __call__(self, pid, sig):
+ if self.delays.get(pid, 0) > 0:
+ self.delays[pid] -= 1
+ raise OSError
+ self.kills.append((pid, sig))
+ shutil.rmtree("/proc/%d" % pid)
+
+
+class KillFixture(MonkeyPatch):
+
+ def __init__(self, delays=None):
+ super(KillFixture, self).__init__("os.kill", Kill(delays=delays))
+
+ @property
+ def kills(self):
+ return self.new_value.kills
=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_binarypackage.py 2017-08-04 13:10:42 +0000
@@ -152,6 +152,7 @@
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=False)
def assertUnmountsSanely(self):
@@ -252,6 +253,7 @@
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=False)
self.assertFalse(self.slave.wasCalled('buildFail'))
@@ -271,6 +273,7 @@
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=False)
self.assertFalse(self.slave.wasCalled('builderFail'))
reap_subprocess = self.buildmanager._subprocess
@@ -310,6 +313,7 @@
self.assertState(
BinaryPackageBuildState.INIT,
['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=False)
self.buildmanager.iterate(0)
=== modified file 'lpbuildd/tests/test_debian.py'
--- lpbuildd/tests/test_debian.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_debian.py 2017-08-04 13:10:42 +0000
@@ -161,6 +161,7 @@
self.assertEqual(MockBuildState.MAIN, self.getState())
self.assertEqual(
(['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=xenial', '--arch=amd64',
self.buildid],
None),
self.buildmanager.commands[-1])
@@ -280,6 +281,7 @@
self.assertEqual(MockBuildState.MAIN, self.getState())
self.assertEqual(
(['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=xenial', '--arch=amd64',
self.buildid],
None),
self.buildmanager.commands[-1])
=== modified file 'lpbuildd/tests/test_livefs.py'
--- lpbuildd/tests/test_livefs.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_livefs.py 2017-08-04 13:10:42 +0000
@@ -100,6 +100,7 @@
self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/scan-for-processes", "scan-for-processes",
+ "--backend=chroot", "--series=saucy", "--arch=i386",
self.buildid,
]
self.assertEqual(
=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_snap.py 2017-08-04 13:10:42 +0000
@@ -112,6 +112,7 @@
self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/scan-for-processes", "scan-for-processes",
+ "--backend=chroot", "--series=xenial", "--arch=i386",
self.buildid,
]
self.assertEqual(SnapBuildState.BUILD_SNAP, self.getState())
@@ -157,6 +158,7 @@
self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/scan-for-processes", "scan-for-processes",
+ "--backend=chroot", "--series=xenial", "--arch=i386",
self.buildid,
]
self.assertEqual(SnapBuildState.BUILD_SNAP, self.getState())
=== modified file 'lpbuildd/tests/test_sourcepackagerecipe.py'
--- lpbuildd/tests/test_sourcepackagerecipe.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_sourcepackagerecipe.py 2017-08-04 13:10:42 +0000
@@ -126,6 +126,7 @@
self.buildmanager.iterate(0)
expected_command = [
'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=maverick', '--arch=i386',
self.buildid,
]
self.assertEqual(
@@ -167,6 +168,7 @@
self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
expected_command = [
'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=maverick', '--arch=i386',
self.buildid,
]
self.assertEqual(
@@ -205,6 +207,7 @@
self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
expected_command = [
'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=maverick', '--arch=i386',
self.buildid,
]
self.assertEqual(
=== modified file 'lpbuildd/tests/test_translationtemplatesbuildmanager.py'
--- lpbuildd/tests/test_translationtemplatesbuildmanager.py 2017-08-04 13:10:42 +0000
+++ lpbuildd/tests/test_translationtemplatesbuildmanager.py 2017-08-04 13:10:42 +0000
@@ -105,6 +105,7 @@
self.buildmanager.iterate(0)
expected_command = [
'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=xenial', '--arch=i386',
self.buildid,
]
self.assertEqual(
@@ -170,6 +171,7 @@
self.buildmanager.iterate(-1)
expected_command = [
'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ '--backend=chroot', '--series=xenial', '--arch=i386',
self.buildid,
]
self.assertEqual(
