Merge lp:~mbp/launchpad/800295-buildd-split into lp:launchpad

Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: 14248
Proposed branch: lp:~mbp/launchpad/800295-buildd-split
Merge into: lp:launchpad
Diff against target: 172 lines (+45/-19)
5 files modified
lib/canonical/buildd/buildd-slave.tac (+3/-10)
lib/canonical/buildd/debian/changelog (+8/-0)
lib/canonical/buildd/debian/rules (+1/-5)
lib/canonical/buildd/tests/harness.py (+12/-4)
lib/canonical/launchpad/daemons/tachandler.py (+21/-0)
To merge this branch: bzr merge lp:~mbp/launchpad/800295-buildd-split
Reviewer Review Type Date Requested Status
Julian Edwards (community) Needs Information
Review via email: mp+81111@code.launchpad.net

Commit message

[r=mbp][bug=800295] [no-qa] prune dependencies between buildd and the rest of Launchpad

Description of the change

This is a partial fix for bug 800295, splitting all the files shared between buildd and lp proper into one or the other.

Beyond this we might like to split them in to separate trees, but I thought this would be easier to review and test by itself.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

This will fail tests. the daemon is used and tested from the lp tree,
which is what readyservice is for.

Revision history for this message
Martin Pool (mbp) wrote :

I should probably move 'wait for the port' up to the base class.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

I can't see where _hasDaemonStarted() is getting called from here. It's called in the whole tree from only one place in lib/canonical/launchpad/daemons/tachandler.py but I can't see how it's replacing the readyService in the buildd code.

Am I missing something or are you? :)

review: Needs Information
Revision history for this message
Martin Pool (mbp) wrote :

_hasDaemonStarted is called from the TestTacServer base class; I'm overriding it here so that the fixture decides the server is ready not when it writes to the log, but rather when the daemon starts listening on its port. That general concept could usefully go into the base class, with the subclass just defining the port.

This patch as it is passes all tests, and it removes the need to actually copy files from the lp tree into the buildd directory. It doesn't make it possible to run the buildd tests without the Launchpad tree, which would be useful to do to make it totally split out.

Revision history for this message
Martin Pool (mbp) wrote :

The current version of this does everything short of cutting things in to separate packages. The buildd code no longer (I think) depends on anything from the rest of lp except TacTestSetup.

The next steps are to split out TacTestSetup into one package, and then to split the buildd itself.

Revision history for this message
Martin Pool (mbp) wrote :

... I'd like to do those splits as separate mps because they'll be harder to reviews (since they will mostly just look like deletions.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== renamed file 'daemons/buildd-slave.tac' => 'lib/canonical/buildd/buildd-slave.tac'
--- daemons/buildd-slave.tac 2011-06-21 17:07:05 +0000
+++ lib/canonical/buildd/buildd-slave.tac 2011-11-04 01:08:26 +0000
@@ -1,12 +1,9 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# XXX: JonathanLange 2011-06-21 bug=800295: The only modules in the Launchpad4# CAUTION: The only modules in the Launchpad tree that this is permitted to
5# tree that this is permitted to depend on are canonical.buildd and5# depend on are canonical.buildd, since buildds are deployed by copying that
6# canonical.launchpad.daemons.readyservice, since canonical.buildd is deployed6# directory only. (See also bug=800295.)
7# by copying lib/canonical/buildd,
8# lib/canonical/launchpad/daemons/readyservice.py and daemons/buildd-slave.tac
9# only.
107
11# Buildd Slave implementation8# Buildd Slave implementation
12# XXX: dsilvers: 2005/01/21: Currently everything logged in the slave gets9# XXX: dsilvers: 2005/01/21: Currently everything logged in the slave gets
@@ -19,7 +16,6 @@
19 SourcePackageRecipeBuildManager)16 SourcePackageRecipeBuildManager)
20from canonical.buildd.translationtemplates import (17from canonical.buildd.translationtemplates import (
21 TranslationTemplatesBuildManager)18 TranslationTemplatesBuildManager)
22from canonical.launchpad.daemons import readyservice
2319
24from twisted.web import server, resource, static20from twisted.web import server, resource, static
25from ConfigParser import SafeConfigParser21from ConfigParser import SafeConfigParser
@@ -42,9 +38,6 @@
42application = service.Application('BuildDSlave')38application = service.Application('BuildDSlave')
43builddslaveService = service.IServiceCollection(application)39builddslaveService = service.IServiceCollection(application)
4440
45# Service that announces when the daemon is ready
46readyservice.ReadyService().setServiceParent(builddslaveService)
47
48root = resource.Resource()41root = resource.Resource()
49root.putChild('rpc', slave)42root.putChild('rpc', slave)
50root.putChild('filecache', static.File(conf.get('slave', 'filecache')))43root.putChild('filecache', static.File(conf.get('slave', 'filecache')))
5144
=== modified file 'lib/canonical/buildd/debian/changelog'
--- lib/canonical/buildd/debian/changelog 2011-10-17 13:12:47 +0000
+++ lib/canonical/buildd/debian/changelog 2011-11-04 01:08:26 +0000
@@ -1,3 +1,11 @@
1launchpad-buildd (82) hardy-cat; urgency=low
2
3 * Cut out readyservice from the buildds. LP: #800295
4 * buildrecipe shows the bzr and bzr-builder versions. LP: #884092
5 * buildrecipe shows bzr rusage. LP: 884997
6
7 -- Martin Pool <mbp@canonical.com> Thu, 03 Nov 2011 17:11:25 +1100
8
1launchpad-buildd (81) hardy-cat; urgency=low9launchpad-buildd (81) hardy-cat; urgency=low
210
3 * generate-translation-templates: switch to Python 2.7.11 * generate-translation-templates: switch to Python 2.7.
412
=== modified file 'lib/canonical/buildd/debian/rules'
--- lib/canonical/buildd/debian/rules 2010-10-20 18:43:29 +0000
+++ lib/canonical/buildd/debian/rules 2011-11-04 01:08:26 +0000
@@ -14,7 +14,6 @@
14target = debian/launchpad-buildd14target = debian/launchpad-buildd
15topdir = ../../..15topdir = ../../..
1616
17daemons = $(topdir)/daemons
18buildd = $(topdir)/lib/canonical/buildd17buildd = $(topdir)/lib/canonical/buildd
1918
20targetshare = $(target)/usr/share/launchpad-buildd19targetshare = $(target)/usr/share/launchpad-buildd
@@ -45,8 +44,7 @@
45 # Do installs here44 # Do installs here
46 touch $(pytarget)/../launchpad/__init__.py45 touch $(pytarget)/../launchpad/__init__.py
47 touch $(pytarget)/../launchpad/daemons/__init__.py46 touch $(pytarget)/../launchpad/daemons/__init__.py
48 cp launchpad-files/readyservice.py $(pytarget)/../launchpad/daemons/47 install -m644 buildd-slave.tac $(targetshare)/buildd-slave.tac
49 install -m644 launchpad-files/buildd-slave.tac $(targetshare)/buildd-slave.tac
50 cp -r pottery $(pytarget)48 cp -r pottery $(pytarget)
51 for pyfile in $(pyfiles); do \49 for pyfile in $(pyfiles); do \
52 install -m644 ./$$pyfile $(pytarget)/$$pyfile; \50 install -m644 ./$$pyfile $(pytarget)/$$pyfile; \
@@ -90,8 +88,6 @@
90 dh_clean88 dh_clean
9189
92prepare:90prepare:
93 install -m644 $(daemons)/buildd-slave.tac launchpad-files/buildd-slave.tac
94 cp ../launchpad/daemons/readyservice.py launchpad-files/readyservice.py
9591
96package: prepare92package: prepare
97 debuild -uc -us -S93 debuild -uc -us -S
9894
=== modified file 'lib/canonical/buildd/tests/harness.py'
--- lib/canonical/buildd/tests/harness.py 2010-09-28 11:05:14 +0000
+++ lib/canonical/buildd/tests/harness.py 2011-11-04 01:08:26 +0000
@@ -1,9 +1,8 @@
1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
5__all__ = [5__all__ = [
6 'BuildlogSecurityTests',
7 'BuilddTestCase',6 'BuilddTestCase',
8 ]7 ]
98
@@ -13,6 +12,7 @@
13from ConfigParser import SafeConfigParser12from ConfigParser import SafeConfigParser
1413
15import canonical14import canonical
15import canonical.buildd
1616
17from canonical.buildd.slave import BuildDSlave17from canonical.buildd.slave import BuildDSlave
18from canonical.launchpad.daemons.tachandler import TacTestSetup18from canonical.launchpad.daemons.tachandler import TacTestSetup
@@ -119,8 +119,8 @@
119 @property119 @property
120 def tacfile(self):120 def tacfile(self):
121 return os.path.abspath(os.path.join(121 return os.path.abspath(os.path.join(
122 os.path.dirname(canonical.__file__), os.pardir, os.pardir,122 os.path.dirname(canonical.buildd.__file__),
123 'daemons/buildd-slave.tac'123 'buildd-slave.tac'
124 ))124 ))
125125
126 @property126 @property
@@ -130,3 +130,11 @@
130 @property130 @property
131 def logfile(self):131 def logfile(self):
132 return '/var/tmp/build-slave.log'132 return '/var/tmp/build-slave.log'
133
134 def _hasDaemonStarted(self):
135 """Called by the superclass to check if the daemon is listening.
136
137 The slave is ready when it's accepting connections.
138 """
139 # This must match buildd-slave-test.conf.
140 return self._isPortListening('localhost', 8221)
133141
=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
--- lib/canonical/launchpad/daemons/tachandler.py 2011-08-19 15:36:31 +0000
+++ lib/canonical/launchpad/daemons/tachandler.py 2011-11-04 01:08:26 +0000
@@ -11,7 +11,9 @@
11 ]11 ]
1212
1313
14import errno
14import os15import os
16import socket
15import subprocess17import subprocess
16import sys18import sys
17import time19import time
@@ -115,6 +117,25 @@
115 else:117 else:
116 return False118 return False
117119
120 def _isPortListening(self, host, port):
121 """True if a tcp port is accepting connections.
122
123 This can be used by subclasses overriding _hasDaemonStarted, if they
124 want to check the port is up rather than for the contents of the log
125 file.
126 """
127 try:
128 s = socket.socket()
129 s.settimeout(2.0)
130 s.connect((host, port))
131 s.close()
132 return True
133 except socket.error, e:
134 if e.errno == errno.ECONNREFUSED:
135 return False
136 else:
137 raise
138
118 def _waitForDaemonStartup(self):139 def _waitForDaemonStartup(self):
119 """ Wait for the daemon to fully start.140 """ Wait for the daemon to fully start.
120141
121142
=== renamed file 'lib/canonical/buildd/tests/test_generate_translation_templates.py' => 'lib/lp/translations/tests/test_generate_translation_templates.py'