Merge lp:~cjwatson/launchpad/snap-builds into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 17663
Proposed branch: lp:~cjwatson/launchpad/snap-builds
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/snapbuild-basic-model
Diff against target: 517 lines (+375/-10)
4 files modified
lib/lp/snappy/interfaces/snap.py (+10/-0)
lib/lp/snappy/model/snap.py (+106/-7)
lib/lp/snappy/tests/test_snap.py (+255/-0)
lib/lp/testing/factory.py (+4/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-builds
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+265698@code.launchpad.net

This proposal supersedes a proposal from 2015-07-23.

Commit message

Flesh out Snap.requestBuild, Snap.*builds, and Snap.destroySelf methods.

Description of the change

Flesh out Snap.requestBuild, Snap.*builds, and Snap.destroySelf methods.

Now that both Snap and SnapBuild are modelled, we can link them together.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/snappy/interfaces/snap.py'
2--- lib/lp/snappy/interfaces/snap.py 2015-07-30 14:57:39 +0000
3+++ lib/lp/snappy/interfaces/snap.py 2015-07-30 14:57:39 +0000
4@@ -15,6 +15,7 @@
5 'SNAP_FEATURE_FLAG',
6 'SnapBuildAlreadyPending',
7 'SnapBuildArchiveOwnerMismatch',
8+ 'SnapBuildDisallowedArchitecture',
9 'SnapFeatureDisabled',
10 'SnapNotOwner',
11 ]
12@@ -82,6 +83,15 @@
13 "if the snap package owner and the archive owner are equal.")
14
15
16+class SnapBuildDisallowedArchitecture(Exception):
17+ """A build was requested for a disallowed architecture."""
18+
19+ def __init__(self, das):
20+ super(SnapBuildDisallowedArchitecture, self).__init__(
21+ "This snap package is not allowed to build for %s." %
22+ das.displayname)
23+
24+
25 class SnapFeatureDisabled(Unauthorized):
26 """Only certain users can create new snap-related objects."""
27
28
29=== modified file 'lib/lp/snappy/model/snap.py'
30--- lib/lp/snappy/model/snap.py 2015-07-30 14:57:39 +0000
31+++ lib/lp/snappy/model/snap.py 2015-07-30 14:57:39 +0000
32@@ -11,15 +11,18 @@
33 from storm.locals import (
34 Bool,
35 DateTime,
36+ Desc,
37 Int,
38+ Not,
39 Reference,
40+ Store,
41 Storm,
42 Unicode,
43 )
44-from storm.store import Store
45 from zope.component import getUtility
46 from zope.interface import implementer
47
48+from lp.buildmaster.enums import BuildStatus
49 from lp.buildmaster.interfaces.processor import IProcessorSet
50 from lp.buildmaster.model.processor import Processor
51 from lp.registry.interfaces.role import IHasOwner
52@@ -31,16 +34,32 @@
53 IMasterStore,
54 IStore,
55 )
56+from lp.services.database.stormexpr import (
57+ Greatest,
58+ NullsLast,
59+ )
60 from lp.services.features import getFeatureFlag
61+from lp.services.webapp.interfaces import ILaunchBag
62 from lp.snappy.interfaces.snap import (
63+ CannotDeleteSnap,
64 DuplicateSnapName,
65 ISnap,
66 ISnapSet,
67 SNAP_FEATURE_FLAG,
68+ SnapBuildAlreadyPending,
69+ SnapBuildArchiveOwnerMismatch,
70+ SnapBuildDisallowedArchitecture,
71 SnapFeatureDisabled,
72 SnapNotOwner,
73 NoSuchSnap,
74 )
75+from lp.snappy.interfaces.snapbuild import ISnapBuildSet
76+from lp.snappy.model.snapbuild import SnapBuild
77+from lp.soyuz.interfaces.archive import ArchiveDisabled
78+from lp.soyuz.model.archive import (
79+ Archive,
80+ get_enabled_archive_filter,
81+ )
82
83
84 def snap_modified(snap, event):
85@@ -132,33 +151,113 @@
86
87 processors = property(_getProcessors, setProcessors)
88
89+ def _getAllowedArchitectures(self):
90+ """Return all distroarchseries that this package can build for.
91+
92+ :return: Sequence of `IDistroArchSeries` instances.
93+ """
94+ return [
95+ das for das in self.distro_series.buildable_architectures
96+ if (
97+ das.enabled
98+ and das.processor in self.processors
99+ and (
100+ das.processor.supports_virtualized
101+ or not self.require_virtualized))]
102+
103 def requestBuild(self, requester, archive, distro_arch_series, pocket):
104 """See `ISnap`."""
105- raise NotImplementedError
106+ if not requester.inTeam(self.owner):
107+ raise SnapNotOwner(
108+ "%s cannot create snap package builds owned by %s." %
109+ (requester.displayname, self.owner.displayname))
110+ if not archive.enabled:
111+ raise ArchiveDisabled(archive.displayname)
112+ if distro_arch_series not in self._getAllowedArchitectures():
113+ raise SnapBuildDisallowedArchitecture(distro_arch_series)
114+ if archive.private and self.owner != archive.owner:
115+ # See rationale in `SnapBuildArchiveOwnerMismatch` docstring.
116+ raise SnapBuildArchiveOwnerMismatch()
117+
118+ pending = IStore(self).find(
119+ SnapBuild,
120+ SnapBuild.snap_id == self.id,
121+ SnapBuild.archive_id == archive.id,
122+ SnapBuild.distro_arch_series_id == distro_arch_series.id,
123+ SnapBuild.pocket == pocket,
124+ SnapBuild.status == BuildStatus.NEEDSBUILD)
125+ if pending.any() is not None:
126+ raise SnapBuildAlreadyPending
127+
128+ build = getUtility(ISnapBuildSet).new(
129+ requester, self, archive, distro_arch_series, pocket)
130+ build.queueBuild()
131+ return build
132+
133+ def _getBuilds(self, filter_term, order_by):
134+ """The actual query to get the builds."""
135+ query_args = [
136+ SnapBuild.snap == self,
137+ SnapBuild.archive_id == Archive.id,
138+ Archive._enabled == True,
139+ get_enabled_archive_filter(
140+ getUtility(ILaunchBag).user, include_public=True,
141+ include_subscribed=True)
142+ ]
143+ if filter_term is not None:
144+ query_args.append(filter_term)
145+ result = Store.of(self).find(SnapBuild, *query_args)
146+ result.order_by(order_by)
147+ return result
148
149 @property
150 def builds(self):
151 """See `ISnap`."""
152- return []
153+ order_by = (
154+ NullsLast(Desc(Greatest(
155+ SnapBuild.date_started,
156+ SnapBuild.date_finished))),
157+ Desc(SnapBuild.date_created),
158+ Desc(SnapBuild.id))
159+ return self._getBuilds(None, order_by)
160
161 @property
162 def _pending_states(self):
163 """All the build states we consider pending (non-final)."""
164- raise NotImplementedError
165+ return [
166+ BuildStatus.NEEDSBUILD,
167+ BuildStatus.BUILDING,
168+ BuildStatus.UPLOADING,
169+ BuildStatus.CANCELLING,
170+ ]
171
172 @property
173 def completed_builds(self):
174 """See `ISnap`."""
175- return []
176+ filter_term = (Not(SnapBuild.status.is_in(self._pending_states)))
177+ order_by = (
178+ NullsLast(Desc(Greatest(
179+ SnapBuild.date_started,
180+ SnapBuild.date_finished))),
181+ Desc(SnapBuild.id))
182+ return self._getBuilds(filter_term, order_by)
183
184 @property
185 def pending_builds(self):
186 """See `ISnap`."""
187- return []
188+ filter_term = (SnapBuild.status.is_in(self._pending_states))
189+ # We want to order by date_created but this is the same as ordering
190+ # by id (since id increases monotonically) and is less expensive.
191+ order_by = Desc(SnapBuild.id)
192+ return self._getBuilds(filter_term, order_by)
193
194 def destroySelf(self):
195 """See `ISnap`."""
196- raise NotImplementedError
197+ if not self.builds.is_empty():
198+ raise CannotDeleteSnap("Cannot delete a snap package with builds.")
199+ store = IStore(Snap)
200+ store.find(SnapArch, SnapArch.snap == self).remove()
201+ store.remove(self)
202
203
204 class SnapArch(Storm):
205
206=== modified file 'lib/lp/snappy/tests/test_snap.py'
207--- lib/lp/snappy/tests/test_snap.py 2015-07-30 14:57:39 +0000
208+++ lib/lp/snappy/tests/test_snap.py 2015-07-30 14:57:39 +0000
209@@ -9,22 +9,35 @@
210
211 from lazr.lifecycle.event import ObjectModifiedEvent
212 import pytz
213+from storm.locals import Store
214 import transaction
215 from zope.component import getUtility
216 from zope.event import notify
217 from zope.security.proxy import removeSecurityProxy
218
219+from lp.buildmaster.enums import (
220+ BuildQueueStatus,
221+ BuildStatus,
222+ )
223+from lp.buildmaster.interfaces.buildqueue import IBuildQueue
224 from lp.buildmaster.interfaces.processor import IProcessorSet
225+from lp.buildmaster.model.buildqueue import BuildQueue
226+from lp.registry.interfaces.pocket import PackagePublishingPocket
227 from lp.services.database.constants import UTC_NOW
228 from lp.services.features.testing import FeatureFixture
229 from lp.snappy.interfaces.snap import (
230+ CannotDeleteSnap,
231 ISnap,
232 ISnapSet,
233 SNAP_FEATURE_FLAG,
234+ SnapBuildAlreadyPending,
235+ SnapBuildDisallowedArchitecture,
236 SnapFeatureDisabled,
237 )
238+from lp.snappy.interfaces.snapbuild import ISnapBuild
239 from lp.testing import (
240 admin_logged_in,
241+ person_logged_in,
242 TestCaseWithFactory,
243 )
244 from lp.testing.layers import (
245@@ -74,6 +87,248 @@
246 removeSecurityProxy(snap), snap, [ISnap["name"]]))
247 self.assertSqlAttributeEqualsDate(snap, "date_last_modified", UTC_NOW)
248
249+ def makeBuildableDistroArchSeries(self, **kwargs):
250+ das = self.factory.makeDistroArchSeries(**kwargs)
251+ fake_chroot = self.factory.makeLibraryFileAlias(
252+ filename="fake_chroot.tar.gz", db_only=True)
253+ das.addOrUpdateChroot(fake_chroot)
254+ return das
255+
256+ def test_requestBuild(self):
257+ # requestBuild creates a new SnapBuild.
258+ processor = self.factory.makeProcessor(supports_virtualized=True)
259+ distroarchseries = self.makeBuildableDistroArchSeries(
260+ processor=processor)
261+ snap = self.factory.makeSnap(
262+ distroseries=distroarchseries.distroseries,
263+ processors=[distroarchseries.processor])
264+ build = snap.requestBuild(
265+ snap.owner, snap.distro_series.main_archive, distroarchseries,
266+ PackagePublishingPocket.RELEASE)
267+ self.assertTrue(ISnapBuild.providedBy(build))
268+ self.assertEqual(snap.owner, build.requester)
269+ self.assertEqual(snap.distro_series.main_archive, build.archive)
270+ self.assertEqual(distroarchseries, build.distro_arch_series)
271+ self.assertEqual(PackagePublishingPocket.RELEASE, build.pocket)
272+ self.assertEqual(BuildStatus.NEEDSBUILD, build.status)
273+ store = Store.of(build)
274+ store.flush()
275+ build_queue = store.find(
276+ BuildQueue,
277+ BuildQueue._build_farm_job_id ==
278+ removeSecurityProxy(build).build_farm_job_id).one()
279+ self.assertProvides(build_queue, IBuildQueue)
280+ self.assertEqual(
281+ snap.distro_series.main_archive.require_virtualized,
282+ build_queue.virtualized)
283+ self.assertEqual(BuildQueueStatus.WAITING, build_queue.status)
284+
285+ def test_requestBuild_score(self):
286+ # Build requests have a relatively low queue score (2505).
287+ processor = self.factory.makeProcessor(supports_virtualized=True)
288+ distroarchseries = self.makeBuildableDistroArchSeries(
289+ processor=processor)
290+ snap = self.factory.makeSnap(
291+ distroseries=distroarchseries.distroseries,
292+ processors=[distroarchseries.processor])
293+ build = snap.requestBuild(
294+ snap.owner, snap.distro_series.main_archive, distroarchseries,
295+ PackagePublishingPocket.RELEASE)
296+ queue_record = build.buildqueue_record
297+ queue_record.score()
298+ self.assertEqual(2505, queue_record.lastscore)
299+
300+ def test_requestBuild_relative_build_score(self):
301+ # Offsets for archives are respected.
302+ processor = self.factory.makeProcessor(supports_virtualized=True)
303+ distroarchseries = self.makeBuildableDistroArchSeries(
304+ processor=processor)
305+ snap = self.factory.makeSnap(
306+ distroseries=distroarchseries.distroseries, processors=[processor])
307+ archive = self.factory.makeArchive(owner=snap.owner)
308+ removeSecurityProxy(archive).relative_build_score = 100
309+ build = snap.requestBuild(
310+ snap.owner, archive, distroarchseries,
311+ PackagePublishingPocket.RELEASE)
312+ queue_record = build.buildqueue_record
313+ queue_record.score()
314+ self.assertEqual(2605, queue_record.lastscore)
315+
316+ def test_requestBuild_rejects_repeats(self):
317+ # requestBuild refuses if there is already a pending build.
318+ distroseries = self.factory.makeDistroSeries()
319+ procs = []
320+ arches = []
321+ for i in range(2):
322+ procs.append(self.factory.makeProcessor(supports_virtualized=True))
323+ arches.append(self.makeBuildableDistroArchSeries(
324+ distroseries=distroseries, processor=procs[i]))
325+ snap = self.factory.makeSnap(
326+ distroseries=distroseries, processors=[procs[0], procs[1]])
327+ old_build = snap.requestBuild(
328+ snap.owner, snap.distro_series.main_archive, arches[0],
329+ PackagePublishingPocket.RELEASE)
330+ self.assertRaises(
331+ SnapBuildAlreadyPending, snap.requestBuild,
332+ snap.owner, snap.distro_series.main_archive, arches[0],
333+ PackagePublishingPocket.RELEASE)
334+ # We can build for a different archive.
335+ snap.requestBuild(
336+ snap.owner, self.factory.makeArchive(owner=snap.owner), arches[0],
337+ PackagePublishingPocket.RELEASE)
338+ # We can build for a different distroarchseries.
339+ snap.requestBuild(
340+ snap.owner, snap.distro_series.main_archive, arches[1],
341+ PackagePublishingPocket.RELEASE)
342+ # Changing the status of the old build allows a new build.
343+ old_build.updateStatus(BuildStatus.BUILDING)
344+ old_build.updateStatus(BuildStatus.FULLYBUILT)
345+ snap.requestBuild(
346+ snap.owner, snap.distro_series.main_archive, arches[0],
347+ PackagePublishingPocket.RELEASE)
348+
349+ def test_requestBuild_rejects_unconfigured_arch(self):
350+ # Snap.requestBuild only allows dispatching a build for one of the
351+ # configured architectures.
352+ distroseries = self.factory.makeDistroSeries()
353+ procs = []
354+ arches = []
355+ for i in range(2):
356+ procs.append(self.factory.makeProcessor(supports_virtualized=True))
357+ arches.append(self.makeBuildableDistroArchSeries(
358+ distroseries=distroseries, processor=procs[i]))
359+ snap = self.factory.makeSnap(
360+ distroseries=distroseries, processors=[procs[0]])
361+ snap.requestBuild(
362+ snap.owner, snap.distro_series.main_archive, arches[0],
363+ PackagePublishingPocket.RELEASE)
364+ self.assertRaises(
365+ SnapBuildDisallowedArchitecture, snap.requestBuild,
366+ snap.owner, snap.distro_series.main_archive, arches[1],
367+ PackagePublishingPocket.RELEASE)
368+
369+ def test_requestBuild_virtualization(self):
370+ # New builds are virtualized if any of the processor, snap or
371+ # archive require it.
372+ proc_arches = {}
373+ for proc_nonvirt in True, False:
374+ processor = self.factory.makeProcessor(
375+ supports_virtualized=True,
376+ supports_nonvirtualized=proc_nonvirt)
377+ distroarchseries = self.makeBuildableDistroArchSeries(
378+ processor=processor)
379+ proc_arches[proc_nonvirt] = (processor, distroarchseries)
380+ for proc_nonvirt, snap_virt, archive_virt, build_virt in (
381+ (True, False, False, False),
382+ (True, False, True, True),
383+ (True, True, False, True),
384+ (True, True, True, True),
385+ (False, False, False, True),
386+ (False, False, True, True),
387+ (False, True, False, True),
388+ (False, True, True, True),
389+ ):
390+ processor, distroarchseries = proc_arches[proc_nonvirt]
391+ snap = self.factory.makeSnap(
392+ distroseries=distroarchseries.distroseries,
393+ require_virtualized=snap_virt, processors=[processor])
394+ archive = self.factory.makeArchive(
395+ distribution=distroarchseries.distroseries.distribution,
396+ owner=snap.owner, virtualized=archive_virt)
397+ build = snap.requestBuild(
398+ snap.owner, archive, distroarchseries,
399+ PackagePublishingPocket.RELEASE)
400+ self.assertEqual(build_virt, build.virtualized)
401+
402+ def test_requestBuild_nonvirtualized(self):
403+ # A non-virtualized processor can build a snap package iff the snap
404+ # has require_virtualized set to False.
405+ processor = self.factory.makeProcessor(
406+ supports_virtualized=False, supports_nonvirtualized=True)
407+ distroarchseries = self.makeBuildableDistroArchSeries(
408+ processor=processor)
409+ snap = self.factory.makeSnap(
410+ distroseries=distroarchseries.distroseries, processors=[processor])
411+ self.assertRaises(
412+ SnapBuildDisallowedArchitecture, snap.requestBuild,
413+ snap.owner, snap.distro_series.main_archive, distroarchseries,
414+ PackagePublishingPocket.RELEASE)
415+ with admin_logged_in():
416+ snap.require_virtualized = False
417+ snap.requestBuild(
418+ snap.owner, snap.distro_series.main_archive, distroarchseries,
419+ PackagePublishingPocket.RELEASE)
420+
421+ def test_getBuilds(self):
422+ # Test the various getBuilds methods.
423+ snap = self.factory.makeSnap()
424+ builds = [self.factory.makeSnapBuild(snap=snap) for x in range(3)]
425+ # We want the latest builds first.
426+ builds.reverse()
427+
428+ self.assertEqual(builds, list(snap.builds))
429+ self.assertEqual([], list(snap.completed_builds))
430+ self.assertEqual(builds, list(snap.pending_builds))
431+
432+ # Change the status of one of the builds and retest.
433+ builds[0].updateStatus(BuildStatus.BUILDING)
434+ builds[0].updateStatus(BuildStatus.FULLYBUILT)
435+ self.assertEqual(builds, list(snap.builds))
436+ self.assertEqual(builds[:1], list(snap.completed_builds))
437+ self.assertEqual(builds[1:], list(snap.pending_builds))
438+
439+ def test_getBuilds_cancelled_never_started_last(self):
440+ # A cancelled build that was never even started sorts to the end.
441+ snap = self.factory.makeSnap()
442+ fullybuilt = self.factory.makeSnapBuild(snap=snap)
443+ instacancelled = self.factory.makeSnapBuild(snap=snap)
444+ fullybuilt.updateStatus(BuildStatus.BUILDING)
445+ fullybuilt.updateStatus(BuildStatus.FULLYBUILT)
446+ instacancelled.updateStatus(BuildStatus.CANCELLED)
447+ self.assertEqual([fullybuilt, instacancelled], list(snap.builds))
448+ self.assertEqual(
449+ [fullybuilt, instacancelled], list(snap.completed_builds))
450+ self.assertEqual([], list(snap.pending_builds))
451+
452+ def test_getBuilds_privacy(self):
453+ # The various getBuilds methods exclude builds against invisible
454+ # archives.
455+ snap = self.factory.makeSnap()
456+ archive = self.factory.makeArchive(
457+ distribution=snap.distro_series.distribution, owner=snap.owner,
458+ private=True)
459+ with person_logged_in(snap.owner):
460+ build = self.factory.makeSnapBuild(snap=snap, archive=archive)
461+ self.assertEqual([build], list(snap.builds))
462+ self.assertEqual([build], list(snap.pending_builds))
463+ self.assertEqual([], list(snap.builds))
464+ self.assertEqual([], list(snap.pending_builds))
465+
466+ def test_delete_without_builds(self):
467+ # A snap package with no builds can be deleted.
468+ owner = self.factory.makePerson()
469+ distroseries = self.factory.makeDistroSeries()
470+ snap = self.factory.makeSnap(
471+ registrant=owner, owner=owner, distroseries=distroseries,
472+ name=u"condemned")
473+ self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
474+ with person_logged_in(snap.owner):
475+ snap.destroySelf()
476+ self.assertFalse(getUtility(ISnapSet).exists(owner, u"condemned"))
477+
478+ def test_delete_with_builds(self):
479+ # A snap package with builds cannot be deleted.
480+ owner = self.factory.makePerson()
481+ distroseries = self.factory.makeDistroSeries()
482+ snap = self.factory.makeSnap(
483+ registrant=owner, owner=owner, distroseries=distroseries,
484+ name=u"condemned")
485+ self.factory.makeSnapBuild(snap=snap)
486+ self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
487+ with person_logged_in(snap.owner):
488+ self.assertRaises(CannotDeleteSnap, snap.destroySelf)
489+ self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
490+
491
492 class TestSnapSet(TestCaseWithFactory):
493
494
495=== modified file 'lib/lp/testing/factory.py'
496--- lib/lp/testing/factory.py 2015-07-30 14:57:39 +0000
497+++ lib/lp/testing/factory.py 2015-07-30 14:57:39 +0000
498@@ -4536,7 +4536,8 @@
499
500 def makeSnap(self, registrant=None, owner=None, distroseries=None,
501 name=None, branch=None, git_ref=None,
502- require_virtualized=True, date_created=DEFAULT):
503+ require_virtualized=True, processors=None,
504+ date_created=DEFAULT):
505 """Make a new Snap."""
506 if registrant is None:
507 registrant = self.makePerson()
508@@ -4556,8 +4557,8 @@
509 kwargs["git_path"] = git_ref.path
510 snap = getUtility(ISnapSet).new(
511 registrant, owner, distroseries, name,
512- require_virtualized=require_virtualized, date_created=date_created,
513- **kwargs)
514+ require_virtualized=require_virtualized, processors=processors,
515+ date_created=date_created, **kwargs)
516 IStore(snap).flush()
517 return snap
518