Merge lp:~cjwatson/launchpad/configurable-germinate-base into lp:launchpad

Proposed by Colin Watson
Status: Rejected
Rejected by: Colin Watson
Proposed branch: lp:~cjwatson/launchpad/configurable-germinate-base
Merge into: lp:launchpad
Diff against target: 195 lines (+29/-22)
4 files modified
configs/development/launchpad-lazr.conf (+1/-0)
lib/lp/archivepublisher/scripts/generate_extra_overrides.py (+9/-8)
lib/lp/archivepublisher/tests/test_generate_extra_overrides.py (+15/-14)
lib/lp/services/config/schema-lazr.conf (+4/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/configurable-germinate-base
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Resubmitting
William Grant Needs Information
Review via email: mp+194340@code.launchpad.net

Commit message

Make germinate's seed base location configurable.

Description of the change

== Summary ==

Allow us to configure the seed base URL used by germinate in generate-extra-overrides (by default "http://people.canonical.com/~ubuntu-archive/seeds/") so that we can have it point to http://archive-team.internal/seeds/ instead in production. This will avoid the publisher being reliant on two other machines rather than one for proper functioning, since http://people.canonical.com/~ubuntu-archive/... is now handled by a proxy.

== Tests ==

bin/test -vvct lp.archivepublisher.tests.test_generate_extra_overrides

== Demo and Q/A ==

Run generate-extra-overrides on DF without configuration and verify that it still works. Configure DF to use some different URL with modified seeds instead and verify that the new output of generate-extra-overrides reflects these.

To post a comment you must log in.
Revision history for this message
Adam Conrad (adconrad) wrote :

maintenance-check.py also hits people.canonical.com for the same reason, should probably be fixed to use the same config option.

Testing this should perhaps be done by intentionally firewalling off people.c.c from dogfood, doing a publisher run, and scraping logs to make sure we didn't miss anything. :P

Revision history for this message
Colin Watson (cjwatson) wrote :

On Thu, Nov 07, 2013 at 09:25:47PM -0000, Adam Conrad wrote:
> maintenance-check.py also hits people.canonical.com for the same reason, should probably be fixed to use the same config option.

It probably needs an additional one, because it points to
germinate-output as well as seeds and I'm a bit wary of hardcoding their
relative locations.

I'm also wary of changing anything substantial in maintenance-check
until it has tests worth the name, though. I should finish that
refactoring branch ...

Revision history for this message
William Grant (wgrant) wrote :

Overriding the config should usually only been done in high-level integration tests. What was the rationale behind having makeSeedStructures() look up the config option, rather than doing it in generateExtraOverrides() or even process()? That would surely make testing less ugly.

review: Needs Information
Revision history for this message
Colin Watson (cjwatson) wrote :

I'll redo this on top of lp:ubuntu-archive-publishing, and improve the configuration strategy in the process.

review: Needs Resubmitting

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2013-04-09 11:51:24 +0000
3+++ configs/development/launchpad-lazr.conf 2013-11-07 13:43:38 +0000
4@@ -7,6 +7,7 @@
5
6 [archivepublisher]
7 run_parts_location: none
8+germinate_seed_base: none
9
10 [builddmaster]
11 root: /var/tmp/builddmaster/
12
13=== modified file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
14--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py 2012-11-10 02:21:31 +0000
15+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py 2013-11-07 13:43:38 +0000
16@@ -1,4 +1,4 @@
17-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
18+# Copyright 2011-2013 Canonical Ltd. This software is licensed under the
19 # GNU Affero General Public License version 3 (see the file LICENSE).
20
21 """Generate extra overrides using Germinate."""
22@@ -27,6 +27,7 @@
23 from lp.archivepublisher.config import getPubConfig
24 from lp.registry.interfaces.distribution import IDistributionSet
25 from lp.registry.interfaces.series import SeriesStatus
26+from lp.services.config import config
27 from lp.services.database.policy import (
28 DatabaseBlockedPolicy,
29 SlaveOnlyDatabasePolicy,
30@@ -155,7 +156,9 @@
31 for component in series.component_names
32 if component != "partner"]
33
34- def makeSeedStructures(self, series_name, flavours, seed_bases=None):
35+ def makeSeedStructures(self, series_name, flavours):
36+ seed_base = config.archivepublisher.germinate_seed_base
37+ seed_bases = None if seed_base is None else [seed_base]
38 structures = {}
39 for flavour in flavours:
40 try:
41@@ -343,9 +346,8 @@
42 os.remove(output)
43
44 def generateExtraOverrides(self, series_name, components, architectures,
45- flavours, seed_bases=None):
46- structures = self.makeSeedStructures(
47- series_name, flavours, seed_bases=seed_bases)
48+ flavours):
49+ structures = self.makeSeedStructures(series_name, flavours)
50
51 if structures:
52 seed_outputs = set()
53@@ -359,7 +361,7 @@
54 flavours, structures, seed_outputs=seed_outputs)
55 self.removeStaleOutputs(series_name, seed_outputs)
56
57- def process(self, seed_bases=None):
58+ def process(self):
59 """Do the bulk of the work."""
60 self.setUp()
61
62@@ -374,8 +376,7 @@
63 self.txn.commit()
64 with DatabaseBlockedPolicy():
65 self.generateExtraOverrides(
66- series_name, components, architectures, self.args,
67- seed_bases=seed_bases)
68+ series_name, components, architectures, self.args)
69
70 def main(self):
71 """See `LaunchpadScript`."""
72
73=== modified file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
74--- lib/lp/archivepublisher/tests/test_generate_extra_overrides.py 2013-09-24 05:45:06 +0000
75+++ lib/lp/archivepublisher/tests/test_generate_extra_overrides.py 2013-11-07 13:43:38 +0000
76@@ -1,4 +1,4 @@
77-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
78+# Copyright 2011-2013 Canonical Ltd. This software is licensed under the
79 # GNU Affero General Public License version 3 (see the file LICENSE).
80
81 """Test for the `generate-extra-overrides` script."""
82@@ -10,6 +10,7 @@
83 from optparse import OptionValueError
84 import os
85 import tempfile
86+from textwrap import dedent
87
88 from germinate import (
89 archive,
90@@ -29,6 +30,7 @@
91 from lp.archivepublisher.utils import RepositoryIndexFile
92 from lp.registry.interfaces.pocket import PackagePublishingPocket
93 from lp.registry.interfaces.series import SeriesStatus
94+from lp.services.config import config
95 from lp.services.log.logger import DevNullLogger
96 from lp.services.osutils import (
97 ensure_directory_exists,
98@@ -85,6 +87,10 @@
99 def setUp(self):
100 super(TestGenerateExtraOverrides, self).setUp()
101 self.seeddir = self.makeTemporaryDirectory()
102+ config.push("seed-base", dedent("""\
103+ [archivepublisher]
104+ germinate_seed_base: file://%s
105+ """ % self.seeddir))
106 # XXX cjwatson 2011-12-06 bug=694140: Make sure germinate doesn't
107 # lose its loggers between tests, due to Launchpad's messing with
108 # global log state.
109@@ -322,8 +328,7 @@
110 series_name = self.distroseries[0].name
111 flavour = self.factory.getUniqueString()
112
113- structures = self.script.makeSeedStructures(
114- series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
115+ structures = self.script.makeSeedStructures(series_name, [flavour])
116 self.assertEqual({}, structures)
117
118 def test_make_seed_structures_empty_seed_structure(self):
119@@ -333,8 +338,7 @@
120 flavour = self.factory.getUniqueString()
121 self.makeSeedStructure(flavour, series_name, [])
122
123- structures = self.script.makeSeedStructures(
124- series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
125+ structures = self.script.makeSeedStructures(series_name, [flavour])
126 self.assertEqual({}, structures)
127
128 def test_make_seed_structures_valid_seeds(self):
129@@ -346,15 +350,12 @@
130 self.makeSeedStructure(flavour, series_name, [seed])
131 self.makeSeed(flavour, series_name, seed, [])
132
133- structures = self.script.makeSeedStructures(
134- series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
135+ structures = self.script.makeSeedStructures(series_name, [flavour])
136 self.assertIn(flavour, structures)
137
138 def fetchGerminatedOverrides(self, script, distroseries, arch, flavours):
139 """Helper to call script.germinateArch and return overrides."""
140- structures = script.makeSeedStructures(
141- distroseries.name, flavours,
142- seed_bases=["file://%s" % self.seeddir])
143+ structures = script.makeSeedStructures(distroseries.name, flavours)
144
145 override_fd, override_path = tempfile.mkstemp()
146 with os.fdopen(override_fd, "w") as override_file:
147@@ -579,7 +580,7 @@
148 self.makeSeedStructure(flavour, self.distroseries[1].name, [seed])
149 self.makeSeed(flavour, self.distroseries[1].name, seed, [])
150
151- self.script.process(seed_bases=["file://%s" % self.seeddir])
152+ self.script.process()
153 self.assertFalse(os.path.exists(os.path.join(
154 self.script.config.miscroot,
155 "more-extra.override.%s.main" % self.distroseries[0].name)))
156@@ -602,13 +603,13 @@
157 seed_new = self.factory.getUniqueString()
158 self.makeSeedStructure(flavour, series_name, [seed_old])
159 self.makeSeed(flavour, series_name, seed_old, [])
160- self.script.process(seed_bases=["file://%s" % self.seeddir])
161+ self.script.process()
162 output = partial(
163 self.script.composeOutputPath, flavour, series_name, arch)
164 self.assertTrue(os.path.exists(output(seed_old)))
165 self.makeSeedStructure(flavour, series_name, [seed_new])
166 self.makeSeed(flavour, series_name, seed_new, [])
167- self.script.process(seed_bases=["file://%s" % self.seeddir])
168+ self.script.process()
169 self.assertTrue(os.path.exists(os.path.join(self.script.log_file)))
170 self.assertTrue(os.path.exists(output("structure")))
171 self.assertTrue(os.path.exists(output("all")))
172@@ -652,7 +653,7 @@
173 flavour, series_name, seed, [package.name],
174 headers=["Task-Description: task"])
175
176- self.script.process(seed_bases=["file://%s" % self.seeddir])
177+ self.script.process()
178 override_path = os.path.join(
179 self.script.config.miscroot,
180 "more-extra.override.%s.main" % series_name)
181
182=== modified file 'lib/lp/services/config/schema-lazr.conf'
183--- lib/lp/services/config/schema-lazr.conf 2013-08-27 01:30:47 +0000
184+++ lib/lp/services/config/schema-lazr.conf 2013-11-07 13:43:38 +0000
185@@ -32,6 +32,10 @@
186 # datatype: string
187 run_parts_location: none
188
189+# Base URL from which generate-extra-overrides fetches seeds using
190+# germinate. If "none", use germinate's built-in default.
191+germinate_seed_base: none
192+
193
194 [auditor]
195 host: localhost